From: Christoph Hellwig <hch@lst.de>
To: Mike Snitzer <snitzer@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>,
linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
Chuck Lever <cel@kernel.org>, Jeff Layton <jlayton@kernel.org>,
linux-nfs@vger.kernel.org, NeilBrown <neil@brown.name>,
Olga Kornievskaia <okorniev@redhat.com>,
Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
Jonathan Corbet <corbet@lwn.net>
Subject: Re: [PATCH v9 1/6] mm: take a folio off WB_DONTCACHE_DIRTY only if it was counted there
Date: Tue, 3 Nov 2026 11:46:29 +0100 [thread overview]
Message-ID: <20261103104629.GA7296@lst.de> (raw)
In-Reply-To: <20261024221152.6965-2-snitzer@kernel.org>
Please write a concise commit log. This is a long meandering thought
of eletronic dreams making no sense.
> +/*
> + * 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. Every path that uncounts a dirty folio holds the
> + * folio lock. So with the folio locked and the i_pages lock held, the folio
> + * is counted when both PG_dirty and the dirty tag are set. PG_dirty alone
> + * is not enough: a dirtier past folio_test_set_dirty() but not yet counting
> + * finds the flag clear and does not count the folio. The tag alone is not
> + * enough either: __folio_cancel_dirty() uncounts a folio and clears
> + * PG_dirty, but leaves the tag until writeback or truncation clears it. A
> + * folio another task has locked keeps the flag, as it would have had this
> + * lookup not run.
> + */
And this comment isn't much better.
> - 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);
Please don't mix code moves and non-trivial behavior changes.
Add a prep patch moving the code, and only then modify it.
next prev parent reply other threads:[~2026-11-03 10:46 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-24 22:11 [PATCH v9 0/6] mm, iomap, NFSD: stop uncached writes reading back shared pages Mike Snitzer
2026-10-24 22:11 ` [PATCH v9 1/6] mm: take a folio off WB_DONTCACHE_DIRTY only if it was counted there Mike Snitzer
2026-11-03 10:46 ` Christoph Hellwig [this message]
2026-10-24 22:11 ` [PATCH v9 2/6] mm: expose WB_DONTCACHE_DIRTY in bdi debugfs stats Mike Snitzer
2026-10-24 22:11 ` [PATCH v9 3/6] mm: add folio_mark_dropbehind() to mark a folio dropbehind and account it Mike Snitzer
2026-10-25 22:12 ` sashiko-bot
2026-10-26 6:42 ` Mike Snitzer
2026-10-24 22:11 ` [PATCH v9 4/6] mm, iomap: keep a partly covered page until an uncached write completes it Mike Snitzer
2026-11-03 10:50 ` Christoph Hellwig
2026-10-24 22:11 ` [PATCH v9 5/6] mm, iomap: let an uncached writer complete its shared pages itself Mike Snitzer
2026-10-24 22:11 ` [PATCH v9 6/6] NFSD: drop a shared boundary page only once both WRITEs have written it Mike Snitzer
2026-10-25 22:12 ` sashiko-bot
2026-10-26 6:45 ` Mike Snitzer
2026-11-03 10:51 ` Christoph Hellwig
2026-11-03 18:25 ` Mike Snitzer
2026-10-27 14:31 ` [PATCH v9 0/6] mm, iomap, NFSD: stop uncached writes reading back shared pages Jeff Layton
2026-10-27 15:36 ` Chuck Lever
2026-10-28 5:12 ` Mike Snitzer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261103104629.GA7296@lst.de \
--to=hch@lst.de \
--cc=Dai.Ngo@oracle.com \
--cc=axboe@kernel.dk \
--cc=cel@kernel.org \
--cc=corbet@lwn.net \
--cc=jlayton@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neil@brown.name \
--cc=okorniev@redhat.com \
--cc=snitzer@kernel.org \
--cc=tom@talpey.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox