Skip to contentSkip to conversation

fix NVMe multipath partition diskstats

PATCH In review John Garry 3 participants 8 messages Sep 21 – Sep 28

Subsystems (2)
lore
Share & tools
lore

John Garry

author
#0 lore raw

v4 cover fix NVMe multipath partition diskstats

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.

Difference to v3:
- Add block device operation disk changed CB

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

John Garry (4):
block: support cloning partitions
block: don't show partition diskstats for GENHD_FL_HIDDEN
nvme-multipath: clone head disk partitions to per-path disks
nvme-multipath: fix diskstats for partitions

 block/genhd.c                 |  3 +-
 block/partitions/core.c       | 43 ++++++++++++++++++
 drivers/nvme/host/multipath.c | 83 ++++++++++++++++++++++++++++++-----
 include/linux/blkdev.h        |  2 +
 4 files changed, 120 insertions(+), 11 deletions(-)
Signature · 1 line
2.43.0
author
#1 lore raw

PATCH v4 1/4 block: support cloning partitions

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

Even though for hidden disks there is no 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.

Add struct block_device_operations.disk_changed_notify callback to allow block drivers be notified of when partition layout may have changed. In the NVMe multipath case, the head disk would use this callback to re-clone the partitions onto the per-path disks.

---
 block/partitions/core.c | 43 +++++++++++++++++++++++++++++++++++++++++
 include/linux/blkdev.h  |  2 ++
 2 files changed, 45 insertions(+)
block/partitions/core.c +43 -0
index b5c59b79ca7c..b03b206bc050 100644
@@ -703,6 +703,9 @@ int bdev_disk_changed(struct gendisk *disk, bool invalidate)
 		kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE);
 	}
 
+	if (disk->fops->disk_changed_notify)
+		disk->fops->disk_changed_notify(disk);
+
 	return ret;
 }
 /*
@@ -711,6 +714,46 @@ int bdev_disk_changed(struct gendisk *disk, bool invalidate)
  */
 EXPORT_SYMBOL_GPL(bdev_disk_changed);
 
