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
