Re: [PATCH v2 03/17] mm: Add folio_end_read()

[Matthew Wilcox <willy@infradead.org>]

On Sat, Feb 24, 2024 at 12:26:41AM +0900, Tetsuo Handa wrote:
> During a bisection of a different problem, I noticed that
> commit 0b237047d5a7 ("mm: add folio_end_read()") added folio_end_read() as
> 
> ----------------------------------------
> +void folio_end_read(struct folio *folio, bool success)
> +{
> +       if (likely(success))
> +               folio_mark_uptodate(folio);
> +       folio_unlock(folio);
> +}
> ----------------------------------------
> 
> but commit f8174a118122 ("ext4: use folio_end_read()") for unknown reason
> removed folio_clear_uptodate() call when bio->bi_status != 0.
> 
> ----------------------------------------
> -       bio_for_each_folio_all(fi, bio) {
> -               struct folio *folio = fi.folio;
> -
> -               if (bio->bi_status)
> -                       folio_clear_uptodate(folio);
> -               else
> -                       folio_mark_uptodate(folio);
> -               folio_unlock(folio);
> -       }
> +       bio_for_each_folio_all(fi, bio)
> +               folio_end_read(fi.folio, bio->bi_status == 0);
> ----------------------------------------
> 
> Why
> 
> 	else
> 		folio_clear_uptodate(folio);
> 
> part is missing in folio_end_read() ?

Because the folio already has its uptodate flag clear.  We don't re-read
folios which have the uptodate flag set.  This was always dead code.
Now we assert it's true in folio_end_read():

        VM_BUG_ON_FOLIO(folio_test_uptodate(folio), folio);