Re: [PATCH v2 4/8] fs/mpage: use blocks_per_folio instead of blocks_per_page

Hannes Reinecke
2025-02-18T15:02:46Z

On 2/17/25 22:58, Matthew Wilcox wrote:
> On Tue, Feb 04, 2025 at 03:12:05PM -0800, Luis Chamberlain wrote:
>> @@ -182,7 +182,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
>> goto confused;
>>
>> block_in_file = folio_pos(folio) >> blkbits;
>> - last_block = block_in_file + args->nr_pages * blocks_per_page;
>> + last_block = block_in_file + args->nr_pages * blocks_per_folio;
>
> In mpage_readahead(), we set args->nr_pages to the nunber of pages (not
> folios) being requested. In mpage_read_folio() we currently set it to
> 1. So this is going to read too far ahead for readahead if using large
> folios.
>
> I think we need to make nr_pages continue to mean nr_pages. Or we pass
> in nr_bytes or nr_blocks.
>
I had been pondering this, too, while developing the patch.
The idea I had here was to change counting by pages over to counting by
folios, as then the logic is essentially unchanged.

Not a big fan of 'nr_pages', as then the question really is how much
data we should read at the end of the day. So I'd rather go with
'nr_blocks' to avoid any confusion.

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