Re: [PATCH V7 03/19] dax: add fsdev.c driver for fs-dax on character dax

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Tue, 17 Feb 2026 11:56:20 -0600
John Groves <John@groves.net> wrote:

> On 26/02/13 03:05PM, Ira Weiny wrote:
> > John Groves wrote:  
> > > From: John Groves <john@groves.net>
> > > 
> > > The new fsdev driver provides pages/folios initialized compatibly with
> > > fsdax - normal rather than devdax-style refcounting, and starting out
> > > with order-0 folios.
> > > 
> > > When fsdev binds to a daxdev, it is usually (always?) switching from the
> > > devdax mode (device.c), which pre-initializes compound folios according
> > > to its alignment. Fsdev uses fsdev_clear_folio_state() to switch the
> > > folios into a fsdax-compatible state.
> > > 
> > > A side effect of this is that raw mmap doesn't (can't?) work on an fsdev
> > > dax instance. Accordingly, The fsdev driver does not provide raw mmap -
> > > devices must be put in 'devdax' mode (drivers/dax/device.c) to get raw
> > > mmap capability.
> > > 
> > > In this commit is just the framework, which remaps pages/folios compatibly
> > > with fsdax.
> > > 
> > > Enabling dax changes:
> > > 
> > > - bus.h: add DAXDRV_FSDEV_TYPE driver type
> > > - bus.c: allow DAXDRV_FSDEV_TYPE drivers to bind to daxdevs
> > > - dax.h: prototype inode_dax(), which fsdev needs
> > > 
> > > Suggested-by: Dan Williams <dan.j.williams@intel.com>
> > > Suggested-by: Gregory Price <gourry@gourry.net>
> > > Signed-off-by: John Groves <john@groves.net>
> > > ---
> > >  MAINTAINERS          |   8 ++
> > >  drivers/dax/Makefile |   6 ++
> > >  drivers/dax/bus.c    |   4 +
> > >  drivers/dax/bus.h    |   1 +
> > >  drivers/dax/fsdev.c  | 242 +++++++++++++++++++++++++++++++++++++++++++
> > >  fs/dax.c             |   1 +
> > >  include/linux/dax.h  |   5 +
> > >  7 files changed, 267 insertions(+)
> > >  create mode 100644 drivers/dax/fsdev.c
> > >   
> > 
> > [snip]
> >   
> > > +
> > > +static int fsdev_dax_probe(struct dev_dax *dev_dax)
> > > +{
> > > +	struct dax_device *dax_dev = dev_dax->dax_dev;
> > > +	struct device *dev = &dev_dax->dev;
> > > +	struct dev_pagemap *pgmap;
> > > +	u64 data_offset = 0;
> > > +	struct inode *inode;
> > > +	struct cdev *cdev;
> > > +	void *addr;
> > > +	int rc, i;
> > > +
> > > +	if (static_dev_dax(dev_dax))  {
> > > +		if (dev_dax->nr_range > 1) {
> > > +			dev_warn(dev, "static pgmap / multi-range device conflict\n");
> > > +			return -EINVAL;
> > > +		}
> > > +
> > > +		pgmap = dev_dax->pgmap;
> > > +	} else {
> > > +		size_t pgmap_size;
> > > +
> > > +		if (dev_dax->pgmap) {
> > > +			dev_warn(dev, "dynamic-dax with pre-populated page map\n");
> > > +			return -EINVAL;
> > > +		}
> > > +
> > > +		pgmap_size = struct_size(pgmap, ranges, dev_dax->nr_range - 1);
> > > +		pgmap = devm_kzalloc(dev, pgmap_size,  GFP_KERNEL);
> > > +		if (!pgmap)
> > > +			return -ENOMEM;
> > > +
> > > +		pgmap->nr_range = dev_dax->nr_range;
> > > +		dev_dax->pgmap = pgmap;
> > > +
> > > +		for (i = 0; i < dev_dax->nr_range; i++) {
> > > +			struct range *range = &dev_dax->ranges[i].range;
> > > +
> > > +			pgmap->ranges[i] = *range;
> > > +		}
> > > +	}
> > > +
> > > +	for (i = 0; i < dev_dax->nr_range; i++) {
> > > +		struct range *range = &dev_dax->ranges[i].range;
> > > +
> > > +		if (!devm_request_mem_region(dev, range->start,
> > > +					range_len(range), dev_name(dev))) {
> > > +			dev_warn(dev, "mapping%d: %#llx-%#llx could not reserve range\n",
> > > +				 i, range->start, range->end);
> > > +			return -EBUSY;
> > > +		}
> > > +	}  
> > 
> > All of the above code is AFAICT exactly the same as the dev_dax driver.
> > Isn't there a way to make this common?
> > 
> > The rest of the common code is simple enough.  
> 
> dev_dax_probe() and fsdev_dax_probe() do indeed have some "same code" - 
> range validity checking and pgmap setup, from the top of probe through 
> the for loop above. After that they're different. Also, I just did a scan 
> and the probe function seems like the only remaining common code between 
> device.c and fsdev.c.
> 
> These are separate kmods; that code could certainly be factored out and 
> shared, but it would need to go somewhere common (maybe bus.c)?

Given I made a similar comment on new version. I'll reply here.
Could move it to core code, or if you want to keep stuff kmod, it's common
enough to have helper / library modules.  They are non userselectable
Kconfig options that are selected by the visible parts that need them.
Then dependency management ensures the helper gets loaded first.

> 
> So both device.c and fsdev.c would call bus.c:dax_prepare_pgmap() or
> some such.
> 
> I feel like this might not be worth factoring out, but I'm happy to do it
> if you and/or the dax team prefer it factored out and shared.

I think I'd like to see what it looks like. Maybe as a series on top.
But not my area so over to Dax folk ;)

Jonathan


>