Re: [PATCH 05/46] mm/truncate: look again if a folio left the mapping

[Kiryl Shutsemau]

On Thu, Oct 29, 2026 at 03:18:23PM +0100, David Hildenbrand (Arm) wrote:
> On 10/29/26 12:37, Kiryl Shutsemau wrote:
> > On Wed, Oct 28, 2026 at 10:45:34PM +0100, David Hildenbrand (Arm) wrote:
> >> My brain hurts, does the above make sense?
> > 
> > Yes, I missed that the loop only goes back to start when a lookup
> > comes back empty below end. When the last batch reaches end, it exits
> > without another look. It doesn't need the split either: any punch hole
> > whose last page is in the page cache gets there.
> > 
> > That exit looks unintended to me. b9a8a4195c7d ("truncate,shmem: Handle
> > truncates that split large folios") turned the "if (start >= end) goto
> > out" early exit into the loop condition. Before it, the loop went back
> > to start until a lookup from start found nothing; Hugh kept that on
> > purpose for hole punch in 792ceaefe621 ("mm/fs: fix pessimization in
> > hole-punching pagecache").
> > 
> > Restoring it is enough, and makes the "restart to make sure all gone"
> > comment true again:
> > 
> > +       if (start >= end)
> > +               return;
> > +
> >         index = start;
> > -       while (index < end) {
> > +       for (;;) {
> > 
> > Then a replaced folio is found on the next round, and truncate needs no
> > special case for it.
> > 
> 
> But that's then another lookup even if not really required?
> 
> Why do you thing that is that better?
> 
> I agree that the code is simpler, but at least it's consistent with the other
> handling.

The extra lookup only happens when the second loop had work to do.
Normally the first pass removes everything, the first lookup from start
comes back empty, and both versions stop there. When the second loop did
run to end, it costs one more lookup over a range that is now empty.

What I like about it is one way to look again instead of two: the loop
already goes back to start when the last batch ends below end, and the
patch adds a second way for when it ends at end. truncate is not like
shmem here: shmem_undo_range() stops after one sweep on hole punch on
purpose, so it needs the rewind; truncate was meant to sweep until
nothing is left.

Your version fixes the bug too, so I'll leave the choice to you.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov