All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.