Re: [PATCH v4 10/13] mm/collapse: open-code collapse_single_pmd() in its two callers

[David Hildenbrand (Arm)]

On 9/28/26 12:06, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@xxxxxxxxxx>
> 
> A scan and a collapse want different things from mmap_lock.  The scan
> reads one PTE table under the lock the caller holds, refuses most of the
> time, and the caller moves on to the next table without letting go.  The
> collapse allocates, may sleep in writeback and takes the lock for write
> itself, so the lock it is handed is of no use to it.
> 
> collapse_single_pmd() kept that boundary inside itself.  It dropped the
> lock on some paths and not others, and reported which by way of a bool
> its callers had to carry along and then act on. 

Let me think this through. collapse_scan_file: it doesn't actually need the MM at
all. The only reason is to do tracing. Rather stupid, it just should not consume
the MM at all anymore. Consequently it doesn't even need the mmap lock. But the
caller needs the mmap lock to figure out the file + range from the vma (the
per-vma lock would also be sufficient for that).

Also, I guess we can convert some of the scanning to use per-vma locks in the
future, whereby we would actually want to scan with the per-vma lock held.

I do wonder about one thing: should we really care so much about keeping the
mmap lock locked? Meaning, why not provide a single collapse_single_pmd() that

* Is always called without the mmap lock (as is)
* Always returns with the mmap lock unlocked (change)

Sure, we drop+re-acquire the mmap lock a couple of times and lookup the vma, but
isn't that actually being nice to the other parts of the system? In the future
 it would simply get called with the per-vma lock and would return with it unlocked.

In the good old days, looking up VMAs was expensive, but nowadays ... not sure
if it still matters?

khugepaged? Not sure if this matters. madvise? I suspect many real users operate
on a single PMD only (e.g., tcmalloc, jemalloc). For the other ones, not sure if
dropping the lock every PMD is really a problem?

IOW, how bad would the following simplification be (prototype that needs more
work and thought):