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

[Lance Yang <lance.yang@linux.dev>]

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