| Message ID | 20150430221144.26376.30479.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
[ adding Linda, Toshi, and Robert ] On Thu, Apr 30, 2015 at 3:15 PM, Dan Williams <dan.j.williams@intel.com> wrote: > >From Toshi: > > We have seen a case that the region#->pmem# binding becomes > inconsistent across a reboot when there are 8 NVDIMM cards > (reported by Robert Elliott). This leads user to access a > wrong device. > > While it is a bug for userspace to depend on stable device names from > boot-to-boot, we should try not to give random results in the case where > the configuration has not changed. Now that PMEM only ever attaches to > 'region' devices emitted by libnd, we can simply carry the region-id all > the way down to the pmem device name. > > Since BLK-regions allow multiple namespaces per-region we need to carry > the <region-id>.<namespace-index> in the block device name if we want > stable(ish) device names. > > Cc: Christoph Hellwig <hch@lst.de> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com> > Reported-by: Toshi Kani <toshi.kani@hp.com> > Reported-by: Robert Elliott <Elliott@hp.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > > Will fold this into the v3 posting. v3 will be a rebase on top of the > upcoming ACPICA NFIT enabling. > > drivers/block/nd/blk.c | 14 ++------------ > drivers/block/nd/pmem.c | 21 +++++---------------- > 2 files changed, 7 insertions(+), 28 deletions(-) > > diff --git a/drivers/block/nd/blk.c b/drivers/block/nd/blk.c > index 8536ee8b2009..9a2b93bdeccc 100644 > --- a/drivers/block/nd/blk.c > +++ b/drivers/block/nd/blk.c > @@ -28,11 +28,9 @@ struct nd_blk_device { > struct nd_blk_region *ndbr; > struct nd_io ndio; > size_t disk_size; > - int id; > }; > > static int nd_blk_major; > -static DEFINE_IDA(nd_blk_ida); > > static resource_size_t to_dev_offset(struct nd_namespace_blk *nsblk, > resource_size_t ns_offset, unsigned int len) > @@ -139,6 +137,7 @@ static const struct block_device_operations nd_blk_fops = { > static int nd_blk_probe(struct device *dev) > { > struct nd_namespace_blk *nsblk = to_nd_namespace_blk(dev); > + struct nd_region *nd_region = to_nd_region(dev->parent); > struct nd_blk_device *blk_dev; > resource_size_t disk_size; > struct gendisk *disk; > @@ -152,12 +151,6 @@ static int nd_blk_probe(struct device *dev) > if (!blk_dev) > return -ENOMEM; > > - blk_dev->id = ida_simple_get(&nd_blk_ida, 0, 0, GFP_KERNEL); > - if (blk_dev->id < 0) { > - err = blk_dev->id; > - goto err_ida; > - } > - > blk_dev->disk_size = disk_size; > > blk_dev->queue = blk_alloc_queue(GFP_KERNEL); > @@ -186,7 +179,7 @@ static int nd_blk_probe(struct device *dev) > disk->private_data = blk_dev; > disk->queue = blk_dev->queue; > disk->flags = GENHD_FL_EXT_DEVT; > - sprintf(disk->disk_name, "ndblk%d", blk_dev->id); > + sprintf(disk->disk_name, "ndblk%d.%d", nd_region->id, nsblk->id); > set_capacity(disk, disk_size >> SECTOR_SHIFT); > > nd_bus_lock(dev); > @@ -202,8 +195,6 @@ static int nd_blk_probe(struct device *dev) > err_alloc_disk: > blk_cleanup_queue(blk_dev->queue); > err_alloc_queue: > - ida_simple_remove(&nd_blk_ida, blk_dev->id); > - err_ida: > kfree(blk_dev); > return err; > } > @@ -219,7 +210,6 @@ static int nd_blk_remove(struct device *dev) > del_gendisk(blk_dev->disk); > put_disk(blk_dev->disk); > blk_cleanup_queue(blk_dev->queue); > - ida_simple_remove(&nd_blk_ida, blk_dev->id); > kfree(blk_dev); > > return 0; > diff --git a/drivers/block/nd/pmem.c b/drivers/block/nd/pmem.c > index 900dad61a6b9..5814c8aed481 100644 > --- a/drivers/block/nd/pmem.c > +++ b/drivers/block/nd/pmem.c > @@ -37,11 +37,9 @@ struct pmem_device { > phys_addr_t phys_addr; > void *virt_addr; > size_t size; > - int id; > }; > > static int pmem_major; > -static DEFINE_IDA(pmem_ida); > > static void pmem_do_bvec(struct pmem_device *pmem, struct page *page, > unsigned int len, unsigned int off, int rw, > @@ -142,7 +140,7 @@ static const struct block_device_operations pmem_fops = { > .direct_access = pmem_direct_access, > }; > > -static struct pmem_device *pmem_alloc(struct device *dev, struct resource *res) > +static struct pmem_device *pmem_alloc(struct device *dev, struct resource *res, int id) > { > struct pmem_device *pmem; > struct gendisk *disk; > @@ -153,19 +151,13 @@ static struct pmem_device *pmem_alloc(struct device *dev, struct resource *res) > if (!pmem) > goto out; > > - pmem->id = ida_simple_get(&pmem_ida, 0, 0, GFP_KERNEL); > - if (pmem->id < 0) { > - err = pmem->id; > - goto out_free_dev; > - } > - > pmem->phys_addr = res->start; > pmem->size = resource_size(res); > > err = -EINVAL; > if (!request_mem_region(pmem->phys_addr, pmem->size, "pmem")) { > dev_warn(dev, "could not reserve region [0x%pa:0x%zx]\n", &pmem->phys_addr, pmem->size); > - goto out_free_ida; > + goto out_free_dev; > } > > /* > @@ -190,12 +182,12 @@ static struct pmem_device *pmem_alloc(struct device *dev, struct resource *res) > goto out_free_queue; > > disk->major = pmem_major; > - disk->first_minor = PMEM_MINORS * pmem->id; > + disk->first_minor = PMEM_MINORS * id; > disk->fops = &pmem_fops; > disk->private_data = pmem; > disk->queue = pmem->pmem_queue; > disk->flags = GENHD_FL_EXT_DEVT; > - sprintf(disk->disk_name, "pmem%d", pmem->id); > + sprintf(disk->disk_name, "pmem%d", id); > disk->driverfs_dev = dev; > set_capacity(disk, pmem->size >> 9); > pmem->pmem_disk = disk; > @@ -208,8 +200,6 @@ out_unmap: > iounmap(pmem->virt_addr); > out_release_region: > release_mem_region(pmem->phys_addr, pmem->size); > -out_free_ida: > - ida_simple_remove(&pmem_ida, pmem->id); > out_free_dev: > kfree(pmem); > out: > @@ -223,7 +213,6 @@ static void pmem_free(struct pmem_device *pmem) > blk_cleanup_queue(pmem->pmem_queue); > iounmap(pmem->virt_addr); > release_mem_region(pmem->phys_addr, pmem->size); > - ida_simple_remove(&pmem_ida, pmem->id); > kfree(pmem); > } > > @@ -250,7 +239,7 @@ static int nd_pmem_probe(struct device *dev) > } > } > > - pmem = pmem_alloc(dev, &nsio->res); > + pmem = pmem_alloc(dev, &nsio->res, nd_region->id); > if (IS_ERR(pmem)) > return PTR_ERR(pmem); > > > _______________________________________________ > Linux-nvdimm mailing list > Linux-nvdimm@lists.01.org > https://lists.01.org/mailman/listinfo/linux-nvdimm
diff --git a/drivers/block/nd/blk.c b/drivers/block/nd/blk.c index 8536ee8b2009..9a2b93bdeccc 100644 --- a/drivers/block/nd/blk.c +++ b/drivers/block/nd/blk.c @@ -28,11 +28,9 @@ struct nd_blk_device { struct nd_blk_region *ndbr; struct nd_io ndio; size_t disk_size; - int id; }; static int nd_blk_major; -static DEFINE_IDA(nd_blk_ida); static resource_size_t to_dev_offset(struct nd_namespace_blk *nsblk, resource_size_t ns_offset, unsigned int len) @@ -139,6 +137,7 @@ static const struct block_device_operations nd_blk_fops = { static int nd_blk_probe(struct device *dev) { struct nd_namespace_blk *nsblk = to_nd_namespace_blk(dev); + struct nd_region *nd_region = to_nd_region(dev->parent); struct nd_blk_device *blk_dev; resource_size_t disk_size; struct gendisk *disk; @@ -152,12 +151,6 @@ static int nd_blk_probe(struct device *dev) if (!blk_dev) return -ENOMEM; - blk_dev->id = ida_simple_get(&nd_blk_ida, 0, 0, GFP_KERNEL); - if (blk_dev->id < 0) { - err = blk_dev->id; - goto err_ida; - } - blk_dev->disk_size = disk_size; blk_dev->queue = blk_alloc_queue(GFP_KERNEL); @@ -186,7 +179,7 @@ static int nd_blk_probe(struct device *dev) disk->private_data = blk_dev; disk->queue = blk_dev->queue; disk->flags = GENHD_FL_EXT_DEVT; - sprintf(disk->disk_name, "ndblk%d", blk_dev->id); + sprintf(disk->disk_name, "ndblk%d.%d", nd_region->id, nsblk->id); set_capacity(disk, disk_size >> SECTOR_SHIFT); nd_bus_lock(dev); @@ -202,8 +195,6 @@ static int nd_blk_probe(struct device *dev) err_alloc_disk: blk_cleanup_queue(blk_dev->queue); err_alloc_queue: - ida_simple_remove(&nd_blk_ida, blk_dev->id); - err_ida: kfree(blk_dev); return err; } @@ -219,7 +210,6 @@ static int nd_blk_remove(struct device *dev) del_gendisk(blk_dev->disk); put_disk(blk_dev->disk); blk_cleanup_queue(blk_dev->queue); - ida_simple_remove(&nd_blk_ida, blk_dev->id); kfree(blk_dev); return 0; diff --git a/drivers/block/nd/pmem.c b/drivers/block/nd/pmem.c index 900dad61a6b9..5814c8aed481 100644 --- a/drivers/block/nd/pmem.c +++ b/drivers/block/nd/pmem.c @@ -37,11 +37,9 @@ struct pmem_device { phys_addr_t phys_addr; void *virt_addr; size_t size; - int id; }; static int pmem_major; -static DEFINE_IDA(pmem_ida); static void pmem_do_bvec(struct pmem_device *pmem, struct page *page, unsigned int len, unsigned int off, int rw, @@ -142,7 +140,7 @@ static const struct block_device_operations pmem_fops = { .direct_access = pmem_direct_access, }; -static struct pmem_device *pmem_alloc(struct device *dev, struct resource *res) +static struct pmem_device *pmem_alloc(struct device *dev, struct resource *res, int id) { struct pmem_device *pmem; struct gendisk *disk; @@ -153,19 +151,13 @@ static struct pmem_device *pmem_alloc(struct device *dev, struct resource *res) if (!pmem) goto out; - pmem->id = ida_simple_get(&pmem_ida, 0, 0, GFP_KERNEL); - if (pmem->id < 0) { - err = pmem->id; - goto out_free_dev; - } - pmem->phys_addr = res->start; pmem->size = resource_size(res); err = -EINVAL; if (!request_mem_region(pmem->phys_addr, pmem->size, "pmem")) { dev_warn(dev, "could not reserve region [0x%pa:0x%zx]\n", &pmem->phys_addr, pmem->size); - goto out_free_ida; + goto out_free_dev; } /* @@ -190,12 +182,12 @@ static struct pmem_device *pmem_alloc(struct device *dev, struct resource *res) goto out_free_queue; disk->major = pmem_major; - disk->first_minor = PMEM_MINORS * pmem->id; + disk->first_minor = PMEM_MINORS * id; disk->fops = &pmem_fops; disk->private_data = pmem; disk->queue = pmem->pmem_queue; disk->flags = GENHD_FL_EXT_DEVT; - sprintf(disk->disk_name, "pmem%d", pmem->id); + sprintf(disk->disk_name, "pmem%d", id); disk->driverfs_dev = dev; set_capacity(disk, pmem->size >> 9); pmem->pmem_disk = disk; @@ -208,8 +200,6 @@ out_unmap: iounmap(pmem->virt_addr); out_release_region: release_mem_region(pmem->phys_addr, pmem->size); -out_free_ida: - ida_simple_remove(&pmem_ida, pmem->id); out_free_dev: kfree(pmem); out: @@ -223,7 +213,6 @@ static void pmem_free(struct pmem_device *pmem) blk_cleanup_queue(pmem->pmem_queue); iounmap(pmem->virt_addr); release_mem_region(pmem->phys_addr, pmem->size); - ida_simple_remove(&pmem_ida, pmem->id); kfree(pmem); } @@ -250,7 +239,7 @@ static int nd_pmem_probe(struct device *dev) } } - pmem = pmem_alloc(dev, &nsio->res); + pmem = pmem_alloc(dev, &nsio->res, nd_region->id); if (IS_ERR(pmem)) return PTR_ERR(pmem);