All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Brian Foster <bfoster@redhat.com>,
	Carlos Maiolino <cem@kernel.org>,
	linux-xfs@vger.kernel.org
Subject: Re: [PATCH 02/12] xfs: consolidate buffer locking in xfs_buf_get_map
Date: Fri, 24 Jul 2026 09:42:18 -0700	[thread overview]
Message-ID: <20260724164218.GP2901224@frogsfrogsfrogs> (raw)
In-Reply-To: <20260720075902.GA18645@lst.de>

On Mon, Jul 20, 2026 at 09:59:02AM +0200, Christoph Hellwig wrote:
> On Fri, Jul 17, 2026 at 10:13:10AM -0400, Brian Foster wrote:
> > I see that you're intentionally separating out the lookup from the buf
> > data locking, but it does still look like we're making a potential
> > functional tradeoff here by changing the insertion side locking.
> 
> Yes.  That commit log tries to cover that.
> 
> > Note
> > that I don't object or know that it matters in any way, I'd just suggest
> > to add a note about this in the commit log in case somebody has to refer
> > back to this in the future.
> 
> ... but I guess the wording wasn't goot enough.  I'll try to improve it.

It could emphasize that b_lockref is used to make rcu lookups safe
against a freed buffer whereas b_sema is used to prevent racing threads
from doing anything non-trivial with the buffer once they've gotten
themselves a lockref, and that's why the comment is obsolete and can go
away.

Or modify the comment?

	/*
	 * The creator of a new buffer holds a lockref to that buffer.
	 * This ensures that the buffer is owned by the caller and
	 * racing RCU lookups right after inserting into the hash table
	 * are safe against freeing.
	 *
	 * Racing threads trying to look up the buffer will have to wait
	 * for b_sema to do anything non-trivial.  Note that b_sema is
	 * not locked at creation time.
	 */

Is there a subtlety here that the buffer creator might not be the one to
read the buffer contents in from disk if another thread happens to find
the buffer in the cache, take b_sema, notice that it's not XBF_DONE, and
start the read on its own?

--D

  reply	other threads:[~2026-07-24 16:42 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-15 14:50 misc buffer cache improvements Christoph Hellwig
2026-07-15 14:50 ` [PATCH 01/12] xfs: don't get a pag reference in xfs_buf_get_map Christoph Hellwig
2026-07-24 16:32   ` Darrick J. Wong
2026-07-15 14:50 ` [PATCH 02/12] xfs: consolidate buffer locking " Christoph Hellwig
2026-07-16 13:12   ` Brian Foster
2026-07-17  8:59     ` Christoph Hellwig
2026-07-17 14:13       ` Brian Foster
2026-07-20  7:59         ` Christoph Hellwig
2026-07-24 16:42           ` Darrick J. Wong [this message]
2026-07-28  4:38             ` Christoph Hellwig
2026-07-15 14:50 ` [PATCH 03/12] xfs: split out a lower-level xfs_buf_get_map helper from xfs_find_get_buf Christoph Hellwig
2026-07-24 16:44   ` Darrick J. Wong
2026-07-28  4:39     ` Christoph Hellwig
2026-07-15 14:50 ` [PATCH 04/12] xfs: remove spurious XBF_DONE clearing on readahead validation failure Christoph Hellwig
2026-07-24 16:45   ` Darrick J. Wong
2026-07-15 14:50 ` [PATCH 05/12] xfs: remove _XBF_LOGRECOVERY Christoph Hellwig
2026-07-24 16:49   ` Darrick J. Wong
2026-07-28  4:39     ` Christoph Hellwig
2026-07-15 14:50 ` [PATCH 06/12] xfs: hide b_flags manipulation from code outside of xfs_buf.c Christoph Hellwig
2026-07-24 16:49   ` Darrick J. Wong
2026-07-15 14:51 ` [PATCH 07/12] xfs: use WRITE_ONCE to update b_flags Christoph Hellwig
2026-07-24 16:53   ` Darrick J. Wong
2026-07-28  4:40     ` Christoph Hellwig
2026-07-15 14:51 ` [PATCH 08/12] xfs: don't reverify buffers in xfs_buf_readahead_map Christoph Hellwig
2026-07-24 16:56   ` Darrick J. Wong
2026-07-15 14:51 ` [PATCH 09/12] xfs: use goto based error unwinding in xfs_buf_read_map Christoph Hellwig
2026-07-24 16:57   ` Darrick J. Wong
2026-07-15 14:51 ` [PATCH 10/12] xfs: merge xfs_buf_reverify into xfs_buf_read_map Christoph Hellwig
2026-07-17 14:13   ` Brian Foster
2026-07-24 16:58   ` Darrick J. Wong
2026-07-15 14:51 ` [PATCH 11/12] xfs: move buffer locking out of xfs_find_get_buf Christoph Hellwig
2026-07-24 16:59   ` Darrick J. Wong
2026-07-15 14:51 ` [PATCH 12/12] xfs: add lockless xfs_buf_readahead_map fast path Christoph Hellwig
2026-07-24 17:03   ` Darrick J. Wong
2026-07-28  4:41     ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2026-07-28  8:11 misc buffer cache improvements v2 Christoph Hellwig
2026-07-28  8:11 ` [PATCH 02/12] xfs: consolidate buffer locking in xfs_buf_get_map Christoph Hellwig

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=20260724164218.GP2901224@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=bfoster@redhat.com \
    --cc=cem@kernel.org \
    --cc=hch@lst.de \
    --cc=linux-xfs@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.