Re: [PATCH v2] mm/readahead: align the async window in page_cache_ra_order()

[Jan Kara]

 On Tue 03-11-26 13:19:26, Daejun Park via B4 Relay wrote:
> From: Daejun Park <daejun7.park@xxxxxxxxxxx>
> 
> page_cache_async_ra() rounds each window end down to a multiple of
> 1 << ra->order pages before page_cache_ra_order() clamps the folio
> order. A window starting just below a boundary shrinks to a few pages.
> With read_ahead_kb=128 and max_sectors_kb=512, 1 MiB reads after a
> 4 KiB read at offset 0 hit this on one window in three. On a 512 MiB
> XFS file over QEMU NVMe (cold cache), that takes 0.251 s, 3329 device
> reads, and 9721 folios (none of order 7).
> 
> Pass a flag in struct readahead_control and align the window end in
> page_cache_ra_order(), stepping the order down to keep at least half of
> the window. The read then takes 0.200 s, 1031 device reads, and 1039
> folios (1021 of order 7).
> 
> Fixes: 18ebe55a9236 ("mm/readahead: terminate async readahead on natural boundary")
> Suggested-by: Jan Kara <jack@xxxxxxx>
> Link: https://lore.kernel.org/r/747jhqta3plrrx3bfikceoltsewshdpejdei2qjpn3onnvtu3p@cptmb4phbn54
> Signed-off-by: Daejun Park <daejun7.park@xxxxxxxxxxx>
...

> diff --git a/mm/readahead.c b/mm/readahead.c
> index 6e55632902..959c9a7b48 100644
> --- a/mm/readahead.c
> +++ b/mm/readahead.c
> @@ -498,6 +498,35 @@ void page_cache_ra_order(struct readahead_control *ractl,
>  		goto fallback;
>  	}
>  
> +	new_order = min(mapping_max_folio_order(mapping), new_order);
> +	new_order = min_t(unsigned int, new_order, ilog2(ra->size));
> +	new_order = max(new_order, min_order);
> +
> +	/*
> +	 * The caller allows the window to shrink so that it ends on a folio
> +	 * boundary, rather than ending in small folios. Keep at least half of
> +	 * the window: one that starts just below a boundary would otherwise
> +	 * shrink to a few pages. Use a smaller order instead.
> +	 */
> +	if (ractl->_align_end) {
> +		pgoff_t end = start + ra->size;
> +
> +		while (new_order > min_order) {
> +			pgoff_t aligned_end = round_down(end, 1UL << new_order);
> +
> +			if (aligned_end <= start)
> +				break;
> +			if (2 * (aligned_end - start) >= ra->size) {
> +				ra->size = aligned_end - start;
> +				ra->async_size = ra->size;
> +				new_order = min_t(unsigned int, new_order,
> +						  ilog2(ra->size));
> +				break;
> +			}
> +			new_order--;
> +		}
> +	}

Um, this isn't quite what I meant because this is still quite ad hoc.
Rather something like attached change (plus the _align_end bits from this
patch). It will also end up guaranteeing at least half of the original
window is read but it is also implicitly taking into account alignment of
the starting offset and other possible factors.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR