On 25.09.20 21:11, Dan Williams wrote: > The passed in dev_pagemap is only required in the pmem case as the > libnvdimm core may have reserved a vmem_altmap for dev_memremap_pages() to > place the memmap in pmem directly. In the hmem case there is no agent > reserving an altmap so it can all be handled by a core internal default. > > Pass the resource range via a new @range property of 'struct > dev_dax_data'. > > Link: https://lkml.kernel.org/r/159643099958.4062302.10379230791041872886.stgit@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx > Cc: David Hildenbrand <david@xxxxxxxxxx> > Cc: Vishal Verma <vishal.l.verma@xxxxxxxxx> > Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx> > Cc: Pavel Tatashin <pasha.tatashin@xxxxxxxxxx> > Cc: Brice Goglin <Brice.Goglin@xxxxxxxx> > Cc: Dave Jiang <dave.jiang@xxxxxxxxx> > Cc: David Hildenbrand <david@xxxxxxxxxx> > Cc: Ira Weiny <ira.weiny@xxxxxxxxx> > Cc: Jia He <justin.he@xxxxxxx> > Cc: Joao Martins <joao.m.martins@xxxxxxxxxx> > Cc: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> > --- > drivers/dax/bus.c | 29 +++++++++++++++-------------- > drivers/dax/bus.h | 2 ++ > drivers/dax/dax-private.h | 9 ++++++++- > drivers/dax/device.c | 28 +++++++++++++++++++--------- > drivers/dax/hmem/hmem.c | 8 ++++---- > drivers/dax/kmem.c | 12 ++++++------ > drivers/dax/pmem/core.c | 4 ++++ > tools/testing/nvdimm/dax-dev.c | 8 ++++---- > 8 files changed, 62 insertions(+), 38 deletions(-) > > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c > index dffa4655e128..96bd64ba95a5 100644 > --- a/drivers/dax/bus.c > +++ b/drivers/dax/bus.c > @@ -271,7 +271,7 @@ static ssize_t size_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > struct dev_dax *dev_dax = to_dev_dax(dev); > - unsigned long long size = resource_size(&dev_dax->region->res); > + unsigned long long size = range_len(&dev_dax->range); > > return sprintf(buf, "%llu\n", size); > } > @@ -293,19 +293,12 @@ static ssize_t target_node_show(struct device *dev, > } > static DEVICE_ATTR_RO(target_node); > > -static unsigned long long dev_dax_resource(struct dev_dax *dev_dax) > -{ > - struct dax_region *dax_region = dev_dax->region; > - > - return dax_region->res.start; > -} > - > static ssize_t resource_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > struct dev_dax *dev_dax = to_dev_dax(dev); > > - return sprintf(buf, "%#llx\n", dev_dax_resource(dev_dax)); > + return sprintf(buf, "%#llx\n", dev_dax->range.start); > } > static DEVICE_ATTR(resource, 0400, resource_show, NULL); > > @@ -376,6 +369,7 @@ static void dev_dax_release(struct device *dev) > > dax_region_put(dax_region); > put_dax(dax_dev); > + kfree(dev_dax->pgmap); > kfree(dev_dax); > } > > @@ -412,7 +406,12 @@ struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data) > if (!dev_dax) > return ERR_PTR(-ENOMEM); > > - memcpy(&dev_dax->pgmap, data->pgmap, sizeof(struct dev_pagemap)); > + if (data->pgmap) { > + dev_dax->pgmap = kmemdup(data->pgmap, > + sizeof(struct dev_pagemap), GFP_KERNEL); > + if (!dev_dax->pgmap) > + goto err_pgmap; > + } > > /* > * No 'host' or dax_operations since there is no access to this > @@ -421,18 +420,19 @@ struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data) > dax_dev = alloc_dax(dev_dax, NULL, NULL, DAXDEV_F_SYNC); > if (IS_ERR(dax_dev)) { > rc = PTR_ERR(dax_dev); > - goto err; > + goto err_alloc_dax; > } > > /* a device_dax instance is dead while the driver is not attached */ > kill_dax(dax_dev); > > - /* from here on we're committed to teardown via dax_dev_release() */ > + /* from here on we're committed to teardown via dev_dax_release() */ > dev = &dev_dax->dev; > device_initialize(dev); > > dev_dax->dax_dev = dax_dev; > dev_dax->region = dax_region; > + dev_dax->range = data->range; > dev_dax->target_node = dax_region->target_node; > kref_get(&dax_region->kref); > > @@ -458,8 +458,9 @@ struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data) > return ERR_PTR(rc); > > return dev_dax; > - > - err: > +err_alloc_dax: > + kfree(dev_dax->pgmap); > +err_pgmap: > kfree(dev_dax); > > return ERR_PTR(rc); > diff --git a/drivers/dax/bus.h b/drivers/dax/bus.h > index 299c2e7fac09..4aeb36da83a4 100644 > --- a/drivers/dax/bus.h > +++ b/drivers/dax/bus.h > @@ -3,6 +3,7 @@ > #ifndef __DAX_BUS_H__ > #define __DAX_BUS_H__ > #include <linux/device.h> > +#include <linux/range.h> > > struct dev_dax; > struct resource; > @@ -21,6 +22,7 @@ struct dev_dax_data { > struct dax_region *dax_region; > struct dev_pagemap *pgmap; > enum dev_dax_subsys subsys; > + struct range range; > int id; > }; > > diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h > index 8a4c40ccd2ef..6779f683671d 100644 > --- a/drivers/dax/dax-private.h > +++ b/drivers/dax/dax-private.h > @@ -41,6 +41,7 @@ struct dax_region { > * @target_node: effective numa node if dev_dax memory range is onlined > * @dev - device core > * @pgmap - pgmap for memmap setup / lifetime (driver owned) > + * @range: resource range for the instance > * @dax_mem_res: physical address range of hotadded DAX memory > * @dax_mem_name: name for hotadded DAX memory via add_memory_driver_managed() > */ > @@ -49,10 +50,16 @@ struct dev_dax { > struct dax_device *dax_dev; > int target_node; > struct device dev; > - struct dev_pagemap pgmap; > + struct dev_pagemap *pgmap; > + struct range range; > struct resource *dax_kmem_res; > }; > > +static inline u64 range_len(struct range *range) > +{ > + return range->end - range->start + 1; > +} include/linux/range.h seems to have this function - why is this here needed? -- Thanks, David / dhildenb