linux-bcache.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* remove GENHD_FL_UP
@ 2021-08-09  6:40 Christoph Hellwig
  2021-08-09  6:40 ` [PATCH 1/8] mmc: block: let device_add_disk create disk attributes Christoph Hellwig
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-08-09  6:40 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Coly Li, Song Liu, Ulf Hansson, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

Hi Jens,

this series first cleans up various drivers to not rely on the
internal GENHD_FL_UP to decided if they need to call del_gendisk
and then removes the flag entirely.

Diffstat:
 block/genhd.c                 |    6 -
 block/partitions/core.c       |    4 -
 drivers/block/sx8.c           |    2 
 drivers/md/bcache/super.c     |   26 ++++---
 drivers/md/md.h               |    4 -
 drivers/mmc/core/block.c      |  143 +++++++++++++++++-------------------------
 drivers/nvme/host/core.c      |   16 ++--
 drivers/nvme/host/multipath.c |    2 
 fs/block_dev.c                |    2 
 include/linux/genhd.h         |    9 +-
 10 files changed, 93 insertions(+), 121 deletions(-)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/8] mmc: block: let device_add_disk create disk attributes
  2021-08-09  6:40 remove GENHD_FL_UP Christoph Hellwig
@ 2021-08-09  6:40 ` Christoph Hellwig
  2021-08-09 11:30   ` Ulf Hansson
  2021-08-09  6:40 ` [PATCH 2/8] mmc: block: cleanup gendisk creation Christoph Hellwig
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2021-08-09  6:40 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Coly Li, Song Liu, Ulf Hansson, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

Pass the attribute group for the attributes on the gendisk to
device_add_disk so that they are created atomically with the
disk creation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/mmc/core/block.c | 102 +++++++++++++++++----------------------
 1 file changed, 45 insertions(+), 57 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index ce8aed562929..4ac3e1b93e7e 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -128,8 +128,6 @@ struct mmc_blk_data {
 	 * track of the current selected device partition.
 	 */
 	unsigned int	part_curr;
-	struct device_attribute force_ro;
-	struct device_attribute power_ro_lock;
 	int	area_type;
 
 	/* debugfs files (only in main mmc_blk_data) */
@@ -281,6 +279,9 @@ static ssize_t power_ro_lock_store(struct device *dev,
 	return count;
 }
 
+static DEVICE_ATTR(ro_lock_until_next_power_on, 0,
+		power_ro_lock_show, power_ro_lock_store);
+
 static ssize_t force_ro_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
@@ -313,6 +314,44 @@ static ssize_t force_ro_store(struct device *dev, struct device_attribute *attr,
 	return ret;
 }
 
+static DEVICE_ATTR(force_ro, 0644, force_ro_show, force_ro_store);
+
+static struct attribute *mmc_disk_attrs[] = {
+	&dev_attr_force_ro.attr,
+	&dev_attr_ro_lock_until_next_power_on.attr,
+	NULL,
+};
+
+static umode_t mmc_disk_attrs_is_visible(struct kobject *kobj,
+		struct attribute *a, int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct mmc_blk_data *md = mmc_blk_get(dev_to_disk(dev));
+	umode_t mode = a->mode;
+
+	if (a == &dev_attr_ro_lock_until_next_power_on.attr &&
+	    (md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
+	    md->queue.card->ext_csd.boot_ro_lockable) {
+		mode = S_IRUGO;
+		if (!(md->queue.card->ext_csd.boot_ro_lock &
+				EXT_CSD_BOOT_WP_B_PWR_WP_DIS))
+			mode |= S_IWUSR;
+	}
+
+	mmc_blk_put(md);
+	return mode;
+}
+
+static const struct attribute_group mmc_disk_attr_group = {
+	.is_visible	= mmc_disk_attrs_is_visible,
+	.attrs		= mmc_disk_attrs,
+};
+
+static const struct attribute_group *mmc_disk_attr_groups[] = {
+	&mmc_disk_attr_group,
+	NULL,
+};
+
 static int mmc_blk_open(struct block_device *bdev, fmode_t mode)
 {
 	struct mmc_blk_data *md = mmc_blk_get(bdev->bd_disk);
@@ -2644,15 +2683,8 @@ static void mmc_blk_remove_req(struct mmc_blk_data *md)
 		 * from being accepted.
 		 */
 		card = md->queue.card;
-		if (md->disk->flags & GENHD_FL_UP) {
-			device_remove_file(disk_to_dev(md->disk), &md->force_ro);
-			if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
-					card->ext_csd.boot_ro_lockable)
-				device_remove_file(disk_to_dev(md->disk),
-					&md->power_ro_lock);
-
+		if (md->disk->flags & GENHD_FL_UP)
 			del_gendisk(md->disk);
-		}
 		mmc_cleanup_queue(&md->queue);
 		mmc_blk_put(md);
 	}
@@ -2679,51 +2711,6 @@ static void mmc_blk_remove_parts(struct mmc_card *card,
 	}
 }
 
