On Wed, Oct 16, 2024 at 12:57:44PM +0100, Matthew Wilcox wrote: > On Wed, Oct 16, 2024 at 03:35:27PM +0530, Pankaj Raghav (Samsung) wrote: > > > > - 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. > > > > Got it. I saw this while running generic/476 on XFS with 64k block size. > > > > Let's assume the following values: > > index = 128 > > nr_to_read = 16 > > lookahead_size = 28 > > mapping_min_order = 4 (16 pages) > > > > The lookahead_size is actually lying outside the current readahead > > window. The calculation without this patch will result in incorrect mark > > as follows: > > > > ra_folio_index = round_up(128 + 16 - 28, 16) = 128; > > mark = 128 - 128 = 0; > > > > So we will be marking the folio on 0th index with RA flag, even though Oops. I shouldn't have said 0th index. I meant at offset 0 from the index. > > we shouldn't have. Does that make sense? > > But we don't go back and find the folio for index 0. We only consider > the folios we're actually reading for marking. So if 'mark' lies > outside the readahead window, we simply won't mark any of them. So I `mark` is the offset from index. So we compare `mark` with the iterator `i`, which starts at 0. So we will set the RA flag on index 128 in this example, which is not correct. if (i == mark) folio_set_readahead(folio); With this patch, mark will be ULONG_MAX and not 0. > don't think your patch changes anything. Or did I miss something? Does that make sense?
