Re: [f2fs-dev] [PATCH] f2fs: release a removed xattr node in roll-forward recovery

[Chao Yu <chao@kernel.org>]

On 10/13/26 10:09, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
> 
> When xattrs shrink enough to fit in the inline xattr area,
> write_all_xattrs() truncates the xattr node and clears i_xattr_nid. If
> the file is then fsynced without a checkpoint, only the inode block is
> written, with i_xattr_nid of 0.
> 
> Roll-forward recovery starts from the checkpointed inode, whose
> i_xattr_nid still points to the old xattr node.
> f2fs_recover_inline_xattr() copies only the inline xattr area, and
> f2fs_update_inode() writes the old i_xattr_nid back. So the inode keeps
> the stale xattr node:
> 
>  - The node block is never released, and fsck reports "nonzero bytes in
>    xattr space after end of list".
>  - If the inline xattr list ends exactly at the end of the inline area,
>    its terminator is read from the stale node, and the removed xattrs
>    come back after recovery.
> 
> When the fsynced inode has no xattr node but the checkpointed one has,
> release the old xattr node in the same way as f2fs_recover_xattr_data().
> 
> Fixes: 28cdce0459cc ("f2fs: recover inline xattr data in roll-forward process")
> Cc: stable@vger.kernel.org
> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> ---
>  fs/f2fs/node.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 55008f18afd..590fdedf9c2 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -2831,6 +2831,29 @@ int f2fs_recover_inline_xattr(struct inode *inode, struct f2fs_cached_block *ent
>  	f2fs_cache_wait_writeback(ientry);
>  	memcpy(dst_addr, src_addr, inline_size);
>  update_inode:
> +	/*
> +	 * The xattr node of the checkpoint was removed before fsync, e.g. the
> +	 * xattrs became small enough to fit inline. Release it as well, or
> +	 * the inode keeps pointing to the stale xattrs.
> +	 */
> +	if (!ri->i_xattr_nid && F2FS_I(inode)->i_xattr_nid) {
> +		struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> +		struct node_info ni;
> +		int err;
> +
> +		err = f2fs_get_node_info(sbi, F2FS_I(inode)->i_xattr_nid,
> +								&ni, false);
> +		if (err) {
> +			f2fs_put_cache(ientry, true);
> +			return err;
> +		}
> +
> +		f2fs_invalidate_blocks(sbi, ni.blk_addr, 1);
> +		dec_valid_node_count(sbi, inode, false);
> +		set_node_addr(sbi, &ni, NULL_ADDR, false);
> +		f2fs_i_xnid_write(inode, 0);

I think we need to clear dirty on xnode as well, refer to truncate_node(), otherwise
f2fs_writeback will write a dirty xnode w/ blkaddr = NULL_ADDR.

Thanks,

> +	}
> +
>  	f2fs_update_inode(inode, ientry);
>  	f2fs_put_cache(ientry, true);
>  	return 0;