Re: [PATCH v2 1/8] fs/buffer: simplify block_read_full_folio() with bh_offset()

[Hannes Reinecke <hare@suse.de>]

On 2/5/25 00:12, Luis Chamberlain wrote:
> When we read over all buffers in a folio we currently use the
> buffer index on the folio and blocksize to get the offset. Simplify
> this with bh_offset(). This simplifies the loop while making no
> functional changes.
> 
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>   fs/buffer.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index cc8452f60251..b99560e8a142 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2381,7 +2381,6 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block)
>   	lblock = div_u64(limit + blocksize - 1, blocksize);
>   	bh = head;
>   	nr = 0;
> -	i = 0;
>   
>   	do {
>   		if (buffer_uptodate(bh))
> @@ -2398,7 +2397,7 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block)
>   					page_error = true;
>   			}
>   			if (!buffer_mapped(bh)) {
> -				folio_zero_range(folio, i * blocksize,
> +				folio_zero_range(folio, bh_offset(bh),
>   						blocksize);
>   				if (!err)
>   					set_buffer_uptodate(bh);
> @@ -2412,7 +2411,7 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block)
>   				continue;
>   		}
>   		arr[nr++] = bh;
> -	} while (i++, iblock++, (bh = bh->b_this_page) != head);
> +	} while (iblock++, (bh = bh->b_this_page) != head);
>   
>   	if (fully_mapped)
>   		folio_set_mappedtodisk(folio);

One wonders: shouldn't we use plugging here to make I/O more efficient?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich