On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
> text sections), the pages may still be dirty from recent writes. The
> current code triggers an async flush via filemap_flush() and returns
> SCAN_FAIL, requiring userspace to retry the operation.
>
> This is problematic for userspace that wants to collapse text pages into
> THPs to reduce ITLB pressure. The first madvise() call always fails with
> EINVAL, and only subsequent calls succeed after writeback completes.
>
> For direct MADV_COLLAPSE calls (!cc->is_khugepaged), perform a synchronous
> writeback using filemap_write_and_wait_range() before scanning the folios.
> This ensures that folios are clean on the first attempt.
>
> Reported-by: Branden Moore <[email protected]>
> Closes:
> https://lore.kernel.org/all/[email protected]
> Fixes: 34488399fa08 ("mm/madvise: add file and shmem support to
> MADV_COLLAPSE")
> Suggested-by: David Hildenbrand <[email protected]>
> Signed-off-by: Shivank Garg <[email protected]>
> ---
> Applies cleanly on:
> 6.18-rc5
> mm-stable:e9a6fb0bc
Please base on mm-unstable. mm-stable is usually out of date until very close to
merge window.
>
>
> mm/khugepaged.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index abe54f0043c7..d08ed6eb9ce1 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -21,6 +21,7 @@
> #include <linux/shmem_fs.h>
> #include <linux/dax.h>
> #include <linux/ksm.h>
> +#include <linux/backing-dev.h>
>
> #include <asm/tlb.h>
> #include <asm/pgalloc.h>
> @@ -1845,6 +1846,7 @@ static int collapse_file(struct mm_struct *mm, unsigned
> long addr,
> struct page *dst;
> struct folio *folio, *tmp, *new_folio;
> pgoff_t index = 0, end = start + HPAGE_PMD_NR;
> + loff_t range_start, range_end;
> LIST_HEAD(pagelist);
> XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
> int nr_none = 0, result = SCAN_SUCCEED;
> @@ -1853,6 +1855,21 @@ static int collapse_file(struct mm_struct *mm,
> unsigned long addr,
> VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
> VM_BUG_ON(start & (HPAGE_PMD_NR - 1));
>
> + /*
> + * For MADV_COLLAPSE on regular files, do a synchronous writeback
> + * to ensure dirty folios are flushed before we attempt collapse.
> + * This is a best-effort approach to avoid failing on the first
> + * attempt when freshly-written executable text is still dirty.
> + */
> + if (!is_shmem && cc && !cc->is_khugepaged &&
> mapping_can_writeback(mapping)) {
> + range_start = (loff_t)start << PAGE_SHIFT;
> + range_end = ((loff_t)end << PAGE_SHIFT) - 1;
> + if (filemap_write_and_wait_range(mapping, range_start,
> range_end)) {
> + result = SCAN_FAIL;
> + goto out;
> + }
> + }
I feel this is the wrong level of abstraction.
We explicitly invoke this oth from khugepaged and madvise(..., MADV_COLLAPSE):
khugepaged_scan_mm_slot() / madvise_collapse()
-> hpage_collapse_scan_file()
-> collapse_file()
ofc you are addressing this with the !cc->is_khugepaged, but feels like we'd be
better off just doing it in madvise_collapse().
I wonder also if doing I/O without getting the mmap lock again and revalidating
is wise, as the state of things might have changed significantly.
So maybe need a hugepage_vma_revalidate() as well?
> +
> result = alloc_charge_folio(&new_folio, mm, cc);
> if (result != SCAN_SUCCEED)
> goto out;
>
> base-commit: e9a6fb0bcdd7609be6969112f3fbfcce3b1d4a7c
> --
> 2.43.0
>
Thanks, Lorenzo
