| Message ID | 20231004165317.1061855-4-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add folio_end_read | expand |
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() ?
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);
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 351c3b7f93a1..5bb2f5f802bc 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -1129,6 +1129,7 @@ static inline void wait_on_page_locked(struct page *page) folio_wait_locked(page_folio(page)); } +void folio_end_read(struct folio *folio, bool success); void wait_on_page_writeback(struct page *page); void folio_wait_writeback(struct folio *folio); int folio_wait_writeback_killable(struct folio *folio); diff --git a/mm/filemap.c b/mm/filemap.c index f0a15ce1bd1b..ea317cdf9532 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1527,6 +1527,28 @@ void folio_unlock(struct folio *folio) } EXPORT_SYMBOL(folio_unlock); +/** + * folio_end_read - End read on a folio. + * @folio: The folio. + * @success: True if all reads completed successfully. + * + * When all reads against a folio have completed, filesystems should + * call this function to let the pagecache know that no more reads + * are outstanding. This will unlock the folio and wake up any thread + * sleeping on the lock. The folio will also be marked uptodate if all + * reads succeeded. + * + * Context: May be called from interrupt or process context. May not be + * called from NMI context. + */ +void folio_end_read(struct folio *folio, bool success) +{ + if (likely(success)) + folio_mark_uptodate(folio); + folio_unlock(folio); +} +EXPORT_SYMBOL(folio_end_read); + /** * folio_end_private_2 - Clear PG_private_2 and wake any waiters. * @folio: The folio.
Provide a function for filesystems to call when they have finished reading an entire folio. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- include/linux/pagemap.h | 1 + mm/filemap.c | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+)