linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* partition iteration simplifications
@ 2021-04-06  6:22 Christoph Hellwig
  2021-04-06  6:22 ` [PATCH 01/11] dasd: use bdev_disk_changed instead of blk_drop_partitions Christoph Hellwig
                   ` (11 more replies)
  0 siblings, 12 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-04-06  6:22 UTC (permalink / raw)
  To: Jens Axboe, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

Hi all,

with the switch to an xarray for storing the partitions of a gendisk we
can now use the xarray iterators for iterating over the partitions and don't
really need the disk_part_iter_* scheme.  Also clean up the teardown of
partitions where I think we have a (very unlikely to hit) race currently.

Diffstat:
 block/blk.h                     |    1 
 block/genhd.c                   |  183 +++++++++++-----------------------------
 block/partitions/core.c         |   54 +++++------
 drivers/s390/block/dasd_genhd.c |    3 
 fs/block_dev.c                  |    8 +
 include/linux/genhd.h           |   21 ----
 6 files changed, 84 insertions(+), 186 deletions(-)

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

* [PATCH 01/11] dasd: use bdev_disk_changed instead of blk_drop_partitions
  2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
@ 2021-04-06  6:22 ` Christoph Hellwig
  2021-04-08 14:31   ` Stefan Haberland
  2021-04-06  6:22 ` [PATCH 02/11] block: remove invalidate_partition Christoph Hellwig
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2021-04-06  6:22 UTC (permalink / raw)
  To: Jens Axboe, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

Use the more general interface - the behavior is the same except
that now a change uevent is sent, which is the right thing to do
when the device becomes unusable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/partitions/core.c         | 4 ----
 drivers/s390/block/dasd_genhd.c | 3 +--
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/block/partitions/core.c b/block/partitions/core.c
index 1a7558917c47d6..22a0dab17ed343 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -544,10 +544,6 @@ int blk_drop_partitions(struct block_device *bdev)
 
 	return 0;
 }
-#ifdef CONFIG_S390
-/* for historic reasons in the DASD driver */
-EXPORT_SYMBOL_GPL(blk_drop_partitions);
-#endif
 
 static bool blk_add_partition(struct gendisk *disk, struct block_device *bdev,
 		struct parsed_partitions *state, int p)
diff --git a/drivers/s390/block/dasd_genhd.c b/drivers/s390/block/dasd_genhd.c
index a9698fba9b76ce..8d6587ec73e208 100644
--- a/drivers/s390/block/dasd_genhd.c
+++ b/drivers/s390/block/dasd_genhd.c
@@ -146,12 +146,11 @@ void dasd_destroy_partitions(struct dasd_block *block)
 	block->bdev = NULL;
 
 	mutex_lock(&bdev->bd_mutex);
-	blk_drop_partitions(bdev);
+	bdev_disk_changed(bdev, true);
 	mutex_unlock(&bdev->bd_mutex);
 
 	/* Matching blkdev_put to the blkdev_get in dasd_scan_partitions. */
 	blkdev_put(bdev, FMODE_READ);
-	set_capacity(block->gdp, 0);
 }
 
 int dasd_gendisk_init(void)
-- 
2.30.1


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

* [PATCH 02/11] block: remove invalidate_partition
  2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
  2021-04-06  6:22 ` [PATCH 01/11] dasd: use bdev_disk_changed instead of blk_drop_partitions Christoph Hellwig
@ 2021-04-06  6:22 ` Christoph Hellwig
  2021-04-06  6:22 ` [PATCH 03/11] block: move more syncing and invalidation to delete_partition Christoph Hellwig
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-04-06  6:22 UTC (permalink / raw)
  To: Jens Axboe, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

invalidate_partition has two callers, one of which already performs
the remove_inode_hash just after the call.  Just open code the
function in the two callsites.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 8c8f543572e64e..9b121b1f79982f 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -646,18 +646,6 @@ void device_add_disk_no_queue_reg(struct device *parent, struct gendisk *disk)
 }
 EXPORT_SYMBOL(device_add_disk_no_queue_reg);
 
