Re: [PATCH V12 01/12] dax: replace exported dax_dev_get() with non-allocating dax_dev_find()

"John Groves" <[email protected]>
Newsgroups dev.linux.lists.nvdimm,dev.linux.lists.fuse-devel,org.kernel.vger.linux-cxl,org.kernel.vger.linux-doc,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>

On Mon, Aug 3, 2026, at 2:13 PM, Alison Schofield wrote:
> On Mon, Aug 03, 2026 at 02:28:26AM +0000, John Groves wrote:
> > From: John Groves <[email protected]>
> > 
> > This fix is in response to a Sashiko review, and some subsequent
> > analysis.
> > 
> > dax_dev_get() uses iget5_locked() which creates a new inode if no
> > matching one exists. This is correct for the internal caller
> > (alloc_dax), but dangerous for external callers that look up devices
> > from user-supplied or metadata-supplied dev_t values:
> > 
> > 1. A new inode is created with DAXDEV_ALIVE set but no backing driver,
> >    no ops, and no IDA-allocated minor number.
> > 
> > 2. On teardown, dax_destroy_inode() warns because kill_dax() was never
> >    called, and dax_free_inode() calls ida_free() for a minor that was
> >    never ida_alloc'd -- potentially freeing the minor of a real device.
> > 
> > Add dax_dev_find() which uses ilookup5() for lookup-only semantics:
> > it returns an existing dax_device with an elevated inode reference, or
> > NULL if no device with the given dev_t exists. It never creates inodes.
> > A dax_alive() check under dax_read_lock() guards against returning a
> > device that is concurrently being torn down by kill_dax().
> > 
> > Make dax_dev_get() static again (internal to super.c for alloc_dax),
> > export dax_dev_find() instead, and update the two external callers
> > (famfs_inode.c, famfs.c). Also add the missing CONFIG_DAX=n stub.
> 
> There are no external callers yet as those arrive in subsequent
> famfs patches.
> 
> 
> > 
> > About the 'fixes' tag: this removes the export of dax_dev_get(),
> > which was flawed, and replaces is with dax_dev_find(). It feels like
> > the fixes tag makes sense for correcting an ABI error.
> > 
> > Fixes: 2ae624d5a555d ("dax: export dax_dev_get()")
> 
> Hi John,
> 
> I think this should be split.
> 
> Please send a standalone DAX patch that only removes the dax_dev_get()
> export (make it static again, drop the header declaration). It's unused
> in-tree and unsafe for its intended use, so it stands on its own with
> no FAMFS dependency. I'll take it through the DAX tree for 7.3.
> 
> Please drop the Fixes: tag on the removal. IIUC the stable team uses
> it to pick backports, and this shouldn't land in 7.2.y. Removing this
> fixes nothing since no in-tree code calls the symbol. Name the commit
> in prose instead, something like:
> 
>   Commit 2ae624d5a555 ("dax: export dax_dev_get()") exported
>   dax_dev_get() in v7.2 for famfs, which has not merged. The export
>   has never had an in-tree caller, so make dax_dev_get() static again.
> 
> Keep the dax_dev_find() addition in this famfs series, so the new export
> lands with famfs.
> 
> -- Alison
> 
> > 
> > Reviewed-by: Dave Jiang <[email protected]>
> > Reviewed-by: Alison Schofield <[email protected]>
> 
> I think you can carry the tags for both patches because the code
> should end up byte identical.
> 
> snip
> 

Thanks Alison, done in my local tree.

The patch that drops dev_dax_get() should hit your inbox today or tomorrow;
the one that adds dev_dax_find() will stay with this series.

Best,
John
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.