Re: [f2fs-dev] [PATCH 08/14] f2fs: optimize small block size large folio read

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

On 8/26/26 16:26, 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. If the contiguous mapping covers the whole
> folio, skip f2fs_folio_state allocation for that folio.
> 
> Signed-off-by: Nanzhe Zhao <zhaonanzhe@xiaomi.com>
> ---
>   fs/f2fs/data.c | 62 ++++++++++++++++++++++++++++++++++++++------------
>   1 file changed, 47 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 0e54b1e25893..48c1bb6c02e3 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -153,6 +153,7 @@ static void f2fs_finish_read_bio(struct bio *bio, bool in_task)
>   		struct folio *folio = fi.folio;
>   		unsigned int nr_pages = fi.length >> PAGE_SHIFT;
>   		bool finished = true;
> +		bool uptodate = bio->bi_status == BLK_STS_OK;
>   
>   		if (!folio_test_large(folio) &&
>   		    f2fs_is_compressed_page(folio)) {
> @@ -163,10 +164,14 @@ static void f2fs_finish_read_bio(struct bio *bio, bool in_task)
>   			continue;
>   		}
>   
> -		if (folio_test_large(folio)) {
> -			struct f2fs_folio_state *ffs = folio->private;
> +		if (f2fs_folio_has_ffs(folio)) {
> +			struct f2fs_folio_state *ffs =
> +				(struct f2fs_folio_state *)folio->private;
>   
>   			spin_lock_irqsave(&ffs->state_lock, flags);
> +			if (bio->bi_status == BLK_STS_OK)
> +				uptodate = __ffs_mark_subrange_uptodate(folio, ffs,
> +						fi.offset, fi.length);
>   			ffs->read_pages_pending -= nr_pages;
>   			finished = !ffs->read_pages_pending;
>   			spin_unlock_irqrestore(&ffs->state_lock, flags);
> @@ -182,7 +187,7 @@ static void f2fs_finish_read_bio(struct bio *bio, bool in_task)
>   			bio->bi_status = BLK_STS_IOERR;
>   
>   		if (finished)
> -			folio_end_read(folio, bio->bi_status == BLK_STS_OK);

folio_end_read(folio, bio->bi_status == BLK_STS_OK && uptodate);
in case f2fs_sanity_check_node_footer() failed.

Thanks,

> +			folio_end_read(folio, uptodate);
>   	}
>   
>   	if (ctx)
> @@ -2887,8 +2892,15 @@ static int f2fs_read_data_large_folio(struct inode *inode,
>   	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;
> +		bool whole_folio_in_bio;
> +		unsigned int i;
> +
> +		len_blks = 1;
> +
>   		/*
>   		 * Map blocks using the previous result first.
>   		 */
> @@ -2917,13 +2929,31 @@ 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;
> +				}
>   			}
> +
> +			/*
> +			 * If an entire folio is added to one bio,
> +			 * folio_end_read() can complete the folio read status
> +			 * without relying on f2fs_folio_state.
> +			 */
> +			whole_folio_in_bio = offset == 0 &&
> +					len_blks == folio_nr_pages(folio);
> +
>   		} 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;
> @@ -2933,14 +2963,14 @@ 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.
>   		 */
> -		if (folio_test_large(folio)) {
> +		if (folio_test_large(folio) && !whole_folio_in_bio) {
>   			ffs = f2fs_ffs_find_or_alloc(folio);
>   
>   			/* set the bitmap to wait */
>   			spin_lock_irq(&ffs->state_lock);
> -			ffs->read_pages_pending++;
> +			ffs->read_pages_pending += len_blks;
>   			spin_unlock_irq(&ffs->state_lock);
>   		}
>   
> @@ -2965,17 +2995,19 @@ 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);
>   
> -		if (!bio_add_folio(bio, folio, F2FS_BLKSIZE,
> +		if (!bio_add_folio(bio, folio, len_blks * F2FS_BLKSIZE,
>   					offset << PAGE_SHIFT))
>   			goto submit_and_realloc;
>   
>   		folio_in_bio = true;
> -		inc_page_count(F2FS_I_SB(inode), F2FS_RD_DATA);
> +		for (i = 0; i < len_blks; i++)
> +			inc_page_count(F2FS_I_SB(inode), F2FS_RD_DATA);
>   		f2fs_update_iostat(F2FS_I_SB(inode), NULL, FS_DATA_READ_IO,
> -				F2FS_BLKSIZE);
> -		last_block_in_bio = block_nr;
> +				len_blks * F2FS_BLKSIZE);
> +		last_block_in_bio = block_nr + len_blks - 1;
>   	}
>   	trace_f2fs_read_folio(folio, DATA);
>   err_out:



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