Re: [PATCH v7 2/5] mm: add folio_mark_dropbehind() to mark a folio dropbehind and account it

[Mike Snitzer <snitzer@kernel.org>]

On Wed, Oct 21, 2026 at 06:52:59AM -0400, Jeff Layton wrote:
> 
> Chuck's sashiko had a comment on this one:
> 
> [Severity: Medium]
> Could this overcount WB_DONTCACHE_DIRTY if folio_mark_dropbehind() races with
> filemap_dirty_folio() in mm/page-writeback.c? The dirtying path can set
> PG_dirty before folio_account_dirtied() checks PG_dropbehind, and it need not
> hold the folio lock. If folio_mark_dropbehind() sets PG_dropbehind after that
> dirty transition but before the accounting check, both paths increment the
> counter, while folio_account_cleaned() subtracts it only once. The later NFSD
> use of this helper retains the same interleaving. Could the flag update and
> dirty accounting be serialized so the counter is incremented only once?

Yes, it can double count.  Every path that takes a folio off
WB_DONTCACHE_DIRTY holds the folio lock, as folio_mark_dropbehind()'s
callers do, but folio_account_dirtied() adds under the i_pages lock
after filemap_dirty_folio() has set PG_dirty without it.  So
PG_dirty is the wrong test: it is set before the folio is counted.
Reaching it needs a dirtier that doesn't hold the folio lock, such as a
zap of a dirty pte in a shared file mapping or bio_set_pages_dirty()
after a direct read into one; the cost is a counter the DONTCACHE kick
reads too high, not data.

v8 will test what folio_account_dirtied() leaves behind instead: the
dirty tag, which __folio_mark_dirty() sets in the same i_pages section
that counts the folio.  With the folio locked nothing can clean it, so
the tag is set exactly when the folio has been counted, and taking the
i_pages lock around setting the flag and testing the tag orders it
against that section:

        xa_lock_irqsave(&mapping->i_pages, flags);
        folio_set_dropbehind(folio);
        if (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);

A dirtier past folio_test_set_dirty() but not yet in that section finds
the flag set and counts the folio itself.

The same interleaving undercounts in the clear path that 88d6f128d06d
("mm: track DONTCACHE dirty pages per bdi_writeback") added to
__filemap_get_folio_mpol(): it clears the flag and subtracts if
PG_dirty is set, which a dirtier that has set PG_dirty but not yet
counted the folio makes it do for a folio that was never added.  The
same change, clearing instead of setting, fixes that; I will send it
as its own fix for 88d6f128d06d, at the start of this series for v8.

Thanks,
Mike