Re:Re: [PATCH v7 08/15] f2fs: make GC migration large-folio aware

["Nanzhe Zhao" <nzzhao@126.com>]

At 2026-10-27 17:37:22, "Chao Yu" <chao@kernel.org> wrote:
>On 10/26/26 11:37, Nanzhe Zhao wrote:
>> From: Nanzhe Zhao <zhaonanzhe@xiaomi.com>
>> 
>> GC can operate on a 4K block that is cached inside a large folio.
>> The data lookup helpers therefore need to test and update uptodate
>> state for the addressed subpage instead of rejecting large folios or
>> treating the whole folio as the target block.
>> 
>> Let f2fs_get_read_data_folio() and f2fs_get_lock_data_folio() to use
>> subpage uptodate state. Submit single-block reads at the requested
>> folio offset and zero only the addressed 4K range for NEW_ADDR.
>> 
>> Also update `move_data_page` to mark, clear, and restore dirty
>> state for the target subpage, and submit write I/O with the subpage
>> offset recorded in f2fs_io_info.
>> 
>> The write_pages_pending reference is taken by f2fs_do_write_data_page()
>> and dropped from its common error path, so a block that is truncated
>> concurrently no longer leaks the counter.
>> 
>> Reported-by: kernel test robot <yi1.lai@intel.com>
>> Closes: https://lore.kernel.org/oe-lkp/202610081027.b13dcf10-lkp@intel.com
>> Signed-off-by: Nanzhe Zhao <zhaonanzhe@xiaomi.com>
>> ---
>>  fs/f2fs/data.c    | 70 +++++++++++++++++++++++++++++++++--------------
>>  fs/f2fs/f2fs.h    |  1 +
>>  fs/f2fs/gc.c      | 28 +++++++++++++++----
>>  fs/f2fs/segment.c |  2 +-
>>  4 files changed, 75 insertions(+), 26 deletions(-)
>> 
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index d027c949823b..b3ada22cb0cb 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1679,20 +1679,16 @@ struct folio *f2fs_get_read_data_folio(struct inode *inode, pgoff_t index,
>>  	struct dnode_of_data dn;
>>  	struct folio *folio;
>>  	int err;
>> -retry:
>> +
>>  	folio = f2fs_grab_cache_folio(mapping, index, for_write);
>>  	if (IS_ERR(folio))
>>  		return folio;
>>  
>>  	if (folio_test_large(folio)) {
>> -		pgoff_t folio_index = mapping_align_index(mapping, index);
>> -		unsigned long nr_pages = folio_nr_pages(folio);
>> -
>> -		f2fs_folio_put(folio, true);
>> -		invalidate_inode_pages2_range(mapping, folio_index,
>> -				folio_index + nr_pages - 1);
>> -		f2fs_schedule_timeout(DEFAULT_SCHEDULE_TIMEOUT);
>> -		goto retry;
>> +		if (!f2fs_ffs_find_or_alloc(folio)) {
>> +			err = -ENOMEM;
>> +			goto put_err;
>> +		}
>>  	}
>>  
>>  	if (f2fs_lookup_read_extent_cache_block(inode, index,
>> @@ -1728,7 +1724,7 @@ struct folio *f2fs_get_read_data_folio(struct inode *inode, pgoff_t index,
>>  		goto put_err;
>>  	}
>>  got_it:
>> -	if (folio_test_uptodate(folio)) {
>> +	if (f2fs_ffs_test_blk_uptodate(folio, index)) {
>>  		folio_unlock(folio);
>>  		return folio;
>>  	}
>> @@ -1741,9 +1737,11 @@ struct folio *f2fs_get_read_data_folio(struct inode *inode, pgoff_t index,
>>  	 * f2fs_init_inode_metadata.
>>  	 */
>>  	if (dn.data_blkaddr == NEW_ADDR) {
>> -		folio_zero_segment(folio, 0, folio_size(folio));
>> -		if (!folio_test_uptodate(folio))
>> -			folio_mark_uptodate(folio);
>> +		size_t offset = offset_in_folio(folio,
>> +						(loff_t)index << PAGE_SHIFT);
>> +
>> +		folio_zero_segment(folio, offset, offset + PAGE_SIZE);
>> +		f2fs_ffs_mark_subrange_uptodate(folio, offset, PAGE_SIZE);
>>  		folio_unlock(folio);
>>  		return folio;
>>  	}
>> @@ -1803,7 +1801,6 @@ struct folio *f2fs_get_lock_data_folio(struct inode *inode, pgoff_t index,
>>  
>>  	/* wait for read completion */
>>  	folio_lock(folio);
>> -
>>  	if (unlikely(folio->mapping != mapping)) {
>>  		f2fs_folio_put(folio, true);
>>  		return ERR_PTR(-EIO);
>> @@ -1812,7 +1809,7 @@ struct folio *f2fs_get_lock_data_folio(struct inode *inode, pgoff_t index,
>>  	/*
>>  	 * ffs can be released while the folio was unlocked above.  Rebuild it
>>  	 * for a fully uptodate folio; a partially uptodate one cannot be
>> -	 * rebuilt and will fail the uptodate check below.
>> +	 * rebuilt and will fail the subpage check below.
>>  	 */
>>  	if (folio_test_large(folio) && !f2fs_folio_has_ffs(folio)) {
>>  		if (!f2fs_ffs_find_or_alloc(folio)) {
>> @@ -1821,7 +1818,7 @@ struct folio *f2fs_get_lock_data_folio(struct inode *inode, pgoff_t index,
>>  		}
>>  	}
>>  
>> -	if (unlikely(!folio_test_uptodate(folio))) {
>> +	if (unlikely(!f2fs_ffs_test_blk_uptodate(folio, index))) {
>>  		f2fs_folio_put(folio, true);
>>  		return ERR_PTR(-EIO);
>>  	}
>> @@ -2970,6 +2967,22 @@ static void f2fs_ffs_mark_subrange_uptodate(struct folio *folio, size_t offset,
>>  		folio_mark_uptodate(folio);
>>  }
>>  
>> +bool f2fs_ffs_test_blk_dirty(const struct folio *folio, pgoff_t index)
>> +{
>> +	struct f2fs_folio_state *ffs;
>> +	unsigned int idx, nr_subpages;
>> +
>> +	f2fs_bug_on(F2FS_F_SB(folio), !folio_contains(folio, index));
>> +
>> +	if (!f2fs_folio_has_ffs(folio))
>> +		return folio_test_dirty(folio);
>> +
>> +	ffs = (struct f2fs_folio_state *)folio->private;
>> +	nr_subpages = folio_nr_pages(folio);
>> +	idx = index - folio->index;
>> +	return test_bit(nr_subpages + idx, ffs->state);
>> +}
>> +
>>  static unsigned int __ffs_mark_subrange_dirty(struct folio *folio,
>>  			struct f2fs_folio_state *ffs, size_t offset, size_t len)
>>  {
>> @@ -3063,7 +3076,6 @@ bool f2fs_ffs_clear_subrange_dirty_and_test(struct folio *folio, size_t offset,
>>  
>>  	return dirty;
>>  }
>> -
>>  static unsigned int ffs_next_dirty_subpage(struct f2fs_folio_state *ffs,
>>  			const struct folio *folio, unsigned int start,
>>  			unsigned int end)
>> @@ -3561,7 +3573,7 @@ bool f2fs_should_update_outplace(struct inode *inode, struct f2fs_io_info *fio)
>>  		return true;
>>  
>>  	if (fio) {
>> -		if (page_private_gcing(fio->page))
>> +		if (folio_test_f2fs_gcing(fio->folio))
>>  			return true;
>>  		if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED) &&
>>  			f2fs_is_checkpointed_data(sbi, fio->old_blkaddr)))
>> @@ -3587,8 +3599,11 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>>  	struct dnode_of_data dn;
>>  	struct node_info ni;
>>  	struct f2fs_lock_context lc;
>> +	struct f2fs_folio_state *ffs;
>>  	bool ipu_force = false;
>>  	bool atomic_commit;
>> +	bool pending_added = false;
>> +	bool submitted = false;
>>  	int err = 0;
>>  
>>  	/* Use COW inode to make dnode_of_data for atomic write */
>> @@ -3617,8 +3632,11 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>>  		return -EINVAL;
>>  
>>  	/* Deadlock due to between page->lock and f2fs_lock_op */
>> -	if (fio->need_lock == LOCK_REQ && !f2fs_trylock_op(fio->sbi, &lc))
>> +	if (fio->need_lock == LOCK_REQ && !f2fs_trylock_op(fio->sbi, &lc)) {
>> +		if (!f2fs_ffs_has_dirty_subpage(folio))
>> +			folio_clear_f2fs_gcing(folio);
>>  		return -EAGAIN;
>> +	}
>>  
>>  	err = f2fs_get_dnode_of_data(&dn, F2FS_FIO_LBLK(fio),
>>  				     LOOKUP_NODE);
>> @@ -3630,7 +3648,6 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>>  	/* This page is already truncated */
>>  	if (fio->old_blkaddr == NULL_ADDR) {
>>  		folio_clear_uptodate(folio);
>> -		folio_clear_f2fs_gcing(folio);
>>  		goto out_writepage;
>>  	}
>>  got_it:
>> @@ -3680,6 +3697,11 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>>  	fio->version = ni.version;
>>  
>>  	__folio_start_writeback(folio, folio_test_dirty(folio));
>> +	if (f2fs_folio_has_ffs(folio)) {
>> +		ffs = (struct f2fs_folio_state *)folio->private;
>> +		atomic_inc(&ffs->write_pages_pending);
>> +		pending_added = true;
>> +	}
>>  
>>  	if (fio->compr_blocks && fio->old_blkaddr == COMPRESS_ADDR)
>>  		f2fs_i_compr_blocks_update(inode, fio->compr_blocks - 1, false);
>> @@ -3694,6 +3716,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>>  		err = -EIO;
>>  		goto out_writepage;
>>  	}
>> +	submitted = true;
>>  	set_inode_flag(inode, FI_APPEND_WRITE);
>>  	if (atomic_commit)
>>  		folio_clear_f2fs_atomic(folio);
>> @@ -3702,6 +3725,13 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>>  out:
>>  	if (fio->need_lock == LOCK_REQ)
>>  		f2fs_unlock_op(fio->sbi, &lc);
>> +	if (!submitted) {
>> +		if (pending_added) {
>> +			f2fs_ffs_put_write_pending(folio, 1);
>> +		} else if (!f2fs_ffs_has_dirty_subpage(folio)) {
>> +			folio_clear_f2fs_gcing(folio);
>
>If f2fs_trylock_op() failed:
>
>	if (fio->need_lock == LOCK_RETRY) {
>		if (!f2fs_trylock_op(fio->sbi, &lc)) {
>			err = -EAGAIN;
>			goto out_writepage;
>		}
>		fio->need_lock = LOCK_REQ;
>	}
>
>For order-0 folio, we may loss gcing flag becuase f2fs_ffs_has_dirty_subpage()
>return false?
>

But GC path which calls move_data_page will clear f2fs gcing whatever error code
f2fs_do_write_page returns right？ I think we may need to having an parameter,
or some infomation carried in fio to judge whether we call f2fs_do_write_data_page
in gc context?