Re: [PATCH v4] khugepaged: fix readahead lock inversion in collapse_file()

[Lance Yang]

On 2026/10/27 21:58, David Hildenbrand (Arm) wrote:
> On 10/27/26 14:05, Pedro Falcato wrote:
> > On Tue, Oct 27, 2026 at 12:06:05PM +0100, David Hildenbrand (Arm) wrote:
> > > On 10/26/26 16:20, Lance Yang wrote:
> > > > collapse_file() can call page_cache_sync_readahead() while still holding
> > > > source folios locked. If truncate holds mapping->invalidate_lock
> > > > exclusively and waits for one of those folios, readahead waits for
> > > > invalidate_lock and neither side can make progress.
> > > >
> > > > We need to release those folio locks before starting readahead. On a page
> > > > cache miss, roll back first, populate the collapse range, and retry once.
> > > > Doing readahead after rollback avoids an extra pass when all folios are
> > > > already present. If another miss occurs on retry, give up rather than keep
> > > > retrying while truncate or reclaim removes folios.
> > >
> > > Makes sense. So we simply try readahead on all folios.
> > >
> > > Any reason we don't simply add a second loop after the first loop where we don't
> > > take the folio locks and do the swapin/readahead dance, then a second loop where
> > > we do all the other stuff?
> >
> > I'm not sure I understand. You want to nest two loops? Or do you want a loop
> > upfront? I don't think we want a loop upfont. That's slow and involves
> > starting/waiting for IO. Fallback logic makes more sense IMO.
>
> I'd have an earlier loop where we focus on filling holes, and grabbing folios
> then in the second loop only from the pagecache + trylocking them.
>
> That would make the second easier to the eye.
>
> collapse_scan() file could just indicate if there are any holes and whether
> looping makes sense at all. It already does a scan.
>
> So you could just skip the loop when there is no reason to readahead at all.
>
> This function is pretty horrible and the retry logic doesn't quite improve the
> situation.
>
> But no strong opinion for this fix as long as we clean it up properly later.
>
> (looking forward to reworks)

IIUC, we're good with the fix, so let's keep v4 as is and leave the cleanup
for the rework :)

Cheers, Lance