Re: [f2fs-dev] [PATCH 12/14] f2fs: handle block cloning within the same large folio

[Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>]

On 8/26/26 21:09, Nanzhe Zhao wrote:
> With large folios, the source and destination block in __clone_blkaddrs()
> can belong to the same folio during same-inode operations such as
> collapse range and insert range.
> 
> In that case, locking the source folio and then looking up the
> destination folio can try to lock the same folio again.  It also copies
> from and to offset 0, which is only correct for order-0 folios.
> 
> Detect the same-inode, same-folio case before looking up the destination
> folio.  Then in this case,we reserve the destination block explicitly.
> 
> Signed-off-by: Nanzhe Zhao <zhaonanzhe@xiaomi.com>
> ---
>   fs/f2fs/file.c | 76 ++++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 64 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 4272013dbe38..b82acbc3240f 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1581,25 +1581,77 @@ static int __clone_blkaddrs(struct inode *src_inode, struct inode *dst_inode,
>   			f2fs_put_dnode(&dn);
>   		} else {
>   			struct folio *fsrc, *fdst;
> +			size_t src_off, dst_off;
>   
>   			fsrc = f2fs_get_lock_data_folio(src_inode,
>   							src + i, true);
>   			if (IS_ERR(fsrc))
>   				return PTR_ERR(fsrc);
> -			fdst = f2fs_get_new_data_folio(dst_inode, NULL, dst + i,
> -								true);
> -			if (IS_ERR(fdst)) {
> -				f2fs_folio_put(fsrc, true);
> -				return PTR_ERR(fdst);
> -			}
>   
> -			f2fs_folio_wait_writeback(fdst, DATA, true, true);
> +			src_off = offset_in_folio(fsrc,
> +						(loff_t)(src + i) << PAGE_SHIFT);
> +
> +			/*
> +			 * For same-inode operations (collapse/insert), src and
> +			 * dst may fall within the same large folio. Detect this
> +			 * to avoid self-deadlock on folio lock.
> +			 */
> +			if (src_inode == dst_inode &&
> +			    folio_contains(fsrc, dst + i)) {
> +				struct dnode_of_data dn;

A lot of codes are redundant, can we add fdst = fsrc;

So that we can replace fsrc w/ fdst, and clean up the redundant codes blow:

Thanks,

> +
> +				/* Reserve block for dst before copying */
> +				set_new_dnode(&dn, dst_inode, NULL, NULL, 0);
> +				ret = f2fs_reserve_block(&dn, dst + i);
> +				if (ret) {
> +					f2fs_folio_put(fsrc, true);
> +					return ret;
> +				}
> +
> +				dst_off = offset_in_folio(fsrc,
> +						(loff_t)(dst + i) << PAGE_SHIFT);
> +				f2fs_folio_wait_writeback(fsrc, DATA, true, true);
> +				memcpy_folio(fsrc, dst_off, fsrc, src_off,
> +					    PAGE_SIZE);
> +				if (folio_test_large(fsrc)) {
> +					f2fs_ffs_find_or_alloc(fsrc);
> +					f2fs_ffs_mark_subrange_uptodate(fsrc,
> +								dst_off, PAGE_SIZE);
> +					f2fs_ffs_mark_subrange_dirty(fsrc, dst_off,
> +								    PAGE_SIZE);
> +				}
> +				folio_mark_dirty(fsrc);
> +				if (i_size_read(dst_inode) <
> +				    ((loff_t)(dst + i + 1) << PAGE_SHIFT))
> +					f2fs_i_size_write(dst_inode,
> +						((loff_t)(dst + i + 1) << PAGE_SHIFT));
> +				folio_set_f2fs_gcing(fsrc);
> +				f2fs_folio_put(fsrc, true);
> +			} else {
> +				fdst = f2fs_get_new_data_folio(dst_inode, NULL,
> +							      dst + i, true);
> +				if (IS_ERR(fdst)) {
> +					f2fs_folio_put(fsrc, true);
> +					return PTR_ERR(fdst);
> +				}
>   
> -			memcpy_folio(fdst, 0, fsrc, 0, PAGE_SIZE);
> -			folio_mark_dirty(fdst);
> -			folio_set_f2fs_gcing(fdst);
> -			f2fs_folio_put(fdst, true);
> -			f2fs_folio_put(fsrc, true);
> +				dst_off = offset_in_folio(fdst,
> +						(loff_t)(dst + i) << PAGE_SHIFT);
> +				f2fs_folio_wait_writeback(fdst, DATA, true, true);
> +				memcpy_folio(fdst, dst_off, fsrc, src_off,
> +					    PAGE_SIZE);
> +				if (folio_test_large(fdst)) {
> +					f2fs_ffs_find_or_alloc(fdst);
> +					f2fs_ffs_mark_subrange_uptodate(fdst,
> +								dst_off, PAGE_SIZE);
> +					f2fs_ffs_mark_subrange_dirty(fdst, dst_off,
> +								    PAGE_SIZE);
> +				}
> +				folio_mark_dirty(fdst);
> +				folio_set_f2fs_gcing(fdst);
> +				f2fs_folio_put(fdst, true);
> +				f2fs_folio_put(fsrc, true);
> +			}
>   
>   			ret = f2fs_truncate_hole(src_inode,
>   						src + i, src + i + 1);



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel