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

[Chao Yu]

Thanks, better.

On 9/7/26 17:57, Nanzhe Zhao wrote:
> On Mon, 31 Aug 2026 16:19:05 +0800, Chao Yu wrote:
> > 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
> > below:
> > Thanks,
>
> Thanks for the review. I'll restructure the else branch as suggested:
>
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -56,25 +56,66 @@
>   			f2fs_put_dnode(&dn);
>   		} else {
>   			struct folio *fsrc, *fdst;
> +			size_t src_off, dst_off;
> +			bool same_folio;
>   
>   			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);
> +
> +			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: in that case the
> +			 * destination folio is fsrc itself and only its block
> +			 * needs reservation.
> +			 */
> +			same_folio = src_inode == dst_inode &&
> +					folio_contains(fsrc, dst + i);
> +			if (same_folio) {
> +				struct dnode_of_data dn;
> +
> +				/* 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;
> +				}
> +				fdst = fsrc;
> +			} 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);
> +				}
>   			}
>   
> +			dst_off = offset_in_folio(fdst,
> +					(loff_t)(dst + i) << PAGE_SHIFT);
>   			f2fs_folio_wait_writeback(fdst, DATA, true, true);
> -
> -			memcpy_folio(fdst, 0, fsrc, 0, PAGE_SIZE);
> +			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);
> +			if (same_folio && 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(fdst);
>   			f2fs_folio_put(fdst, true);
> -			f2fs_folio_put(fsrc, true);
> +			if (!same_folio)
> +				f2fs_folio_put(fsrc, true);
>   
>   			ret = f2fs_truncate_hole(src_inode,
>   						src + i, src + i + 1);
>
> Thanks,
> Nanzhe