-static void invalidate_partition(struct block_device *bdev)
-{
-	fsync_bdev(bdev);
-	__invalidate_device(bdev, true);
-
-	/*
-	 * Unhash the bdev inode for this device so that it can't be looked
-	 * up any more even if openers still hold references to it.
-	 */
-	remove_inode_hash(bdev->bd_inode);
-}
-
 /**
  * del_gendisk - remove the gendisk
  * @disk: the struct gendisk to remove
@@ -699,12 +687,21 @@ void del_gendisk(struct gendisk *disk)
 	/* invalidate stuff */
 	disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY);
 	while ((part = disk_part_iter_next(&piter))) {
-		invalidate_partition(part);
+		fsync_bdev(part);
+		__invalidate_device(part, true);
 		delete_partition(part);
 	}
 	disk_part_iter_exit(&piter);
 
-	invalidate_partition(disk->part0);
+	fsync_bdev(disk->part0);
+	__invalidate_device(disk->part0, true);
+
+	/*
+	 * Unhash the bdev inode for this device so that it can't be looked
+	 * up any more even if openers still hold references to it.
+	 */
+	remove_inode_hash(disk->part0->bd_inode);
+
 	set_capacity(disk, 0);
 	disk->flags &= ~GENHD_FL_UP;
 	up_write(&bdev_lookup_sem);
-- 
2.30.1


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

* [PATCH 03/11] block: move more syncing and invalidation to delete_partition
  2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
  2021-04-06  6:22 ` [PATCH 01/11] dasd: use bdev_disk_changed instead of blk_drop_partitions Christoph Hellwig
  2021-04-06  6:22 ` [PATCH 02/11] block: remove invalidate_partition Christoph Hellwig
@ 2021-04-06  6:22 ` Christoph Hellwig
  2021-04-06  6:22 ` [PATCH 04/11] block: refactor blk_drop_partitions Christoph Hellwig
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-04-06  6:22 UTC (permalink / raw)
  To: Jens Axboe, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

Move the calls to fsync_bdev and __invalidate_device from del_gendisk to
delete_partition.  For the other two callers that check that there are
no openers for the delete partitions(s) the callouts are a no-op as no
file system can be mounted, but this keeps all the cleanup in one
place.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c           | 5 +----
 block/partitions/core.c | 6 +++---
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 9b121b1f79982f..15f99da4543f6d 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -686,11 +686,8 @@ void del_gendisk(struct gendisk *disk)
 
 	/* invalidate stuff */
 	disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY);
-	while ((part = disk_part_iter_next(&piter))) {
-		fsync_bdev(part);
-		__invalidate_device(part, true);
+	while ((part = disk_part_iter_next(&piter)))
 		delete_partition(part);
-	}
 	disk_part_iter_exit(&piter);
 
 	fsync_bdev(disk->part0);
