Re: [PATCH] nilfs2: clear folio dirty flag when copying back from the shadow map

[Viacheslav Dubeyko]

On Tue, 2026-09-01 at 21:44 +0800, Jiaming Zhang wrote:
> While garbage collection runs, nilfs2 keeps a shadow copy of the DAT
> metadata file's page cache so that it can roll the file back if GC
> fails.  The rollback has two steps: nilfs_clear_dirty_pages() drops
> the
> dirty state of the folios in the DAT cache, then
> nilfs_copy_back_pages()
> overwrites them with the saved contents.  The second step warns if it
> still finds a dirty folio, because the first step is supposed to have
> cleared every one of them:
> 
> 	/* overwrite existing folio in the destination cache */
> 	WARN_ON(folio_test_dirty(dfolio));
> 
> Clearing has been best-effort since commit ca76bb226bf4 ("nilfs2: do
> not
> force clear folio if buffer is referenced"):
> nilfs_clear_folio_dirty()
> leaves a folio dirty if a buffer head under it is still busy. 
> Reading
> metadata creates such buffers.  nilfs_mdt_read_block() submits
> read-ahead for the blocks following the one it was asked for and
> waits
> only for that one, so the read-ahead buffers are still locked when it
> returns.  When the block size is smaller than the page size, several
> metadata blocks share a folio, so a single folio can hold both a
> dirty
> block and a locked read-ahead buffer.  Such a folio survives the
> clearing step, and the copy-back warns on it.
> 
> Use __nilfs_clear_folio_dirty() to clear the dirty flag of the
> destination folio before overwriting it, rather than making
> nilfs_clear_folio_dirty() force-clear busy buffer heads again.
> 
> Fixes: ca76bb226bf4 ("nilfs2: do not force clear folio if buffer is
> referenced")
> Closes:
> https://lore.kernel.org/lkml/CANypQFZSYrtcshnUzOPiqatyLd-M8_OReOewQoAi_V5yY0dTtg@xxxxxxxxxxxxxx/
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Claude Code:claude-opus-5
> Signed-off-by: Jiaming Zhang <r772577952@xxxxxxxxx>
> ---
>  fs/nilfs2/page.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nilfs2/page.c b/fs/nilfs2/page.c
> index cf4f1c6798f5..b26d9c3bda6d 100644
> --- a/fs/nilfs2/page.c
> +++ b/fs/nilfs2/page.c
> @@ -328,7 +328,8 @@ void nilfs_copy_back_pages(struct address_space
> *dmap,
>  		dfolio = filemap_lock_folio(dmap, index);
>  		if (!IS_ERR(dfolio)) {
>  			/* overwrite existing folio in the
> destination cache */
> -			WARN_ON(folio_test_dirty(dfolio));
> +			if (unlikely(folio_test_dirty(dfolio)))
> +				__nilfs_clear_folio_dirty(dfolio);

Maybe, I am missing something. But the main point of this WARN() if we
have found that folio is dirty, then potentially we are trying to re-
write already modified data by already obsolete state. How can we
distinguish that folio has been survived the clearing step or it can be
somehow modified in the background? Even if it is survived the
clearing, then it sounds that something is wrong in another logic. What
do you think?

Thanks,
Slava.

>  			nilfs_copy_folio(dfolio, folio, false);
>  			folio_unlock(dfolio);
>  			folio_put(dfolio);