Re: [PATCH] mm/readahead: keep at least half of the async window when aligning it

[Jan Kara]

On Fri 30-10-26 12:52:57, Daejun Park via B4 Relay wrote:
> From: Daejun Park <daejun7.park@xxxxxxxxxxx>
> 
> page_cache_async_ra() trims the end of each new window to a multiple of
> 1 << ra->order pages. A window that starts just below such a boundary
> keeps only 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 about one window
> in three. They issue three times as many device reads.
> 
> Halve the alignment until the trim keeps at least half of the window.
> 
> Fixes: 18ebe55a9236 ("mm/readahead: terminate async readahead on natural boundary")
> Signed-off-by: Daejun Park <daejun7.park@xxxxxxxxxxx>
> ---
> Tested on mm-new in a QEMU guest with XFS on an emulated NVMe,
> read_ahead_kb 128 and a cold cache.
> 
> - max_sectors_kb 512, median of 5 runs: reading a 512 MiB file this
>   way takes 0.247 s before and 0.180 s after. The device sees 3329 and
>   1032 reads.
> 
> - Other reads with max_sectors_kb 512: without the 4 KiB read, and with
>   64 KiB reads, the times do not change. Random 4 KiB and 64 KiB reads,
>   two interleaved streams on one file, and 2 MiB reads at random
>   offsets also take the same time. They read the same amount from the
>   device, within 0.3%.
> 
> With the device's transfer limit at 2 MiB:
> 
> - max_sectors_kb 256: 512 KiB reads after the 4 KiB read take 0.626 s
>   before and 0.322 s after. The device sees 6147 and 2056 reads.
> 
> - max_sectors_kb 1024 or 2048: 1 MiB reads are not trimmed.
> 
> - max_sectors_kb 2048, 3 MiB reads: the device sees 343 instead of 271
>   reads. The time is 0.120 s instead of 0.118 s, the median of 40 runs.
>   The interquartile range is about 0.10 to 0.13 s for both. The window
>   stays at 3/4 of the maximum, off the 2 MiB boundary. mm-new reaches
>   that boundary by trimming deeply.

OK, I can see how it easily happens once that we trim the readahead
significantly due to alignment. I'm not fully clear how it happens
periodically. Is it that you do 1 MiB misaligned reads and so we
periodically end up triggering async readahead from positions that are not
aligned to the current value of ra->order?

Anyway, what we want is to align the readahead so that we avoid creating
folios of unnecessarily small order at the end of the readahead window (and
I actually miss evaluation how order of folios changed after your change
because that's also an important metric for this logic). I think the
problem might be that the attempt to align in page_cache_async_ra() is
actually not reflecting what page_cache_ra_order() will end up doing 
(it may reduce ra->order based on readahead size, starting index, etc.) so
the alignment we do in page_cache_async_ra() may be unnecessarily large.
Instead of trying to second-guess the logic of page_cache_ra_order() in
page_cache_async_ra() I think we could pass a flag to it in ractl telling
it it's allowed to shrink the window instead of allocating small folios
and then handle the alignment in page_cache_ra_order(). Hum?

								Honza

> ---
>  mm/readahead.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/mm/readahead.c b/mm/readahead.c
> index 6e55632902..f4da9b4606 100644
> --- a/mm/readahead.c
> +++ b/mm/readahead.c
> @@ -713,6 +713,17 @@ void page_cache_async_ra(struct readahead_control *ractl,
>  	align = 1UL << min(ra->order, ffs(max_pages) - 1);
>  	end = ra->start + ra->size;
>  	aligned_end = round_down(end, align);
> +	/*
> +	 * Keep at least half of the window when aligning its end. A
> +	 * window that starts just below a large boundary would otherwise
> +	 * shrink to a few pages and cap the folio order. Use a smaller
> +	 * alignment instead.
> +	 */
> +	while (aligned_end > ra->start &&
> +	       2 * (aligned_end - ra->start) < ra->size) {
> +		align >>= 1;
> +		aligned_end = round_down(end, align);
> +	}
>  	if (aligned_end > ra->start)
>  		ra->size -= end - aligned_end;
>  	ra->async_size = ra->size;
> 
> ---
> base-commit: 9c449f885765cf8d80b2c60b1971e774bba51a3f
> change-id: 20261029-mm-readahead-async-align-e491b3dc16a6
> 
> Best regards,
> -- 
> Daejun Park <daejun7.park@xxxxxxxxxxx>
> 
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR