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);
