Re: Subject: [BUG/RFC] write-open file THP cache purge can discard dirty page cache

[Matthew Wilcox]

On Tue, Jun 30, 2026 at 08:55:12PM +0100, Pedro Falcato wrote:
> Gregg, if you could test this patch, it would be much appreciated. This patch
> (hopefully) makes it so no dirty folio will ever coexist with a ro-THP, thus
> hopefully sidestepping the entire issue in a simple way. Only compile-tested
> and not reviewed.

I'd suggest this is slightly misplaced; it's actually called in some
failure places (eg:

                if (!folio_isolate_lru(folio)) {
                        result = SCAN_DEL_PAGE_LRU;
                        goto out_unlock;
...
out_unlock:
                folio_unlock(folio);
                folio_put(folio);
                goto xa_unlocked;
...
xa_unlocked:
        try_to_unmap_flush();


I'd expect to see your new code inside the existing 'if (!is_shmem) {' block
immediately before the xa_locked label.  I'd also follow the
inode_is_open_for_write() case and decrement nr_thps there rather than
introducing a new SCAN code.

> -- 
> Pedro

> >From 43d90a937f0b24656a8d0405035c3efcfdf0961e Mon Sep 17 00:00:00 2001
> From: Pedro Falcato <pfalcato@xxxxxxx>
> Date: Tue, 30 Jun 2026 20:48:41 +0100
> Subject: [PATCH] mm/khugepaged: write all dirty folios when collapsing
> 
> Signed-off-by: Pedro Falcato <pfalcato@xxxxxxx>
> ---
>  mm/khugepaged.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index a97b20617869..3f0f90ab16ba 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -60,6 +60,7 @@ enum scan_result {
>  	SCAN_STORE_FAILED,
>  	SCAN_COPY_MC,
>  	SCAN_PAGE_FILLED,
> +	SCAN_WRITEBACK_FAIL,
>  };
>  
>  #define CREATE_TRACE_POINTS
> @@ -1812,7 +1813,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
>  	pgoff_t index = 0, end = start + HPAGE_PMD_NR;
>  	LIST_HEAD(pagelist);
>  	XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
> -	int nr_none = 0, result = SCAN_SUCCEED;
> +	int nr_none = 0, result = SCAN_SUCCEED, err;
>  	bool is_shmem = shmem_file(file);
>  
>  	VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
> @@ -2043,6 +2044,17 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
>  	 */
>  	try_to_unmap_flush();
>  
> +	/*
> +	 * If collapse looks to be successful, flush any dirty pages
> +	 * out the page cache. With the nr_thps incremented, there won't be
> +	 * any new writers (nor new dirties).
> +	 */
> +	if (result == SCAN_SUCCEED && !is_shmem) {
> +		err = filemap_write_and_wait(mapping);
> +		if (err)
> +			result = SCAN_WRITEBACK_FAIL;
> +	}
> +
>  	if (result == SCAN_SUCCEED && nr_none &&
>  	    !shmem_charge(mapping->host, nr_none))
>  		result = SCAN_FAIL;
> @@ -2210,9 +2222,10 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
>  	/*
>  	 * Undo the updates of filemap_nr_thps_inc for non-SHMEM
>  	 * file only. This undo is not needed unless failure is
> -	 * due to SCAN_COPY_MC.
> +	 * due to SCAN_COPY_MC or SCAN_WRITEBACK_FAIL.
>  	 */
> -	if (!is_shmem && result == SCAN_COPY_MC) {
> +	if (!is_shmem && (result == SCAN_COPY_MC ||
> +	    result == SCAN_WRITEBACK_FAIL)) {
>  		filemap_nr_thps_dec(mapping);
>  		/*
>  		 * Paired with the fence in do_dentry_open() -> get_write_access()
> -- 
> 2.55.0
>