Re: [PATCH v14] mm: don't set readahead flag on a folio when lookahead_size > nr_to_read

[Matthew Wilcox <willy@infradead.org>]

On Tue, Oct 15, 2024 at 06:41:06PM +0200, Pankaj Raghav (Samsung) wrote:

v14?  Where are v1..13 of this patch?  It's the first time I've seen it.

> The readahead flag is set on a folio based on the lookahead_size and
> nr_to_read. For example, when the readahead happens from index to index
> + nr_to_read, then the readahead `mark` offset from index is set at
> nr_to_read - lookahead_size.
> 
> There are some scenarios where the lookahead_size > nr_to_read. If this
> happens, readahead flag is not set on any folio on the current
> readahead window.

Please describe those scenarios, as that's the important bit.

> There are two problems at the moment in the way `mark` is calculated
> when lookahead_size > nr_to_read:
> 
> - unsigned long `mark` will be assigned a negative value which can lead
>   to unexpected results in extreme cases due to wrap around.

Can such an extreme case happen?

> - The current calculation for `mark` with mapping_min_order > 0 gives
>   incorrect results when lookahead_size > nr_to_read due to rounding
>   up operation.
> 
> Explicitly initialize `mark` to be ULONG_MAX and only calculate it
> when lookahead_size is within the readahead window.

You haven't really spelled out the consequences of this properly.
Perhaps a worked example would help.

I think the worst case scenario is that we set the flag on the wrong
folio, causing readahead to occur when it should not.  But maybe I'm
wrong?