From 450c6633e874c4d38112b39647831f67b41a8067 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 28 Nov 2016 11:15:18 -0800 Subject: libnvdimm: use consistent naming for request_mem_region() Here is an example /proc/iomem listing for a system with 2 namespaces, one in "sector" mode and one in "memory" mode: 1fc000000-2fbffffff : Persistent Memory (legacy) 1fc000000-2fbffffff : namespace1.0 340000000-34fffffff : Persistent Memory 340000000-34fffffff : btt0.1 Here is the corresponding ndctl listing: # ndctl list [ { "dev":"namespace1.0", "mode":"memory", "size":4294967296, "blockdev":"pmem1" }, { "dev":"namespace0.0", "mode":"sector", "size":267091968, "uuid":"f7594f86-badb-4592-875f-ded577da2eaf", "sector_size":4096, "blockdev":"pmem0s" } ] Notice that the ndctl listing is purely in terms of namespace devices, while the iomem listing leaks the internal "btt0.1" implementation detail. Given that ndctl requires the namespace device name to change the mode, for example: # ndctl create-namespace --reconfig=namespace0.0 --mode=raw --force ...use the namespace name in the iomem listing to keep the claiming device name consistent across different mode settings. Cc: Vishal Verma Signed-off-by: Dan Williams --- drivers/dax/pmem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/dax') diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c index 9630d8837ba9..3ff84784249a 100644 --- a/drivers/dax/pmem.c +++ b/drivers/dax/pmem.c @@ -87,7 +87,8 @@ static int dax_pmem_probe(struct device *dev) pfn_sb = nd_pfn->pfn_sb; if (!devm_request_mem_region(dev, nsio->res.start, - resource_size(&nsio->res), dev_name(dev))) { + resource_size(&nsio->res), + dev_name(&ndns->dev))) { dev_warn(dev, "could not reserve region %pR\n", &nsio->res); return -EBUSY; } -- cgit v1.2.3 From d7fe1a67f658b50ec98ee1afb86df7b35c2b2593 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sat, 17 Dec 2016 14:50:04 -0800 Subject: dax: add region 'id', 'size', and 'align' attributes While this information is available by looking at the nvdimm parent device that may not always be the case when/if we add support for other memory regions. Tooling should not depend on walking a given ancestor topology that is not guaranteed by the device's class. For example, a device-dax instance will always have a dax_region parent, but it may not always have a libnvdimm "dax" device as a grandparent. Reported-by: Johannes Thumshirn Signed-off-by: Dan Williams --- drivers/dax/dax.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) (limited to 'drivers/dax') diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c index 0e499bfca41c..369fccda3b1b 100644 --- a/drivers/dax/dax.c +++ b/drivers/dax/dax.c @@ -75,6 +75,73 @@ struct dax_dev { struct resource res[0]; }; +static ssize_t id_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct dax_region *dax_region; + ssize_t rc = -ENXIO; + + device_lock(dev); + dax_region = dev_get_drvdata(dev); + if (dax_region) + rc = sprintf(buf, "%d\n", dax_region->id); + device_unlock(dev); + + return rc; +} +static DEVICE_ATTR_RO(id); + +static ssize_t region_size_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct dax_region *dax_region; + ssize_t rc = -ENXIO; + + device_lock(dev); + dax_region = dev_get_drvdata(dev); + if (dax_region) + rc = sprintf(buf, "%llu\n", (unsigned long long) + resource_size(&dax_region->res)); + device_unlock(dev); + + return rc; +} +static struct device_attribute dev_attr_region_size = __ATTR(size, 0444, + region_size_show, NULL); + +static ssize_t align_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct dax_region *dax_region; + ssize_t rc = -ENXIO; + + device_lock(dev); + dax_region = dev_get_drvdata(dev); + if (dax_region) + rc = sprintf(buf, "%u\n", dax_region->align); + device_unlock(dev); + + return rc; +} +static DEVICE_ATTR_RO(align); + +static struct attribute *dax_region_attributes[] = { + &dev_attr_region_size.attr, + &dev_attr_align.attr, + &dev_attr_id.attr, + NULL, +}; + +static const struct attribute_group dax_region_attribute_group = { + .name = "dax_region", + .attrs = dax_region_attributes, +}; + +static const struct attribute_group *dax_region_attribute_groups[] = { + &dax_region_attribute_group, + NULL, +}; + static struct inode *dax_alloc_inode(struct super_block *sb) { return kmem_cache_alloc(dax_cache, GFP_KERNEL); @@ -200,12 +267,31 @@ void dax_region_put(struct dax_region *dax_region) } EXPORT_SYMBOL_GPL(dax_region_put); +static void dax_region_unregister(void *region) +{ + struct dax_region *dax_region = region; + + sysfs_remove_groups(&dax_region->dev->kobj, + dax_region_attribute_groups); + dax_region_put(dax_region); +} + struct dax_region *alloc_dax_region(struct device *parent, int region_id, struct resource *res, unsigned int align, void *addr, unsigned long pfn_flags) { struct dax_region *dax_region; + /* + * The DAX core assumes that it can store its private data in + * parent->driver_data. This WARN is a reminder / safeguard for + * developers of device-dax drivers. + */ + if (dev_get_drvdata(parent)) { + dev_WARN(parent, "dax core failed to setup private data\n"); + return NULL; + } + if (!IS_ALIGNED(res->start, align) || !IS_ALIGNED(resource_size(res), align)) return NULL; @@ -214,6 +300,7 @@ struct dax_region *alloc_dax_region(struct device *parent, int region_id, if (!dax_region) return NULL; + dev_set_drvdata(parent, dax_region); memcpy(&dax_region->res, res, sizeof(*res)); dax_region->pfn_flags = pfn_flags; kref_init(&dax_region->kref); @@ -222,7 +309,14 @@ struct dax_region *alloc_dax_region(struct device *parent, int region_id, dax_region->align = align; dax_region->dev = parent; dax_region->base = addr; + if (sysfs_create_groups(&parent->kobj, dax_region_attribute_groups)) { + kfree(dax_region); + return NULL;; + } + kref_get(&dax_region->kref); + if (devm_add_action_or_reset(parent, dax_region_unregister, dax_region)) + return NULL; return dax_region; } EXPORT_SYMBOL_GPL(alloc_dax_region); -- cgit v1.2.3