-static int mmc_add_disk(struct mmc_blk_data *md)
-{
-	int ret;
-	struct mmc_card *card = md->queue.card;
-
-	device_add_disk(md->parent, md->disk, NULL);
-	md->force_ro.show = force_ro_show;
-	md->force_ro.store = force_ro_store;
-	sysfs_attr_init(&md->force_ro.attr);
-	md->force_ro.attr.name = "force_ro";
-	md->force_ro.attr.mode = S_IRUGO | S_IWUSR;
-	ret = device_create_file(disk_to_dev(md->disk), &md->force_ro);
-	if (ret)
-		goto force_ro_fail;
-
-	if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
-	     card->ext_csd.boot_ro_lockable) {
-		umode_t mode;
-
-		if (card->ext_csd.boot_ro_lock & EXT_CSD_BOOT_WP_B_PWR_WP_DIS)
-			mode = S_IRUGO;
-		else
-			mode = S_IRUGO | S_IWUSR;
-
-		md->power_ro_lock.show = power_ro_lock_show;
-		md->power_ro_lock.store = power_ro_lock_store;
-		sysfs_attr_init(&md->power_ro_lock.attr);
-		md->power_ro_lock.attr.mode = mode;
-		md->power_ro_lock.attr.name =
-					"ro_lock_until_next_power_on";
-		ret = device_create_file(disk_to_dev(md->disk),
-				&md->power_ro_lock);
-		if (ret)
-			goto power_ro_lock_fail;
-	}
-	return ret;
-
-power_ro_lock_fail:
-	device_remove_file(disk_to_dev(md->disk), &md->force_ro);
-force_ro_fail:
-	del_gendisk(md->disk);
-
-	return ret;
-}
-
 #ifdef CONFIG_DEBUG_FS
 
 static int mmc_dbg_card_status_get(void *data, u64 *val)
@@ -2919,12 +2906,13 @@ static int mmc_blk_probe(struct mmc_card *card)
 
 	dev_set_drvdata(&card->dev, md);
 
-	ret = mmc_add_disk(md);
+	device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
 	if (ret)
 		goto out;
 
 	list_for_each_entry(part_md, &md->part, part) {
-		ret = mmc_add_disk(part_md);
+		device_add_disk(part_md->parent, part_md->disk,
+				mmc_disk_attr_groups);
 		if (ret)
 			goto out;
 	}
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/8] mmc: block: cleanup gendisk creation
  2021-08-09  6:40 remove GENHD_FL_UP Christoph Hellwig
  2021-08-09  6:40 ` [PATCH 1/8] mmc: block: let device_add_disk create disk attributes Christoph Hellwig
@ 2021-08-09  6:40 ` Christoph Hellwig
  2021-08-09 11:30   ` Ulf Hansson
  2021-08-09  6:40 ` [PATCH 3/8] nvme: remove the GENHD_FL_UP check in nvme_ns_remove Christoph Hellwig
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2021-08-09  6:40 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Coly Li, Song Liu, Ulf Hansson, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

Restructure mmc_blk_probe to avoid a failure path with a half created
disk.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/mmc/core/block.c | 49 ++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 32 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 4ac3e1b93e7e..4c11f171e56d 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2328,7 +2328,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 					      sector_t size,
 					      bool default_ro,
 					      const char *subname,
-					      int area_type)
+					      int area_type,
+					      unsigned int part_type)
 {
 	struct mmc_blk_data *md;
 	int devidx, ret;
@@ -2375,6 +2376,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	kref_init(&md->kref);
 
 	md->queue.blkdata = md;
+	md->part_type = part_type;
 
 	md->disk->major	= MMC_BLOCK_MAJOR;
 	md->disk->minors = perdev_minors;
@@ -2427,6 +2429,10 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 		md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
 		cap_str, md->read_only ? "(ro)" : "");
 
+	/* used in ->open, must be set before add_disk: */
+	if (area_type == MMC_BLK_DATA_AREA_MAIN)
+		dev_set_drvdata(&card->dev, md);
+	device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
 	return md;
 
  err_kfree:
