On Thu, Oct 1, 2020 at 1:41 AM David Hildenbrand <david@xxxxxxxxxx> wrote: > > 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? It's there because I add it later in this series. I waited until "mm/memremap_pages: convert to 'struct range'" to make it global as that's the first kernel-wide visible usage of it.
