Re: [PATCH v3 10/17] f2fs: optimize small block size large folio read

[Chao Yu <chao@kernel.org>]

On 10/10/26 00:12, Nanzhe Zhao wrote:
> The original f2fs_read_data_large_folio() implementation has limited
> benefit with a 4KB block size, mainly because updating
> read_pages_pending greatly increases the number of spinlock
> operations.
> 
> Use len_blks to batch read_pages_pending and iostat updates for
> contiguous mapped blocks.
> 
> Signed-off-by: Nanzhe Zhao <zhaonanzhe@xiaomi.com>
> ---
>   fs/f2fs/data.c | 48 ++++++++++++++++++++++++++++++++----------------
>   1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 171b6a38e5cc..6f6f5223951d 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -158,7 +158,7 @@ static void f2fs_finish_read_bio(struct bio *bio, bool in_task)
>   			continue;
>   		}
>   
> -		if (folio_test_large(folio)) {
> +		if (f2fs_folio_has_ffs(folio)) {
>   			struct f2fs_folio_state *ffs =
>   				(struct f2fs_folio_state *)folio->private;
>   
> @@ -3081,7 +3081,7 @@ static int f2fs_read_data_large_folio(struct inode *inode,
>   	pgoff_t index, offset, next_pgofs = 0;
>   	unsigned max_nr_pages = rac ? readahead_count(rac) :
>   				folio_nr_pages(folio);
> -	unsigned nrpages;
> +	unsigned nrpages, len_blks;
>   	struct f2fs_folio_state *ffs;
>   	int ret = 0;
>   	bool folio_in_bio = false;
> @@ -3105,11 +3105,16 @@ static int f2fs_read_data_large_folio(struct inode *inode,
>   	folio_in_bio = false;
>   	index = folio->index;
>   	offset = 0;
> -	ffs = NULL;
>   	nrpages = folio_nr_pages(folio);
>   
> -	for (; nrpages; nrpages--, max_nr_pages--, index++, offset++) {
> +	for (; nrpages;
> +	     nrpages -= len_blks, max_nr_pages -= len_blks,
> +	     index += len_blks, offset += len_blks) {
>   		sector_t block_nr;
> +		unsigned int i;
> +
> +		len_blks = 1;
> +
>   		/*
>   		 * Map blocks using the previous result first.
>   		 */
> @@ -3138,13 +3143,23 @@ static int f2fs_read_data_large_folio(struct inode *inode,
>   got_it:
>   		if ((map.m_flags & F2FS_MAP_MAPPED)) {
>   			block_nr = map.m_pblk + index - map.m_lblk;
> -			if (!f2fs_is_valid_blkaddr(F2FS_I_SB(inode), block_nr,
> +
> +			len_blks = min_t(unsigned int, nrpages, max_nr_pages);
> +			len_blks = min_t(unsigned int, len_blks,
> +					(unsigned int)(map.m_lblk + map.m_len - index));
> +
> +			for (i = 0; i < len_blks; i++) {
> +				if (!f2fs_is_valid_blkaddr(F2FS_I_SB(inode),
> +						block_nr + i,
>   						DATA_GENERIC_ENHANCE_READ)) {
> -				ret = -EFSCORRUPTED;
> -				goto err_out;
> +					ret = -EFSCORRUPTED;
> +					goto err_out;
> +				}
>   			}
> +
>   		} else {
>   			size_t page_offset = offset << PAGE_SHIFT;
> +
>   			folio_zero_range(folio, page_offset, PAGE_SIZE);
>   			if (vi && !fsverity_verify_blocks(vi, folio, PAGE_SIZE, page_offset)) {
>   				ret = -EIO;
> @@ -3154,7 +3169,7 @@ static int f2fs_read_data_large_folio(struct inode *inode,
>   		}
>   
>   		/* We must increment read_pages_pending before possible BIOs submitting
> -		 * to prevent from premature folio_end_read() call on folio
> +		 * to prevent from premature folio_end_read() call on folio.

It's not worth to change this line only by adding a "."

>   		 */
>   		if (folio_test_large(folio)) {
>   			ffs = f2fs_ffs_find_or_alloc(folio);
> @@ -3164,9 +3179,7 @@ static int f2fs_read_data_large_folio(struct inode *inode,
>   			}
>   
>   			/* set the bitmap to wait */
> -			spin_lock_irq(&ffs->state_lock);
> -			ffs->read_pages_pending++;
> -			spin_unlock_irq(&ffs->state_lock);
> +			f2fs_update_read_folio_pending(folio, len_blks);
>   		}
>   
>   		/*
> @@ -3190,17 +3203,20 @@ static int f2fs_read_data_large_folio(struct inode *inode,
>   		 * If the page is under writeback, we need to wait for
>   		 * its completion to see the correct decrypted data.
>   		 */
> -		f2fs_wait_on_block_writeback(inode, block_nr);
> +		for (i = 0; i < len_blks; i++)
> +			f2fs_wait_on_block_writeback(inode, block_nr + i);

f2fs_wait_on_block_writeback_range()

>   
> -		if (!bio_add_folio(bio, folio, F2FS_BLKSIZE(F2FS_I_SB(inode)),
> +		if (!bio_add_folio(bio, folio,
> +				len_blks * F2FS_BLKSIZE(F2FS_I_SB(inode)),
>   				offset << PAGE_SHIFT))
>   			goto submit_and_realloc;
>   
>   		folio_in_bio = true;
> -		inc_cache_count(F2FS_I_SB(inode), F2FS_RD_DATA);
> +		for (i = 0; i < len_blks; i++)
> +			inc_cache_count(F2FS_I_SB(inode), F2FS_RD_DATA);
>   		f2fs_update_iostat(F2FS_I_SB(inode), NULL, FS_DATA_READ_IO,
> -				F2FS_BLKSIZE(F2FS_I_SB(inode)));
> -		last_block_in_bio = block_nr;
> +				len_blks * F2FS_BLKSIZE(F2FS_I_SB(inode)));
> +		last_block_in_bio = block_nr + len_blks - 1;
>   	}
>   	trace_f2fs_read_folio(folio, DATA);
>   err_out: