[RFC 0/5] block: use flag enhancement for del_gendisk()

[RFC 0/5] block: use flag enhancement for del_gendisk()

[Luis Chamberlain]

This is the third group of driver conversions for adding *add_disk*()
error handling. In this series we address the new flag added and use
on del_gendisk() so that it's easier to deal with error handling.

Luis Chamberlain (5):
  bcache: remove no longer needed add_disk() check
  bcache: add error handling support for add_disk()
  block/sx8: remove the GENHD_FL_UP check before del_gendisk()
  block/sx8: add helper carm_free_all_disks()
  block/sx8: add error handling support for add_disk()

 drivers/block/sx8.c       | 58 +++++++++++++++++++++++----------------
 drivers/md/bcache/super.c | 23 +++++++++-------
 2 files changed, 47 insertions(+), 34 deletions(-)

-- 
2.27.0

[RFC 1/5] bcache: remove no longer needed add_disk() check

[Luis Chamberlain]

After the patch "block: add flag for add_disk() completion
notation" we no longer need to check the disk was added prior
to calling del_gendisk(), del_gendisk() now deals with the
check for us.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/md/bcache/super.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 185246a0d855..70ee7a22b2a3 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -885,11 +885,7 @@ static void bcache_device_free(struct bcache_device *d)
 		bcache_device_detach(d);
 
 	if (disk) {
-		bool disk_added = (disk->flags & GENHD_FL_UP) != 0;
-
-		if (disk_added)
-			del_gendisk(disk);
-
+		del_gendisk(disk);
 		blk_cleanup_disk(disk);
 		ida_simple_remove(&bcache_device_idx,
 				  first_minor_to_idx(disk->first_minor));
-- 
2.27.0

[RFC 2/5] bcache: add error handling support for add_disk()

[Luis Chamberlain]

We never checked for errors on add_disk() as this function
returned void. Now that this is fixed, use the shiny new
error handling.

This driver doesn't do any unwinding with blk_cleanup_disk()
even on errors after add_disk() and so we follow that
tradition.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/md/bcache/super.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 70ee7a22b2a3..85da34e234d6 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1077,7 +1077,9 @@ int bch_cached_dev_run(struct cached_dev *dc)
 		closure_sync(&cl);
 	}
 
-	add_disk(d->disk);
+	ret = add_disk(d->disk);
+	if (ret)
+		goto out;
 	bd_link_disk_holder(dc->bdev, dc->disk.disk);
 	/*
 	 * won't show up in the uevent file, use udevadm monitor -e instead
@@ -1526,10 +1528,11 @@ static void flash_dev_flush(struct closure *cl)
 
 static int flash_dev_run(struct cache_set *c, struct uuid_entry *u)
 {
+	int err = -ENOMEM;
 	struct bcache_device *d = kzalloc(sizeof(struct bcache_device),
 					  GFP_KERNEL);
 	if (!d)
-		return -ENOMEM;
+		goto err_ret;
 
 	closure_init(&d->cl, NULL);
 	set_closure_fn(&d->cl, flash_dev_flush, system_wq);
@@ -1543,9 +1546,12 @@ static int flash_dev_run(struct cache_set *c, struct uuid_entry *u)
 	bcache_device_attach(d, c, u - c->uuids);
 	bch_sectors_dirty_init(d);
 	bch_flash_dev_request_init(d);
-	add_disk(d->disk);
+	err = add_disk(d->disk);
+	if (err)
+		goto err;
 
-	if (kobject_add(&d->kobj, &disk_to_dev(d->disk)->kobj, "bcache"))
+	err = kobject_add(&d->kobj, &disk_to_dev(d->disk)->kobj, "bcache");
+	if (err)
 		goto err;
 
 	bcache_device_link(d, c, "volume");
@@ -1559,7 +1565,8 @@ static int flash_dev_run(struct cache_set *c, struct uuid_entry *u)
 	return 0;
 err:
 	kobject_put(&d->kobj);
-	return -ENOMEM;
+err_ret:
+	return err;
 }
 
 static int flash_devs_run(struct cache_set *c)
-- 
2.27.0

[RFC 3/5] block/sx8: remove the GENHD_FL_UP check before del_gendisk()

[Luis Chamberlain]

This is no longer required, given the block layer adds a flag
now for us on a disk once add_disk() completes successfully,
and so del_gendisk() will only really run if that flag is set.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/sx8.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/block/sx8.c b/drivers/block/sx8.c
index 7b54353ee92b..e4dfee5acf08 100644
--- a/drivers/block/sx8.c
+++ b/drivers/block/sx8.c
@@ -1373,8 +1373,7 @@ static void carm_free_disk(struct carm_host *host, unsigned int port_no)
 	if (!disk)
 		return;
 
-	if (disk->flags & GENHD_FL_UP)
-		del_gendisk(disk);
+	del_gendisk(disk);
 	blk_cleanup_disk(disk);
 }
 
-- 
2.27.0

[RFC 4/5] block/sx8: add helper carm_free_all_disks()

[Luis Chamberlain]

Share the code of unregistering disks in a common helper.
Code is shifted a above so that we can later re-use this
helper in other places.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/sx8.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/block/sx8.c b/drivers/block/sx8.c
index e4dfee5acf08..6a6dc3fffa5c 100644
--- a/drivers/block/sx8.c
+++ b/drivers/block/sx8.c
@@ -1092,6 +1092,27 @@ static irqreturn_t carm_interrupt(int irq, void *__host)
 	return IRQ_RETVAL(handled);
 }
 
+static void carm_free_disk(struct carm_host *host, unsigned int port_no)
+{
+	struct carm_port *port = &host->port[port_no];
+	struct gendisk *disk = port->disk;
+
+	if (!disk)
+		return;
+
+	del_gendisk(disk);
+	blk_cleanup_disk(disk);
+}
+
+static void carm_free_all_disks(struct carm_host *host)
+{
+	unsigned int i;
+
+	for (i = 0; i < CARM_MAX_PORTS; i++)
+		carm_free_disk(host, i);
+	unregister_blkdev(host->major, host->name);
+}
+
 static void carm_fsm_task (struct work_struct *work)
 {
 	struct carm_host *host =
@@ -1365,18 +1386,6 @@ static int carm_init_disk(struct carm_host *host, unsigned int port_no)
 	return 0;
 }
 
-static void carm_free_disk(struct carm_host *host, unsigned int port_no)
-{
-	struct carm_port *port = &host->port[port_no];
-	struct gendisk *disk = port->disk;
-
-	if (!disk)
-		return;
-
-	del_gendisk(disk);
-	blk_cleanup_disk(disk);
-}
-
 static int carm_init_shm(struct carm_host *host)
 {
 	host->shm = dma_alloc_coherent(&host->pdev->dev, CARM_SHM_SIZE,
@@ -1520,9 +1529,7 @@ static int carm_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 err_out_free_irq:
 	free_irq(pdev->irq, host);
 err_out_blkdev_disks:
-	for (i = 0; i < CARM_MAX_PORTS; i++)
-		carm_free_disk(host, i);
-	unregister_blkdev(host->major, host->name);
+	carm_free_all_disks(host);
 err_out_free_majors:
 	if (host->major == 160)
 		clear_bit(0, &carm_major_alloc);
@@ -1546,7 +1553,6 @@ static int carm_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 static void carm_remove_one (struct pci_dev *pdev)
 {
 	struct carm_host *host = pci_get_drvdata(pdev);
-	unsigned int i;
 
 	if (!host) {
 		printk(KERN_ERR PFX "BUG: no host data for PCI(%s)\n",
@@ -1555,9 +1561,7 @@ static void carm_remove_one (struct pci_dev *pdev)
 	}
 
 	free_irq(pdev->irq, host);
-	for (i = 0; i < CARM_MAX_PORTS; i++)
-		carm_free_disk(host, i);
-	unregister_blkdev(host->major, host->name);
+	carm_free_all_disks(host);
 	if (host->major == 160)
 		clear_bit(0, &carm_major_alloc);
 	else if (host->major == 161)
-- 
2.27.0

[RFC 5/5] block/sx8: add error handling support for add_disk()

[Luis Chamberlain]

We never checked for errors on add_disk() as this function
returned void. Now that this is fixed, use the shiny new
error handling.

A completion is used to notify the initial probe what is
happening and so we must defer error handling on completion.
Do this by remembering the error and using the shared cleanup
function.

The tags are shared and so are hanlded later for the
driver already.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/sx8.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/block/sx8.c b/drivers/block/sx8.c
index 6a6dc3fffa5c..34bbd033e2fd 100644
--- a/drivers/block/sx8.c
+++ b/drivers/block/sx8.c
@@ -297,6 +297,7 @@ struct carm_host {
 
 	struct work_struct		fsm_task;
 
+	int probe_err;
 	struct completion		probe_comp;
 };
 
@@ -1202,8 +1203,11 @@ static void carm_fsm_task (struct work_struct *work)
 				struct gendisk *disk = port->disk;
 
 				set_capacity(disk, port->capacity);
-				add_disk(disk);
-				activated++;
+				host->probe_err = add_disk(disk);
+				if (!host->probe_err)
+					activated++;
+				else
+					break;
 			}
 
 		printk(KERN_INFO DRV_NAME "(%s): %d ports activated\n",
@@ -1213,11 +1217,9 @@ static void carm_fsm_task (struct work_struct *work)
 		reschedule = 1;
 		break;
 	}
-
 	case HST_PROBE_FINISHED:
 		complete(&host->probe_comp);
 		break;
-
 	case HST_ERROR:
 		/* FIXME: TODO */
 		break;
@@ -1515,7 +1517,10 @@ static int carm_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_out_free_irq;
 
 	DPRINTK("waiting for probe_comp\n");
+	host->probe_err = -ENODEV;
 	wait_for_completion(&host->probe_comp);
+	if (host->probe_err)
+		goto err_out_disks;
 
 	printk(KERN_INFO "%s: pci %s, ports %d, io %llx, irq %u, major %d\n",
 	       host->name, pci_name(pdev), (int) CARM_MAX_PORTS,
@@ -1526,6 +1531,8 @@ static int carm_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 	pci_set_drvdata(pdev, host);
 	return 0;
 
+err_out_disks:
+	carm_free_all_disks(host);
 err_out_free_irq:
 	free_irq(pdev->irq, host);
 err_out_blkdev_disks:
-- 
2.27.0