diff --git a/block/partitions/core.c b/block/partitions/core.c
index 22a0dab17ed343..8c173529294081 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -287,6 +287,9 @@ struct device_type part_type = {
  */
 void delete_partition(struct block_device *part)
 {
+	fsync_bdev(part);
+	__invalidate_device(part, true);
+
 	xa_erase(&part->bd_disk->part_tbl, part->bd_partno);
 	kobject_put(part->bd_holder_dir);
 	device_del(&part->bd_device);
@@ -468,9 +471,6 @@ int bdev_del_partition(struct block_device *bdev, int partno)
 	if (part->bd_openers)
 		goto out_unlock;
 
-	sync_blockdev(part);
-	invalidate_bdev(part);
-
 	delete_partition(part);
 	ret = 0;
 out_unlock:
-- 
2.30.1


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

* [PATCH 04/11] block: refactor blk_drop_partitions
  2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
                   ` (2 preceding siblings ...)
  2021-04-06  6:22 ` [PATCH 03/11] block: move more syncing and invalidation to delete_partition Christoph Hellwig
@ 2021-04-06  6:22 ` Christoph Hellwig
  2021-04-06  6:22 ` [PATCH 05/11] block: take bd_mutex around delete_partitions in del_gendisk Christoph Hellwig
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-04-06  6:22 UTC (permalink / raw)
  To: Jens Axboe, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

Move the busy check and disk-wide sync into the only caller, so that
the remainder can be shared with del_gendisk.  Also pass the gendisk
instead of the bdev as that is all that is needed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk.h             |  1 -
 block/genhd.c           | 11 +----------
 block/partitions/core.c | 14 +++-----------
 fs/block_dev.c          |  8 +++++---
 include/linux/genhd.h   |  2 +-
 5 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/block/blk.h b/block/blk.h
index 3b53e44b967e4e..900cb246d5f384 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -346,7 +346,6 @@ char *disk_name(struct gendisk *hd, int partno, char *buf);
 #define ADDPART_FLAG_NONE	0
 #define ADDPART_FLAG_RAID	1
 #define ADDPART_FLAG_WHOLEDISK	2
-void delete_partition(struct block_device *part);
 int bdev_add_partition(struct block_device *bdev, int partno,
 		sector_t start, sector_t length);
 int bdev_del_partition(struct block_device *bdev, int partno);
diff --git a/block/genhd.c b/block/genhd.c
index 15f99da4543f6d..8303ec67fd7099 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -667,9 +667,6 @@ EXPORT_SYMBOL(device_add_disk_no_queue_reg);
  */
 void del_gendisk(struct gendisk *disk)
 {
-	struct disk_part_iter piter;
-	struct block_device *part;
-
 	might_sleep();
 
 	if (WARN_ON_ONCE(!disk->queue))
@@ -683,13 +680,7 @@ void del_gendisk(struct gendisk *disk)
 	 * disk is marked as dead (GENHD_FL_UP cleared).
 	 */
 	down_write(&bdev_lookup_sem);
-
-	/* invalidate stuff */
-	disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY);
-	while ((part = disk_part_iter_next(&piter)))
-		delete_partition(part);
-	disk_part_iter_exit(&piter);
-
+	blk_drop_partitions(disk);
 	fsync_bdev(disk->part0);
 	__invalidate_device(disk->part0, true);
 
diff --git a/block/partitions/core.c b/block/partitions/core.c
index 8c173529294081..536f7c5bb0b6d2 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -285,7 +285,7 @@ struct device_type part_type = {
  * Must be called either with bd_mutex held, before a disk can be opened or
  * after all disk users are gone.
  */
-void delete_partition(struct block_device *part)
+static void delete_partition(struct block_device *part)
 {
 	fsync_bdev(part);
 	__invalidate_device(part, true);
@@ -526,23 +526,15 @@ static bool disk_unlock_native_capacity(struct gendisk *disk)
 	}
 }
 
-int blk_drop_partitions(struct block_device *bdev)
+void blk_drop_partitions(struct gendisk *disk)
 {
 	struct disk_part_iter piter;
 	struct block_device *part;
 
-	if (bdev->bd_part_count)
-		return -EBUSY;
-
-	sync_blockdev(bdev);
-	invalidate_bdev(bdev);
-
-	disk_part_iter_init(&piter, bdev->bd_disk, DISK_PITER_INCL_EMPTY);
+	disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY);
 	while ((part = disk_part_iter_next(&piter)))
 		delete_partition(part);
 	disk_part_iter_exit(&piter);
-
-	return 0;
 }
 
 static bool blk_add_partition(struct gendisk *disk, struct block_device *bdev,
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 92ed7d5df67744..594a1bee9dd9ac 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1243,9 +1243,11 @@ int bdev_disk_changed(struct block_device *bdev, bool invalidate)
 	clear_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
 
 rescan:
-	ret = blk_drop_partitions(bdev);
-	if (ret)
-		return ret;
+	if (bdev->bd_part_count)
+		return -EBUSY;
+	sync_blockdev(bdev);
+	invalidate_bdev(bdev);
+	blk_drop_partitions(disk);
 
 	/*
 	 * Historically we only set the capacity to zero for devices that
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index f364619092cca0..16178a935c4041 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -273,7 +273,7 @@ static inline sector_t get_capacity(struct gendisk *disk)
 
 int bdev_disk_changed(struct block_device *bdev, bool invalidate);
 int blk_add_partitions(struct gendisk *disk, struct block_device *bdev);
-int blk_drop_partitions(struct block_device *bdev);
+void blk_drop_partitions(struct gendisk *disk);
 
 extern struct gendisk *__alloc_disk_node(int minors, int node_id);
 extern void put_disk(struct gendisk *disk);
-- 
2.30.1


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

* [PATCH 05/11] block: take bd_mutex around delete_partitions in del_gendisk
  2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
                   ` (3 preceding siblings ...)
  2021-04-06  6:22 ` [PATCH 04/11] block: refactor blk_drop_partitions Christoph Hellwig
@ 2021-04-06  6:22 ` Christoph Hellwig
  2021-04-06  6:22 ` [PATCH 06/11] block: simplify partition removal Christoph Hellwig
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-04-06  6:22 UTC (permalink / raw)
  To: Jens Axboe, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

There is nothing preventing an ioctl from trying do delete partition
concurrenly with del_gendisk, so take open_mutex to serialize against
that.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c           | 4 ++++
 block/partitions/core.c | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/block/genhd.c b/block/genhd.c
index 8303ec67fd7099..e3f3c2321773f9 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -680,7 +680,11 @@ void del_gendisk(struct gendisk *disk)
 	 * disk is marked as dead (GENHD_FL_UP cleared).
 	 */
 	down_write(&bdev_lookup_sem);
+
+	mutex_lock(&disk->part0->bd_mutex);
 	blk_drop_partitions(disk);
+	mutex_unlock(&disk->part0->bd_mutex);
+
 	fsync_bdev(disk->part0);
 	__invalidate_device(disk->part0, true);
 
diff --git a/block/partitions/core.c b/block/partitions/core.c
index 536f7c5bb0b6d2..9fbaec466b516d 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -531,6 +531,8 @@ void blk_drop_partitions(struct gendisk *disk)
 	struct disk_part_iter piter;
 	struct block_device *part;
 
+	lockdep_assert_held(&disk->part0->bd_mutex);
+
 	disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY);
 	while ((part = disk_part_iter_next(&piter)))
 		delete_partition(part);
-- 
2.30.1


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

* [PATCH 06/11] block: simplify partition removal
  2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
                   ` (4 preceding siblings ...)
  2021-04-06  6:22 ` [PATCH 05/11] block: take bd_mutex around delete_partitions in del_gendisk Christoph Hellwig
@ 2021-04-06  6:22 ` Christoph Hellwig
  2021-04-06  6:22 ` [PATCH 07/11] block: simplify partition_overlaps Christoph Hellwig
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-04-06  6:22 UTC (permalink / raw)
  To: Jens Axboe, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

Always look up the first available entry instead of the complicated
stateful traversal.

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

diff --git a/block/partitions/core.c b/block/partitions/core.c
index 9fbaec466b516d..927144d4e59d39 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -528,15 +528,17 @@ static bool disk_unlock_native_capacity(struct gendisk *disk)
 
 void blk_drop_partitions(struct gendisk *disk)
 {
-	struct disk_part_iter piter;
 	struct block_device *part;
+	unsigned long idx;
 
 	lockdep_assert_held(&disk->part0->bd_mutex);
 
-	disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY);
-	while ((part = disk_part_iter_next(&piter)))
+	xa_for_each_start(&disk->part_tbl, idx, part, 1) {
+		if (!bdgrab(part))
+			continue;
 		delete_partition(part);
-	disk_part_iter_exit(&piter);
+		bdput(part);
+	}
 }
 
 static bool blk_add_partition(struct gendisk *disk, struct block_device *bdev,
-- 
2.30.1


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

* [PATCH 07/11] block: simplify partition_overlaps
  2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
                   ` (5 preceding siblings ...)
  2021-04-06  6:22 ` [PATCH 06/11] block: simplify partition removal Christoph Hellwig
@ 2021-04-06  6:22 ` Christoph Hellwig
  2021-04-06  6:22 ` [PATCH 08/11] block: simplify printk_all_partitions Christoph Hellwig
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-04-06  6:22 UTC (permalink / raw)
  To: Jens Axboe, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

Just use xa_for_each to iterate over the partitions as there is no need
to grab a reference to each partition.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/partitions/core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/block/partitions/core.c b/block/partitions/core.c
index 927144d4e59d39..0f8454b93c6e4c 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -420,21 +420,21 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
 static bool partition_overlaps(struct gendisk *disk, sector_t start,
 		sector_t length, int skip_partno)
 {
-	struct disk_part_iter piter;
 	struct block_device *part;
 	bool overlap = false;
+	unsigned long idx;
 
-	disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY);
-	while ((part = disk_part_iter_next(&piter))) {
-		if (part->bd_partno == skip_partno ||
-		    start >= part->bd_start_sect + bdev_nr_sectors(part) ||
-		    start + length <= part->bd_start_sect)
-			continue;
-		overlap = true;
-		break;
+	rcu_read_lock();
+	xa_for_each_start(&disk->part_tbl, idx, part, 1) {
+		if (part->bd_partno != skip_partno &&
+		    start < part->bd_start_sect + bdev_nr_sectors(part) &&
+		    start + length > part->bd_start_sect) {
+			overlap = true;
+			break;
+		}
 	}
+	rcu_read_unlock();
 
-	disk_part_iter_exit(&piter);
 	return overlap;
 }
 
-- 
2.30.1


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

* [PATCH 08/11] block: simplify printk_all_partitions
  2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
                   ` (6 preceding siblings ...)
  2021-04-06  6:22 ` [PATCH 07/11] block: simplify partition_overlaps Christoph Hellwig
@ 2021-04-06  6:22 ` Christoph Hellwig
  2021-04-06  6:23 ` [PATCH 09/11] block: simplify show_partition Christoph Hellwig
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-04-06  6:22 UTC (permalink / raw)
  To: Jens Axboe, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

Just use xa_for_each to iterate over the partitions as there is no need
to grab a reference to each partition.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index e3f3c2321773f9..409ff4710f9215 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -806,10 +806,10 @@ void __init printk_all_partitions(void)
 	class_dev_iter_init(&iter, &block_class, NULL, &disk_type);
 	while ((dev = class_dev_iter_next(&iter))) {
 		struct gendisk *disk = dev_to_disk(dev);
-		struct disk_part_iter piter;
 		struct block_device *part;
 		char name_buf[BDEVNAME_SIZE];
 		char devt_buf[BDEVT_SIZE];
+		unsigned long idx;
 
 		/*
 		 * Don't show empty devices or things that have been
@@ -820,30 +820,29 @@ void __init printk_all_partitions(void)
 			continue;
 
 		/*
-		 * Note, unlike /proc/partitions, I am showing the
-		 * numbers in hex - the same format as the root=
-		 * option takes.
+		 * Note, unlike /proc/partitions, I am showing the numbers in
+		 * hex - the same format as the root= option takes.
 		 */
-		disk_part_iter_init(&piter, disk, DISK_PITER_INCL_PART0);
-		while ((part = disk_part_iter_next(&piter))) {
-			bool is_part0 = part == disk->part0;
-
-			printk("%s%s %10llu %s %s", is_part0 ? "" : "  ",
+		rcu_read_lock();
+		xa_for_each(&disk->part_tbl, idx, part) {
+			if (!bdev_nr_sectors(part))
+				continue;
+			printk("%s%s %10llu %s %s",
+			       bdev_is_partition(part) ? "  " : "",
 			       bdevt_str(part->bd_dev, devt_buf),
 			       bdev_nr_sectors(part) >> 1,
 			       disk_name(disk, part->bd_partno, name_buf),
 			       part->bd_meta_info ?
 					part->bd_meta_info->uuid : "");
-			if (is_part0) {
-				if (dev->parent && dev->parent->driver)
-					printk(" driver: %s\n",
-					      dev->parent->driver->name);
-				else
-					printk(" (driver?)\n");
-			} else
+			if (bdev_is_partition(part))
 				printk("\n");
+			else if (dev->parent && dev->parent->driver)
+				printk(" driver: %s\n",
+					dev->parent->driver->name);
+			else
+				printk(" (driver?)\n");
 		}
-		disk_part_iter_exit(&piter);
+		rcu_read_unlock();
 	}
 	class_dev_iter_exit(&iter);
 }
-- 
2.30.1


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

* [PATCH 09/11] block: simplify show_partition
  2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
                   ` (7 preceding siblings ...)
  2021-04-06  6:22 ` [PATCH 08/11] block: simplify printk_all_partitions Christoph Hellwig
@ 2021-04-06  6:23 ` Christoph Hellwig
  2021-04-06  6:23 ` [PATCH 10/11] block: simplify diskstats_show Christoph Hellwig
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-04-06  6:23 UTC (permalink / raw)
  To: Jens Axboe, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

Just use xa_for_each to iterate over the partitions as there is no need
to grab a reference to each partition.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 409ff4710f9215..5726714ef82fcf 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -907,8 +907,8 @@ static void *show_partition_start(struct seq_file *seqf, loff_t *pos)
 static int show_partition(struct seq_file *seqf, void *v)
 {
 	struct gendisk *sgp = v;
-	struct disk_part_iter piter;
 	struct block_device *part;
+	unsigned long idx;
 	char buf[BDEVNAME_SIZE];
 
 	/* Don't show non-partitionable removeable devices or empty devices */
@@ -918,15 +918,16 @@ static int show_partition(struct seq_file *seqf, void *v)
 	if (sgp->flags & GENHD_FL_SUPPRESS_PARTITION_INFO)
 		return 0;
 
-	/* show the full disk and all non-0 size partitions of it */
-	disk_part_iter_init(&piter, sgp, DISK_PITER_INCL_PART0);
-	while ((part = disk_part_iter_next(&piter)))
+	rcu_read_lock();
+	xa_for_each(&sgp->part_tbl, idx, part) {
+		if (!bdev_nr_sectors(part))
+			continue;
 		seq_printf(seqf, "%4d  %7d %10llu %s\n",
 			   MAJOR(part->bd_dev), MINOR(part->bd_dev),
 			   bdev_nr_sectors(part) >> 1,
 			   disk_name(sgp, part->bd_partno, buf));
-	disk_part_iter_exit(&piter);
-
+	}
+	rcu_read_unlock();
 	return 0;
 }
 
-- 
2.30.1


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

* [PATCH 10/11] block: simplify diskstats_show
  2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
                   ` (8 preceding siblings ...)
  2021-04-06  6:23 ` [PATCH 09/11] block: simplify show_partition Christoph Hellwig
@ 2021-04-06  6:23 ` Christoph Hellwig
  2021-04-06  6:23 ` [PATCH 11/11] block: remove disk_part_iter Christoph Hellwig
  2021-04-08 16:24 ` partition iteration simplifications Jens Axboe
  11 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-04-06  6:23 UTC (permalink / raw)
  To: Jens Axboe, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

Just use xa_for_each to iterate over the partitions as there is no need
to grab a reference to each partition.

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

diff --git a/block/genhd.c b/block/genhd.c
index 5726714ef82fcf..cbfe1ff19360b3 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1236,11 +1236,11 @@ const struct device_type disk_type = {
 static int diskstats_show(struct seq_file *seqf, void *v)
 {
 	struct gendisk *gp = v;
-	struct disk_part_iter piter;
 	struct block_device *hd;
 	char buf[BDEVNAME_SIZE];
 	unsigned int inflight;
 	struct disk_stats stat;
+	unsigned long idx;
 
 	/*
 	if (&disk_to_dev(gp)->kobj.entry == block_class.devices.next)
@@ -1250,8 +1250,10 @@ static int diskstats_show(struct seq_file *seqf, void *v)
 				"\n\n");
 	*/
 
-	disk_part_iter_init(&piter, gp, DISK_PITER_INCL_EMPTY_PART0);
-	while ((hd = disk_part_iter_next(&piter))) {
+	rcu_read_lock();
+	xa_for_each(&gp->part_tbl, idx, hd) {
+		if (bdev_is_partition(hd) && !bdev_nr_sectors(hd))
+			continue;
 		part_stat_read_all(hd, &stat);
 		if (queue_is_mq(gp->queue))
 			inflight = blk_mq_in_flight(gp->queue, hd);
@@ -1294,7 +1296,7 @@ static int diskstats_show(struct seq_file *seqf, void *v)
 						 NSEC_PER_MSEC)
 			);
 	}
-	disk_part_iter_exit(&piter);
+	rcu_read_unlock();
 
 	return 0;
 }
-- 
2.30.1


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

* [PATCH 11/11] block: remove disk_part_iter
  2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
                   ` (9 preceding siblings ...)
  2021-04-06  6:23 ` [PATCH 10/11] block: simplify diskstats_show Christoph Hellwig
@ 2021-04-06  6:23 ` Christoph Hellwig
  2021-04-08 16:24 ` partition iteration simplifications Jens Axboe
  11 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2021-04-06  6:23 UTC (permalink / raw)
  To: Jens Axboe, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

Just open code the xa_for_each in the remaining user.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c         | 92 ++++++-------------------------------------
 include/linux/genhd.h | 19 ---------
 2 files changed, 13 insertions(+), 98 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index cbfe1ff19360b3..39ca97b0edc612 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -161,81 +161,6 @@ static void part_in_flight_rw(struct block_device *part,
 		inflight[1] = 0;
 }
 
-/**
- * disk_part_iter_init - initialize partition iterator
- * @piter: iterator to initialize
- * @disk: disk to iterate over
- * @flags: DISK_PITER_* flags
- *
- * Initialize @piter so that it iterates over partitions of @disk.
- *
- * CONTEXT:
- * Don't care.
- */
-void disk_part_iter_init(struct disk_part_iter *piter, struct gendisk *disk,
-			  unsigned int flags)
-{
-	piter->disk = disk;
-	piter->part = NULL;
-	if (flags & (DISK_PITER_INCL_PART0 | DISK_PITER_INCL_EMPTY_PART0))
-		piter->idx = 0;
-	else
-		piter->idx = 1;
-	piter->flags = flags;
-}
-
-/**
- * disk_part_iter_next - proceed iterator to the next partition and return it
- * @piter: iterator of interest
- *
- * Proceed @piter to the next partition and return it.
- *
- * CONTEXT:
- * Don't care.
- */
-struct block_device *disk_part_iter_next(struct disk_part_iter *piter)
-{
-	struct block_device *part;
-	unsigned long idx;
-
-	/* put the last partition */
-	disk_part_iter_exit(piter);
-
-	rcu_read_lock();
-	xa_for_each_start(&piter->disk->part_tbl, idx, part, piter->idx) {
-		if (!bdev_nr_sectors(part) &&
-		    !(piter->flags & DISK_PITER_INCL_EMPTY) &&
-		    !(piter->flags & DISK_PITER_INCL_EMPTY_PART0 &&
-		      piter->idx == 0))
-			continue;
-
-		piter->part = bdgrab(part);
-		if (!piter->part)
-			continue;
-		piter->idx = idx + 1;
-		break;
-	}
-	rcu_read_unlock();
-
-	return piter->part;
-}
-
-/**
- * disk_part_iter_exit - finish up partition iteration
- * @piter: iter of interest
- *
- * Called when iteration is over.  Cleans up @piter.
- *
- * CONTEXT:
- * Don't care.
- */
-void disk_part_iter_exit(struct disk_part_iter *piter)
-{
-	if (piter->part)
-		bdput(piter->part);
-	piter->part = NULL;
-}
-
 /*
  * Can be deleted altogether. Later.
  *
@@ -472,13 +397,22 @@ static char *bdevt_str(dev_t devt, char *buf)
 
 void disk_uevent(struct gendisk *disk, enum kobject_action action)
 {
-	struct disk_part_iter piter;
 	struct block_device *part;
+	unsigned long idx;
+
+	rcu_read_lock();
+	xa_for_each(&disk->part_tbl, idx, part) {
+		if (bdev_is_partition(part) && !bdev_nr_sectors(part))
+			continue;
+		if (!bdgrab(part))
+			continue;
 
-	disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY_PART0);
-	while ((part = disk_part_iter_next(&piter)))
+		rcu_read_unlock();
 		kobject_uevent(bdev_kobj(part), action);
-	disk_part_iter_exit(&piter);
+		bdput(part);
+		rcu_read_lock();
+	}
+	rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(disk_uevent);
 
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 16178a935c4041..7e9660ea967d59 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -204,25 +204,6 @@ static inline dev_t disk_devt(struct gendisk *disk)
 
 void disk_uevent(struct gendisk *disk, enum kobject_action action);
 
-/*
- * Smarter partition iterator without context limits.
- */
-#define DISK_PITER_INCL_EMPTY	(1 << 1) /* include 0-sized parts */
-#define DISK_PITER_INCL_PART0	(1 << 2) /* include partition 0 */
-#define DISK_PITER_INCL_EMPTY_PART0 (1 << 3) /* include empty partition 0 */
-
-struct disk_part_iter {
-	struct gendisk		*disk;
-	struct block_device	*part;
-	unsigned long		idx;
-	unsigned int		flags;
-};
-
-extern void disk_part_iter_init(struct disk_part_iter *piter,
-				 struct gendisk *disk, unsigned int flags);
-struct block_device *disk_part_iter_next(struct disk_part_iter *piter);
-extern void disk_part_iter_exit(struct disk_part_iter *piter);
-
 /* block/genhd.c */
 extern void device_add_disk(struct device *parent, struct gendisk *disk,
 			    const struct attribute_group **groups);
-- 
2.30.1


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

* Re: [PATCH 01/11] dasd: use bdev_disk_changed instead of blk_drop_partitions
  2021-04-06  6:22 ` [PATCH 01/11] dasd: use bdev_disk_changed instead of blk_drop_partitions Christoph Hellwig
@ 2021-04-08 14:31   ` Stefan Haberland
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Haberland @ 2021-04-08 14:31 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

Am 06.04.21 um 08:22 schrieb Christoph Hellwig:
> Use the more general interface - the behavior is the same except
> that now a change uevent is sent, which is the right thing to do
> when the device becomes unusable.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/partitions/core.c         | 4 ----
>  drivers/s390/block/dasd_genhd.c | 3 +--
>  2 files changed, 1 insertion(+), 6 deletions(-)

Acked-by: Stefan Haberland <sth@linux.ibm.com>

Thanks,
Stefan



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

* Re: partition iteration simplifications
  2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
                   ` (10 preceding siblings ...)
  2021-04-06  6:23 ` [PATCH 11/11] block: remove disk_part_iter Christoph Hellwig
@ 2021-04-08 16:24 ` Jens Axboe
  11 siblings, 0 replies; 14+ messages in thread
From: Jens Axboe @ 2021-04-08 16:24 UTC (permalink / raw)
  To: Christoph Hellwig, Stefan Haberland, Jan Hoeppner
  Cc: Tejun Heo, linux-block, linux-s390

On 4/6/21 12:22 AM, Christoph Hellwig wrote:
> Hi all,
> 
> with the switch to an xarray for storing the partitions of a gendisk we
> can now use the xarray iterators for iterating over the partitions and don't
> really need the disk_part_iter_* scheme.  Also clean up the teardown of
> partitions where I think we have a (very unlikely to hit) race currently.

Applied, thanks.

-- 
Jens Axboe


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

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

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-06  6:22 partition iteration simplifications Christoph Hellwig
2021-04-06  6:22 ` [PATCH 01/11] dasd: use bdev_disk_changed instead of blk_drop_partitions Christoph Hellwig
2021-04-08 14:31   ` Stefan Haberland
2021-04-06  6:22 ` [PATCH 02/11] block: remove invalidate_partition Christoph Hellwig
2021-04-06  6:22 ` [PATCH 03/11] block: move more syncing and invalidation to delete_partition Christoph Hellwig
2021-04-06  6:22 ` [PATCH 04/11] block: refactor blk_drop_partitions Christoph Hellwig
2021-04-06  6:22 ` [PATCH 05/11] block: take bd_mutex around delete_partitions in del_gendisk Christoph Hellwig
2021-04-06  6:22 ` [PATCH 06/11] block: simplify partition removal Christoph Hellwig
2021-04-06  6:22 ` [PATCH 07/11] block: simplify partition_overlaps Christoph Hellwig
2021-04-06  6:22 ` [PATCH 08/11] block: simplify printk_all_partitions Christoph Hellwig
2021-04-06  6:23 ` [PATCH 09/11] block: simplify show_partition Christoph Hellwig
2021-04-06  6:23 ` [PATCH 10/11] block: simplify diskstats_show Christoph Hellwig
2021-04-06  6:23 ` [PATCH 11/11] block: remove disk_part_iter Christoph Hellwig
2021-04-08 16:24 ` partition iteration simplifications 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).