[PATCH RFC v3 0/3] fix NVMe multipath partition diskstats

[PATCH RFC v3 0/3] fix NVMe multipath partition diskstats

[John Garry]

Currently the NVMe multipath partition diskstats are not maintained.
This is because the functionality to update the diskstats only updates
for the whole disk (and not per partition).

As an example:
$ ./fio_read nvme1n1p1 # run traffic on /dev/nvme1n1p1
...
$ more /proc/diskstats | grep nvme1
 259       2 nvme1c1n1 49857 0 400344 768565 0 0 0 0 0 2334 768565 0 0 0 0 0 0
 259       3 nvme1n1 99710 0 800680 1599285 0 0 0 0 0 2346 1599285 0 0 0 0 0 0
 259       5 nvme1n1p1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
 259       4 nvme1c2n1 49853 0 400336 831472 0 0 0 0 0 2315 831472 0 0 0 0 0 0

Notice how all values are 0 for nvme1n1p1.

This series addresses that problem by first updating the gendisk code to
create the partition table for hidden disks, as suggested by Keith.

This is done by cloning the partition table from the multipath disk.

Secondly the NVMe multipath functionality to handle the diskstats is
modified to handle the gendisk partition, by looking up this partition
from the per-path disk partition.

Setting as RFC as I am not too happy with the block partition cloning
code.

Difference to v2:
- don't scan per-path disk, and clone from multipath disk instead

John Garry (3):
  block: support linking disks to clone partitions
  block: don't show partition diskstats for GENHD_FL_HIDDEN
  nvme-multipath: fix diskstats for partitions

 block/blk.h                   |  1 +
 block/genhd.c                 | 35 +++++++++++++++++++++++-
 block/partitions/core.c       | 45 +++++++++++++++++++++++++++++++
 drivers/nvme/host/core.c      |  1 +
 drivers/nvme/host/multipath.c | 51 ++++++++++++++++++++++++++++-------
 include/linux/blkdev.h        |  8 ++++++
 6 files changed, 130 insertions(+), 11 deletions(-)

-- 
2.43.7

[PATCH RFC v3 1/3] block: support linking disks to clone partitions

[John Garry]

From: John Garry <john.garry@linux.dev>

For NVMe multipath support, per-path gendisks have flag GENHD_FL_HIDDEN
set. This means that no partition scan we be run for those disks. However
the head disk will still have its partitions scanned.

Even though for hidden disks there is not partition scan, it can be useful
to have the partition table available. Such is a case for NVMe multipath
when we want to send bios to specific partitions.

Support allowing a disk to clone the partitions from a multipath head disk.
The head disk would be a mirror (of that same disk).

The head disk must keep track of the mirror disks, as if the partition
table for the head disk is updated, the partition table for the mirrors
must automatically be updated as well.

Similarly, the mirror must keep a reference to the head disk to allow
updating its partition table when that mirror disk is added.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk.h             |  1 +
 block/genhd.c           | 32 +++++++++++++++++++++++++++++
 block/partitions/core.c | 45 +++++++++++++++++++++++++++++++++++++++++
 include/linux/blkdev.h  |  8 ++++++++
 4 files changed, 86 insertions(+)

diff --git a/block/blk.h b/block/blk.h
index fb95d3c58950c..9e9a0d3b9950d 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -638,6 +638,7 @@ int bdev_del_partition(struct gendisk *disk, int partno);
 int bdev_resize_partition(struct gendisk *disk, int partno, sector_t start,
 		sector_t length);
 void drop_partition(struct block_device *part);
+int bdev_clone_partitions(struct gendisk *disk);
 
 void bdev_set_nr_sectors(struct block_device *bdev, sector_t sectors);
 
diff --git a/block/genhd.c b/block/genhd.c
index 30ac0ffe65174..79cf26e8f5dbe 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -423,6 +423,17 @@ static void add_disk_final(struct gendisk *disk)
 		disk_uevent(disk, KOBJ_ADD);
 	}
 
+	if (disk->head) {
+		int ret;
+
+		mutex_lock(&disk->head->open_mutex);
+		ret = bdev_clone_partitions(disk);
+		mutex_unlock(&disk->head->open_mutex);
+		if (ret)
+			dev_err(disk_to_dev(disk), "could not clone partitions (%d)\n",
+				ret);
+	}
+
 	blk_apply_bdi_limits(disk->bdi, &disk->queue->limits);
 	disk_add_events(disk);
 	set_bit(GD_ADDED, &disk->state);
