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

[David Hildenbrand (Arm)]

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.

-- 
Cheers,

David