@@ -2456,7 +2462,7 @@ static struct mmc_blk_data *mmc_blk_alloc(struct mmc_card *card)
 	}
 
 	return mmc_blk_alloc_req(card, &card->dev, size, false, NULL,
-					MMC_BLK_DATA_AREA_MAIN);
+					MMC_BLK_DATA_AREA_MAIN, 0);
 }
 
 static int mmc_blk_alloc_part(struct mmc_card *card,
@@ -2470,10 +2476,9 @@ static int mmc_blk_alloc_part(struct mmc_card *card,
 	struct mmc_blk_data *part_md;
 
 	part_md = mmc_blk_alloc_req(card, disk_to_dev(md->disk), size, default_ro,
-				    subname, area_type);
+				    subname, area_type, part_type);
 	if (IS_ERR(part_md))
 		return PTR_ERR(part_md);
-	part_md->part_type = part_type;
 	list_add(&part_md->part, &md->part);
 
 	return 0;
@@ -2674,20 +2679,13 @@ static int mmc_blk_alloc_parts(struct mmc_card *card, struct mmc_blk_data *md)
 
 static void mmc_blk_remove_req(struct mmc_blk_data *md)
 {
-	struct mmc_card *card;
-
-	if (md) {
-		/*
-		 * Flush remaining requests and free queues. It
-		 * is freeing the queue that stops new requests
-		 * from being accepted.
-		 */
-		card = md->queue.card;
-		if (md->disk->flags & GENHD_FL_UP)
-			del_gendisk(md->disk);
-		mmc_cleanup_queue(&md->queue);
-		mmc_blk_put(md);
-	}
+	/*
+	 * Flush remaining requests and free queues. It is freeing the queue
+	 * that stops new requests from being accepted.
+	 */
+	del_gendisk(md->disk);
+	mmc_cleanup_queue(&md->queue);
+	mmc_blk_put(md);
 }
 
 static void mmc_blk_remove_parts(struct mmc_card *card,
@@ -2876,7 +2874,7 @@ static void mmc_blk_remove_debugfs(struct mmc_card *card,
 
 static int mmc_blk_probe(struct mmc_card *card)
 {
-	struct mmc_blk_data *md, *part_md;
+	struct mmc_blk_data *md;
 	int ret = 0;
 
 	/*
@@ -2904,19 +2902,6 @@ static int mmc_blk_probe(struct mmc_card *card)
 	if (ret)
 		goto out;
 
-	dev_set_drvdata(&card->dev, md);
-
-	device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
-	if (ret)
-		goto out;
-
-	list_for_each_entry(part_md, &md->part, part) {
-		device_add_disk(part_md->parent, part_md->disk,
-				mmc_disk_attr_groups);
-		if (ret)
-			goto out;
-	}
-
 	/* Add two debugfs entries */
 	mmc_blk_add_debugfs(card, md);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/8] nvme: remove the GENHD_FL_UP check in nvme_ns_remove
  2021-08-09  6:40 remove GENHD_FL_UP Christoph Hellwig
  2021-08-09  6:40 ` [PATCH 1/8] mmc: block: let device_add_disk create disk attributes Christoph Hellwig
  2021-08-09  6:40 ` [PATCH 2/8] mmc: block: cleanup gendisk creation Christoph Hellwig
@ 2021-08-09  6:40 ` Christoph Hellwig
  2021-08-09  6:40 ` [PATCH 4/8] nvme: replace the GENHD_FL_UP check in nvme_mpath_shutdown_disk Christoph Hellwig
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-08-09  6:40 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Coly Li, Song Liu, Ulf Hansson, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

Early probe failure never reaches nvme_ns_remove, so GENHD_FL_UP must
be set at this point.  Remove the check.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/core.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index dfd9dec0c1f6..e24a0143fd87 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3826,14 +3826,12 @@ static void nvme_ns_remove(struct nvme_ns *ns)
 	nvme_mpath_clear_current_path(ns);
 	synchronize_srcu(&ns->head->srcu); /* wait for concurrent submissions */
 
-	if (ns->disk->flags & GENHD_FL_UP) {
-		if (!nvme_ns_head_multipath(ns->head))
-			nvme_cdev_del(&ns->cdev, &ns->cdev_device);
-		del_gendisk(ns->disk);
-		blk_cleanup_queue(ns->queue);
-		if (blk_get_integrity(ns->disk))
-			blk_integrity_unregister(ns->disk);
-	}
+	if (!nvme_ns_head_multipath(ns->head))
+		nvme_cdev_del(&ns->cdev, &ns->cdev_device);
+	del_gendisk(ns->disk);
+	blk_cleanup_queue(ns->queue);
+	if (blk_get_integrity(ns->disk))
+		blk_integrity_unregister(ns->disk);
 
 	down_write(&ns->ctrl->namespaces_rwsem);
 	list_del_init(&ns->list);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/8] nvme: replace the GENHD_FL_UP check in nvme_mpath_shutdown_disk
  2021-08-09  6:40 remove GENHD_FL_UP Christoph Hellwig
                   ` (2 preceding siblings ...)
  2021-08-09  6:40 ` [PATCH 3/8] nvme: remove the GENHD_FL_UP check in nvme_ns_remove Christoph Hellwig
@ 2021-08-09  6:40 ` Christoph Hellwig
  2021-08-09  6:40 ` [PATCH 5/8] sx8: use the internal state machine to check if del_gendisk needs to be called Christoph Hellwig
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-08-09  6:40 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Coly Li, Song Liu, Ulf Hansson, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

Use the nvme-internal NVME_NSHEAD_DISK_LIVE flag instead of abusing
the block layer state.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/multipath.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 3f32c5e86bfc..37ce3e8b1db2 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -765,7 +765,7 @@ void nvme_mpath_shutdown_disk(struct nvme_ns_head *head)
 	if (!head->disk)
 		return;
 	kblockd_schedule_work(&head->requeue_work);
-	if (head->disk->flags & GENHD_FL_UP) {
+	if (test_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
 		nvme_cdev_del(&head->cdev, &head->cdev_device);
 		del_gendisk(head->disk);
 	}
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/8] sx8: use the internal state machine to check if del_gendisk needs to be called
  2021-08-09  6:40 remove GENHD_FL_UP Christoph Hellwig
                   ` (3 preceding siblings ...)
  2021-08-09  6:40 ` [PATCH 4/8] nvme: replace the GENHD_FL_UP check in nvme_mpath_shutdown_disk Christoph Hellwig
@ 2021-08-09  6:40 ` Christoph Hellwig
  2021-08-09  6:40 ` [PATCH 6/8] bcache: add proper error unwinding in bcache_device_init Christoph Hellwig
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-08-09  6:40 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Coly Li, Song Liu, Ulf Hansson, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

Remove usage of the block layer internal GENHD_FL_UP by just looking
at the host state.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/sx8.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/8] bcache: add proper error unwinding in bcache_device_init
  2021-08-09  6:40 remove GENHD_FL_UP Christoph Hellwig
                   ` (4 preceding siblings ...)
  2021-08-09  6:40 ` [PATCH 5/8] sx8: use the internal state machine to check if del_gendisk needs to be called Christoph Hellwig
@ 2021-08-09  6:40 ` Christoph Hellwig
  2021-08-12 15:48   ` Coly Li
  2021-08-09  6:40 ` [PATCH 7/8] bcache: move the del_gendisk call out of bcache_device_free Christoph Hellwig
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2021-08-09  6:40 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Coly Li, Song Liu, Ulf Hansson, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

Except for the IDA none of the allocations in bcache_device_init is
unwound on error, fix that.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/bcache/super.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 185246a0d855..d0f08e946453 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -931,20 +931,20 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
 	n = BITS_TO_LONGS(d->nr_stripes) * sizeof(unsigned long);
 	d->full_dirty_stripes = kvzalloc(n, GFP_KERNEL);
 	if (!d->full_dirty_stripes)
-		return -ENOMEM;
+		goto out_free_stripe_sectors_dirty;
 
 	idx = ida_simple_get(&bcache_device_idx, 0,
 				BCACHE_DEVICE_IDX_MAX, GFP_KERNEL);
 	if (idx < 0)
-		return idx;
+		goto out_free_full_dirty_stripes;
 
 	if (bioset_init(&d->bio_split, 4, offsetof(struct bbio, bio),
 			BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
-		goto err;
+		goto out_ida_remove;
 
 	d->disk = blk_alloc_disk(NUMA_NO_NODE);
 	if (!d->disk)
-		goto err;
+		goto out_bioset_exit;
 
 	set_capacity(d->disk, sectors);
 	snprintf(d->disk->disk_name, DISK_NAME_LEN, "bcache%i", idx);
@@ -987,8 +987,14 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
 
 	return 0;
 
-err:
+out_bioset_exit:
+	bioset_exit(&d->bio_split);
+out_ida_remove:
 	ida_simple_remove(&bcache_device_idx, idx);
+out_free_full_dirty_stripes:
+	kvfree(d->full_dirty_stripes);
+out_free_stripe_sectors_dirty:
+	kvfree(d->stripe_sectors_dirty);
 	return -ENOMEM;
 
 }
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 7/8] bcache: move the del_gendisk call out of bcache_device_free
  2021-08-09  6:40 remove GENHD_FL_UP Christoph Hellwig
                   ` (5 preceding siblings ...)
  2021-08-09  6:40 ` [PATCH 6/8] bcache: add proper error unwinding in bcache_device_init Christoph Hellwig
@ 2021-08-09  6:40 ` Christoph Hellwig
  2021-08-12  8:38   ` Coly Li
  2021-08-09  6:40 ` [PATCH 8/8] block: remove GENHD_FL_UP Christoph Hellwig
  2021-08-12 16:30 ` Jens Axboe
  8 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2021-08-09  6:40 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Coly Li, Song Liu, Ulf Hansson, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

Let the callers call del_gendisk so that we can check if add_disk
has been called properly for the cached device case instead of relying
on the block layer internal GENHD_FL_UP flag.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/bcache/super.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index d0f08e946453..f2874c77ff79 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -885,11 +885,6 @@ 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);
-
 		blk_cleanup_disk(disk);
 		ida_simple_remove(&bcache_device_idx,
 				  first_minor_to_idx(disk->first_minor));
@@ -1371,8 +1366,10 @@ static void cached_dev_free(struct closure *cl)
 
 	mutex_lock(&bch_register_lock);
 
-	if (atomic_read(&dc->running))
+	if (atomic_read(&dc->running)) {
 		bd_unlink_disk_holder(dc->bdev, dc->disk.disk);
+		del_gendisk(dc->disk.disk);
+	}
 	bcache_device_free(&dc->disk);
 	list_del(&dc->list);
 
@@ -1518,6 +1515,7 @@ static void flash_dev_free(struct closure *cl)
 	mutex_lock(&bch_register_lock);
 	atomic_long_sub(bcache_dev_sectors_dirty(d),
 			&d->c->flash_dev_dirty_sectors);
+	del_gendisk(d->disk);
 	bcache_device_free(d);
 	mutex_unlock(&bch_register_lock);
 	kobject_put(&d->kobj);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 8/8] block: remove GENHD_FL_UP
  2021-08-09  6:40 remove GENHD_FL_UP Christoph Hellwig
                   ` (6 preceding siblings ...)
  2021-08-09  6:40 ` [PATCH 7/8] bcache: move the del_gendisk call out of bcache_device_free Christoph Hellwig
@ 2021-08-09  6:40 ` Christoph Hellwig
  2021-08-12 16:30 ` Jens Axboe
  8 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-08-09  6:40 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Coly Li, Song Liu, Ulf Hansson, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

Just check inode_unhashed on the whole device bdev inode instead,
and provide a helper to check for that information.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c            | 6 ++----
 block/partitions/core.c  | 4 ++--
 drivers/md/md.h          | 4 +---
 drivers/nvme/host/core.c | 2 +-
 fs/block_dev.c           | 2 +-
 include/linux/genhd.h    | 9 +++++----
 6 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index a4817e42f3a3..9021c8ce3869 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -77,7 +77,8 @@ bool set_capacity_and_notify(struct gendisk *disk, sector_t size)
 	 * initial capacity during probing.
 	 */
 	if (size == capacity ||
-	    (disk->flags & (GENHD_FL_UP | GENHD_FL_HIDDEN)) != GENHD_FL_UP)
+	    !disk_live(disk) ||
+	    (disk->flags & GENHD_FL_HIDDEN))
 		return false;
 
 	pr_info("%s: detected capacity change from %lld to %lld\n",
@@ -519,8 +520,6 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
 		disk->flags |= GENHD_FL_EXT_DEVT;
 	}
 
-	disk->flags |= GENHD_FL_UP;
-
 	disk_alloc_events(disk);
 
 	if (disk->flags & GENHD_FL_HIDDEN) {
@@ -604,7 +603,6 @@ void del_gendisk(struct gendisk *disk)
 
 	mutex_lock(&disk->open_mutex);
 	remove_inode_hash(disk->part0->bd_inode);
-	disk->flags &= ~GENHD_FL_UP;
 	blk_drop_partitions(disk);
 	mutex_unlock(&disk->open_mutex);
 
diff --git a/block/partitions/core.c b/block/partitions/core.c
index fb3a556cacce..c6738ccbcee5 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -459,7 +459,7 @@ int bdev_add_partition(struct block_device *bdev, int partno,
 	int ret;
 
 	mutex_lock(&disk->open_mutex);
-	if (!(disk->flags & GENHD_FL_UP)) {
+	if (!disk_live(disk)) {
 		ret = -ENXIO;
 		goto out;
 	}
@@ -669,7 +669,7 @@ int bdev_disk_changed(struct gendisk *disk, bool invalidate)
 
 	lockdep_assert_held(&disk->open_mutex);
 
-	if (!(disk->flags & GENHD_FL_UP))
+	if (!disk_live(disk))
 		return -ENXIO;
 
 rescan:
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 832547cf038f..4c96c36bd01a 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -764,9 +764,7 @@ struct md_rdev *md_find_rdev_rcu(struct mddev *mddev, dev_t dev);
 
 static inline bool is_mddev_broken(struct md_rdev *rdev, const char *md_type)
 {
-	int flags = rdev->bdev->bd_disk->flags;
-
-	if (!(flags & GENHD_FL_UP)) {
+	if (!disk_live(rdev->bdev->bd_disk)) {
 		if (!test_and_set_bit(MD_BROKEN, &rdev->mddev->flags))
 			pr_warn("md: %s: %s array has a missing/failed member\n",
 				mdname(rdev->mddev), md_type);
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e24a0143fd87..cb12d8b94e82 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1822,7 +1822,7 @@ static void nvme_update_disk_info(struct gendisk *disk,
 static inline bool nvme_first_scan(struct gendisk *disk)
 {
 	/* nvme_alloc_ns() scans the disk prior to adding it */
-	return !(disk->flags & GENHD_FL_UP);
+	return !disk_live(disk);
 }
 
 static void nvme_set_chunk_sectors(struct nvme_ns *ns, struct nvme_id_ns *id)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 6658f40ae492..ab50428b328b 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1374,7 +1374,7 @@ struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder)
 
 	mutex_lock(&disk->open_mutex);
 	ret = -ENXIO;
-	if (!(disk->flags & GENHD_FL_UP))
+	if (!disk_live(disk))
 		goto abort_claiming;
 	if (bdev_is_partition(bdev))
 		ret = blkdev_get_part(bdev, mode);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 849486de81c6..ddd02def1ed4 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -60,9 +60,6 @@ struct partition_meta_info {
  * device.
  * Affects responses to the ``CDROM_GET_CAPABILITY`` ioctl.
  *
- * ``GENHD_FL_UP`` (0x0010): indicates that the block device is "up",
- * with a similar meaning to network interfaces.
- *
  * ``GENHD_FL_SUPPRESS_PARTITION_INFO`` (0x0020): don't include
  * partition information in ``/proc/partitions`` or in the output of
  * printk_all_partitions().
@@ -97,7 +94,6 @@ struct partition_meta_info {
 /* 2 is unused (used to be GENHD_FL_DRIVERFS) */
 /* 4 is unused (used to be GENHD_FL_MEDIA_CHANGE_NOTIFY) */
 #define GENHD_FL_CD				0x0008
-#define GENHD_FL_UP				0x0010
 #define GENHD_FL_SUPPRESS_PARTITION_INFO	0x0020
 #define GENHD_FL_EXT_DEVT			0x0040
 #define GENHD_FL_NATIVE_CAPACITY		0x0080
@@ -175,6 +171,11 @@ struct gendisk {
 	u64 diskseq;
 };
 
+static inline bool disk_live(struct gendisk *disk)
+{
+	return !inode_unhashed(disk->part0->bd_inode);
+}
+
 /*
  * The gendisk is refcounted by the part0 block_device, and the bd_device
  * therein is also used for device model presentation in sysfs.
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/8] mmc: block: let device_add_disk create disk attributes
  2021-08-09  6:40 ` [PATCH 1/8] mmc: block: let device_add_disk create disk attributes Christoph Hellwig
@ 2021-08-09 11:30   ` Ulf Hansson
  0 siblings, 0 replies; 14+ messages in thread
From: Ulf Hansson @ 2021-08-09 11:30 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Coly Li, Song Liu, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

On Mon, 9 Aug 2021 at 08:43, Christoph Hellwig <hch@lst.de> wrote:
>
> Pass the attribute group for the attributes on the gendisk to
> device_add_disk so that they are created atomically with the
> disk creation.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Let's try to funnel this via Jens' tree. As long as his tree is based
upon v5.14-rc3 or later I don't think we should have any problem with
conflicts.

Kind regards
Uffe


> ---
>  drivers/mmc/core/block.c | 102 +++++++++++++++++----------------------
>  1 file changed, 45 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index ce8aed562929..4ac3e1b93e7e 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -128,8 +128,6 @@ struct mmc_blk_data {
>          * track of the current selected device partition.
>          */
>         unsigned int    part_curr;
> -       struct device_attribute force_ro;
> -       struct device_attribute power_ro_lock;
>         int     area_type;
>
>         /* debugfs files (only in main mmc_blk_data) */
> @@ -281,6 +279,9 @@ static ssize_t power_ro_lock_store(struct device *dev,
>         return count;
>  }
>
> +static DEVICE_ATTR(ro_lock_until_next_power_on, 0,
> +               power_ro_lock_show, power_ro_lock_store);
> +
>  static ssize_t force_ro_show(struct device *dev, struct device_attribute *attr,
>                              char *buf)
>  {
> @@ -313,6 +314,44 @@ static ssize_t force_ro_store(struct device *dev, struct device_attribute *attr,
>         return ret;
>  }
>
> +static DEVICE_ATTR(force_ro, 0644, force_ro_show, force_ro_store);
> +
> +static struct attribute *mmc_disk_attrs[] = {
> +       &dev_attr_force_ro.attr,
> +       &dev_attr_ro_lock_until_next_power_on.attr,
> +       NULL,
> +};
> +
> +static umode_t mmc_disk_attrs_is_visible(struct kobject *kobj,
> +               struct attribute *a, int n)
> +{
> +       struct device *dev = container_of(kobj, struct device, kobj);
> +       struct mmc_blk_data *md = mmc_blk_get(dev_to_disk(dev));
> +       umode_t mode = a->mode;
> +
> +       if (a == &dev_attr_ro_lock_until_next_power_on.attr &&
> +           (md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
> +           md->queue.card->ext_csd.boot_ro_lockable) {
> +               mode = S_IRUGO;
> +               if (!(md->queue.card->ext_csd.boot_ro_lock &
> +                               EXT_CSD_BOOT_WP_B_PWR_WP_DIS))
> +                       mode |= S_IWUSR;
> +       }
> +
> +       mmc_blk_put(md);
> +       return mode;
> +}
> +
> +static const struct attribute_group mmc_disk_attr_group = {
> +       .is_visible     = mmc_disk_attrs_is_visible,
> +       .attrs          = mmc_disk_attrs,
> +};
> +
> +static const struct attribute_group *mmc_disk_attr_groups[] = {
> +       &mmc_disk_attr_group,
> +       NULL,
> +};
> +
>  static int mmc_blk_open(struct block_device *bdev, fmode_t mode)
>  {
>         struct mmc_blk_data *md = mmc_blk_get(bdev->bd_disk);
> @@ -2644,15 +2683,8 @@ static void mmc_blk_remove_req(struct mmc_blk_data *md)
>                  * from being accepted.
>                  */
>                 card = md->queue.card;
> -               if (md->disk->flags & GENHD_FL_UP) {
> -                       device_remove_file(disk_to_dev(md->disk), &md->force_ro);
> -                       if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
> -                                       card->ext_csd.boot_ro_lockable)
> -                               device_remove_file(disk_to_dev(md->disk),
> -                                       &md->power_ro_lock);
> -
> +               if (md->disk->flags & GENHD_FL_UP)
>                         del_gendisk(md->disk);
> -               }
>                 mmc_cleanup_queue(&md->queue);
>                 mmc_blk_put(md);
>         }
> @@ -2679,51 +2711,6 @@ static void mmc_blk_remove_parts(struct mmc_card *card,
>         }
>  }
>
> -static int mmc_add_disk(struct mmc_blk_data *md)
> -{
> -       int ret;
> -       struct mmc_card *card = md->queue.card;
> -
> -       device_add_disk(md->parent, md->disk, NULL);
> -       md->force_ro.show = force_ro_show;
> -       md->force_ro.store = force_ro_store;
> -       sysfs_attr_init(&md->force_ro.attr);
> -       md->force_ro.attr.name = "force_ro";
> -       md->force_ro.attr.mode = S_IRUGO | S_IWUSR;
> -       ret = device_create_file(disk_to_dev(md->disk), &md->force_ro);
> -       if (ret)
> -               goto force_ro_fail;
> -
> -       if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
> -            card->ext_csd.boot_ro_lockable) {
> -               umode_t mode;
> -
> -               if (card->ext_csd.boot_ro_lock & EXT_CSD_BOOT_WP_B_PWR_WP_DIS)
> -                       mode = S_IRUGO;
> -               else
> -                       mode = S_IRUGO | S_IWUSR;
> -
> -               md->power_ro_lock.show = power_ro_lock_show;
> -               md->power_ro_lock.store = power_ro_lock_store;
> -               sysfs_attr_init(&md->power_ro_lock.attr);
> -               md->power_ro_lock.attr.mode = mode;
> -               md->power_ro_lock.attr.name =
> -                                       "ro_lock_until_next_power_on";
> -               ret = device_create_file(disk_to_dev(md->disk),
> -                               &md->power_ro_lock);
> -               if (ret)
> -                       goto power_ro_lock_fail;
> -       }
> -       return ret;
> -
> -power_ro_lock_fail:
> -       device_remove_file(disk_to_dev(md->disk), &md->force_ro);
> -force_ro_fail:
> -       del_gendisk(md->disk);
> -
> -       return ret;
> -}
> -
>  #ifdef CONFIG_DEBUG_FS
>
>  static int mmc_dbg_card_status_get(void *data, u64 *val)
> @@ -2919,12 +2906,13 @@ static int mmc_blk_probe(struct mmc_card *card)
>
>         dev_set_drvdata(&card->dev, md);
>
> -       ret = mmc_add_disk(md);
> +       device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
>         if (ret)
>                 goto out;
>
>         list_for_each_entry(part_md, &md->part, part) {
> -               ret = mmc_add_disk(part_md);
> +               device_add_disk(part_md->parent, part_md->disk,
> +                               mmc_disk_attr_groups);
>                 if (ret)
>                         goto out;
>         }
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/8] mmc: block: cleanup gendisk creation
  2021-08-09  6:40 ` [PATCH 2/8] mmc: block: cleanup gendisk creation Christoph Hellwig
@ 2021-08-09 11:30   ` Ulf Hansson
  0 siblings, 0 replies; 14+ messages in thread
From: Ulf Hansson @ 2021-08-09 11:30 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Coly Li, Song Liu, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

On Mon, 9 Aug 2021 at 08:44, Christoph Hellwig <hch@lst.de> wrote:
>
> Restructure mmc_blk_probe to avoid a failure path with a half created
> disk.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Let's try to funnel this via Jens' tree. As long as his tree is based
upon v5.14-rc3 or later I don't think we should have any problem with
conflicts.

Kind regards
Uffe


> ---
>  drivers/mmc/core/block.c | 49 ++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 4ac3e1b93e7e..4c11f171e56d 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2328,7 +2328,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>                                               sector_t size,
>                                               bool default_ro,
>                                               const char *subname,
> -                                             int area_type)
> +                                             int area_type,
> +                                             unsigned int part_type)
>  {
>         struct mmc_blk_data *md;
>         int devidx, ret;
> @@ -2375,6 +2376,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>         kref_init(&md->kref);
>
>         md->queue.blkdata = md;
> +       md->part_type = part_type;
>
>         md->disk->major = MMC_BLOCK_MAJOR;
>         md->disk->minors = perdev_minors;
> @@ -2427,6 +2429,10 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>                 md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
>                 cap_str, md->read_only ? "(ro)" : "");
>
> +       /* used in ->open, must be set before add_disk: */
> +       if (area_type == MMC_BLK_DATA_AREA_MAIN)
> +               dev_set_drvdata(&card->dev, md);
> +       device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
>         return md;
>
>   err_kfree:
> @@ -2456,7 +2462,7 @@ static struct mmc_blk_data *mmc_blk_alloc(struct mmc_card *card)
>         }
>
>         return mmc_blk_alloc_req(card, &card->dev, size, false, NULL,
> -                                       MMC_BLK_DATA_AREA_MAIN);
> +                                       MMC_BLK_DATA_AREA_MAIN, 0);
>  }
>
>  static int mmc_blk_alloc_part(struct mmc_card *card,
> @@ -2470,10 +2476,9 @@ static int mmc_blk_alloc_part(struct mmc_card *card,
>         struct mmc_blk_data *part_md;
>
>         part_md = mmc_blk_alloc_req(card, disk_to_dev(md->disk), size, default_ro,
> -                                   subname, area_type);
> +                                   subname, area_type, part_type);
>         if (IS_ERR(part_md))
>                 return PTR_ERR(part_md);
> -       part_md->part_type = part_type;
>         list_add(&part_md->part, &md->part);
>
>         return 0;
> @@ -2674,20 +2679,13 @@ static int mmc_blk_alloc_parts(struct mmc_card *card, struct mmc_blk_data *md)
>
>  static void mmc_blk_remove_req(struct mmc_blk_data *md)
>  {
> -       struct mmc_card *card;
> -
> -       if (md) {
> -               /*
> -                * Flush remaining requests and free queues. It
> -                * is freeing the queue that stops new requests
> -                * from being accepted.
> -                */
> -               card = md->queue.card;
> -               if (md->disk->flags & GENHD_FL_UP)
> -                       del_gendisk(md->disk);
> -               mmc_cleanup_queue(&md->queue);
> -               mmc_blk_put(md);
> -       }
> +       /*
> +        * Flush remaining requests and free queues. It is freeing the queue
> +        * that stops new requests from being accepted.
> +        */
> +       del_gendisk(md->disk);
> +       mmc_cleanup_queue(&md->queue);
> +       mmc_blk_put(md);
>  }
>
>  static void mmc_blk_remove_parts(struct mmc_card *card,
> @@ -2876,7 +2874,7 @@ static void mmc_blk_remove_debugfs(struct mmc_card *card,
>
>  static int mmc_blk_probe(struct mmc_card *card)
>  {
> -       struct mmc_blk_data *md, *part_md;
> +       struct mmc_blk_data *md;
>         int ret = 0;
>
>         /*
> @@ -2904,19 +2902,6 @@ static int mmc_blk_probe(struct mmc_card *card)
>         if (ret)
>                 goto out;
>
> -       dev_set_drvdata(&card->dev, md);
> -
> -       device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
> -       if (ret)
> -               goto out;
> -
> -       list_for_each_entry(part_md, &md->part, part) {
> -               device_add_disk(part_md->parent, part_md->disk,
> -                               mmc_disk_attr_groups);
> -               if (ret)
> -                       goto out;
> -       }
> -
>         /* Add two debugfs entries */
>         mmc_blk_add_debugfs(card, md);
>
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 7/8] bcache: move the del_gendisk call out of bcache_device_free
  2021-08-09  6:40 ` [PATCH 7/8] bcache: move the del_gendisk call out of bcache_device_free Christoph Hellwig
@ 2021-08-12  8:38   ` Coly Li
  0 siblings, 0 replies; 14+ messages in thread
From: Coly Li @ 2021-08-12  8:38 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Song Liu, Ulf Hansson, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

On 8/9/21 2:40 PM, Christoph Hellwig wrote:
> Let the callers call del_gendisk so that we can check if add_disk
> has been called properly for the cached device case instead of relying
> on the block layer internal GENHD_FL_UP flag.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

It looks good to me.

Reviewed-by: Coly Li <colyli@suse.de>

Thanks.

Coly Li

> ---
>  drivers/md/bcache/super.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index d0f08e946453..f2874c77ff79 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -885,11 +885,6 @@ 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);
> -
>  		blk_cleanup_disk(disk);
>  		ida_simple_remove(&bcache_device_idx,
>  				  first_minor_to_idx(disk->first_minor));
> @@ -1371,8 +1366,10 @@ static void cached_dev_free(struct closure *cl)
>  
>  	mutex_lock(&bch_register_lock);
>  
> -	if (atomic_read(&dc->running))
> +	if (atomic_read(&dc->running)) {
>  		bd_unlink_disk_holder(dc->bdev, dc->disk.disk);
> +		del_gendisk(dc->disk.disk);
> +	}
>  	bcache_device_free(&dc->disk);
>  	list_del(&dc->list);
>  
> @@ -1518,6 +1515,7 @@ static void flash_dev_free(struct closure *cl)
>  	mutex_lock(&bch_register_lock);
>  	atomic_long_sub(bcache_dev_sectors_dirty(d),
>  			&d->c->flash_dev_dirty_sectors);
> +	del_gendisk(d->disk);
>  	bcache_device_free(d);
>  	mutex_unlock(&bch_register_lock);
>  	kobject_put(&d->kobj);


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/8] bcache: add proper error unwinding in bcache_device_init
  2021-08-09  6:40 ` [PATCH 6/8] bcache: add proper error unwinding in bcache_device_init Christoph Hellwig
@ 2021-08-12 15:48   ` Coly Li
  0 siblings, 0 replies; 14+ messages in thread
From: Coly Li @ 2021-08-12 15:48 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Song Liu, Ulf Hansson, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

On 8/9/21 2:40 PM, Christoph Hellwig wrote:
> Except for the IDA none of the allocations in bcache_device_init is
> unwound on error, fix that.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Coly Li <colyli@suse.de>

Thanks.

Coly Li

> ---
>  drivers/md/bcache/super.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 185246a0d855..d0f08e946453 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -931,20 +931,20 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
>  	n = BITS_TO_LONGS(d->nr_stripes) * sizeof(unsigned long);
>  	d->full_dirty_stripes = kvzalloc(n, GFP_KERNEL);
>  	if (!d->full_dirty_stripes)
> -		return -ENOMEM;
> +		goto out_free_stripe_sectors_dirty;
>  
>  	idx = ida_simple_get(&bcache_device_idx, 0,
>  				BCACHE_DEVICE_IDX_MAX, GFP_KERNEL);
>  	if (idx < 0)
> -		return idx;
> +		goto out_free_full_dirty_stripes;
>  
>  	if (bioset_init(&d->bio_split, 4, offsetof(struct bbio, bio),
>  			BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
> -		goto err;
> +		goto out_ida_remove;
>  
>  	d->disk = blk_alloc_disk(NUMA_NO_NODE);
>  	if (!d->disk)
> -		goto err;
> +		goto out_bioset_exit;
>  
>  	set_capacity(d->disk, sectors);
>  	snprintf(d->disk->disk_name, DISK_NAME_LEN, "bcache%i", idx);
> @@ -987,8 +987,14 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
>  
>  	return 0;
>  
> -err:
> +out_bioset_exit:
> +	bioset_exit(&d->bio_split);
> +out_ida_remove:
>  	ida_simple_remove(&bcache_device_idx, idx);
> +out_free_full_dirty_stripes:
> +	kvfree(d->full_dirty_stripes);
> +out_free_stripe_sectors_dirty:
> +	kvfree(d->stripe_sectors_dirty);
>  	return -ENOMEM;
>  
>  }


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: remove GENHD_FL_UP
  2021-08-09  6:40 remove GENHD_FL_UP Christoph Hellwig
                   ` (7 preceding siblings ...)
  2021-08-09  6:40 ` [PATCH 8/8] block: remove GENHD_FL_UP Christoph Hellwig
@ 2021-08-12 16:30 ` Jens Axboe
  8 siblings, 0 replies; 14+ messages in thread
From: Jens Axboe @ 2021-08-12 16:30 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Coly Li, Song Liu, Ulf Hansson, Luis Chamberlain, linux-block,
	linux-bcache, linux-raid, linux-mmc, linux-nvme

On 8/9/21 12:40 AM, Christoph Hellwig wrote:
> Hi Jens,
> 
> this series first cleans up various drivers to not rely on the
> internal GENHD_FL_UP to decided if they need to call del_gendisk
> and then removes the flag entirely.

Applied, thanks.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-08-12 16:30 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-09  6:40 remove GENHD_FL_UP Christoph Hellwig
2021-08-09  6:40 ` [PATCH 1/8] mmc: block: let device_add_disk create disk attributes Christoph Hellwig
2021-08-09 11:30   ` Ulf Hansson
2021-08-09  6:40 ` [PATCH 2/8] mmc: block: cleanup gendisk creation Christoph Hellwig
2021-08-09 11:30   ` Ulf Hansson
2021-08-09  6:40 ` [PATCH 3/8] nvme: remove the GENHD_FL_UP check in nvme_ns_remove Christoph Hellwig
2021-08-09  6:40 ` [PATCH 4/8] nvme: replace the GENHD_FL_UP check in nvme_mpath_shutdown_disk Christoph Hellwig
2021-08-09  6:40 ` [PATCH 5/8] sx8: use the internal state machine to check if del_gendisk needs to be called Christoph Hellwig
2021-08-09  6:40 ` [PATCH 6/8] bcache: add proper error unwinding in bcache_device_init Christoph Hellwig
2021-08-12 15:48   ` Coly Li
2021-08-09  6:40 ` [PATCH 7/8] bcache: move the del_gendisk call out of bcache_device_free Christoph Hellwig
2021-08-12  8:38   ` Coly Li
2021-08-09  6:40 ` [PATCH 8/8] block: remove GENHD_FL_UP Christoph Hellwig
2021-08-12 16:30 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).