From 8d7c22ac0c036978a072b7e13c607b5402c474e0 Mon Sep 17 00:00:00 2001 From: Toshi Kani Date: Wed, 19 Oct 2016 08:19:44 -0600 Subject: libnvdimm: use generic iostat interfaces nd_iostat_start() and nd_iostat_end() implement the same functionality that generic_start_io_acct() and generic_end_io_acct() already provide. Change nd_iostat_start() and nd_iostat_end() to call the generic iostat interfaces. There is no change in the nd interfaces. Signed-off-by: Toshi Kani Cc: Andrew Morton Cc: Alexander Viro Cc: Dave Chinner Cc: Ross Zwisler Signed-off-by: Dan Williams --- drivers/nvdimm/core.c | 29 ----------------------------- drivers/nvdimm/nd.h | 11 +++++++++-- 2 files changed, 9 insertions(+), 31 deletions(-) (limited to 'drivers') diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c index 7ceba08774b6..9303cfeb8bee 100644 --- a/drivers/nvdimm/core.c +++ b/drivers/nvdimm/core.c @@ -317,35 +317,6 @@ ssize_t nd_sector_size_store(struct device *dev, const char *buf, } } -void __nd_iostat_start(struct bio *bio, unsigned long *start) -{ - struct gendisk *disk = bio->bi_bdev->bd_disk; - const int rw = bio_data_dir(bio); - int cpu = part_stat_lock(); - - *start = jiffies; - part_round_stats(cpu, &disk->part0); - part_stat_inc(cpu, &disk->part0, ios[rw]); - part_stat_add(cpu, &disk->part0, sectors[rw], bio_sectors(bio)); - part_inc_in_flight(&disk->part0, rw); - part_stat_unlock(); -} -EXPORT_SYMBOL(__nd_iostat_start); - -void nd_iostat_end(struct bio *bio, unsigned long start) -{ - struct gendisk *disk = bio->bi_bdev->bd_disk; - unsigned long duration = jiffies - start; - const int rw = bio_data_dir(bio); - int cpu = part_stat_lock(); - - part_stat_add(cpu, &disk->part0, ticks[rw], duration); - part_round_stats(cpu, &disk->part0); - part_dec_in_flight(&disk->part0, rw); - part_stat_unlock(); -} -EXPORT_SYMBOL(nd_iostat_end); - static ssize_t commands_show(struct device *dev, struct device_attribute *attr, char *buf) { diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h index d3b2fca8deec..065abf1b8f32 100644 --- a/drivers/nvdimm/nd.h +++ b/drivers/nvdimm/nd.h @@ -377,10 +377,17 @@ static inline bool nd_iostat_start(struct bio *bio, unsigned long *start) if (!blk_queue_io_stat(disk->queue)) return false; - __nd_iostat_start(bio, start); + *start = jiffies; + generic_start_io_acct(bio_data_dir(bio), + bio_sectors(bio), &disk->part0); return true; } -void nd_iostat_end(struct bio *bio, unsigned long start); +static inline void nd_iostat_end(struct bio *bio, unsigned long start) +{ + struct gendisk *disk = bio->bi_bdev->bd_disk; + + generic_end_io_acct(bio_data_dir(bio), &disk->part0, start); +} static inline bool is_bad_pmem(struct badblocks *bb, sector_t sector, unsigned int len) { -- cgit v1.2.3 From 42237e393f64d619ed56e17fbf8fd27526485695 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sat, 15 Oct 2016 15:33:52 -0700 Subject: libnvdimm: allow a platform to force enable label support Platforms like QEMU-KVM implement an NFIT table and label DSMs. However, since that environment does not define an aliased configuration, the labels are currently ignored and the kernel registers a single full-sized pmem-namespace per region. Now that the kernel supports sub-divisions of pmem regions the labels have a purpose. Arrange for the labels to be honored when we find an existing / valid namespace index block. Cc: Cc: Haozhong Zhang Cc: Xiao Guangrong Signed-off-by: Dan Williams --- drivers/nvdimm/dimm.c | 2 ++ drivers/nvdimm/dimm_devs.c | 7 +++++++ drivers/nvdimm/nd.h | 1 + 3 files changed, 10 insertions(+) (limited to 'drivers') diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c index 619834e144d1..ee0b412827bf 100644 --- a/drivers/nvdimm/dimm.c +++ b/drivers/nvdimm/dimm.c @@ -64,6 +64,8 @@ static int nvdimm_probe(struct device *dev) nd_label_copy(ndd, to_next_namespace_index(ndd), to_current_namespace_index(ndd)); rc = nd_label_reserve_dpa(ndd); + if (ndd->ns_current >= 0) + nvdimm_set_aliasing(dev); nvdimm_bus_unlock(dev); if (rc) diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c index d614493ad5ac..0eedc49e0d47 100644 --- a/drivers/nvdimm/dimm_devs.c +++ b/drivers/nvdimm/dimm_devs.c @@ -184,6 +184,13 @@ int nvdimm_set_config_data(struct nvdimm_drvdata *ndd, size_t offset, return rc; } +void nvdimm_set_aliasing(struct device *dev) +{ + struct nvdimm *nvdimm = to_nvdimm(dev); + + nvdimm->flags |= NDD_ALIASING; +} + static void nvdimm_release(struct device *dev) { struct nvdimm *nvdimm = to_nvdimm(dev); diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h index 065abf1b8f32..35dd75057e16 100644 --- a/drivers/nvdimm/nd.h +++ b/drivers/nvdimm/nd.h @@ -238,6 +238,7 @@ int nvdimm_set_config_data(struct nvdimm_drvdata *ndd, size_t offset, void *buf, size_t len); long nvdimm_clear_poison(struct device *dev, phys_addr_t phys, unsigned int len); +void nvdimm_set_aliasing(struct device *dev); struct nd_btt *to_nd_btt(struct device *dev); struct nd_gen_sb { -- cgit v1.2.3 From 82bf1037f2cab2d6960a08ae08513f2c3c0b335a Mon Sep 17 00:00:00 2001 From: Dave Jiang Date: Fri, 11 Nov 2016 12:37:36 -0700 Subject: libnvdimm: check and clear poison before writing to pmem We need to clear any poison when we are writing to pmem. The granularity will be sector size. If it's less then we can't do anything about it barring corruption. Signed-off-by: Dave Jiang Reviewed-by: Vishal Verma [djbw: fixup 0-length write request to succeed] Signed-off-by: Dan Williams --- drivers/nvdimm/claim.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c index d5dc80c48b4c..8d66fbb779ed 100644 --- a/drivers/nvdimm/claim.c +++ b/drivers/nvdimm/claim.c @@ -226,6 +226,12 @@ static int nsio_rw_bytes(struct nd_namespace_common *ndns, resource_size_t offset, void *buf, size_t size, int rw) { struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev); + unsigned int sz_align = ALIGN(size + (offset & (512 - 1)), 512); + sector_t sector = offset >> 9; + int rc = 0; + + if (unlikely(!size)) + return 0; if (unlikely(offset + size > nsio->size)) { dev_WARN_ONCE(&ndns->dev, 1, "request out of range\n"); @@ -233,17 +239,33 @@ static int nsio_rw_bytes(struct nd_namespace_common *ndns, } if (rw == READ) { - unsigned int sz_align = ALIGN(size + (offset & (512 - 1)), 512); - - if (unlikely(is_bad_pmem(&nsio->bb, offset / 512, sz_align))) + if (unlikely(is_bad_pmem(&nsio->bb, sector, sz_align))) return -EIO; return memcpy_from_pmem(buf, nsio->addr + offset, size); } else { + + if (unlikely(is_bad_pmem(&nsio->bb, sector, sz_align))) { + if (IS_ALIGNED(offset, 512) && IS_ALIGNED(size, 512)) { + long cleared; + + cleared = nvdimm_clear_poison(&ndns->dev, + offset, size); + if (cleared != size) { + size = cleared; + rc = -EIO; + } + + badblocks_clear(&nsio->bb, sector, + cleared >> 9); + } else + rc = -EIO; + } + memcpy_to_pmem(nsio->addr + offset, buf, size); nvdimm_flush(to_nd_region(ndns->dev.parent)); } - return 0; + return rc; } int devm_nsio_enable(struct device *dev, struct nd_namespace_io *nsio) -- cgit v1.2.3 From 2d9a02744f5a2e8325fc0c3e5533593261cead0a Mon Sep 17 00:00:00 2001 From: Nicolas Iooss Date: Sat, 29 Oct 2016 13:28:52 +0200 Subject: nvdimm: use the right length of "pmem" In order to test that the name of a resource begins with "pmem", call strncmp() with 4 as length instead of 3 to match the whole prefix. Signed-off-by: Nicolas Iooss Signed-off-by: Dan Williams --- drivers/nvdimm/label.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c index fac7cabe8f56..dd615345699f 100644 --- a/drivers/nvdimm/label.c +++ b/drivers/nvdimm/label.c @@ -938,7 +938,7 @@ int nd_pmem_namespace_label_update(struct nd_region *nd_region, } for_each_dpa_resource(ndd, res) - if (strncmp(res->name, "pmem", 3) == 0) + if (strncmp(res->name, "pmem", 4) == 0) count++; WARN_ON_ONCE(!count); -- cgit v1.2.3 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 ++- drivers/nvdimm/claim.c | 2 +- drivers/nvdimm/pmem.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers') 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; } diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c index 8d66fbb779ed..4638b9ea5229 100644 --- a/drivers/nvdimm/claim.c +++ b/drivers/nvdimm/claim.c @@ -275,7 +275,7 @@ int devm_nsio_enable(struct device *dev, struct nd_namespace_io *nsio) nsio->size = resource_size(res); if (!devm_request_mem_region(dev, res->start, resource_size(res), - dev_name(dev))) { + dev_name(&ndns->dev))) { dev_warn(dev, "could not reserve region %pR\n", res); return -EBUSY; } diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index 42b3a8217073..34f16a17c07b 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -266,7 +266,7 @@ static int pmem_attach_disk(struct device *dev, dev_warn(dev, "unable to guarantee persistence of writes\n"); if (!devm_request_mem_region(dev, res->start, resource_size(res), - dev_name(dev))) { + dev_name(&ndns->dev))) { dev_warn(dev, "could not reserve region %pR\n", res); return -EBUSY; } -- cgit v1.2.3 From 238b323a681dd4a5ea0c651fdf4e6ee91a09a9ba Mon Sep 17 00:00:00 2001 From: Nicolas Iooss Date: Sat, 26 Nov 2016 20:18:04 +0100 Subject: libnvdimm, namespace: fix the type of name variable In create_namespace_blk(), the local variable "name" is defined as an array of NSLABEL_NAME_LEN pointers: char *name[NSLABEL_NAME_LEN]; This variable is then used in calls to memcpy() and kmemdup() as if it were char[NSLABEL_NAME_LEN]. Remove the star in the variable definition to makes it look right. Signed-off-by: Nicolas Iooss Reviewed-by: Ross Zwisler Signed-off-by: Dan Williams --- drivers/nvdimm/namespace_devs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c index 3509cff68ef9..61636d135105 100644 --- a/drivers/nvdimm/namespace_devs.c +++ b/drivers/nvdimm/namespace_devs.c @@ -1997,7 +1997,7 @@ struct device *create_namespace_blk(struct nd_region *nd_region, struct nd_mapping *nd_mapping = &nd_region->mapping[0]; struct nvdimm_drvdata *ndd = to_ndd(nd_mapping); struct nd_namespace_blk *nsblk; - char *name[NSLABEL_NAME_LEN]; + char name[NSLABEL_NAME_LEN]; struct device *dev = NULL; struct resource *res; -- cgit v1.2.3 From d37806dc37b42b05515849b0444f09f493cb4cba Mon Sep 17 00:00:00 2001 From: Fabian Frederick Date: Sun, 4 Dec 2016 10:45:13 -0800 Subject: libnvdimm: remove else after return in nsio_rw_bytes() else after return is not needed. Signed-off-by: Fabian Frederick [djbw: removed some now unnecessary newlines] Signed-off-by: Dan Williams --- drivers/nvdimm/claim.c | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) (limited to 'drivers') diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c index 4638b9ea5229..3376da1fb263 100644 --- a/drivers/nvdimm/claim.c +++ b/drivers/nvdimm/claim.c @@ -242,29 +242,26 @@ static int nsio_rw_bytes(struct nd_namespace_common *ndns, if (unlikely(is_bad_pmem(&nsio->bb, sector, sz_align))) return -EIO; return memcpy_from_pmem(buf, nsio->addr + offset, size); - } else { - - if (unlikely(is_bad_pmem(&nsio->bb, sector, sz_align))) { - if (IS_ALIGNED(offset, 512) && IS_ALIGNED(size, 512)) { - long cleared; - - cleared = nvdimm_clear_poison(&ndns->dev, - offset, size); - if (cleared != size) { - size = cleared; - rc = -EIO; - } - - badblocks_clear(&nsio->bb, sector, - cleared >> 9); - } else + } + + if (unlikely(is_bad_pmem(&nsio->bb, sector, sz_align))) { + if (IS_ALIGNED(offset, 512) && IS_ALIGNED(size, 512)) { + long cleared; + + cleared = nvdimm_clear_poison(&ndns->dev, offset, size); + if (cleared != size) { + size = cleared; rc = -EIO; - } + } - memcpy_to_pmem(nsio->addr + offset, buf, size); - nvdimm_flush(to_nd_region(ndns->dev.parent)); + badblocks_clear(&nsio->bb, sector, cleared >> 9); + } else + rc = -EIO; } + memcpy_to_pmem(nsio->addr + offset, buf, size); + nvdimm_flush(to_nd_region(ndns->dev.parent)); + return rc; } -- cgit v1.2.3 From 0a3f27b9a6a8f76f1df270e1a18b096f8f5d5dbc Mon Sep 17 00:00:00 2001 From: Fabian Frederick Date: Sun, 4 Dec 2016 10:48:58 -0800 Subject: libnvdimm, namespace: avoid multiple sector calculations Use sector_t for cleared Suggested-by: Dan Williams Signed-off-by: Fabian Frederick Signed-off-by: Dan Williams --- drivers/nvdimm/pmem.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index 34f16a17c07b..bcc359a4e64d 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -51,17 +51,16 @@ static void pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset, unsigned int len) { struct device *dev = to_dev(pmem); - sector_t sector; - long cleared; + sector_t sector, cleared; sector = (offset - pmem->data_offset) / 512; - cleared = nvdimm_clear_poison(dev, pmem->phys_addr + offset, len); + cleared = nvdimm_clear_poison(dev, pmem->phys_addr + offset, len) / 512; - if (cleared > 0 && cleared / 512) { + if (cleared) { dev_dbg(dev, "%s: %#llx clear %ld sector%s\n", __func__, (unsigned long long) sector, - cleared / 512, cleared / 512 > 1 ? "s" : ""); - badblocks_clear(&pmem->bb, sector, cleared / 512); + cleared, cleared > 1 ? "s" : ""); + badblocks_clear(&pmem->bb, sector, cleared); } invalidate_pmem(pmem->virt_addr + offset, len); } -- cgit v1.2.3 From b44fe760433a58f2284d2a544afd91dd685ac677 Mon Sep 17 00:00:00 2001 From: Fabian Frederick Date: Sun, 4 Dec 2016 10:54:08 -0800 Subject: libnvdimm, namespace: use octal for permissions According to commit f90774e1fd27 ("checkpatch: look for symbolic permissions and suggest octal instead") Signed-off-by: Fabian Frederick Signed-off-by: Dan Williams --- drivers/nvdimm/namespace_devs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c index 61636d135105..8817f8a0cf38 100644 --- a/drivers/nvdimm/namespace_devs.c +++ b/drivers/nvdimm/namespace_devs.c @@ -1132,7 +1132,7 @@ static ssize_t size_show(struct device *dev, return sprintf(buf, "%llu\n", (unsigned long long) nvdimm_namespace_capacity(to_ndns(dev))); } -static DEVICE_ATTR(size, S_IRUGO, size_show, size_store); +static DEVICE_ATTR(size, 0444, size_show, size_store); static u8 *namespace_to_uuid(struct device *dev) { @@ -1456,7 +1456,7 @@ static umode_t namespace_visible(struct kobject *kobj, if (is_namespace_pmem(dev) || is_namespace_blk(dev)) { if (a == &dev_attr_size.attr) - return S_IWUSR | S_IRUGO; + return 0644; if (is_namespace_pmem(dev) && a == &dev_attr_sector_size.attr) return 0; -- cgit v1.2.3 From 3a71b3c8946fd44cd2599d9e87ac8f90c71318e1 Mon Sep 17 00:00:00 2001 From: Johannes Thumshirn Date: Mon, 5 Dec 2016 09:23:20 +0100 Subject: libnvdimm, e820: use module_platform_driver Use module_platform_driver for the e820 driver instead of open-coding it. Signed-off-by: Johannes Thumshirn Signed-off-by: Dan Williams --- drivers/nvdimm/e820.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) (limited to 'drivers') diff --git a/drivers/nvdimm/e820.c b/drivers/nvdimm/e820.c index 11ea90120542..6f9a6ffd7cde 100644 --- a/drivers/nvdimm/e820.c +++ b/drivers/nvdimm/e820.c @@ -84,18 +84,8 @@ static struct platform_driver e820_pmem_driver = { }, }; -static __init int e820_pmem_init(void) -{ - return platform_driver_register(&e820_pmem_driver); -} - -static __exit void e820_pmem_exit(void) -{ - platform_driver_unregister(&e820_pmem_driver); -} +module_platform_driver(e820_pmem_driver); MODULE_ALIAS("platform:e820_pmem*"); MODULE_LICENSE("GPL v2"); MODULE_AUTHOR("Intel Corporation"); -module_init(e820_pmem_init); -module_exit(e820_pmem_exit); -- cgit v1.2.3 From af7d9f0c57941b465043681cb5c3410f7f3f1a41 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sat, 10 Dec 2016 08:12:05 -0800 Subject: libnvdimm, pfn: fix align attribute Fix the format specifier so that the attribute can be parsed correctly. Currently it returns decimal 1000 for a 4096-byte alignment. Cc: Reported-by: Dave Jiang Fixes: 315c562536c4 ("libnvdimm, pfn: add 'align' attribute, default to HPAGE_SIZE") Signed-off-by: Dan Williams --- drivers/nvdimm/pfn_devs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c index cea8350fbc7e..a2ac9e641aa9 100644 --- a/drivers/nvdimm/pfn_devs.c +++ b/drivers/nvdimm/pfn_devs.c @@ -108,7 +108,7 @@ static ssize_t align_show(struct device *dev, { struct nd_pfn *nd_pfn = to_nd_pfn_safe(dev); - return sprintf(buf, "%lx\n", nd_pfn->align); + return sprintf(buf, "%ld\n", nd_pfn->align); } static ssize_t __align_store(struct nd_pfn *nd_pfn, const char *buf) -- cgit v1.2.3 From 9cf8bd529c6ba81402ebf6b7a56307b0787e4f93 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 15 Dec 2016 20:04:31 -0800 Subject: libnvdimm: replace mutex_is_locked() warnings with lockdep_assert_held For warnings that should only ever trigger during development and testing replace WARN statements with lockdep_assert_held. The lockdep pattern is prevalent, and these paths are are well covered by libnvdimm unit tests. Reported-by: Johannes Thumshirn Reviewed-by: Johannes Thumshirn Signed-off-by: Dan Williams --- drivers/nvdimm/claim.c | 10 ++++------ drivers/nvdimm/namespace_devs.c | 2 +- drivers/nvdimm/region_devs.c | 2 +- 3 files changed, 6 insertions(+), 8 deletions(-) (limited to 'drivers') diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c index 3376da1fb263..97d1772774a8 100644 --- a/drivers/nvdimm/claim.c +++ b/drivers/nvdimm/claim.c @@ -22,9 +22,8 @@ void __nd_detach_ndns(struct device *dev, struct nd_namespace_common **_ndns) { struct nd_namespace_common *ndns = *_ndns; - dev_WARN_ONCE(dev, !mutex_is_locked(&ndns->dev.mutex) - || ndns->claim != dev, - "%s: invalid claim\n", __func__); + lockdep_assert_held(&ndns->dev.mutex); + dev_WARN_ONCE(dev, ndns->claim != dev, "%s: invalid claim\n", __func__); ndns->claim = NULL; *_ndns = NULL; put_device(&ndns->dev); @@ -49,9 +48,8 @@ bool __nd_attach_ndns(struct device *dev, struct nd_namespace_common *attach, { if (attach->claim) return false; - dev_WARN_ONCE(dev, !mutex_is_locked(&attach->dev.mutex) - || *_ndns, - "%s: invalid claim\n", __func__); + lockdep_assert_held(&attach->dev.mutex); + dev_WARN_ONCE(dev, *_ndns, "%s: invalid claim\n", __func__); attach->claim = dev; *_ndns = attach; get_device(&attach->dev); diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c index 8817f8a0cf38..de5809a88512 100644 --- a/drivers/nvdimm/namespace_devs.c +++ b/drivers/nvdimm/namespace_devs.c @@ -1653,7 +1653,7 @@ static int select_pmem_id(struct nd_region *nd_region, u8 *pmem_id) u64 hw_start, hw_end, pmem_start, pmem_end; struct nd_label_ent *label_ent; - WARN_ON(!mutex_is_locked(&nd_mapping->lock)); + lockdep_assert_held(&nd_mapping->lock); list_for_each_entry(label_ent, &nd_mapping->labels, list) { nd_label = label_ent->label; if (!nd_label) diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c index 6af5e629140c..7cd705f3247c 100644 --- a/drivers/nvdimm/region_devs.c +++ b/drivers/nvdimm/region_devs.c @@ -509,7 +509,7 @@ void nd_mapping_free_labels(struct nd_mapping *nd_mapping) { struct nd_label_ent *label_ent, *e; - WARN_ON(!mutex_is_locked(&nd_mapping->lock)); + lockdep_assert_held(&nd_mapping->lock); list_for_each_entry_safe(label_ent, e, &nd_mapping->labels, list) { list_del(&label_ent->list); kfree(label_ent); -- cgit v1.2.3 From 868f036fee4b1f934117197fb93461d2c968ffec Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 16 Dec 2016 08:10:31 -0800 Subject: libnvdimm: fix mishandled nvdimm_clear_poison() return value Colin, via static analysis, reports that the length could be negative from nvdimm_clear_poison() in the error case. There was a similar problem with commit 0a3f27b9a6a8 "libnvdimm, namespace: avoid multiple sector calculations" that I noticed when merging the for-4.10/libnvdimm topic branch into libnvdimm-for-next, but I missed this one. Fix both of them to the following procedure: * if we clear a block's worth of media, clear that many blocks in badblocks * if we clear less than the requested size of the transfer return an error * always invalidate cache after any non-error / non-zero nvdimm_clear_poison result Fixes: 82bf1037f2ca ("libnvdimm: check and clear poison before writing to pmem") Fixes: 0a3f27b9a6a8 ("libnvdimm, namespace: avoid multiple sector calculations") Cc: Fabian Frederick Cc: Dave Jiang Reported-by: Colin Ian King Signed-off-by: Dan Williams --- drivers/nvdimm/claim.c | 9 +++++---- drivers/nvdimm/pmem.c | 21 ++++++++++++++------- 2 files changed, 19 insertions(+), 11 deletions(-) (limited to 'drivers') diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c index 97d1772774a8..b3323c0697f6 100644 --- a/drivers/nvdimm/claim.c +++ b/drivers/nvdimm/claim.c @@ -247,12 +247,13 @@ static int nsio_rw_bytes(struct nd_namespace_common *ndns, long cleared; cleared = nvdimm_clear_poison(&ndns->dev, offset, size); - if (cleared != size) { - size = cleared; + if (cleared < size) rc = -EIO; + if (cleared > 0 && cleared / 512) { + cleared /= 512; + badblocks_clear(&nsio->bb, sector, cleared); } - - badblocks_clear(&nsio->bb, sector, cleared >> 9); + invalidate_pmem(nsio->addr + offset, size); } else rc = -EIO; } diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index bcc359a4e64d..ecf79fd64517 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -47,22 +47,29 @@ static struct nd_region *to_region(struct pmem_device *pmem) return to_nd_region(to_dev(pmem)->parent); } -static void pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset, +static int pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset, unsigned int len) { struct device *dev = to_dev(pmem); - sector_t sector, cleared; + sector_t sector; + long cleared; + int rc = 0; sector = (offset - pmem->data_offset) / 512; - cleared = nvdimm_clear_poison(dev, pmem->phys_addr + offset, len) / 512; - if (cleared) { - dev_dbg(dev, "%s: %#llx clear %ld sector%s\n", - __func__, (unsigned long long) sector, - cleared, cleared > 1 ? "s" : ""); + cleared = nvdimm_clear_poison(dev, pmem->phys_addr + offset, len); + if (cleared < len) + rc = -EIO; + if (cleared > 0 && cleared / 512) { + cleared /= 512; + dev_dbg(dev, "%s: %#llx clear %ld sector%s\n", __func__, + (unsigned long long) sector, cleared, + cleared > 1 ? "s" : ""); badblocks_clear(&pmem->bb, sector, cleared); } invalidate_pmem(pmem->virt_addr + offset, len); + + return rc; } static void write_pmem(void *pmem_addr, struct page *page, -- 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') 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