All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Groves <John@groves.net>
To: Richard Cheng <icheng@nvidia.com>
Cc: John Groves <john@jagalactic.com>, Dan Williams <djbw@kernel.org>,
	 John Groves <jgroves@micron.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	 Dave Jiang <dave.jiang@intel.com>,
	Matthew Wilcox <willy@infradead.org>, Jan Kara <jack@suse.cz>,
	 Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>,
	 Miklos Szeredi <miklos@szeredi.hu>,
	Alison Schofield <alison.schofield@intel.com>,
	 Ira Weiny <iweiny@kernel.org>,
	Jonathan Cameron <jic23@kernel.org>,
	 "nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>,
	"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
	 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH V5 7/9] dax: read holder_ops once in dax_holder_notify_failure()
Date: Mon, 15 Jun 2026 08:22:32 -0500	[thread overview]
Message-ID: <ai_70p0O3u3S__gh@groves.net> (raw)
In-Reply-To: <ait2Lymb4y-Wb2ie@MWDK4CY14F>

On 26/06/12 11:02AM, Richard Cheng wrote:
> On Thu, Jun 11, 2026 at 05:32:45PM +0800, John Groves wrote:
> > From: John Groves <John@Groves.net>
> > 
> > dax_holder_notify_failure() reads dax_dev->holder_ops twice without
> > READ_ONCE() -- once for the NULL check and once for the indirect
> > notify_failure() call. A concurrent fs_put_dax() or kill_dax() can clear
> > holder_ops between the two reads, so the check can observe a non-NULL
> > pointer while the call dereferences NULL.
> > 
> 
> Hello John,
> 
> Thanks for this.
> 
> Reviewed-by: Richard Cheng <icheng@nvidia.com>
> 
> Small message nit, kill_dax() isn't a racing clearer.

Right -- kill_dax() clears holder_ops only after synchronize_srcu(), so it
can't race a reader under dax_read_lock(). Reworded the commit message and
the code comment for V6 to name only fs_put_dax().

> Plus I think this only fix holder_ops double-fetch, the fs_put_dax()
> race issue is separate and pre-existing.

Yes, intentionally -- this patch is just the reader-side double-fetch. The
fs_put_dax() race is the separate, pre-existing one handled by the next
patch in the series ("dax: fix holder_ops race in fs_put_dax()").

> 
> Best regards,
> Richard Cheng.

Thanks,
John

<snip>

  reply	other threads:[~2026-06-15 13:22 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260611173057.65868-1-john@jagalactic.com>
2026-06-11 17:31 ` [PATCH V5 0/9] Fixes to the previously-merged drivers/dax/fsdev series John Groves
2026-06-11 17:31   ` [PATCH V5 1/9] dax: fix misleading comment about share/index union in dax_folio_reset_order() John Groves
2026-06-11 17:31   ` [PATCH V5 2/9] dax/fsdev: fix multi-range offset in memory_failure handler John Groves
2026-06-11 17:51     ` sashiko-bot
2026-06-12  3:08     ` Richard Cheng
2026-06-15 13:13       ` John Groves
2026-06-11 17:32   ` [PATCH V5 3/9] dax/fsdev: clear vmemmap_shift when binding static pgmap John Groves
2026-06-11 17:55     ` sashiko-bot
2026-06-12  2:56     ` Richard Cheng
2026-06-15 13:16       ` John Groves
2026-06-11 17:32   ` [PATCH V5 4/9] dax/fsdev: don't leave a dangling dev_dax->pgmap on probe failure John Groves
2026-06-11 18:04     ` sashiko-bot
2026-06-11 17:32   ` [PATCH V5 5/9] dax/fsdev: use __va(phys) for kaddr in direct_access John Groves
2026-06-11 17:32   ` [PATCH V5 6/9] dax/fsdev: fail probe on invalid pgmap offset John Groves
2026-06-11 18:09     ` Gupta, Pankaj
2026-06-15 13:23       ` John Groves
2026-06-11 18:13     ` sashiko-bot
2026-06-11 17:32   ` [PATCH V5 7/9] dax: read holder_ops once in dax_holder_notify_failure() John Groves
2026-06-11 18:13     ` sashiko-bot
2026-06-12  3:02     ` Richard Cheng
2026-06-15 13:22       ` John Groves [this message]
2026-06-11 17:32   ` [PATCH V5 8/9] dax: fix holder_ops race in fs_put_dax() John Groves
2026-06-11 18:28     ` sashiko-bot
2026-06-11 17:33   ` [PATCH V5 9/9] dax: fsdev.c minor formatting cleanup John Groves

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=ai_70p0O3u3S__gh@groves.net \
    --to=john@groves.net \
    --cc=alison.schofield@intel.com \
    --cc=brauner@kernel.org \
    --cc=dave.jiang@intel.com \
    --cc=djbw@kernel.org \
    --cc=icheng@nvidia.com \
    --cc=iweiny@kernel.org \
    --cc=jack@suse.cz \
    --cc=jgroves@micron.com \
    --cc=jic23@kernel.org \
    --cc=john@jagalactic.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=nvdimm@lists.linux.dev \
    --cc=viro@zeniv.linux.org.uk \
    --cc=vishal.l.verma@intel.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 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.