Re: [PATCH] mm/khugepaged: don't map folios that are not uptodate in MADV_COLLAPSE

[David Hildenbrand (Arm)]

On 11/6/26 13:09, Kiryl Shutsemau wrote:
> On Thu, Nov 05, 2026 at 07:16:44PM +0100, David Hildenbrand (Arm) wrote:
>> madvise(MADV_COLLAPSE) can map PMD-sized pagecache folios that are not
>> uptodate. For example, shmem fallocate() can put such folios into the
>> page cache.
>>
>> collapse_file() already refuses to proceed with folios that are not
>> uptodate. Let's just do the same in try_collapse_pte_mapped_thp() and skip
>> such folios, leaving handling to the page faults.
>>
>> Note that collapse_file() effectively clears shmem folios through
>> shmem_get_folio(); let's keep this fix simple and simply let the page
>> fault path deal with that.
>>
>> Fixes: 34488399fa08 ("mm/madvise: add file and shmem support to MADV_COLLAPSE")
>> Reported-by: Chris Mason <mason@xxxxxxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> 
> Reviewed-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> 
> Separately from this fix: nothing on the mapping side catches this.
> Every caller checks uptodate on its own (next_uptodate_folio(),
> filemap_fault(), shmem_get_folio()), and khugepaged was the one that
> went straight from filemap_lock_folio() to do_set_pmd().
> 
> Should do_set_pmd() and set_pte_range() assert it? Something like
> 
>         VM_WARN_ON_ONCE_FOLIO(folio->mapping && !folio_test_anon(folio) &&
>                               !folio_test_uptodate(folio), folio);

I think so. But looking at these hard-to-trigger but in the end very nasty
problems, should we actually reject the operation and do a pr_warn_once() or
something?

-- 
Cheers,

David