@@ -436,6 +447,9 @@ static int __add_disk(struct device *parent, struct gendisk *disk,
 	struct device *ddev = disk_to_dev(disk);
 	int ret;
 
+	if (disk->head && disk_has_partscan(disk))
+		return -EINVAL;
+
 	if (WARN_ON_ONCE(bdev_nr_sectors(disk->part0) > BLK_DEV_MAX_SECTORS))
 		return -EINVAL;
 
@@ -712,6 +726,12 @@ static void __del_gendisk(struct gendisk *disk)
 		bdev_unhash(part);
 	mutex_unlock(&disk->open_mutex);
 
+	if (disk->head) {
+		mutex_lock(&disk->head->open_mutex);
+		hlist_del(&disk->mirror_node);
+		mutex_unlock(&disk->head->open_mutex);
+	}
+
 	/*
 	 * Tell the file system to write back all dirty data and shut down if
 	 * it hasn't been notified earlier.
@@ -1509,6 +1529,18 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 	return NULL;
 }
 
+void disk_add_mirror(struct gendisk *head, struct gendisk *mirror)
+{
+	if (!head)
+		return;
+	WARN_ON_ONCE(test_bit(GD_ADDED, &mirror->state));
+	mirror->head = head;
+	mutex_lock(&head->open_mutex);
+	hlist_add_head(&mirror->mirror_node, &head->mirror_head);
+	mutex_unlock(&head->open_mutex);
+}
+EXPORT_SYMBOL_GPL(disk_add_mirror);
+
 struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
 		struct lock_class_key *lkclass)
 {
diff --git a/block/partitions/core.c b/block/partitions/core.c
index b5c59b79ca7cb..baf792becf699 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -644,6 +644,7 @@ static int blk_add_partitions(struct gendisk *disk)
 int bdev_disk_changed(struct gendisk *disk, bool invalidate)
 {
 	struct block_device *part;
+	struct gendisk *mirror;
 	unsigned long idx;
 	int ret = 0;
 
@@ -703,6 +704,13 @@ int bdev_disk_changed(struct gendisk *disk, bool invalidate)
 		kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE);
 	}
 
+	hlist_for_each_entry(mirror, &disk->mirror_head,
+				mirror_node) {
+		ret = bdev_clone_partitions(mirror);
+		if (ret)
+			break;
+	}
+
 	return ret;
 }
 /*
@@ -711,6 +719,43 @@ int bdev_disk_changed(struct gendisk *disk, bool invalidate)
  */
 EXPORT_SYMBOL_GPL(bdev_disk_changed);
 
+int bdev_clone_partitions(struct gendisk *disk)
+{
+	struct block_device *part;
+	unsigned long idx;
+
+	if (!disk->head)
+		return -EINVAL;
+
+	mutex_lock(&disk->open_mutex);
+	xa_for_each_start(&disk->part_tbl, idx, part, 1) {
+		/* Same as bdev_disk_changed() */
+		bdev_unhash(part);
+		WARN_ON_ONCE(atomic_read(&part->bd_openers));
+		invalidate_bdev(part);
+		drop_partition(part);
+	}
+
+	xa_for_each_start(&disk->head->part_tbl, idx, part, 1) {
+		struct block_device *part_added;
+
+		part_added = add_partition(disk, idx, part->bd_start_sect,
+					part->bd_nr_sectors, ADDPART_FLAG_NONE,
+					part->bd_meta_info);
+		if (IS_ERR(part_added)) {
+			if (PTR_ERR(part_added) != -ENXIO) {
+				dev_err(disk_to_dev(disk), "p%ld could not be added: %pe\n",
+				       idx, part_added);
+			}
+			mutex_unlock(&disk->open_mutex);
+			return PTR_ERR(part_added);
+		}
+	}
+	mutex_unlock(&disk->open_mutex);
+
+	return 0;
+}
+
 void *read_part_sector(struct parsed_partitions *state, sector_t n, Sector *p)
 {
 	struct address_space *mapping = state->disk->part0->bd_mapping;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 9213a5716f95a..3a9502f5e22c6 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -187,6 +187,13 @@ struct gendisk {
 #ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
 	struct list_head slave_bdevs;
 #endif
+	union {
+		struct hlist_head mirror_head;
+		struct {
+			struct hlist_node mirror_node;
+			struct gendisk *head;
+		};
+	};
 	struct timer_rand_state *random;
 	struct disk_events *ev;
 
@@ -977,6 +984,7 @@ static inline unsigned int bdev_nr_zones(struct block_device *bdev)
 }
 
 int bdev_disk_changed(struct gendisk *disk, bool invalidate);
+void disk_add_mirror(struct gendisk *head, struct gendisk *mirror);
 
 void put_disk(struct gendisk *disk);
 struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
-- 
2.43.7

[PATCH RFC v3 2/3] block: don't show partition diskstats for GENHD_FL_HIDDEN

[John Garry]

From: John Garry <john.garry@linux.dev>

When flag GENHD_FL_HIDDEN is set for a disk, normally the partition table
were not available and so we won't get its partitions showing in diskstats.

However, now hidden disks may have partition tables.

The diskstats for partitions on disks with GENHD_FL_HIDDEN can bloat
the output and is not too interesting, so always hide it.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/genhd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/genhd.c b/block/genhd.c
index 79cf26e8f5dbe..896369a96f96c 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1388,7 +1388,8 @@ static int diskstats_show(struct seq_file *seqf, void *v)
 
 	rcu_read_lock();
 	xa_for_each(&gp->part_tbl, idx, hd) {
-		if (bdev_is_partition(hd) && !bdev_nr_sectors(hd))
+		if (bdev_is_partition(hd) &&
+		    (!bdev_nr_sectors(hd) || (gp->flags & GENHD_FL_HIDDEN)))
 			continue;
 
 		inflight = bdev_count_inflight(hd);
-- 
2.43.7