From 3aefb5ee843fbe4789d03bb181e190d462df95e4 Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Wed, 3 Nov 2021 09:58:43 -0700 Subject: nvdimm/btt: do not call del_gendisk() if not needed del_gendisk() should not called if the disk has not been added. Fix this. Fixes: 41cd8b70c37a ("libnvdimm, btt: add support for blk integrity") Reviewed-by: Dan Williams Reviewed-by: Christoph Hellwig Signed-off-by: Luis Chamberlain Link: https://lore.kernel.org/r/20211103165843.1402142-1-mcgrof@kernel.org Signed-off-by: Jens Axboe --- drivers/nvdimm/btt.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/nvdimm') diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c index 4295fa809420..9b9d5e506e11 100644 --- a/drivers/nvdimm/btt.c +++ b/drivers/nvdimm/btt.c @@ -1537,7 +1537,6 @@ static int btt_blk_init(struct btt *btt) int rc = nd_integrity_init(btt->btt_disk, btt_meta_size(btt)); if (rc) { - del_gendisk(btt->btt_disk); blk_cleanup_disk(btt->btt_disk); return rc; } -- cgit v1.2.3 From 2762ff06aa49e3a13fb4b779120f4f8c12c39fd1 Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Wed, 3 Nov 2021 16:04:24 -0700 Subject: nvdimm/btt: use goto error labels on btt_blk_init() This will make it easier to share common error paths. Reviewed-by: Dan Williams Reviewed-by: Christoph Hellwig Signed-off-by: Luis Chamberlain Link: https://lore.kernel.org/r/20211103230437.1639990-2-mcgrof@kernel.org Signed-off-by: Jens Axboe --- drivers/nvdimm/btt.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'drivers/nvdimm') diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c index 9b9d5e506e11..5cb6d7ac6e36 100644 --- a/drivers/nvdimm/btt.c +++ b/drivers/nvdimm/btt.c @@ -1519,6 +1519,7 @@ static int btt_blk_init(struct btt *btt) { struct nd_btt *nd_btt = btt->nd_btt; struct nd_namespace_common *ndns = nd_btt->ndns; + int rc = -ENOMEM; btt->btt_disk = blk_alloc_disk(NUMA_NO_NODE); if (!btt->btt_disk) @@ -1534,19 +1535,22 @@ static int btt_blk_init(struct btt *btt) blk_queue_flag_set(QUEUE_FLAG_NONROT, btt->btt_disk->queue); if (btt_meta_size(btt)) { - int rc = nd_integrity_init(btt->btt_disk, btt_meta_size(btt)); - - if (rc) { - blk_cleanup_disk(btt->btt_disk); - return rc; - } + rc = nd_integrity_init(btt->btt_disk, btt_meta_size(btt)); + if (rc) + goto out_cleanup_disk; } + set_capacity(btt->btt_disk, btt->nlba * btt->sector_size >> 9); device_add_disk(&btt->nd_btt->dev, btt->btt_disk, NULL); + btt->nd_btt->size = btt->nlba * (u64)btt->sector_size; nvdimm_check_and_set_ro(btt->btt_disk); return 0; + +out_cleanup_disk: + blk_cleanup_disk(btt->btt_disk); + return rc; } static void btt_blk_cleanup(struct btt *btt) -- cgit v1.2.3 From 16be7974ff5d0a5cd9f345571c3eac1c3f6ba6de Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Wed, 3 Nov 2021 16:04:25 -0700 Subject: nvdimm/btt: add error handling support for add_disk() We never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling. Reviewed-by: Christoph Hellwig Signed-off-by: Luis Chamberlain Link: https://lore.kernel.org/r/20211103230437.1639990-3-mcgrof@kernel.org Signed-off-by: Jens Axboe --- drivers/nvdimm/btt.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/nvdimm') diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c index 5cb6d7ac6e36..38ed53eeea5e 100644 --- a/drivers/nvdimm/btt.c +++ b/drivers/nvdimm/btt.c @@ -1541,7 +1541,9 @@ static int btt_blk_init(struct btt *btt) } set_capacity(btt->btt_disk, btt->nlba * btt->sector_size >> 9); - device_add_disk(&btt->nd_btt->dev, btt->btt_disk, NULL); + rc = device_add_disk(&btt->nd_btt->dev, btt->btt_disk, NULL); + if (rc) + goto out_cleanup_disk; btt->nd_btt->size = btt->nlba * (u64)btt->sector_size; nvdimm_check_and_set_ro(btt->btt_disk); -- cgit v1.2.3 From b7421afcec0c77ab58633587ddc29d53e6eb95af Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Wed, 3 Nov 2021 16:04:26 -0700 Subject: nvdimm/blk: avoid calling del_gendisk() on early failures If nd_integrity_init() fails we'd get del_gendisk() called, but that's not correct as we should only call that if we're done with device_add_disk(). Fix this by providing unwinding prior to the devm call being registered and moving the devm registration to the very end. This should fix calling del_gendisk() if nd_integrity_init() fails. I only spotted this issue through code inspection. It does not fix any real world bug. Reviewed-by: Christoph Hellwig Signed-off-by: Luis Chamberlain Link: https://lore.kernel.org/r/20211103230437.1639990-4-mcgrof@kernel.org Signed-off-by: Jens Axboe --- drivers/nvdimm/blk.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'drivers/nvdimm') diff --git a/drivers/nvdimm/blk.c b/drivers/nvdimm/blk.c index b6c6866f9259..4eef67918a7e 100644 --- a/drivers/nvdimm/blk.c +++ b/drivers/nvdimm/blk.c @@ -239,6 +239,7 @@ static int nsblk_attach_disk(struct nd_namespace_blk *nsblk) resource_size_t available_disk_size; struct gendisk *disk; u64 internal_nlba; + int rc; internal_nlba = div_u64(nsblk->size, nsblk_internal_lbasize(nsblk)); available_disk_size = internal_nlba * nsblk_sector_size(nsblk); @@ -255,20 +256,26 @@ static int nsblk_attach_disk(struct nd_namespace_blk *nsblk) blk_queue_logical_block_size(disk->queue, nsblk_sector_size(nsblk)); blk_queue_flag_set(QUEUE_FLAG_NONROT, disk->queue); - if (devm_add_action_or_reset(dev, nd_blk_release_disk, disk)) - return -ENOMEM; - if (nsblk_meta_size(nsblk)) { - int rc = nd_integrity_init(disk, nsblk_meta_size(nsblk)); + rc = nd_integrity_init(disk, nsblk_meta_size(nsblk)); if (rc) - return rc; + goto out_before_devm_err; } set_capacity(disk, available_disk_size >> SECTOR_SHIFT); device_add_disk(dev, disk, NULL); + + /* nd_blk_release_disk() is called if this fails */ + if (devm_add_action_or_reset(dev, nd_blk_release_disk, disk)) + return -ENOMEM; + nvdimm_check_and_set_ro(disk); return 0; + +out_before_devm_err: + blk_cleanup_disk(disk); + return rc; } static int nd_blk_probe(struct device *dev) -- cgit v1.2.3 From dc104f4bb2d0a652dee010e47bc89c1ad2ab37c9 Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Wed, 3 Nov 2021 16:04:27 -0700 Subject: nvdimm/blk: add error handling support for add_disk() We never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling. Since nvdimm/blk uses devm we just need to move the devm registration towards the end. And in hindsight, that seems to also provide a fix given del_gendisk() should not be called unless the disk was already added via add_disk(). The probably of that issue happening is low though, like OOM while calling devm_add_action(), so the fix is minor. We manually unwind in case of add_disk() failure prior to the devm registration. Reviewed-by: Dan Williams Reviewed-by: Christoph Hellwig Signed-off-by: Luis Chamberlain Link: https://lore.kernel.org/r/20211103230437.1639990-5-mcgrof@kernel.org Signed-off-by: Jens Axboe --- drivers/nvdimm/blk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/nvdimm') diff --git a/drivers/nvdimm/blk.c b/drivers/nvdimm/blk.c index 4eef67918a7e..228c33b8d1d6 100644 --- a/drivers/nvdimm/blk.c +++ b/drivers/nvdimm/blk.c @@ -264,7 +264,9 @@ static int nsblk_attach_disk(struct nd_namespace_blk *nsblk) } set_capacity(disk, available_disk_size >> SECTOR_SHIFT); - device_add_disk(dev, disk, NULL); + rc = device_add_disk(dev, disk, NULL); + if (rc) + goto out_before_devm_err; /* nd_blk_release_disk() is called if this fails */ if (devm_add_action_or_reset(dev, nd_blk_release_disk, disk)) -- cgit v1.2.3 From accf58afb689f81daadde24080ea1164ad2db75f Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Wed, 3 Nov 2021 16:04:28 -0700 Subject: nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned Prior to devm being able to use pmem_release_disk() there are other failure which can occur for which we must account for and release the disk for. Address those few cases. Fixes: 3dd60fb9d95d ("nvdimm/pmem: stop using q_usage_count as external pgmap refcount") Reviewed-by: Christoph Hellwig Signed-off-by: Luis Chamberlain Link: https://lore.kernel.org/r/20211103230437.1639990-6-mcgrof@kernel.org Signed-off-by: Jens Axboe --- drivers/nvdimm/pmem.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'drivers/nvdimm') diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index a67a3ad1d413..cbb3662391c2 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -457,8 +457,10 @@ static int pmem_attach_disk(struct device *dev, bb_range.end = res->end; } - if (IS_ERR(addr)) - return PTR_ERR(addr); + if (IS_ERR(addr)) { + rc = PTR_ERR(addr); + goto out; + } pmem->virt_addr = addr; blk_queue_write_cache(q, true, fua); @@ -483,7 +485,8 @@ static int pmem_attach_disk(struct device *dev, flags = DAXDEV_F_SYNC; dax_dev = alloc_dax(pmem, disk->disk_name, &pmem_dax_ops, flags); if (IS_ERR(dax_dev)) { - return PTR_ERR(dax_dev); + rc = PTR_ERR(dax_dev); + goto out; } dax_write_cache(dax_dev, nvdimm_has_cache(nd_region)); pmem->dax_dev = dax_dev; @@ -498,8 +501,10 @@ static int pmem_attach_disk(struct device *dev, "badblocks"); if (!pmem->bb_state) dev_warn(dev, "'badblocks' notification disabled\n"); - return 0; +out: + blk_cleanup_disk(pmem->disk); + return rc; } static int nd_pmem_probe(struct device *dev) -- cgit v1.2.3 From 5a192ccc32e2981f721343c750b8cfb4c3f41007 Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Wed, 3 Nov 2021 16:04:29 -0700 Subject: nvdimm/pmem: use add_disk() error handling Now that device_add_disk() supports returning an error, use that. We must unwind alloc_dax() on error. Reviewed-by: Christoph Hellwig Signed-off-by: Luis Chamberlain Link: https://lore.kernel.org/r/20211103230437.1639990-7-mcgrof@kernel.org Signed-off-by: Jens Axboe --- drivers/nvdimm/pmem.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'drivers/nvdimm') diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index cbb3662391c2..6052091f6f59 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -491,7 +491,9 @@ static int pmem_attach_disk(struct device *dev, dax_write_cache(dax_dev, nvdimm_has_cache(nd_region)); pmem->dax_dev = dax_dev; - device_add_disk(dev, disk, pmem_attribute_groups); + rc = device_add_disk(dev, disk, pmem_attribute_groups); + if (rc) + goto out_cleanup_dax; if (devm_add_action_or_reset(dev, pmem_release_disk, pmem)) return -ENOMEM; @@ -502,6 +504,10 @@ static int pmem_attach_disk(struct device *dev, if (!pmem->bb_state) dev_warn(dev, "'badblocks' notification disabled\n"); return 0; + +out_cleanup_dax: + kill_dax(pmem->dax_dev); + put_dax(pmem->dax_dev); out: blk_cleanup_disk(pmem->disk); return rc; -- cgit v1.2.3