linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Dev Jain <dev.jain@arm.com>
To: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@kernel.org>,
	"Liam R . Howlett" <Liam.Howlett@oracle.com>,
	Vlastimil Babka <vbabka@suse.cz>, Mike Rapoport <rppt@kernel.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Michal Hocko <mhocko@suse.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-kselftest@vger.kernel.org, Mark Brown <broonie@kernel.org>,
	aneesh.kumar@kernel.org,
	Anshuman Khandual <anshuman.khandual@arm.com>
Subject: Re: [PATCH] selftests/mm: remove virtual_address_range test
Date: Tue, 20 Jan 2026 15:50:45 +0530	[thread overview]
Message-ID: <27ca39af-27d2-4f99-8988-5c45211b659a@arm.com> (raw)
In-Reply-To: <8acef9ff-c5e2-4111-9437-f50b427db061@lucifer.local>


On 20/01/26 2:13 pm, Lorenzo Stoakes wrote:
> On Tue, Jan 20, 2026 at 10:59:01AM +0530, Dev Jain wrote:
>> I'll reply to everything here otherwise I'll have to repeat myself at different places.
>>
>> "Also exhausting VA space is an inherently silly thing for a test to do, you're
>> making assumptions about existing VMA layout which is absolutely an
>> implementation detail and may even be influence by libc..."
>>
>> The original version only uses mmap() and checks whether we got a high address mmap
>> success when we shouldn't have. There are no assumptions being made here about
>> VA layout. No matter the VA layout, the test will succeed because the kernel
>> must enforce the distinction between low and high addresses (but see point 3 below).
>>
>>
>> "It is therefore ENTIRELY a user-facing and kernel/user API thing that has to be tested from this perspective."
>>
>> So in merge.c, the statements ASSERT_NE(ptrx, MAP_FAILED) surely assert the
>> user-visible API - that mremap must not fail. But there are statements which
>> also assert where a VMA starts and where it ends, testing VMA merging -
>> I was concerned about these. It is not the goal of userspace to minimize the
>> number of VMAs while making a syscall - that is a kernel optimization. My point being,
>> I suspect that the mm selftests *already* test internal details a lot, and I believe
>> that they *need* to! Running selftests is the most convenient way of testing the mm subsystem.
>> Hence this should not be a ground for removal of test.
> Dev I would rather try to be positive not negative in review but again,
> this isn't constructive, we're not talking about merge.c and even if it
> contained the comment /* Ha ha I am a contradiction */ it wouldn't impact
> this discussion.
>
> You're not correct, mremap() has an API requirement that you can't cross
> VMA boundaries for most operations, therefore start/end of VMA's and
> merging _does_ matter. This also impacts how e.g. madvise() behaves.
>
> As I said before:
>
> 	But you do not - both VMA merge and CoW impact API. Re: merging
> 	certain user-facing functions, most notably mremap(), have API
> 	requirements that the user must not cross VMA boundaries. It is
> 	therefore ENTIRELY a user-facing and kernel/user API thing that has
> 	to be tested from this perspective.
>
> Can we drop the subject please?

I think we are talking past each other, and communication by email has not been
the greatest thing, so I'll step back.

>
>> Talking about the recent commits, they can be reverted. So, the ground for
>> removal should be that the ratio of the time taken by the test (exhausting VA
>> space), to the coverage of the test (for arm64, it is testing backward compatibility of 48-bit VA
>> on 52-bit VA, which one can argue is easy to spot if it ever breaks, and easy to fix)
>> is too large and does not justify a selftest. I tend to agree here.
>>
>> "Add a _new_ selftest, named something sensible like mmap_hint.c or something ..."
>>
>> va_high_addr_switch.c tests stuff around the switch boundary. But it does not
>> exhaust VA space. We *must* exhaust VA space if we are to check that the kernel,
>> in a situation of "emergency" (i.e exhausted lower VA space), starts giving out
>> high addresses, when it shouldn't. Again, one may argue that trying to test
>> this out is not worth it.
>>
>> I personally opine that besides testing the back compat of 48 bit VA on 52 bit VA,
>> we are testing something more important here: exhausting the VA space tests whether
>> the kernel can truly distinguish b/w virtual and physical memory - we stress the virtual
>> memory subsystem without touching physical memory, something which the kernel should be able
>> to handle. But again, any such test has the potential for a timeout. I wonder if there is a
>> faster way of filling up VA space.
>>
>> To summarize, I will agree with you that currently
>>
>> 1. The test is in a broken state
>> 2. The test is taking too much time to test something trivial
>> 3. It is a maintenance hazard. It turns out that the original version used
>>    MAP_CHUNK_SIZE of 16GB, but then it was changed to 1GB because (this is the bit
>>    where the dependency on VA layout comes) on some systems even a single 16GB mmap
>>    may fail. So now we are stuck in making a tradeoff between the size of a single
>>    mmap versus the time taken by the test
>>
>> So the better option seems to just remove the test.
> Right let's just focus on that.
>
>> A separate question: Do you think that the functionality advertised by validate_complete_va_space,
>> to check the gaps between VMAs, deserves a test in kunit / tools/testing/vma, or somewhere else?
> I don't think so as it's not a requirement set in stone as far as I
> understand it.
>
> But expectations as to what get_unmapped_area() should be doing makes more
> sense as a kunit test.
>
> Thanks, Lorenzo


  reply	other threads:[~2026-01-20 10:20 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-16 13:20 Lorenzo Stoakes
2026-01-17  1:21 ` SeongJae Park
2026-01-18  7:55 ` Dev Jain
2026-01-18 12:58   ` Lorenzo Stoakes
2026-01-19  6:21     ` Dev Jain
2026-01-19  8:59       ` Lorenzo Stoakes
2026-01-19  9:06         ` Dev Jain
2026-01-19  9:13           ` Lorenzo Stoakes
2026-01-19 11:11         ` Lorenzo Stoakes
2026-01-20  5:29         ` Dev Jain
2026-01-20  8:43           ` Lorenzo Stoakes
2026-01-20 10:20             ` Dev Jain [this message]
2026-01-19 10:39 ` David Hildenbrand (Red Hat)
2026-01-19 11:06   ` Lorenzo Stoakes
2026-01-19 11:11     ` David Hildenbrand (Red Hat)
2026-01-19 11:16       ` Lorenzo Stoakes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=27ca39af-27d2-4f99-8988-5c45211b659a@arm.com \
    --to=dev.jain@arm.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=aneesh.kumar@kernel.org \
    --cc=anshuman.khandual@arm.com \
    --cc=broonie@kernel.org \
    --cc=david@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=mhocko@suse.com \
    --cc=rppt@kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox