Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
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.



  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