A cached lookup in __filemap_get_folio_mpol() that finds a folio marked dropbehind clears the flag, and subtracts the folio from WB_DONTCACHE_DIRTY if PG_dirty is set. But filemap_dirty_folio() sets PG_dirty before folio_account_dirtied() counts the folio, under the i_pages lock and without the folio lock. A lookup between the two subtracts a folio that was never added, and folio_account_dirtied(), finding the flag clear, does not add it either, so the counter the DONTCACHE writeback kick reads falls short and can go negative. A lookup without FGP_LOCK also races with the paths that uncount a dirty folio, which hold the folio lock: folio_clear_dirty_for_io() clears PG_dirty and then tests the flag, so a lookup between the two leaves the folio counted with no one to uncount it. Every path that uncounts a dirty folio holds the folio lock, and folio_account_dirtied() sets the folio's dirty tag in the same i_pages section that counts it. So clear the flag with the folio locked, taking the lock with a trylock for a lookup without FGP_LOCK, and test the dirty tag rather than PG_dirty under the i_pages lock: the folio is uncounted only if it was counted. Under that lock the wb is inode_to_wb(), as in folio_account_dirtied() and inode_do_switch_wbs(), which moves the counter by the same tag. A lookup that finds the folio locked by another task leaves the flag set, as it would have been without the lookup. Fixes: 88d6f128d06d ("mm: track DONTCACHE dirty pages per bdi_writeback") Signed-off-by: Mike Snitzer --- mm/filemap.c | 54 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index 00fd89cf6f55..1cc28224d547 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1927,6 +1927,45 @@ void *filemap_get_entry(struct address_space *mapping, pgoff_t index) return folio; } +/* + * Clear the uncached flag of a folio a cached lookup found, and take the + * folio off WB_DONTCACHE_DIRTY if it is counted there. + * + * folio_account_dirtied() counts a dropbehind folio under the i_pages lock, + * in the section that sets its dirty tag, after PG_dirty has been set + * without the folio lock, and every path that uncounts a dirty folio holds + * the folio lock. So with the folio locked and the i_pages lock held, the + * dirty tag, not PG_dirty, says whether the folio is counted: a dirtier past + * folio_test_set_dirty() but not yet counting finds the flag clear and does + * not count it. A folio another task has locked keeps the flag, as it + * would have had this lookup not run. + */ +static void filemap_clear_dropbehind(struct address_space *mapping, + struct folio *folio, bool locked) +{ + unsigned long flags; + + if (!locked) { + if (!folio_trylock(folio)) + return; + if (unlikely(folio->mapping != mapping)) + goto out; + } + if (!mapping_can_writeback(mapping)) { + folio_clear_dropbehind(folio); + goto out; + } + xa_lock_irqsave(&mapping->i_pages, flags); + if (folio_test_clear_dropbehind(folio) && + xa_get_mark(&mapping->i_pages, folio->index, PAGECACHE_TAG_DIRTY)) + wb_stat_mod(inode_to_wb(mapping->host), WB_DONTCACHE_DIRTY, + -folio_nr_pages(folio)); + xa_unlock_irqrestore(&mapping->i_pages, flags); +out: + if (!locked) + folio_unlock(folio); +} + /** * __filemap_get_folio_mpol - Find and get a reference to a folio. * @mapping: The address_space to search. @@ -2058,19 +2097,8 @@ struct folio *__filemap_get_folio_mpol(struct address_space *mapping, if (!folio) return ERR_PTR(-ENOENT); /* not an uncached lookup, clear uncached if set */ - if (!(fgp_flags & FGP_DONTCACHE) && folio_test_clear_dropbehind(folio)) { - if (folio_test_dirty(folio) && - mapping_can_writeback(mapping)) { - struct inode *inode = mapping->host; - struct bdi_writeback *wb; - struct wb_lock_cookie cookie = {}; - long nr = folio_nr_pages(folio); - - wb = unlocked_inode_to_wb_begin(inode, &cookie); - wb_stat_mod(wb, WB_DONTCACHE_DIRTY, -nr); - unlocked_inode_to_wb_end(inode, &cookie); - } - } + if (!(fgp_flags & FGP_DONTCACHE) && folio_test_dropbehind(folio)) + filemap_clear_dropbehind(mapping, folio, fgp_flags & FGP_LOCK); return folio; } EXPORT_SYMBOL(__filemap_get_folio_mpol); -- 2.52.0