+int bdev_clone_partitions(struct gendisk *disk, struct gendisk *mirror)
+{
+	struct block_device *part;
+	unsigned long idx;
+
+	if (disk_has_partscan(mirror))
+		return -EINVAL;
+
+	lockdep_assert_held(&disk->open_mutex);
+	mutex_lock_nested(&mirror->open_mutex, SINGLE_DEPTH_NESTING);
+
+	xa_for_each_start(&mirror->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->part_tbl, idx, part, 1) {
+		struct block_device *part_added;
+
+		part_added = add_partition(mirror, 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(mirror), "p%ld could not be added: %pe\n",
+				       idx, part_added);
+			}
+			mutex_unlock(&mirror->open_mutex);
+			return PTR_ERR(part_added);
+		}
+	}
+	mutex_unlock(&mirror->open_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(bdev_clone_partitions);
+
 void *read_part_sector(struct parsed_partitions *state, sector_t n, Sector *p)
 {
 	struct address_space *mapping = state->disk->part0->bd_mapping;
include/linux/blkdev.h +2 -0
index d003a9d2d1f6..249a4250a56d 100644
@@ -978,6 +978,7 @@ static inline unsigned int bdev_nr_zones(struct block_device *bdev)
 }
 
 int bdev_disk_changed(struct gendisk *disk, bool invalidate);
+int bdev_clone_partitions(struct gendisk *disk, struct gendisk *mirror);
 
 void put_disk(struct gendisk *disk);
 struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
@@ -1599,6 +1600,7 @@ struct block_device_operations {
 	/* returns the length of the identifier or a negative errno: */
 	int (*get_unique_id)(struct gendisk *disk, u8 id[16],
 			enum blk_unique_id id_type);
+	void (*disk_changed_notify)(struct gendisk *disk);
 	struct module *owner;
 	const struct pr_ops *pr_ops;
 
Signature · 1 line
2.43.0
author
#2 lore raw

PATCH v4 2/4 block: don't show partition diskstats for GENHD_FL_HIDDEN

In reply to John Garry

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

However, now hidden disks may 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.

---
 block/genhd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
block/genhd.c +2 -1
index 10ca8b4d6eea..60dc7a61ea8a 100644
@@ -1376,7 +1376,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);
Signature · 1 line
2.43.0
author
#3 lore raw

PATCH v4 3/4 nvme-multipath: clone head disk partitions to per-path disks

In reply to John Garry

To fix diskstats per partition, it is required that the per-path disk actually has an up-to-date partition table. This partition table would be a clone of the head disk partition table.

At two points the partition table needs to be updated:
- when the per-path disk is added in nvme_mpath_add_disk()
- for partition table updates to the head disk, add nvme_ns_head_disk_changed() to clone the partition table to per-path disks

---
 drivers/nvme/host/multipath.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
drivers/nvme/host/multipath.c +32 -0
index 75dbb58286a3..e871ad40d893 100644
@@ -623,6 +623,23 @@ static int nvme_ns_head_report_zones(struct gendisk *disk, sector_t sector,
 #define nvme_ns_head_report_zones	NULL
 #endif /* CONFIG_BLK_DEV_ZONED */
 
+static void nvme_ns_head_disk_changed(struct gendisk *disk)
+{
+	struct nvme_ns_head *head = disk->private_data;
+	struct nvme_ns *ns;
+	int srcu_idx;
+
+	lockdep_assert_held(&disk->open_mutex);
+
+	srcu_idx = srcu_read_lock(&head->srcu);
+	list_for_each_entry_srcu(ns, &head->list, siblings,
+				 srcu_read_lock_held(&head->srcu)) {
+		if (bdev_clone_partitions(head->disk, ns->disk))
+			clear_bit(NVME_NS_READY, &ns->flags);
+	}
+	srcu_read_unlock(&head->srcu, srcu_idx);
+}
+
 const struct block_device_operations nvme_ns_head_ops = {
 	.owner		= THIS_MODULE,
 	.submit_bio	= nvme_ns_head_submit_bio,
@@ -634,6 +651,7 @@ const struct block_device_operations nvme_ns_head_ops = {
 	.get_unique_id	= nvme_ns_head_get_unique_id,
 	.report_zones	= nvme_ns_head_report_zones,
 	.pr_ops		= &nvme_pr_ops,
+	.disk_changed_notify	= nvme_ns_head_disk_changed,
 };
 
 static const struct file_operations nvme_ns_head_chr_fops = {
@@ -1360,6 +1378,20 @@ void nvme_mpath_remove_sysfs_link(struct nvme_ns *ns)
 
 void nvme_mpath_add_disk(struct nvme_ns *ns, __le32 anagrpid)
 {
+	struct nvme_ns_head *head = ns->head;
+	int ret;
+
+	if (!head->disk)
+		return;
+
+	mutex_lock(&head->disk->open_mutex);
+	ret = bdev_clone_partitions(head->disk, ns->disk);
+	mutex_unlock(&head->disk->open_mutex);
+	if (ret) {
+		clear_bit(NVME_NS_READY, &ns->flags);
+		return;
+	}
+
 	if (nvme_ctrl_use_ana(ns->ctrl)) {
 		struct nvme_ana_group_desc desc = {
 			.grpid = anagrpid,
Signature · 1 line
2.43.0
author
#4 lore raw

PATCH v4 4/4 nvme-multipath: fix diskstats for partitions

In reply to John Garry

Currently diskstats for partitions are never updated:

$ ./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

This is because we only ever update the diskstats for the multipath disk in nvme_mpath_end_request(), and we never take into account that the original bi_bdev may been a partition of this disk.

Functions bdev_start_io_acct() and bdev_start_io_acct() do handle updating diskstats for a partition, in that they also update the whole disk also (if a partition), so use the partition (if applicable) when calling those functions.

Change any functionality which used to lookup the gendisk part0 to now lookup the specific gendisk partition.

---
 drivers/nvme/host/multipath.c | 51 ++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 10 deletions(-)
drivers/nvme/host/multipath.c +41 -10
index e871ad40d893..2969c0e5187c 100644
@@ -144,6 +144,23 @@ void nvme_mpath_start_freeze(struct nvme_subsystem *subsys)
 			blk_freeze_queue_start(h->disk->queue);
 }
 
+static struct block_device *nvme_disk_to_part(struct gendisk *disk,
+						u8 partno)
+{
+	/* Quick lookup for whole disk */
+	if (!partno)
+		return disk->part0;
+	return xa_load(&disk->part_tbl, partno);
+}
+
+static struct block_device *nvme_to_disk_part(struct gendisk *disk,
+					struct block_device *bdev)
+{
+	if (!bdev)
+		return disk->part0;
+	return nvme_disk_to_part(disk, bdev_partno(bdev));
+}
+
 void nvme_failover_req(struct request *req)
 {
 	struct nvme_ns *ns = req->q->queuedata;
@@ -165,8 +182,10 @@ void nvme_failover_req(struct request *req)
 	}
 
 	spin_lock_irqsave(&ns->head->requeue_lock, flags);
-	for (bio = req->bio; bio; bio = bio->bi_next)
-		bio_set_dev(bio, ns->head->disk->part0);
+	for (bio = req->bio; bio; bio = bio->bi_next) {
+		bio_set_dev(bio, nvme_to_disk_part(ns->head->disk,
+				bio->bi_bdev));
+	}
 	blk_steal_bios(&ns->head->requeue_list, req);
 	spin_unlock_irqrestore(&ns->head->requeue_lock, flags);
 
@@ -194,8 +213,9 @@ void nvme_mpath_start_request(struct request *rq)
 		return;
 
 	nvme_req(rq)->flags |= NVME_MPATH_IO_STATS;
-	nvme_req(rq)->start_time = bdev_start_io_acct(disk->part0, req_op(rq),
-						      jiffies);
+	nvme_req(rq)->start_time = bdev_start_io_acct(
+			nvme_to_disk_part(disk, rq->part),
+			req_op(rq), jiffies);
 }
 EXPORT_SYMBOL_GPL(nvme_mpath_start_request);
 
@@ -208,7 +228,8 @@ void nvme_mpath_end_request(struct request *rq)
 
 	if (!(nvme_req(rq)->flags & NVME_MPATH_IO_STATS))
 		return;
-	bdev_end_io_acct(ns->head->disk->part0, req_op(rq),
+	bdev_end_io_acct(nvme_to_disk_part(ns->head->disk,
+			 rq->part), req_op(rq),
 			 blk_rq_bytes(rq) >> SECTOR_SHIFT,
 			 nvme_req(rq)->start_time);
 }
@@ -530,11 +551,21 @@ static bool nvme_available_path(struct nvme_ns_head *head)
 	return nvme_mpath_queue_if_no_path(head);
 }
 
+static struct block_device *nvme_find_path_bdev(struct nvme_ns_head *head,
+						u8 partno)
+{
+	struct nvme_ns *ns = nvme_find_path(head);
+
+	if (!ns)
+		return NULL;
+	return nvme_disk_to_part(ns->disk, partno);
+}
+
 static void nvme_ns_head_submit_bio(struct bio *bio)
 {
 	struct nvme_ns_head *head = bio->bi_bdev->bd_disk->private_data;
 	struct device *dev = disk_to_dev(head->disk);
-	struct nvme_ns *ns;
+	struct block_device *bdev;
 	int srcu_idx;
 
 	/*
@@ -547,9 +578,9 @@ static void nvme_ns_head_submit_bio(struct bio *bio)
 		return;
 
 	srcu_idx = srcu_read_lock(&head->srcu);
-	ns = nvme_find_path(head);
-	if (likely(ns)) {
-		bio_set_dev(bio, ns->disk->part0);
+	bdev = nvme_find_path_bdev(head, bdev_partno(bio->bi_bdev));
+	if (likely(bdev)) {
+		bio_set_dev(bio, bdev);
 		/*
 		 * Use BIO_REMAPPED to skip bio_check_eod() when this bio
 		 * enters submit_bio_noacct() for the per-path device. The EOD
@@ -557,7 +588,7 @@ static void nvme_ns_head_submit_bio(struct bio *bio)
 		 */
 		bio_set_flag(bio, BIO_REMAPPED);
 		bio->bi_opf |= REQ_NVME_MPATH;
-		trace_block_bio_remap(bio, disk_devt(ns->head->disk),
+		trace_block_bio_remap(bio, disk_devt(head->disk),
 				      bio->bi_iter.bi_sector);
 		submit_bio_noacct(bio);
 	} else if (nvme_available_path(head)) {
Signature · 1 line
2.43.0
#5 lore raw
In reply to John Garry

On 21/09/2026 09:37, John Garry wrote:

A reminder on this one ... or are we just going to leave this broken?

Quoted 43 lines

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.

Difference to v3:
- Add block device operation disk changed CB

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

John Garry (4):
block: support cloning partitions
block: don't show partition diskstats for GENHD_FL_HIDDEN
nvme-multipath: clone head disk partitions to per-path disks
nvme-multipath: fix diskstats for partitions

  block/genhd.c                 |  3 +-
  block/partitions/core.c       | 43 ++++++++++++++++++
  drivers/nvme/host/multipath.c | 83 ++++++++++++++++++++++++++++++-----
  include/linux/blkdev.h        |  2 +
  4 files changed, 120 insertions(+), 11 deletions(-)
#6 lore raw
Quoted 3 lines · John Garry

On 21/09/2026 09:37, John Garry wrote:

A reminder on this one ... or are we just going to leave this broken?

I find the amount of overhead and relatively invasive code very offputting, and I still don't understand why we now have a strong urge to address this after 10 years.

author
#7 lore raw
Quoted 8 lines · Christoph Hellwig
Quoted 3 lines · John Garry

On 21/09/2026 09:37, John Garry wrote:

A reminder on this one ... or are we just going to leave this broken?

I find the amount of overhead and relatively invasive code very offputting, and I still don't understand why we now have a strong urge to address this after 10 years.

Nobody noticed (that the values are not maintained), I guess.

It may also be useful to have the per-path disk partition table in future for other reasons.

Anyway, your call, so I'll leave it for now.