* [PATCH v7 1/9] block: expose blk_stat_{enable,disable}_accounting() to drivers
2026-08-09 10:07 [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy Nilay Shroff
@ 2026-08-09 10:07 ` Nilay Shroff
2026-08-10 8:18 ` John Garry
2026-08-09 10:07 ` [PATCH v7 2/9] block: record I/O request start time for passthru request Nilay Shroff
` (9 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: Nilay Shroff @ 2026-08-09 10:07 UTC (permalink / raw)
To: linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, john.g.garry, gjoyce, Nilay Shroff
The functions blk_stat_enable_accounting() and
blk_stat_disable_accounting() are currently exported, but their
prototypes are only defined in a private header. Move these prototypes
into a common header so that block drivers can directly use these APIs.
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
block/blk-stat.h | 4 ----
include/linux/blk-mq.h | 4 ++++
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/blk-stat.h b/block/blk-stat.h
index cc5b66e7ee60..b1614a1ef4ad 100644
--- a/block/blk-stat.h
+++ b/block/blk-stat.h
@@ -70,10 +70,6 @@ void blk_free_queue_stats(struct blk_queue_stats *);
void blk_stat_add(struct request *rq, u64 now);
-/* record time/size info in request but not add a callback */
-void blk_stat_enable_accounting(struct request_queue *q);
-void blk_stat_disable_accounting(struct request_queue *q);
-
/**
* blk_stat_alloc_callback() - Allocate a block statistics callback.
* @timer_fn: Timer callback function.
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index af878597afb8..3956909764bf 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -753,6 +753,10 @@ int blk_rq_poll(struct request *rq, struct io_comp_batch *iob,
bool blk_mq_queue_inflight(struct request_queue *q);
+/* record time/size info in request but not add a callback */
+void blk_stat_enable_accounting(struct request_queue *q);
+void blk_stat_disable_accounting(struct request_queue *q);
+
enum {
/* return when out of requests */
BLK_MQ_REQ_NOWAIT = (__force blk_mq_req_flags_t)(1 << 0),
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v7 1/9] block: expose blk_stat_{enable,disable}_accounting() to drivers
2026-08-09 10:07 ` [PATCH v7 1/9] block: expose blk_stat_{enable,disable}_accounting() to drivers Nilay Shroff
@ 2026-08-10 8:18 ` John Garry
0 siblings, 0 replies; 30+ messages in thread
From: John Garry @ 2026-08-10 8:18 UTC (permalink / raw)
To: Nilay Shroff, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
> The functions blk_stat_enable_accounting() and
> blk_stat_disable_accounting() are currently exported, but their
> prototypes are only defined in a private header. Move these prototypes
> into a common header so that block drivers can directly use these APIs.
It might be worth mentioning somewhere why this is in blk-mq.h (which I
believe is because that these accounting functions are only relevant to
blk-mq queues now, which would not be guessed by the function names).
But I don't feel too strongly about this.
>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
Reviewed-by: John Garry <john.g.garry@oracle.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v7 2/9] block: record I/O request start time for passthru request
2026-08-09 10:07 [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy Nilay Shroff
2026-08-09 10:07 ` [PATCH v7 1/9] block: expose blk_stat_{enable,disable}_accounting() to drivers Nilay Shroff
@ 2026-08-09 10:07 ` Nilay Shroff
2026-08-10 6:44 ` Hannes Reinecke
2026-08-09 10:07 ` [PATCH v7 3/9] nvme-multipath: pass I/O type to nvme_find_path() Nilay Shroff
` (8 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: Nilay Shroff @ 2026-08-09 10:07 UTC (permalink / raw)
To: linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, john.g.garry, gjoyce, Nilay Shroff
While starting an I/O request, blk_mq_start_request() records the
request start timestamp only for non-passthrough requests when
QUEUE_FLAG_STATS is enabled.
However, the latency based multipath policy uses request completion
latency to evaluate path performance, and I/O is issued as passthrough
requests. Since passthru requests never initialize rq->io_start_time_ns,
their latency cannot be computed.
Record io_start_time_ns for all requests whenever QUEUE_FLAG_STATS is
enabled.
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
block/blk-mq.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2c850330a32b..38922209a24f 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1340,12 +1340,14 @@ void blk_mq_start_request(struct request *rq)
trace_block_rq_issue(rq);
- if (test_bit(QUEUE_FLAG_STATS, &q->queue_flags) &&
- !blk_rq_is_passthrough(rq)) {
+ if (test_bit(QUEUE_FLAG_STATS, &q->queue_flags)) {
rq->io_start_time_ns = blk_time_get_ns();
- rq->stats_sectors = blk_rq_sectors(rq);
- rq->rq_flags |= RQF_STATS;
- rq_qos_issue(q, rq);
+
+ if (!blk_rq_is_passthrough(rq)) {
+ rq->stats_sectors = blk_rq_sectors(rq);
+ rq->rq_flags |= RQF_STATS;
+ rq_qos_issue(q, rq);
+ }
}
WARN_ON_ONCE(blk_mq_rq_state(rq) != MQ_RQ_IDLE);
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v7 2/9] block: record I/O request start time for passthru request
2026-08-09 10:07 ` [PATCH v7 2/9] block: record I/O request start time for passthru request Nilay Shroff
@ 2026-08-10 6:44 ` Hannes Reinecke
0 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2026-08-10 6:44 UTC (permalink / raw)
To: Nilay Shroff, linux-nvme
Cc: kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, john.g.garry, gjoyce
On 8/9/26 12:07 PM, Nilay Shroff wrote:
> While starting an I/O request, blk_mq_start_request() records the
> request start timestamp only for non-passthrough requests when
> QUEUE_FLAG_STATS is enabled.
>
> However, the latency based multipath policy uses request completion
> latency to evaluate path performance, and I/O is issued as passthrough
> requests. Since passthru requests never initialize rq->io_start_time_ns,
> their latency cannot be computed.
>
> Record io_start_time_ns for all requests whenever QUEUE_FLAG_STATS is
> enabled.
>
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
> ---
> block/blk-mq.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 2c850330a32b..38922209a24f 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1340,12 +1340,14 @@ void blk_mq_start_request(struct request *rq)
>
> trace_block_rq_issue(rq);
>
> - if (test_bit(QUEUE_FLAG_STATS, &q->queue_flags) &&
> - !blk_rq_is_passthrough(rq)) {
> + if (test_bit(QUEUE_FLAG_STATS, &q->queue_flags)) {
> rq->io_start_time_ns = blk_time_get_ns();
> - rq->stats_sectors = blk_rq_sectors(rq);
> - rq->rq_flags |= RQF_STATS;
> - rq_qos_issue(q, rq);
> +
> + if (!blk_rq_is_passthrough(rq)) {
> + rq->stats_sectors = blk_rq_sectors(rq);
> + rq->rq_flags |= RQF_STATS;
> + rq_qos_issue(q, rq);
> + }
> }
>
> WARN_ON_ONCE(blk_mq_rq_state(rq) != MQ_RQ_IDLE);
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v7 3/9] nvme-multipath: pass I/O type to nvme_find_path()
2026-08-09 10:07 [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy Nilay Shroff
2026-08-09 10:07 ` [PATCH v7 1/9] block: expose blk_stat_{enable,disable}_accounting() to drivers Nilay Shroff
2026-08-09 10:07 ` [PATCH v7 2/9] block: record I/O request start time for passthru request Nilay Shroff
@ 2026-08-09 10:07 ` Nilay Shroff
2026-08-10 8:57 ` John Garry
2026-08-09 10:07 ` [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy Nilay Shroff
` (7 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: Nilay Shroff @ 2026-08-09 10:07 UTC (permalink / raw)
To: linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, john.g.garry, gjoyce, Nilay Shroff
Currently, nvme_find_path() only accepts an nvme_ns_head argument.
However, the upcoming latency-aware I/O policy also needs to know
the I/O type (read/write/other) associated with the request in order
to make path selection decisions.
Update nvme_find_path() to accept an additional argument describing
the I/O type. This patch does not introduce any functional change and
only prepares the interface for subsequent latency-policy changes.
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
drivers/nvme/host/ioctl.c | 38 +++++++++++++++++++++++++++++---
drivers/nvme/host/multipath.c | 9 ++++----
drivers/nvme/host/nvme.h | 41 ++++++++++++++++++++++++++++++++++-
drivers/nvme/host/pr.c | 6 +++--
drivers/nvme/host/sysfs.c | 2 +-
5 files changed, 85 insertions(+), 11 deletions(-)
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 664216eece4a..8a8ed132fab2 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -708,18 +708,29 @@ static int nvme_ns_head_ctrl_ioctl(struct nvme_ns *ns, unsigned int cmd,
int nvme_ns_head_ioctl(struct block_device *bdev, blk_mode_t mode,
unsigned int cmd, unsigned long arg)
{
+ u8 opcode;
struct nvme_ns_head *head = bdev->bd_disk->private_data;
bool open_for_write = mode & BLK_OPEN_WRITE;
void __user *argp = (void __user *)arg;
struct nvme_ns *ns;
int srcu_idx, ret = -EWOULDBLOCK;
unsigned int flags = 0;
+ unsigned int op_type = NVME_STAT_OTHER;
if (bdev_is_partition(bdev))
flags |= NVME_IOCTL_PARTITION;
+ if (cmd == NVME_IOCTL_SUBMIT_IO) {
+ if (get_user(opcode, (u8 *)argp))
+ return -EFAULT;
+ if (opcode == nvme_cmd_write)
+ op_type = NVME_STAT_WRITE;
+ else if (opcode == nvme_cmd_read)
+ op_type = NVME_STAT_READ;
+ }
+
srcu_idx = srcu_read_lock(&head->srcu);
- ns = nvme_find_path(head);
+ ns = nvme_find_path(head, op_type);
if (!ns)
goto out_unlock;
@@ -741,6 +752,7 @@ int nvme_ns_head_ioctl(struct block_device *bdev, blk_mode_t mode,
long nvme_ns_head_chr_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
+ u8 opcode;
bool open_for_write = file->f_mode & FMODE_WRITE;
struct cdev *cdev = file_inode(file)->i_cdev;
struct nvme_ns_head *head =
@@ -748,9 +760,19 @@ long nvme_ns_head_chr_ioctl(struct file *file, unsigned int cmd,
void __user *argp = (void __user *)arg;
struct nvme_ns *ns;
int srcu_idx, ret = -EWOULDBLOCK;
+ unsigned int op_type = NVME_STAT_OTHER;
+
+ if (cmd == NVME_IOCTL_SUBMIT_IO) {
+ if (get_user(opcode, (u8 *)argp))
+ return -EFAULT;
+ if (opcode == nvme_cmd_write)
+ op_type = NVME_STAT_WRITE;
+ else if (opcode == nvme_cmd_read)
+ op_type = NVME_STAT_READ;
+ }
srcu_idx = srcu_read_lock(&head->srcu);
- ns = nvme_find_path(head);
+ ns = nvme_find_path(head, op_type);
if (!ns)
goto out_unlock;
@@ -770,9 +792,19 @@ int nvme_ns_head_chr_uring_cmd(struct io_uring_cmd *ioucmd,
struct cdev *cdev = file_inode(ioucmd->file)->i_cdev;
struct nvme_ns_head *head = container_of(cdev, struct nvme_ns_head, cdev);
int srcu_idx = srcu_read_lock(&head->srcu);
- struct nvme_ns *ns = nvme_find_path(head);
+ struct nvme_ns *ns;
int ret = -EINVAL;
+ const struct nvme_uring_cmd *cmd = io_uring_sqe128_cmd(ioucmd->sqe,
+ struct nvme_uring_cmd);
+ unsigned int op_type = NVME_STAT_OTHER;
+ __u8 opcode = READ_ONCE(cmd->opcode);
+
+ if (opcode == nvme_cmd_write)
+ op_type = NVME_STAT_WRITE;
+ else if (opcode == nvme_cmd_read)
+ op_type = NVME_STAT_READ;
+ ns = nvme_find_path(head, op_type);
if (ns)
ret = nvme_ns_uring_cmd(ns, ioucmd, issue_flags);
srcu_read_unlock(&head->srcu, srcu_idx);
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 9b9a657fa330..8c20ff516e61 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -460,7 +460,8 @@ static struct nvme_ns *nvme_numa_path(struct nvme_ns_head *head)
return ns;
}
-inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head)
+inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head,
+ unsigned int op_type)
{
switch (READ_ONCE(head->subsys->iopolicy)) {
case NVME_IOPOLICY_QD:
@@ -522,7 +523,7 @@ static void nvme_ns_head_submit_bio(struct bio *bio)
return;
srcu_idx = srcu_read_lock(&head->srcu);
- ns = nvme_find_path(head);
+ ns = nvme_find_path(head, __nvme_data_dir(bio_op(bio)));
if (likely(ns)) {
bio_set_dev(bio, ns->disk->part0);
/*
@@ -572,7 +573,7 @@ static int nvme_ns_head_get_unique_id(struct gendisk *disk, u8 id[16],
int srcu_idx, ret = -EWOULDBLOCK;
srcu_idx = srcu_read_lock(&head->srcu);
- ns = nvme_find_path(head);
+ ns = nvme_find_path(head, NVME_STAT_OTHER);
if (ns)
ret = nvme_ns_get_unique_id(ns, id, type);
srcu_read_unlock(&head->srcu, srcu_idx);
@@ -588,7 +589,7 @@ static int nvme_ns_head_report_zones(struct gendisk *disk, sector_t sector,
int srcu_idx, ret = -EWOULDBLOCK;
srcu_idx = srcu_read_lock(&head->srcu);
- ns = nvme_find_path(head);
+ ns = nvme_find_path(head, NVME_STAT_OTHER);
if (ns)
ret = nvme_ns_report_zones(ns, sector, nr_zones, args);
srcu_read_unlock(&head->srcu, srcu_idx);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 824651cc898d..8a9ec502912d 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -520,6 +520,13 @@ struct nvme_ns_ids {
u8 csi;
};
+enum nvme_stat_group {
+ NVME_STAT_READ,
+ NVME_STAT_WRITE,
+ NVME_STAT_OTHER,
+ NVME_NUM_STAT_GROUPS
+};
+
/*
* Anchor structure for namespaces. There is one for each namespace in a
* NVMe subsystem that any of our controllers can see, and the namespace
@@ -1032,7 +1039,39 @@ extern const struct attribute_group *nvme_dev_attr_groups[];
extern const struct block_device_operations nvme_bdev_ops;
void nvme_delete_ctrl_sync(struct nvme_ctrl *ctrl);
-struct nvme_ns *nvme_find_path(struct nvme_ns_head *head);
+struct nvme_ns *nvme_find_path(struct nvme_ns_head *head, unsigned int op_type);
+
+static inline int __nvme_data_dir(const enum req_op op)
+{
+ if (op == REQ_OP_READ)
+ return NVME_STAT_READ;
+ else if (op == REQ_OP_WRITE)
+ return NVME_STAT_WRITE;
+ else
+ return NVME_STAT_OTHER;
+}
+
+static inline int __nvme_data_dir_passthru(enum nvme_opcode op)
+{
+ if (op == nvme_cmd_read)
+ return NVME_STAT_READ;
+ else if (op == nvme_cmd_write)
+ return NVME_STAT_WRITE;
+ else
+ return NVME_STAT_OTHER;
+}
+
+static inline int nvme_data_dir(struct request *req)
+{
+ if (blk_rq_is_passthrough(req)) {
+ struct nvme_request *nr = nvme_req(req);
+
+ return __nvme_data_dir_passthru(nr->cmd->common.opcode);
+ }
+
+ return __nvme_data_dir(req_op(req));
+}
+
#ifdef CONFIG_NVME_MULTIPATH
static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
{
diff --git a/drivers/nvme/host/pr.c b/drivers/nvme/host/pr.c
index fe7dbe264815..80e106f151d2 100644
--- a/drivers/nvme/host/pr.c
+++ b/drivers/nvme/host/pr.c
@@ -53,10 +53,12 @@ static int nvme_send_ns_head_pr_command(struct block_device *bdev,
struct nvme_command *c, void *data, unsigned int data_len)
{
struct nvme_ns_head *head = bdev->bd_disk->private_data;
- int srcu_idx = srcu_read_lock(&head->srcu);
- struct nvme_ns *ns = nvme_find_path(head);
+ int srcu_idx;
+ struct nvme_ns *ns;
int ret = -EWOULDBLOCK;
+ srcu_idx = srcu_read_lock(&head->srcu);
+ ns = nvme_find_path(head, NVME_STAT_OTHER);
if (ns) {
c->common.nsid = cpu_to_le32(ns->head->ns_id);
ret = nvme_submit_sync_cmd(ns->queue, c, data, data_len);
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 75b2d69b5957..b2034e033ac3 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -195,7 +195,7 @@ static int ns_head_update_nuse(struct nvme_ns_head *head)
return 0;
srcu_idx = srcu_read_lock(&head->srcu);
- ns = nvme_find_path(head);
+ ns = nvme_find_path(head, NVME_STAT_OTHER);
if (!ns)
goto out_unlock;
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v7 3/9] nvme-multipath: pass I/O type to nvme_find_path()
2026-08-09 10:07 ` [PATCH v7 3/9] nvme-multipath: pass I/O type to nvme_find_path() Nilay Shroff
@ 2026-08-10 8:57 ` John Garry
2026-08-10 17:19 ` Nilay Shroff
0 siblings, 1 reply; 30+ messages in thread
From: John Garry @ 2026-08-10 8:57 UTC (permalink / raw)
To: Nilay Shroff, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
> @@ -741,6 +752,7 @@ int nvme_ns_head_ioctl(struct block_device *bdev, blk_mode_t mode,
> long nvme_ns_head_chr_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg)
> {
> + u8 opcode;
why declared at the top?
> bool open_for_write = file->f_mode & FMODE_WRITE;
> struct cdev *cdev = file_inode(file)->i_cdev;
> struct nvme_ns_head *head =
> @@ -748,9 +760,19 @@ long nvme_ns_head_chr_ioctl(struct file *file, unsigned int cmd,
> void __user *argp = (void __user *)arg;
> struct nvme_ns *ns;
> int srcu_idx, ret = -EWOULDBLOCK;
> + unsigned int op_type = NVME_STAT_OTHER;
> +
> + if (cmd == NVME_IOCTL_SUBMIT_IO) {
> + if (get_user(opcode, (u8 *)argp))
> + return -EFAULT;
> + if (opcode == nvme_cmd_write)
> + op_type = NVME_STAT_WRITE;
> + else if (opcode == nvme_cmd_read)
> + op_type = NVME_STAT_READ;
> + }
>
> srcu_idx = srcu_read_lock(&head->srcu);
> - ns = nvme_find_path(head);
> + ns = nvme_find_path(head, op_type);
> if (!ns)
> goto out_unlock;
>
> @@ -770,9 +792,19 @@ int nvme_ns_head_chr_uring_cmd(struct io_uring_cmd *ioucmd,
> struct cdev *cdev = file_inode(ioucmd->file)->i_cdev;
> struct nvme_ns_head *head = container_of(cdev, struct nvme_ns_head, cdev);
> int srcu_idx = srcu_read_lock(&head->srcu);
> - struct nvme_ns *ns = nvme_find_path(head);
> + struct nvme_ns *ns;
> int ret = -EINVAL;
> + const struct nvme_uring_cmd *cmd = io_uring_sqe128_cmd(ioucmd->sqe,
> + struct nvme_uring_cmd);
> + unsigned int op_type = NVME_STAT_OTHER;
> + __u8 opcode = READ_ONCE(cmd->opcode);
> +
> + if (opcode == nvme_cmd_write)
> + op_type = NVME_STAT_WRITE;
> + else if (opcode == nvme_cmd_read)
> + op_type = NVME_STAT_READ;
nit: I think that having a final else leg to set op_type is nicer than
setting to NVME_STAT_OTHER at init time (and overwriting in some cases).
>
> + ns = nvme_find_path(head, op_type);
> if (ns)
> ret = nvme_ns_uring_cmd(ns, ioucmd, issue_flags);
> srcu_read_unlock(&head->srcu, srcu_idx);
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 9b9a657fa330..8c20ff516e61 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -460,7 +460,8 @@ static struct nvme_ns *nvme_numa_path(struct nvme_ns_head *head)
> return ns;
> }
>
> -inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head)
> +inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head,
> + unsigned int op_type)
> {
> switch (READ_ONCE(head->subsys->iopolicy)) {
> case NVME_IOPOLICY_QD:
> @@ -522,7 +523,7 @@ static void nvme_ns_head_submit_bio(struct bio *bio)
> return;
>
> srcu_idx = srcu_read_lock(&head->srcu);
> - ns = nvme_find_path(head);
> + ns = nvme_find_path(head, __nvme_data_dir(bio_op(bio)));
It's a but unfortunate that we have to find op_type even for when not
using the latency iopolicy.
> if (likely(ns)) {
> bio_set_dev(bio, ns->disk->part0);
> /*
> @@ -572,7 +573,7 @@ static int nvme_ns_head_get_unique_id(struct gendisk *disk, u8 id[16],
> int srcu_idx, ret = -EWOULDBLOCK;
>
> srcu_idx = srcu_read_lock(&head->srcu);
> - ns = nvme_find_path(head);
> + ns = nvme_find_path(head, NVME_STAT_OTHER);
> if (ns)
> ret = nvme_ns_get_unique_id(ns, id, type);
> srcu_read_unlock(&head->srcu, srcu_idx);
> @@ -588,7 +589,7 @@ static int nvme_ns_head_report_zones(struct gendisk *disk, sector_t sector,
> int srcu_idx, ret = -EWOULDBLOCK;
>
> srcu_idx = srcu_read_lock(&head->srcu);
> - ns = nvme_find_path(head);
> + ns = nvme_find_path(head, NVME_STAT_OTHER);
> if (ns)
> ret = nvme_ns_report_zones(ns, sector, nr_zones, args);
> srcu_read_unlock(&head->srcu, srcu_idx);
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 824651cc898d..8a9ec502912d 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -520,6 +520,13 @@ struct nvme_ns_ids {
> u8 csi;
> };
>
> +enum nvme_stat_group {
> + NVME_STAT_READ,
> + NVME_STAT_WRITE,
> + NVME_STAT_OTHER,
Would NVME_STAT_OTHER ever be used in high frequency scenarios such that
it is worth having its own type? If not, could NVME_STAT_READ be reused?
> + NVME_NUM_STAT_GROUPS
Can these ever be used for non-mulitpath? I just wonder why multipath or
similar is not in the name
> +};
> +
> /*
> * Anchor structure for namespaces. There is one for each namespace in a
> * NVMe subsystem that any of our controllers can see, and the namespace
> @@ -1032,7 +1039,39 @@ extern const struct attribute_group *nvme_dev_attr_groups[];
> extern const struct block_device_operations nvme_bdev_ops;
>
> void nvme_delete_ctrl_sync(struct nvme_ctrl *ctrl);
> -struct nvme_ns *nvme_find_path(struct nvme_ns_head *head);
> +struct nvme_ns *nvme_find_path(struct nvme_ns_head *head, unsigned int op_type);
> +
> +static inline int __nvme_data_dir(const enum req_op op)
> +{
This returns an int (so not strongly typed), which is going to be
NVME_STAT_READ, NVME_STAT_WRITE, or NVME_STAT_OTHER. From the function
name, I am not sure if that it expected. Some might expect READ or WRITE
returned. 'stat' should be in the name, or similar.
> + if (op == REQ_OP_READ)
> + return NVME_STAT_READ;
> + else if (op == REQ_OP_WRITE)
> + return NVME_STAT_WRITE;
> + else
> + return NVME_STAT_OTHER;> +}
> +
> +static inline int __nvme_data_dir_passthru(enum nvme_opcode op)
> +{
As __nvme_data_dir
> + if (op == nvme_cmd_read)
> + return NVME_STAT_READ;
> + else if (op == nvme_cmd_write)
> + return NVME_STAT_WRITE;
> + else
> + return NVME_STAT_OTHER;
> +}
> +
> +static inline int nvme_data_dir(struct request *req)
As __nvme_data_dir
> +{
> + if (blk_rq_is_passthrough(req)) {
> + struct nvme_request *nr = nvme_req(req);
> +
> + return __nvme_data_dir_passthru(nr->cmd->common.opcode);
> + }
> +
> + return __nvme_data_dir(req_op(req));
> +}
> +
> #ifdef CONFIG_NVME_MULTIPATH
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 3/9] nvme-multipath: pass I/O type to nvme_find_path()
2026-08-10 8:57 ` John Garry
@ 2026-08-10 17:19 ` Nilay Shroff
2026-08-11 10:36 ` John Garry
0 siblings, 1 reply; 30+ messages in thread
From: Nilay Shroff @ 2026-08-10 17:19 UTC (permalink / raw)
To: John Garry, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
On 8/10/26 2:27 PM, John Garry wrote:
>> @@ -741,6 +752,7 @@ int nvme_ns_head_ioctl(struct block_device *bdev, blk_mode_t mode,
>> long nvme_ns_head_chr_ioctl(struct file *file, unsigned int cmd,
>> unsigned long arg)
>> {
>> + u8 opcode;
>
> why declared at the top?
>
yes will move it close to its first use.
>> bool open_for_write = file->f_mode & FMODE_WRITE;
>> struct cdev *cdev = file_inode(file)->i_cdev;
>> struct nvme_ns_head *head =
>> @@ -748,9 +760,19 @@ long nvme_ns_head_chr_ioctl(struct file *file, unsigned int cmd,
>> void __user *argp = (void __user *)arg;
>> struct nvme_ns *ns;
>> int srcu_idx, ret = -EWOULDBLOCK;
>> + unsigned int op_type = NVME_STAT_OTHER;
>> +
>> + if (cmd == NVME_IOCTL_SUBMIT_IO) {
>> + if (get_user(opcode, (u8 *)argp))
>> + return -EFAULT;
>> + if (opcode == nvme_cmd_write)
>> + op_type = NVME_STAT_WRITE;
>> + else if (opcode == nvme_cmd_read)
>> + op_type = NVME_STAT_READ;
>> + }
>> srcu_idx = srcu_read_lock(&head->srcu);
>> - ns = nvme_find_path(head);
>> + ns = nvme_find_path(head, op_type);
>> if (!ns)
>> goto out_unlock;
>> @@ -770,9 +792,19 @@ int nvme_ns_head_chr_uring_cmd(struct io_uring_cmd *ioucmd,
>> struct cdev *cdev = file_inode(ioucmd->file)->i_cdev;
>> struct nvme_ns_head *head = container_of(cdev, struct nvme_ns_head, cdev);
>> int srcu_idx = srcu_read_lock(&head->srcu);
>> - struct nvme_ns *ns = nvme_find_path(head);
>> + struct nvme_ns *ns;
>> int ret = -EINVAL;
>> + const struct nvme_uring_cmd *cmd = io_uring_sqe128_cmd(ioucmd->sqe,
>> + struct nvme_uring_cmd);
>> + unsigned int op_type = NVME_STAT_OTHER;
>> + __u8 opcode = READ_ONCE(cmd->opcode);
>> +
>> + if (opcode == nvme_cmd_write)
>> + op_type = NVME_STAT_WRITE;
>> + else if (opcode == nvme_cmd_read)
>> + op_type = NVME_STAT_READ;
>
> nit: I think that having a final else leg to set op_type is nicer than setting to NVME_STAT_OTHER at init time (and overwriting in some cases).
>
okay will address this in next version.
>> + ns = nvme_find_path(head, op_type);
>> if (ns)
>> ret = nvme_ns_uring_cmd(ns, ioucmd, issue_flags);
>> srcu_read_unlock(&head->srcu, srcu_idx);
>> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
>> index 9b9a657fa330..8c20ff516e61 100644
>> --- a/drivers/nvme/host/multipath.c
>> +++ b/drivers/nvme/host/multipath.c
>> @@ -460,7 +460,8 @@ static struct nvme_ns *nvme_numa_path(struct nvme_ns_head *head)
>> return ns;
>> }
>> -inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head)
>> +inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head,
>> + unsigned int op_type)
>> {
>> switch (READ_ONCE(head->subsys->iopolicy)) {
>> case NVME_IOPOLICY_QD:
>> @@ -522,7 +523,7 @@ static void nvme_ns_head_submit_bio(struct bio *bio)
>> return;
>> srcu_idx = srcu_read_lock(&head->srcu);
>> - ns = nvme_find_path(head);
>> + ns = nvme_find_path(head, __nvme_data_dir(bio_op(bio)));
>
> It's a but unfortunate that we have to find op_type even for when not using the latency iopolicy.
>
I looked at a few alternatives to avoid passing op_type into
nvme_find_path(), but couldn't find a cleaner approach.
Fortunately, determining op_type is inexpensive, so I don't
expect it to have any measurable performance impact.
>> if (likely(ns)) {
>> bio_set_dev(bio, ns->disk->part0);
>> /*
>> @@ -572,7 +573,7 @@ static int nvme_ns_head_get_unique_id(struct gendisk *disk, u8 id[16],
>> int srcu_idx, ret = -EWOULDBLOCK;
>> srcu_idx = srcu_read_lock(&head->srcu);
>> - ns = nvme_find_path(head);
>> + ns = nvme_find_path(head, NVME_STAT_OTHER);
>> if (ns)
>> ret = nvme_ns_get_unique_id(ns, id, type);
>> srcu_read_unlock(&head->srcu, srcu_idx);
>> @@ -588,7 +589,7 @@ static int nvme_ns_head_report_zones(struct gendisk *disk, sector_t sector,
>> int srcu_idx, ret = -EWOULDBLOCK;
>> srcu_idx = srcu_read_lock(&head->srcu);
>> - ns = nvme_find_path(head);
>> + ns = nvme_find_path(head, NVME_STAT_OTHER);
>> if (ns)
>> ret = nvme_ns_report_zones(ns, sector, nr_zones, args);
>> srcu_read_unlock(&head->srcu, srcu_idx);
>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>> index 824651cc898d..8a9ec502912d 100644
>> --- a/drivers/nvme/host/nvme.h
>> +++ b/drivers/nvme/host/nvme.h
>> @@ -520,6 +520,13 @@ struct nvme_ns_ids {
>> u8 csi;
>> };
>> +enum nvme_stat_group {
>> + NVME_STAT_READ,
>> + NVME_STAT_WRITE,
>> + NVME_STAT_OTHER,
>
> Would NVME_STAT_OTHER ever be used in high frequency scenarios such that it is worth having its own type? If not, could NVME_STAT_READ be reused?
>
It may not be used in high-throughput scenarios, but treating these
commands as READ or WRITE would unnecessarily skew the latency statistics
for actual read/write workloads. Keeping them in a separate category
avoids that distortion, so I think having NVME_STAT_OTHER makes sense.
>> + NVME_NUM_STAT_GROUPS
>
> Can these ever be used for non-mulitpath? I just wonder why multipath or similar is not in the name
>
Today they're only used by the multipath code. I kept the names generic
because they simply classify NVMe operations into READ/WRITE/OTHER based
on the command opcode, which isn't inherently multipath-specific.
>> +};
>> +
>> /*
>> * Anchor structure for namespaces. There is one for each namespace in a
>> * NVMe subsystem that any of our controllers can see, and the namespace
>> @@ -1032,7 +1039,39 @@ extern const struct attribute_group *nvme_dev_attr_groups[];
>> extern const struct block_device_operations nvme_bdev_ops;
>> void nvme_delete_ctrl_sync(struct nvme_ctrl *ctrl);
>> -struct nvme_ns *nvme_find_path(struct nvme_ns_head *head);
>> +struct nvme_ns *nvme_find_path(struct nvme_ns_head *head, unsigned int op_type);
>> +
>> +static inline int __nvme_data_dir(const enum req_op op)
>> +{
>
> This returns an int (so not strongly typed), which is going to be NVME_STAT_READ, NVME_STAT_WRITE, or NVME_STAT_OTHER. From the function name, I am not sure if that it expected. Some might expect READ or WRITE returned. 'stat' should be in the name, or similar.
Good point. I'll change the return type to enum nvme_stat_group and
rename the helper to better reflect that it returns a stat group
rather than a data direction.
>
>> + if (op == REQ_OP_READ)
>> + return NVME_STAT_READ;
>> + else if (op == REQ_OP_WRITE)
>> + return NVME_STAT_WRITE;
>> + else
> > + return NVME_STAT_OTHER;> +}
>> +
>> +static inline int __nvme_data_dir_passthru(enum nvme_opcode op)
>> +{
>
> As __nvme_data_dir
Yes, I'll make the same change there as well.
>
>> + if (op == nvme_cmd_read)
>> + return NVME_STAT_READ;
>> + else if (op == nvme_cmd_write)
>> + return NVME_STAT_WRITE;
>> + else
>> + return NVME_STAT_OTHER;
>> +}
>> +
>> +static inline int nvme_data_dir(struct request *req)
>
> As __nvme_data_dir
>
Yes, I'll make the same change there as well.
Thanks,
--Nilay
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 3/9] nvme-multipath: pass I/O type to nvme_find_path()
2026-08-10 17:19 ` Nilay Shroff
@ 2026-08-11 10:36 ` John Garry
2026-08-12 8:05 ` Nilay Shroff
0 siblings, 1 reply; 30+ messages in thread
From: John Garry @ 2026-08-11 10:36 UTC (permalink / raw)
To: Nilay Shroff, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
On 10/08/2026 18:19, Nilay Shroff wrote:
>> It's a but unfortunate that we have to find op_type even for when not using the latency iopolicy.
>>
> I looked at a few alternatives to avoid passing op_type into
> nvme_find_path(), but couldn't find a cleaner approach.
> Fortunately, determining op_type is inexpensive, so I don't
> expect it to have any measurable performance impact.
All I can think of is a variant of nvme_find_path() which accepts the
bio, but that means more complication and code duplication - but not too
much.
As you mentioned, the processing should be small for this.
>
>>> if (likely(ns)) {
>>> bio_set_dev(bio, ns->disk->part0);
>>> /*
>>> @@ -572,7 +573,7 @@ static int nvme_ns_head_get_unique_id(struct gendisk *disk, u8 id[16],
>>> int srcu_idx, ret = -EWOULDBLOCK;
>>> srcu_idx = srcu_read_lock(&head->srcu);
>>> - ns = nvme_find_path(head);
>>> + ns = nvme_find_path(head, NVME_STAT_OTHER);
>>> if (ns)
>>> ret = nvme_ns_get_unique_id(ns, id, type);
>>> srcu_read_unlock(&head->srcu, srcu_idx);
>>> @@ -588,7 +589,7 @@ static int nvme_ns_head_report_zones(struct gendisk *disk, sector_t sector,
>>> int srcu_idx, ret = -EWOULDBLOCK;
>>> srcu_idx = srcu_read_lock(&head->srcu);
>>> - ns = nvme_find_path(head);
>>> + ns = nvme_find_path(head, NVME_STAT_OTHER);
>>> if (ns)
>>> ret = nvme_ns_report_zones(ns, sector, nr_zones, args);
>>> srcu_read_unlock(&head->srcu, srcu_idx);
>>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>>> index 824651cc898d..8a9ec502912d 100644
>>> --- a/drivers/nvme/host/nvme.h
>>> +++ b/drivers/nvme/host/nvme.h
>>> @@ -520,6 +520,13 @@ struct nvme_ns_ids {
>>> u8 csi;
>>> };
>>> +enum nvme_stat_group {
>>> + NVME_STAT_READ,
>>> + NVME_STAT_WRITE,
>>> + NVME_STAT_OTHER,
>>
>> Would NVME_STAT_OTHER ever be used in high frequency scenarios such that it is worth having its own type? If not, could NVME_STAT_READ be reused?
>>
> It may not be used in high-throughput scenarios, but treating these
> commands as READ or WRITE would unnecessarily skew the latency statistics
> for actual read/write workloads. Keeping them in a separate category
> avoids that distortion, so I think having NVME_STAT_OTHER makes sense.
I can't imagine that NVME_STAT_OTHER could skew the READ/WRITE results
too much, but I don't know the full range of IO for which
NVME_STAT_OTHER would be used (so I may be wrong).
BTW, it would be nice to mention the reason behind NVME_STAT_OTHER in
the commit message. None of enum nvme_stat_group is described in much
detail.
>
>>> + NVME_NUM_STAT_GROUPS
>>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 3/9] nvme-multipath: pass I/O type to nvme_find_path()
2026-08-11 10:36 ` John Garry
@ 2026-08-12 8:05 ` Nilay Shroff
0 siblings, 0 replies; 30+ messages in thread
From: Nilay Shroff @ 2026-08-12 8:05 UTC (permalink / raw)
To: John Garry, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
On 8/11/26 4:06 PM, John Garry wrote:
> On 10/08/2026 18:19, Nilay Shroff wrote:
>>> It's a but unfortunate that we have to find op_type even for when not using the latency iopolicy.
>>>
>> I looked at a few alternatives to avoid passing op_type into
>> nvme_find_path(), but couldn't find a cleaner approach.
>> Fortunately, determining op_type is inexpensive, so I don't
>> expect it to have any measurable performance impact.
>
> All I can think of is a variant of nvme_find_path() which accepts the bio, but that means more complication and code duplication - but not too much.
>
> As you mentioned, the processing should be small for this.
>
>>
>>>> if (likely(ns)) {
>>>> bio_set_dev(bio, ns->disk->part0);
>>>> /*
>>>> @@ -572,7 +573,7 @@ static int nvme_ns_head_get_unique_id(struct gendisk *disk, u8 id[16],
>>>> int srcu_idx, ret = -EWOULDBLOCK;
>>>> srcu_idx = srcu_read_lock(&head->srcu);
>>>> - ns = nvme_find_path(head);
>>>> + ns = nvme_find_path(head, NVME_STAT_OTHER);
>>>> if (ns)
>>>> ret = nvme_ns_get_unique_id(ns, id, type);
>>>> srcu_read_unlock(&head->srcu, srcu_idx);
>>>> @@ -588,7 +589,7 @@ static int nvme_ns_head_report_zones(struct gendisk *disk, sector_t sector,
>>>> int srcu_idx, ret = -EWOULDBLOCK;
>>>> srcu_idx = srcu_read_lock(&head->srcu);
>>>> - ns = nvme_find_path(head);
>>>> + ns = nvme_find_path(head, NVME_STAT_OTHER);
>>>> if (ns)
>>>> ret = nvme_ns_report_zones(ns, sector, nr_zones, args);
>>>> srcu_read_unlock(&head->srcu, srcu_idx);
>>>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>>>> index 824651cc898d..8a9ec502912d 100644
>>>> --- a/drivers/nvme/host/nvme.h
>>>> +++ b/drivers/nvme/host/nvme.h
>>>> @@ -520,6 +520,13 @@ struct nvme_ns_ids {
>>>> u8 csi;
>>>> };
>>>> +enum nvme_stat_group {
>>>> + NVME_STAT_READ,
>>>> + NVME_STAT_WRITE,
>>>> + NVME_STAT_OTHER,
>>>
>>> Would NVME_STAT_OTHER ever be used in high frequency scenarios such that it is worth having its own type? If not, could NVME_STAT_READ be reused?
>>>
>> It may not be used in high-throughput scenarios, but treating these
>> commands as READ or WRITE would unnecessarily skew the latency statistics
>> for actual read/write workloads. Keeping them in a separate category
>> avoids that distortion, so I think having NVME_STAT_OTHER makes sense.
>
> I can't imagine that NVME_STAT_OTHER could skew the READ/WRITE results too much, but I don't know the full range of IO for which NVME_STAT_OTHER would be used (so I may be wrong).
>
There're many opcodes which falls in other catogory here for instance,
flush/write-zeros/compare/verify/dsm and few others...
> BTW, it would be nice to mention the reason behind NVME_STAT_OTHER in the commit message. None of enum nvme_stat_group is described in much detail.
>
Yes make sense, I'd do the needful.
Thanks,
--Nilay
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy
2026-08-09 10:07 [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy Nilay Shroff
` (2 preceding siblings ...)
2026-08-09 10:07 ` [PATCH v7 3/9] nvme-multipath: pass I/O type to nvme_find_path() Nilay Shroff
@ 2026-08-09 10:07 ` Nilay Shroff
2026-08-10 8:12 ` John Garry
2026-08-10 10:46 ` John Garry
2026-08-09 10:08 ` [PATCH v7 5/9] nvme: add generic debugfs support Nilay Shroff
` (6 subsequent siblings)
10 siblings, 2 replies; 30+ messages in thread
From: Nilay Shroff @ 2026-08-09 10:07 UTC (permalink / raw)
To: linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, john.g.garry, gjoyce, Nilay Shroff
This commit introduces a new I/O policy named "latency". Users can
configure it by writing "latency" to "/sys/class/nvme-subsystem/nvme-
subsystemX/iopolicy"
The "latency" policy dynamically distributes I/O based on measured I/O
completion latency. The main idea is to calculate latency for each path,
derive a weight, and then proportionally forward I/O according to those
weights.
To ensure scalability, path latency is measured per-CPU. Each CPU
maintains its own statistics, and I/O forwarding uses these per-CPU
values. Every ~15 seconds, a simple average latency of per-CPU batched
samples are computed and fed into an Exponentially Weighted Moving
Average (EWMA):
avg_latency = div_u64(batch, batch_count);
new_ewma_latency = (prev_ewma_latency * (WEIGHT-1) + avg_latency)/WEIGHT
With WEIGHT = 8, this assigns 7/8 (~87.5%) weight to the previous
latency value and 1/8 (~12.5%) to the most recent latency. This
smoothing reduces jitter, adapts quickly to changing conditions,
avoids storing historical samples, and works well for both low and
high I/O rates. Path weights are then derived from the smoothed (EWMA)
latency as follows (example with two paths A and B):
path_A_score = NSEC_PER_SEC / path_A_ewma_latency
path_B_score = NSEC_PER_SEC / path_B_ewma_latency
total_score = path_A_score + path_B_score
path_A_weight = (path_A_score * 64) / total_score
path_B_weight = (path_B_score * 64) / total_score
where:
- path_X_ewma_latency is the smoothed latency of a path in nanoseconds
- NSEC_PER_SEC is used as a scaling factor since valid latencies
are < 1 second
- weights are normalized to a 0–64 scale across all paths.
Path credits are refilled based on this weight, with one credit
consumed per I/O. When all credits are consumed, the credits are
refilled again based on the current weight. This ensures that I/O is
distributed across paths proportionally to their calculated weight.
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
drivers/nvme/host/core.c | 15 +-
drivers/nvme/host/multipath.c | 444 +++++++++++++++++++++++++++++++++-
drivers/nvme/host/nvme.h | 52 +++-
3 files changed, 496 insertions(+), 15 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 453c1f0b2dd0..542e2ee036cc 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -690,6 +690,9 @@ static void nvme_free_ns_head(struct kref *ref)
cleanup_srcu_struct(&head->srcu);
nvme_put_subsystem(head->subsys);
kfree(head->plids);
+#ifdef CONFIG_NVME_MULTIPATH
+ free_percpu(head->latency_path);
+#endif
kfree(head);
}
@@ -707,6 +710,7 @@ static void nvme_free_ns(struct kref *kref)
{
struct nvme_ns *ns = container_of(kref, struct nvme_ns, kref);
+ nvme_free_ns_stat(ns);
put_disk(ns->disk);
nvme_put_ns_head(ns->head);
nvme_put_ctrl(ns->ctrl);
@@ -4220,6 +4224,9 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
if (nvme_init_ns_head(ns, info))
goto out_cleanup_disk;
+ if (nvme_alloc_ns_stat(ns))
+ goto out_unlink_ns;
+
/*
* If multipathing is enabled, the device name for all disks and not
* just those that represent shared namespaces needs to be based on the
@@ -4244,7 +4251,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
}
if (nvme_update_ns_info(ns, info))
- goto out_unlink_ns;
+ goto out_free_ns_stat;
mutex_lock(&ctrl->namespaces_lock);
/*
@@ -4253,7 +4260,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
*/
if (test_bit(NVME_CTRL_FROZEN, &ctrl->flags)) {
mutex_unlock(&ctrl->namespaces_lock);
- goto out_unlink_ns;
+ goto out_free_ns_stat;
}
blk_queue_rq_timeout(ns->queue, ctrl->io_timeout);
nvme_ns_add_to_ctrl_list(ns);
@@ -4278,6 +4285,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
list_del_rcu(&ns->list);
mutex_unlock(&ctrl->namespaces_lock);
synchronize_srcu(&ctrl->srcu);
+out_free_ns_stat:
+ nvme_free_ns_stat(ns);
out_unlink_ns:
mutex_lock(&ctrl->subsys->lock);
list_del_rcu(&ns->siblings);
@@ -4317,7 +4326,7 @@ static void nvme_ns_remove(struct nvme_ns *ns)
/*
* Ensure that !NVME_NS_READY is seen by other threads to prevent
- * this ns going back into current_path.
+ * this ns going back into current_path/latency_path.
*/
synchronize_srcu(&ns->head->srcu);
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 8c20ff516e61..8086530b5350 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -6,6 +6,9 @@
#include <linux/backing-dev.h>
#include <linux/moduleparam.h>
#include <linux/vmalloc.h>
+#include <linux/blk-mq.h>
+#include <linux/math64.h>
+#include <linux/rculist.h>
#include <trace/events/block.h>
#include "nvme.h"
@@ -66,9 +69,10 @@ MODULE_PARM_DESC(multipath_always_on,
"create multipath node always except for private namespace with non-unique nsid; note that this also implicitly enables native multipath support");
static const char *nvme_iopolicy_names[] = {
- [NVME_IOPOLICY_NUMA] = "numa",
- [NVME_IOPOLICY_RR] = "round-robin",
- [NVME_IOPOLICY_QD] = "queue-depth",
+ [NVME_IOPOLICY_NUMA] = "numa",
+ [NVME_IOPOLICY_RR] = "round-robin",
+ [NVME_IOPOLICY_QD] = "queue-depth",
+ [NVME_IOPOLICY_LATENCY] = "latency",
};
static int iopolicy = NVME_IOPOLICY_NUMA;
@@ -107,7 +111,7 @@ static int nvme_get_iopolicy(char *buf, const struct kernel_param *kp)
module_param_call(iopolicy, nvme_set_iopolicy, nvme_get_iopolicy,
&iopolicy, 0644);
MODULE_PARM_DESC(iopolicy,
- "Default multipath I/O policy; 'numa' (default), 'round-robin' or 'queue-depth'");
+ "Default multipath I/O policy; 'numa' (default), 'round-robin' or 'queue-depth' or 'latency'");
void nvme_mpath_default_iopolicy(struct nvme_subsystem *subsys)
{
@@ -199,6 +203,203 @@ void nvme_mpath_start_request(struct request *rq)
}
EXPORT_SYMBOL_GPL(nvme_mpath_start_request);
+static void nvme_mpath_weight_work(struct work_struct *weight_work)
+{
+ int cpu, srcu_idx;
+ u32 weight;
+ struct nvme_ns *ns;
+ struct nvme_path_lat_stat *stat;
+ struct nvme_path_lat_work *work = container_of(weight_work,
+ struct nvme_path_lat_work, weight_work);
+ struct nvme_ns_head *head = work->ns->head;
+ int op_type = work->op_type;
+ u64 total_score = 0;
+
+ cpu = get_cpu();
+
+ srcu_idx = srcu_read_lock(&head->srcu);
+ list_for_each_entry_srcu(ns, &head->list, siblings,
+ srcu_read_lock_held(&head->srcu)) {
+
+ stat = &this_cpu_ptr(ns->path_lat)[op_type].stat;
+ if (!READ_ONCE(stat->slat_ns)) {
+ stat->score = 0;
+ continue;
+ }
+ /*
+ * Compute the path score as the inverse of smoothed
+ * latency, scaled by NSEC_PER_SEC. Floating point
+ * math is unavailable in the kernel, so fixed-point
+ * scaling is used instead. NSEC_PER_SEC is chosen
+ * because valid latencies are always < 1 second; longer
+ * latencies are ignored.
+ */
+ stat->score = div_u64(NSEC_PER_SEC, READ_ONCE(stat->slat_ns));
+
+ /* Compute total score. */
+ total_score += stat->score;
+ }
+
+ if (!total_score)
+ goto out;
+
+ /*
+ * After computing the total slatency, we derive per-path weight
+ * (normalized to the range 0–64). The weight represents the
+ * relative share of I/O the path should receive.
+ *
+ * - lower smoothed latency -> higher weight
+ * - higher smoothed slatency -> lower weight
+ *
+ * Next, while forwarding I/O, we assign "credits" to each path
+ * based on its weight (please also refer nvme_latency_path()):
+ * - Initially, credits = weight.
+ * - Each time an I/O is dispatched on a path, its credits are
+ * decremented proportionally.
+ * - When a path runs out of credits, it becomes temporarily
+ * ineligible until credit is refilled.
+ *
+ * I/O distribution is therefore governed by available credits,
+ * ensuring that over time the proportion of I/O sent to each
+ * path matches its weight (and thus its performance).
+ */
+ list_for_each_entry_srcu(ns, &head->list, siblings,
+ srcu_read_lock_held(&head->srcu)) {
+
+ stat = &this_cpu_ptr(ns->path_lat)[op_type].stat;
+ weight = div_u64(stat->score * 64, total_score);
+
+ /*
+ * Ensure the path weight never drops below 1. A weight
+ * of 0 is used only for newly added paths. During
+ * bootstrap, a few I/Os are sent to such paths to
+ * establish an initial weight. Enforcing a minimum
+ * weight of 1 guarantees that no path is forgotten and
+ * that each path is probed at least occasionally.
+ */
+ if (!weight)
+ weight = 1;
+
+ WRITE_ONCE(stat->weight, weight);
+ }
+out:
+ srcu_read_unlock(&head->srcu, srcu_idx);
+ put_cpu();
+}
+
+/*
+ * Formula to calculate the EWMA (Exponentially Weighted Moving Average):
+ * ewma = (old_ewma * (EWMA_SHIFT - 1) + (EWMA_SHIFT)) / EWMA_SHIFT
+ * For instance, with EWMA_SHIFT = 3, this assigns 7/8 (~87.5 %) weight to
+ * the existing/old ewma and 1/8 (~12.5%) weight to the new sample.
+ */
+static inline u64 calc_ewma_update(u64 old, u64 new)
+{
+ return (old * ((1 << NVME_DEFAULT_LATENCY_EWMA_SHIFT) - 1)
+ + new) >> NVME_DEFAULT_LATENCY_EWMA_SHIFT;
+}
+
+static void nvme_mpath_add_sample(struct request *rq, struct nvme_ns *ns)
+{
+ int cpu;
+ unsigned int op_type;
+ struct nvme_path_lat *path_lat;
+ struct nvme_path_lat_stat *stat;
+ u64 now, latency, slat_ns, avg_lat_ns;
+ struct nvme_ns_head *head = ns->head;
+
+ if (list_is_singular(&head->list))
+ return;
+
+ now = ktime_get_ns();
+ latency = now >= rq->io_start_time_ns ? now - rq->io_start_time_ns : 0;
+ if (!latency)
+ return;
+
+ /*
+ * As completion code path is serialized(i.e. no same completion queue
+ * update code could run simultaneously on multiple cpu) we can safely
+ * access per cpu nvme path stat here from another cpu (in case the
+ * completion cpu is different from submission cpu).
+ * The only field which could be accessed simultaneously here is the
+ * path ->weight which may be accessed by this function as well as I/O
+ * submission path during path selection logic and we protect ->weight
+ * using READ_ONCE/WRITE_ONCE. Yes this may not be 100% accurate but
+ * we also don't need to be so accurate here as the path credit would
+ * be anyways refilled, based on path weight, once path consumes all
+ * its credits. And we limit path weight/credit max up to 64. Please
+ * also refer nvme_latency_path().
+ */
+ cpu = blk_mq_rq_cpu(rq);
+ op_type = nvme_data_dir(rq);
+ path_lat = &per_cpu_ptr(ns->path_lat, cpu)[op_type];
+ stat = &path_lat->stat;
+
+ /*
+ * If latency > ~1s then ignore this sample to prevent EWMA from being
+ * skewed by pathological outliers (multi-second waits, controller
+ * timeouts etc.). This keeps path scores representative of normal
+ * performance and avoids instability from rare spikes. If such high
+ * latency is real, ANA state reporting or keep-alive error counters
+ * will mark the path unhealthy and remove it from the head node list,
+ * so we safely skip such sample here.
+ */
+ if (unlikely(latency > NSEC_PER_SEC)) {
+ stat->nr_ignored++;
+ dev_warn_ratelimited(ns->ctrl->device,
+ "ignoring sample with >1s latency (possible controller stall or timeout)\n");
+ return;
+ }
+
+ /*
+ * Accumulate latency samples and increment the batch count for each
+ * ~15 second interval. When the interval expires, compute the simple
+ * average latency over that window, then update the smoothed (EWMA)
+ * latency. The path weight is recalculated based on this smoothed
+ * latency.
+ */
+ stat->batch += latency;
+ stat->batch_count++;
+ stat->nr_samples++;
+
+ if (now > stat->last_batch_ts && ((now - stat->last_batch_ts) >=
+ NVME_DEFAULT_LATENCY_BATCH_TIMEOUT)) {
+
+ /*
+ * Find simple average latency for the last epoch (~15 sec
+ * interval).
+ */
+ avg_lat_ns = div_u64(stat->batch, stat->batch_count);
+ stat->last_batch_ts = now;
+
+ /*
+ * Calculate smooth/EWMA (Exponentially Weighted Moving Average)
+ * latency. EWMA is preferred over simple average latency
+ * because it smooths naturally, reduces jitter from sudden
+ * spikes, and adapts faster to changing conditions. It also
+ * avoids storing historical samples, and works well for both
+ * slow and fast I/O rates.
+ * Formula:
+ * slat_ns = (prev_slat_ns * (WEIGHT - 1) + (latency)) / WEIGHT
+ * With WEIGHT = 8, this assigns 7/8 (~87.5 %) weight to the
+ * existing latency and 1/8 (~12.5%) weight to the new latency.
+ */
+ if (unlikely(!stat->slat_ns))
+ WRITE_ONCE(stat->slat_ns, avg_lat_ns);
+ else {
+ slat_ns = calc_ewma_update(stat->slat_ns, avg_lat_ns);
+ WRITE_ONCE(stat->slat_ns, slat_ns);
+ }
+
+ stat->batch = stat->batch_count = 0;
+
+ /*
+ * Defer calculation of the path weight in per-cpu workqueue.
+ */
+ schedule_work_on(cpu, &path_lat->work.weight_work);
+ }
+}
+
void nvme_mpath_end_request(struct request *rq)
{
struct nvme_ns *ns = rq->q->queuedata;
@@ -206,6 +407,15 @@ void nvme_mpath_end_request(struct request *rq)
if (nvme_req(rq)->flags & NVME_MPATH_CNT_ACTIVE)
atomic_dec_if_positive(&ns->ctrl->nr_active);
+ if (test_bit(NVME_NS_PATH_STAT, &ns->flags)) {
+ int srcu_idx;
+
+ srcu_idx = srcu_read_lock(&ns->head->srcu);
+ if (test_bit(NVME_NS_PATH_STAT, &ns->flags))
+ nvme_mpath_add_sample(rq, ns);
+ srcu_read_unlock(&ns->head->srcu, srcu_idx);
+ }
+
if (!(nvme_req(rq)->flags & NVME_MPATH_IO_STATS))
return;
bdev_end_io_acct(ns->head->disk->part0, req_op(rq),
@@ -239,6 +449,78 @@ static const char *nvme_ana_state_names[] = {
[NVME_ANA_CHANGE] = "change",
};
+static void nvme_reset_ns_latency_stat(struct nvme_ns *ns)
+{
+ int i, cpu;
+ struct nvme_path_lat_stat *stat;
+
+ for_each_possible_cpu(cpu) {
+ for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
+ stat = &per_cpu_ptr(ns->path_lat, cpu)[i].stat;
+ memset(stat, 0, sizeof(struct nvme_path_lat_stat));
+ }
+ }
+}
+
+void nvme_cancel_ns_latency_weight_work(struct nvme_ns *ns)
+{
+ int i, cpu;
+ struct nvme_path_lat *path_lat;
+
+ for_each_possible_cpu(cpu) {
+ for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
+ path_lat = &per_cpu_ptr(ns->path_lat, cpu)[i];
+ cancel_work_sync(&path_lat->work.weight_work);
+ }
+ }
+}
+
+static bool nvme_enable_ns_latency_sampling(struct nvme_ns *ns)
+{
+ struct nvme_ns_head *head = ns->head;
+
+ if (!head->disk ||
+ READ_ONCE(head->subsys->iopolicy) != NVME_IOPOLICY_LATENCY)
+ return false;
+
+ if (test_and_set_bit(NVME_NS_PATH_STAT, &ns->flags))
+ return false;
+
+ blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, ns->queue);
+ blk_stat_enable_accounting(ns->queue);
+ return true;
+}
+
+static bool nvme_disable_ns_latency_sampling(struct nvme_ns *ns)
+{
+ int cpu;
+ struct nvme_ns_head *head = ns->head;
+ bool changed = false;
+
+ if (!test_and_clear_bit(NVME_NS_PATH_STAT, &ns->flags))
+ return false;
+
+ for_each_possible_cpu(cpu) {
+ if (ns == READ_ONCE(*per_cpu_ptr(head->latency_path, cpu))) {
+ WRITE_ONCE(*per_cpu_ptr(head->latency_path, cpu), NULL);
+ changed = true;
+ }
+ }
+
+ blk_stat_disable_accounting(ns->queue);
+ blk_queue_flag_clear(QUEUE_FLAG_SAME_FORCE, ns->queue);
+
+ /*
+ * Ensure that we wait until completion side samplings (if any sneaked
+ * in after we clear NVME_NS_PATH_STAT) are all scheduled before we
+ * start cancelling those.
+ */
+ synchronize_srcu(&head->srcu);
+ nvme_cancel_ns_latency_weight_work(ns);
+ nvme_reset_ns_latency_stat(ns);
+ return changed;
+}
+
bool nvme_mpath_clear_current_path(struct nvme_ns *ns)
{
struct nvme_ns_head *head = ns->head;
@@ -251,6 +533,10 @@ bool nvme_mpath_clear_current_path(struct nvme_ns *ns)
changed = true;
}
}
+
+ if (nvme_disable_ns_latency_sampling(ns))
+ changed = true;
+
return changed;
}
@@ -268,6 +554,45 @@ void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl)
srcu_read_unlock(&ctrl->srcu, srcu_idx);
}
+int nvme_alloc_ns_stat(struct nvme_ns *ns)
+{
+ int i, cpu;
+ struct nvme_path_lat_work *work;
+ gfp_t gfp = GFP_KERNEL | __GFP_ZERO;
+
+ if (!ns->head->disk)
+ return 0;
+
+ ns->path_lat = __alloc_percpu_gfp(NVME_NUM_STAT_GROUPS *
+ sizeof(struct nvme_path_lat),
+ __alignof__(struct nvme_path_lat), gfp);
+ if (!ns->path_lat)
+ return -ENOMEM;
+
+ for_each_possible_cpu(cpu) {
+ for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
+ work = &per_cpu_ptr(ns->path_lat, cpu)[i].work;
+ work->ns = ns;
+ work->op_type = i;
+ INIT_WORK(&work->weight_work, nvme_mpath_weight_work);
+ }
+ }
+
+ return 0;
+}
+
+static void nvme_mpath_set_ctrl_paths(struct nvme_ctrl *ctrl)
+{
+ struct nvme_ns *ns;
+ int srcu_idx;
+
+ srcu_idx = srcu_read_lock(&ctrl->srcu);
+ list_for_each_entry_srcu(ns, &ctrl->namespaces, list,
+ srcu_read_lock_held(&ctrl->srcu))
+ nvme_enable_ns_latency_sampling(ns);
+ srcu_read_unlock(&ctrl->srcu, srcu_idx);
+}
+
void nvme_mpath_revalidate_paths(struct nvme_ns_head *head)
{
sector_t capacity = get_capacity(head->disk);
@@ -280,6 +605,8 @@ void nvme_mpath_revalidate_paths(struct nvme_ns_head *head)
srcu_read_lock_held(&head->srcu)) {
if (capacity != get_capacity(ns->disk))
clear_bit(NVME_NS_READY, &ns->flags);
+
+ nvme_reset_ns_latency_stat(ns);
}
srcu_read_unlock(&head->srcu, srcu_idx);
@@ -404,6 +731,92 @@ static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head)
return found;
}
+static inline bool nvme_state_is_live(enum nvme_ana_state state)
+{
+ return state == NVME_ANA_OPTIMIZED || state == NVME_ANA_NONOPTIMIZED;
+}
+
+static struct nvme_ns *nvme_latency_path(struct nvme_ns_head *head,
+ unsigned int op_type)
+{
+ struct nvme_ns *ns, *start, *found = NULL;
+ struct nvme_path_lat_stat *stat;
+ u32 weight;
+ int cpu;
+
+ cpu = get_cpu();
+ ns = READ_ONCE(*this_cpu_ptr(head->latency_path));
+ if (unlikely(!ns)) {
+ ns = list_first_or_null_rcu(&head->list,
+ struct nvme_ns, siblings);
+ if (unlikely(!ns))
+ goto out;
+ }
+found_ns:
+ start = ns;
+ while (nvme_path_is_disabled(ns) ||
+ !nvme_state_is_live(ns->ana_state)) {
+ ns = list_next_entry_circular(ns, &head->list, siblings);
+
+ /*
+ * If we iterate through all paths in the list but find each
+ * path in list is either disabled or dead then bail out.
+ */
+ if (ns == start)
+ goto out;
+ }
+
+ stat = &this_cpu_ptr(ns->path_lat)[op_type].stat;
+
+ /*
+ * When the head path-list is singular we don't calculate the
+ * only path weight for optimization as we don't need to forward
+ * I/O to more than one path. The another possibility is when the
+ * path is newly added, we don't know its weight. So we go round
+ * -robin for each such path and forward I/O to it.Once we start
+ * getting response for such I/Os, the path weight calculation
+ * would kick in and then we start using path credit for
+ * forwarding I/O.
+ */
+ weight = READ_ONCE(stat->weight);
+ if (!weight) {
+ found = ns;
+ goto out;
+ }
+
+ /*
+ * To keep path selection logic simple, we don't distinguish
+ * between ANA optimized and non-optimized states. The non-
+ * optimized path is expected to have a lower weight, and
+ * therefore fewer credits. As a result, only a small number of
+ * I/Os will be forwarded to paths in the non-optimized state.
+ */
+ if (stat->credit > 0) {
+ --stat->credit;
+ found = ns;
+ goto out;
+ } else {
+ /*
+ * Refill credit from path weight and move to next path. The
+ * refilled credit of the current path will be used next when
+ * all remainng paths exhaust its credits.
+ */
+ weight = READ_ONCE(stat->weight);
+ stat->credit = weight;
+ ns = list_next_entry_circular(ns, &head->list, siblings);
+ if (likely(ns))
+ goto found_ns;
+ }
+out:
+ if (found) {
+ stat->sel++;
+ WRITE_ONCE(*this_cpu_ptr(head->latency_path), found);
+ }
+
+ put_cpu();
+ return found;
+}
+
static struct nvme_ns *nvme_queue_depth_path(struct nvme_ns_head *head)
{
struct nvme_ns *best_opt = NULL, *best_nonopt = NULL, *ns;
@@ -464,6 +877,8 @@ inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head,
unsigned int op_type)
{
switch (READ_ONCE(head->subsys->iopolicy)) {
+ case NVME_IOPOLICY_LATENCY:
+ return nvme_latency_path(head, op_type);
case NVME_IOPOLICY_QD:
return nvme_queue_depth_path(head);
case NVME_IOPOLICY_RR:
@@ -754,6 +1169,10 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
if (!nvme_is_unique_nsid(ctrl, head))
return 0;
+ head->latency_path = alloc_percpu_gfp(struct nvme_ns*, GFP_KERNEL);
+ if (!head->latency_path)
+ return -ENOMEM;
+
blk_set_stacking_limits(&lim);
lim.dma_alignment = 3;
lim.features |= BLK_FEAT_IO_STAT | BLK_FEAT_NOWAIT |
@@ -762,8 +1181,10 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
lim.features |= BLK_FEAT_ZONED;
head->disk = blk_alloc_disk(&lim, ctrl->numa_node);
- if (IS_ERR(head->disk))
+ if (IS_ERR(head->disk)) {
+ free_percpu(head->latency_path);
return PTR_ERR(head->disk);
+ }
head->disk->fops = &nvme_ns_head_ops;
head->disk->private_data = head;
@@ -819,6 +1240,10 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
}
mutex_unlock(&head->lock);
+ mutex_lock(&nvme_subsystems_lock);
+ nvme_enable_ns_latency_sampling(ns);
+ mutex_unlock(&nvme_subsystems_lock);
+
synchronize_srcu(&head->srcu);
kblockd_schedule_work(&head->requeue_work);
}
@@ -867,11 +1292,6 @@ static int nvme_parse_ana_log(struct nvme_ctrl *ctrl, void *data,
return 0;
}
-static inline bool nvme_state_is_live(enum nvme_ana_state state)
-{
- return state == NVME_ANA_OPTIMIZED || state == NVME_ANA_NONOPTIMIZED;
-}
-
static void nvme_update_ns_ana_state(struct nvme_ana_group_desc *desc,
struct nvme_ns *ns)
{
@@ -1049,10 +1469,12 @@ static void nvme_subsys_iopolicy_update(struct nvme_subsystem *subsys,
WRITE_ONCE(subsys->iopolicy, iopolicy);
- /* iopolicy changes clear the mpath by design */
+ /* iopolicy changes clear/reset the mpath by design */
mutex_lock(&nvme_subsystems_lock);
list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry)
nvme_mpath_clear_ctrl_paths(ctrl);
+ list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry)
+ nvme_mpath_set_ctrl_paths(ctrl);
mutex_unlock(&nvme_subsystems_lock);
pr_notice("subsysnqn %s iopolicy changed from %s to %s\n",
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 8a9ec502912d..3c82f67f8926 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -28,7 +28,9 @@ extern unsigned int nvme_io_timeout;
extern unsigned int admin_timeout;
#define NVME_ADMIN_TIMEOUT (admin_timeout * HZ)
-#define NVME_DEFAULT_KATO 5
+#define NVME_DEFAULT_KATO 5
+#define NVME_DEFAULT_LATENCY_EWMA_SHIFT 3
+#define NVME_DEFAULT_LATENCY_BATCH_TIMEOUT (15 * NSEC_PER_SEC)
#ifdef CONFIG_ARCH_NO_SG_CHAIN
#define NVME_INLINE_SG_CNT 0
@@ -483,6 +485,7 @@ enum nvme_iopolicy {
NVME_IOPOLICY_NUMA,
NVME_IOPOLICY_RR,
NVME_IOPOLICY_QD,
+ NVME_IOPOLICY_LATENCY,
};
struct nvme_subsystem {
@@ -527,6 +530,30 @@ enum nvme_stat_group {
NVME_NUM_STAT_GROUPS
};
+struct nvme_path_lat_stat {
+ u64 nr_samples; /* total num of samples processed */
+ u64 nr_ignored; /* num. of samples ignored */
+ u64 slat_ns; /* smoothed (ewma) latency in nanoseconds */
+ u64 score; /* score used for weight calculation */
+ u64 last_batch_ts; /* timestamp when last time avg. latency is calculated */
+ u64 sel; /* num of times this path is selcted for I/O */
+ u64 batch; /* accumulated latency sum for current window */
+ u32 batch_count; /* num of samples accumulated in current window */
+ u32 weight; /* path weight */
+ u32 credit; /* path credit for I/O forwarding */
+};
+
+struct nvme_path_lat_work {
+ struct nvme_ns *ns; /* owning namespace */
+ struct work_struct weight_work; /* deferred work for weight calculation */
+ int op_type; /* op type : READ/WRITE/OTHER */
+};
+
+struct nvme_path_lat {
+ struct nvme_path_lat_stat stat; /* path statistics */
+ struct nvme_path_lat_work work; /* background worker context */
+};
+
/*
* Anchor structure for namespaces. There is one for each namespace in a
* NVMe subsystem that any of our controllers can see, and the namespace
@@ -578,6 +605,8 @@ struct nvme_ns_head {
unsigned int delayed_removal_secs;
atomic_long_t io_requeue_no_usable_path_count;
atomic_long_t io_fail_no_available_path_count;
+ struct nvme_ns * __percpu *latency_path;
+
#define NVME_NSHEAD_DISK_LIVE 0
#define NVME_NSHEAD_QUEUE_IF_NO_PATH 1
#define NVME_NSHEAD_CDEV_LIVE 2
@@ -606,6 +635,7 @@ struct nvme_ns {
enum nvme_ana_state ana_state;
u32 ana_grpid;
atomic_long_t failover;
+ struct nvme_path_lat __percpu *path_lat;
#endif
atomic_long_t retries;
atomic_long_t errors;
@@ -620,6 +650,7 @@ struct nvme_ns {
#define NVME_NS_READY 4
#define NVME_NS_SYSFS_ATTR_LINK 5
#define NVME_NS_CDEV_LIVE 6
+#define NVME_NS_PATH_STAT 7
struct cdev cdev;
struct device cdev_device;
@@ -1100,6 +1131,8 @@ void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl);
void nvme_mpath_remove_disk(struct nvme_ns_head *head);
void nvme_mpath_start_request(struct request *rq);
void nvme_mpath_end_request(struct request *rq);
+int nvme_alloc_ns_stat(struct nvme_ns *ns);
+void nvme_cancel_ns_latency_weight_work(struct nvme_ns *ns);
static inline void nvme_trace_bio_complete(struct request *req)
{
@@ -1130,6 +1163,13 @@ static inline bool nvme_mpath_queue_if_no_path(struct nvme_ns_head *head)
return true;
return false;
}
+static inline void nvme_free_ns_stat(struct nvme_ns *ns)
+{
+ if (!ns->head->disk)
+ return;
+
+ free_percpu(ns->path_lat);
+}
#else
#define multipath false
static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
@@ -1221,6 +1261,16 @@ static inline bool nvme_mpath_queue_if_no_path(struct nvme_ns_head *head)
{
return false;
}
+static inline void nvme_cancel_ns_latency_weight_work(struct nvme_ns *ns)
+{
+}
+static inline int nvme_alloc_ns_stat(struct nvme_ns *ns)
+{
+ return 0;
+}
+static inline void nvme_free_ns_stat(struct nvme_ns *ns)
+{
+}
#endif /* CONFIG_NVME_MULTIPATH */
int nvme_ns_get_unique_id(struct nvme_ns *ns, u8 id[16],
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy
2026-08-09 10:07 ` [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy Nilay Shroff
@ 2026-08-10 8:12 ` John Garry
2026-08-10 17:25 ` Nilay Shroff
2026-08-10 10:46 ` John Garry
1 sibling, 1 reply; 30+ messages in thread
From: John Garry @ 2026-08-10 8:12 UTC (permalink / raw)
To: Nilay Shroff, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
On 09/08/2026 11:07, Nilay Shroff wrote:
> + /*
> + * If latency > ~1s then ignore this sample to prevent EWMA from being
> + * skewed by pathological outliers (multi-second waits, controller
> + * timeouts etc.). This keeps path scores representative of normal
> + * performance and avoids instability from rare spikes. If such high
> + * latency is real, ANA state reporting or keep-alive error counters
> + * will mark the path unhealthy and remove it from the head node list,
> + * so we safely skip such sample here.
> + */
> + if (unlikely(latency > NSEC_PER_SEC)) {
> + stat->nr_ignored++;
> + dev_warn_ratelimited(ns->ctrl->device,
> + "ignoring sample with >1s latency (possible controller stall or timeout)\n");
> + return;
> + }
JFYI, I gave this series a spin and I see this warn a lot:
# ./fio_read.sh nvme1n1
iops: (g=0): rw=read, bs=(R) 4096B-4096B, (W) 4096B-4096B, (T)
4096B-4096B, ioengine=libaio, iodepth=100
...
fio-3.38
Starting 10 processes
[ 130.005374] nvme_mpath_add_sample: 29 callbacks suppressed03h:08m:36s]
[ 130.005387] nvme nvme2: ignoring sample with >1s latency (possible
controller stall or timeout)
[ 130.009968] nvme nvme2: ignoring sample with >1s latency (possible
controller stall or timeout)
[ 130.013176] nvme nvme2: ignoring sample with >1s latency (possible
controller stall or timeout)
[ 130.019576] nvme nvme2: ignoring sample with >1s latency (possible
controller stall or timeout)
[ 130.022275] nvme nvme2: ignoring sample with >1s latency (possible
controller stall or timeout)
[ 130.024762] nvme nvme2: ignoring sample with >1s latency (possible
controller stall or timeout)
[ 130.027268] nvme nvme2: ignoring sample with >1s latency (possible
controller stall or timeout)
[ 130.130140] nvme nvme2: ignoring sample with >1s latency (possible
controller stall or timeout)
[ 130.132316] nvme nvme2: ignoring sample with >1s latency (possible
controller stall or timeout)
[ 130.139279] nvme nvme2: ignoring sample with >1s latency (possible
controller stall or timeout)
^Cbs: 10 (f=10): [R(10)][0.4%][r=35.8MiB/s][r=9171 IOPS][eta 03h:07m:44s]
fio: terminating on signal 2
iops: (groupid=0, jobs=10): err= 0: pid=821: Mon Aug 10 08:08:28 2026
read: IOPS=9239, BW=36.1MiB/s (37.8MB/s)(1434MiB/39744msec)
slat (usec): min=25, max=110836, avg=129.22, stdev=537.18
Note that I do have many heavy kernel debug options enabled, like kasan
and kmemleak, which may influence this.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy
2026-08-10 8:12 ` John Garry
@ 2026-08-10 17:25 ` Nilay Shroff
2026-08-11 10:26 ` John Garry
0 siblings, 1 reply; 30+ messages in thread
From: Nilay Shroff @ 2026-08-10 17:25 UTC (permalink / raw)
To: John Garry, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
On 8/10/26 1:42 PM, John Garry wrote:
> On 09/08/2026 11:07, Nilay Shroff wrote:
>> + /*
>> + * If latency > ~1s then ignore this sample to prevent EWMA from being
>> + * skewed by pathological outliers (multi-second waits, controller
>> + * timeouts etc.). This keeps path scores representative of normal
>> + * performance and avoids instability from rare spikes. If such high
>> + * latency is real, ANA state reporting or keep-alive error counters
>> + * will mark the path unhealthy and remove it from the head node list,
>> + * so we safely skip such sample here.
>> + */
>> + if (unlikely(latency > NSEC_PER_SEC)) {
>> + stat->nr_ignored++;
>> + dev_warn_ratelimited(ns->ctrl->device,
>> + "ignoring sample with >1s latency (possible controller stall or timeout)\n");
>> + return;
>> + }
>
> JFYI, I gave this series a spin and I see this warn a lot:
>
> # ./fio_read.sh nvme1n1
> iops: (g=0): rw=read, bs=(R) 4096B-4096B, (W) 4096B-4096B, (T)
> 4096B-4096B, ioengine=libaio, iodepth=100
> ...
> fio-3.38
> Starting 10 processes
> [ 130.005374] nvme_mpath_add_sample: 29 callbacks suppressed03h:08m:36s]
> [ 130.005387] nvme nvme2: ignoring sample with >1s latency (possible
> controller stall or timeout)
> [ 130.009968] nvme nvme2: ignoring sample with >1s latency (possible
> controller stall or timeout)
> [ 130.013176] nvme nvme2: ignoring sample with >1s latency (possible
> controller stall or timeout)
> [ 130.019576] nvme nvme2: ignoring sample with >1s latency (possible
> controller stall or timeout)
> [ 130.022275] nvme nvme2: ignoring sample with >1s latency (possible
> controller stall or timeout)
> [ 130.024762] nvme nvme2: ignoring sample with >1s latency (possible
> controller stall or timeout)
> [ 130.027268] nvme nvme2: ignoring sample with >1s latency (possible
> controller stall or timeout)
> [ 130.130140] nvme nvme2: ignoring sample with >1s latency (possible
> controller stall or timeout)
> [ 130.132316] nvme nvme2: ignoring sample with >1s latency (possible
> controller stall or timeout)
> [ 130.139279] nvme nvme2: ignoring sample with >1s latency (possible
> controller stall or timeout)
> ^Cbs: 10 (f=10): [R(10)][0.4%][r=35.8MiB/s][r=9171 IOPS][eta 03h:07m:44s]
> fio: terminating on signal 2
>
> iops: (groupid=0, jobs=10): err= 0: pid=821: Mon Aug 10 08:08:28 2026
> read: IOPS=9239, BW=36.1MiB/s (37.8MB/s)(1434MiB/39744msec)
> slat (usec): min=25, max=110836, avg=129.22, stdev=537.18
>
> Note that I do have many heavy kernel debug options enabled, like kasan and kmemleak, which may influence this.
>
Thanks for testing. Given that you're running with KASAN, kmemleak, and other
heavy debug options enabled, I think this is expected. Those configurations
can significantly increase I/O completion latency, causing some samples to
exceed the 1-second threshold. The warning is also rate-limited to avoid
flooding the log. That said, still if it's causing a lot of noise in the logs
then we could change it to dev_dbg_ratelimited() from dev_warn_ratelimited().
Thanks,
--Nilay
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy
2026-08-10 17:25 ` Nilay Shroff
@ 2026-08-11 10:26 ` John Garry
2026-08-12 8:01 ` Nilay Shroff
0 siblings, 1 reply; 30+ messages in thread
From: John Garry @ 2026-08-11 10:26 UTC (permalink / raw)
To: Nilay Shroff, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
On 10/08/2026 18:25, Nilay Shroff wrote:
>>
>> iops: (groupid=0, jobs=10): err= 0: pid=821: Mon Aug 10 08:08:28 2026
>> read: IOPS=9239, BW=36.1MiB/s (37.8MB/s)(1434MiB/39744msec)
>> slat (usec): min=25, max=110836, avg=129.22, stdev=537.18
>>
>> Note that I do have many heavy kernel debug options enabled, like kasan and kmemleak, which may influence this.
>>
>
> Thanks for testing. Given that you're running with KASAN, kmemleak, and other
> heavy debug options enabled, I think this is expected. Those configurations
> can significantly increase I/O completion latency, causing some samples to
> exceed the 1-second threshold. The warning is also rate-limited to avoid
> flooding the log. That said, still if it's causing a lot of noise in the logs
> then we could change it to dev_dbg_ratelimited() from dev_warn_ratelimited().
Note they these warnings go away when I turn off those debug options, as
expected.
The max latency I was seeing was ~1.2 seconds. Maybe you can just
increase this limit for ignoring the sample. Or make it a
dev_warn_once(). Or control the warn print rate with DEFINE_RATELIMIT_STATE.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy
2026-08-11 10:26 ` John Garry
@ 2026-08-12 8:01 ` Nilay Shroff
0 siblings, 0 replies; 30+ messages in thread
From: Nilay Shroff @ 2026-08-12 8:01 UTC (permalink / raw)
To: John Garry, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
On 8/11/26 3:56 PM, John Garry wrote:
> On 10/08/2026 18:25, Nilay Shroff wrote:
>>>
>>> iops: (groupid=0, jobs=10): err= 0: pid=821: Mon Aug 10 08:08:28 2026
>>> read: IOPS=9239, BW=36.1MiB/s (37.8MB/s)(1434MiB/39744msec)
>>> slat (usec): min=25, max=110836, avg=129.22, stdev=537.18
>>>
>>> Note that I do have many heavy kernel debug options enabled, like kasan and kmemleak, which may influence this.
>>>
>>
>> Thanks for testing. Given that you're running with KASAN, kmemleak, and other
>> heavy debug options enabled, I think this is expected. Those configurations
>> can significantly increase I/O completion latency, causing some samples to
>> exceed the 1-second threshold. The warning is also rate-limited to avoid
>> flooding the log. That said, still if it's causing a lot of noise in the logs
>> then we could change it to dev_dbg_ratelimited() from dev_warn_ratelimited().
>
> Note they these warnings go away when I turn off those debug options, as expected.
>
> The max latency I was seeing was ~1.2 seconds. Maybe you can just increase this limit for ignoring the sample. Or make it a dev_warn_once(). Or control the warn print rate with DEFINE_RATELIMIT_STATE.
I think we could make it dev_warn_once() as we also increment the stat
counter nr_ignored when we hit this case. So user could always interrogate
the stat counter to find num of samples those were ignored.
Thanks,
--Nilay
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy
2026-08-09 10:07 ` [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy Nilay Shroff
2026-08-10 8:12 ` John Garry
@ 2026-08-10 10:46 ` John Garry
2026-08-10 15:25 ` Nilay Shroff
1 sibling, 1 reply; 30+ messages in thread
From: John Garry @ 2026-08-10 10:46 UTC (permalink / raw)
To: Nilay Shroff, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
On 09/08/2026 11:07, Nilay Shroff wrote:
> This commit introduces a new I/O policy named "latency". Users can configure it
> by writing "latency" to "/sys/class/nvme-subsystem/nvme- subsystemX/iopolicy"
> The "latency" policy dynamically distributes I/O based on measured I/O
> completion latency.
>
> This commit introduces a new I/O policy named "latency". Users can
> configure it by writing "latency" to "/sys/class/nvme-subsystem/nvme-
> subsystemX/iopolicy"
>
> The "latency" policy dynamically distributes I/O based on measured I/O
> completion latency. The main idea is to calculate latency for each path,
> derive a weight, and then proportionally forward I/O according to those
> weights.
>
> To ensure scalability, path latency is measured per-CPU. Each CPU
> maintains its own statistics, and I/O forwarding uses these per-CPU
> values. Every ~15 seconds, a simple average latency of per-CPU batched
> samples are computed and fed into an Exponentially Weighted Moving
> Average (EWMA):
>
> avg_latency = div_u64(batch, batch_count);
> new_ewma_latency = (prev_ewma_latency * (WEIGHT-1) + avg_latency)/WEIGHT
>
> With WEIGHT = 8, this assigns 7/8 (~87.5%) weight to the previous
> latency value and 1/8 (~12.5%) to the most recent latency. This
> smoothing reduces jitter, adapts quickly to changing conditions,
> avoids storing historical samples, and works well for both low and
> high I/O rates. Path weights are then derived from the smoothed (EWMA)
> latency as follows (example with two paths A and B):
>
> path_A_score = NSEC_PER_SEC / path_A_ewma_latency
> path_B_score = NSEC_PER_SEC / path_B_ewma_latency
> total_score = path_A_score + path_B_score
>
> path_A_weight = (path_A_score * 64) / total_score
> path_B_weight = (path_B_score * 64) / total_score
>
> where:
> - path_X_ewma_latency is the smoothed latency of a path in nanoseconds
> - NSEC_PER_SEC is used as a scaling factor since valid latencies
> are < 1 second
> - weights are normalized to a 0–64 scale across all paths.
>
> Path credits are refilled based on this weight, with one credit
> consumed per I/O. When all credits are consumed, the credits are
> refilled again based on the current weight. This ensures that I/O is
> distributed across paths proportionally to their calculated weight.
>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
> ---
> drivers/nvme/host/core.c | 15 +-
> drivers/nvme/host/multipath.c | 444 +++++++++++++++++++++++++++++++++-
> drivers/nvme/host/nvme.h | 52 +++-
> 3 files changed, 496 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 453c1f0b2dd0..542e2ee036cc 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -690,6 +690,9 @@ static void nvme_free_ns_head(struct kref *ref)
> cleanup_srcu_struct(&head->srcu);
> nvme_put_subsystem(head->subsys);
> kfree(head->plids);
> +#ifdef CONFIG_NVME_MULTIPATH
> + free_percpu(head->latency_path);
> +#endif
Since head->latency_path is allocated in nvme_mpath_alloc_disk(), can
this free be done in nvme_mpath_put_disk()? I know that we would be
doing more than a "put" of the disk, but we do other tidying tasks in
nvme_mpath_put_disk() already.
> kfree(head);
> }
>
> @@ -707,6 +710,7 @@ static void nvme_free_ns(struct kref *kref)
> {
> struct nvme_ns *ns = container_of(kref, struct nvme_ns, kref);
>
> + nvme_free_ns_stat(ns);
> put_disk(ns->disk);
> nvme_put_ns_head(ns->head);
> nvme_put_ctrl(ns->ctrl);
> @@ -4220,6 +4224,9 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
> if (nvme_init_ns_head(ns, info))
> goto out_cleanup_disk;
>
> + if (nvme_alloc_ns_stat(ns))
> + goto out_unlink_ns;
> +
> /*
> * If multipathing is enabled, the device name for all disks and not
> * just those that represent shared namespaces needs to be based on the
> @@ -4244,7 +4251,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
> }
>
> if (nvme_update_ns_info(ns, info))
> - goto out_unlink_ns;
> + goto out_free_ns_stat;
>
> mutex_lock(&ctrl->namespaces_lock);
> /*
> @@ -4253,7 +4260,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
> */
> if (test_bit(NVME_CTRL_FROZEN, &ctrl->flags)) {
> mutex_unlock(&ctrl->namespaces_lock);
> - goto out_unlink_ns;
> + goto out_free_ns_stat;
> }
> blk_queue_rq_timeout(ns->queue, ctrl->io_timeout);
> nvme_ns_add_to_ctrl_list(ns);
> @@ -4278,6 +4285,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
> list_del_rcu(&ns->list);
> mutex_unlock(&ctrl->namespaces_lock);
> synchronize_srcu(&ctrl->srcu);
> +out_free_ns_stat:
> + nvme_free_ns_stat(ns);
> out_unlink_ns:
> mutex_lock(&ctrl->subsys->lock);
> list_del_rcu(&ns->siblings);
> @@ -4317,7 +4326,7 @@ static void nvme_ns_remove(struct nvme_ns *ns)
>
> /*
> * Ensure that !NVME_NS_READY is seen by other threads to prevent
> - * this ns going back into current_path.
> + * this ns going back into current_path/latency_path.
> */
> synchronize_srcu(&ns->head->srcu);
>
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 8c20ff516e61..8086530b5350 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -6,6 +6,9 @@
> #include <linux/backing-dev.h>
> #include <linux/moduleparam.h>
> #include <linux/vmalloc.h>
> +#include <linux/blk-mq.h>
> +#include <linux/math64.h>
> +#include <linux/rculist.h>
maintaining alpabetic ordering is nicer
> #include <trace/events/block.h>
> #include "nvme.h"
>
> @@ -66,9 +69,10 @@ MODULE_PARM_DESC(multipath_always_on,
> "create multipath node always except for private namespace with non-unique nsid; note that this also implicitly enables native multipath support");
>
> static const char *nvme_iopolicy_names[] = {
> - [NVME_IOPOLICY_NUMA] = "numa",
> - [NVME_IOPOLICY_RR] = "round-robin",
> - [NVME_IOPOLICY_QD] = "queue-depth",
> + [NVME_IOPOLICY_NUMA] = "numa",
> + [NVME_IOPOLICY_RR] = "round-robin",
> + [NVME_IOPOLICY_QD] = "queue-depth",
> + [NVME_IOPOLICY_LATENCY] = "latency",
> };
>
> static int iopolicy = NVME_IOPOLICY_NUMA;
> @@ -107,7 +111,7 @@ static int nvme_get_iopolicy(char *buf, const struct kernel_param *kp)
> module_param_call(iopolicy, nvme_set_iopolicy, nvme_get_iopolicy,
> &iopolicy, 0644);
> MODULE_PARM_DESC(iopolicy,
> - "Default multipath I/O policy; 'numa' (default), 'round-robin' or 'queue-depth'");
> + "Default multipath I/O policy; 'numa' (default), 'round-robin' or 'queue-depth' or 'latency'");
>
> void nvme_mpath_default_iopolicy(struct nvme_subsystem *subsys)
> {
> @@ -199,6 +203,203 @@ void nvme_mpath_start_request(struct request *rq)
> }
> EXPORT_SYMBOL_GPL(nvme_mpath_start_request);
>
> +static void nvme_mpath_weight_work(struct work_struct *weight_work)
> +{
> + int cpu, srcu_idx;
> + u32 weight;
> + struct nvme_ns *ns;
> + struct nvme_path_lat_stat *stat;
> + struct nvme_path_lat_work *work = container_of(weight_work,
> + struct nvme_path_lat_work, weight_work);
> + struct nvme_ns_head *head = work->ns->head;
> + int op_type = work->op_type;
> + u64 total_score = 0;
> +
> + cpu = get_cpu();
> +
> + srcu_idx = srcu_read_lock(&head->srcu);
> + list_for_each_entry_srcu(ns, &head->list, siblings,
> + srcu_read_lock_held(&head->srcu)) {
> +
> + stat = &this_cpu_ptr(ns->path_lat)[op_type].stat;
get_cpu() does a raw_smp_processor_id() call to get the cpu. So why not
use the this_cpu_ptr() equivalent which is passed the cpu, which would
save looking up the cpu again? I think that is per_cpu_ptr(), which is
actually used elsewhere
> + if (!READ_ONCE(stat->slat_ns)) {
> + stat->score = 0;
> + continue;
> + }
> + /*
> + * Compute the path score as the inverse of smoothed
> + * latency, scaled by NSEC_PER_SEC. Floating point
> + * math is unavailable in the kernel, so fixed-point
> + * scaling is used instead. NSEC_PER_SEC is chosen
> + * because valid latencies are always < 1 second; longer
> + * latencies are ignored.
> + */
> + stat->score = div_u64(NSEC_PER_SEC, READ_ONCE(stat->slat_ns));
> +
> + /* Compute total score. */
> + total_score += stat->score;
> + }
> +
> + if (!total_score)
> + goto out;
> +
> + /*
> + * After computing the total slatency, we derive per-path weight
> + * (normalized to the range 0–64). The weight represents the
> + * relative share of I/O the path should receive.
> + *
> + * - lower smoothed latency -> higher weight
> + * - higher smoothed slatency -> lower weight
> + *
> + * Next, while forwarding I/O, we assign "credits" to each path
> + * based on its weight (please also refer nvme_latency_path()):
> + * - Initially, credits = weight.
> + * - Each time an I/O is dispatched on a path, its credits are
> + * decremented proportionally.
> + * - When a path runs out of credits, it becomes temporarily
> + * ineligible until credit is refilled.
> + *
> + * I/O distribution is therefore governed by available credits,
> + * ensuring that over time the proportion of I/O sent to each
> + * path matches its weight (and thus its performance).
> + */
> + list_for_each_entry_srcu(ns, &head->list, siblings,
> + srcu_read_lock_held(&head->srcu)) {
> +
> + stat = &this_cpu_ptr(ns->path_lat)[op_type].stat;
> + weight = div_u64(stat->score * 64, total_score);
> +
> + /*
> + * Ensure the path weight never drops below 1. A weight
> + * of 0 is used only for newly added paths. During
> + * bootstrap, a few I/Os are sent to such paths to
> + * establish an initial weight. Enforcing a minimum
> + * weight of 1 guarantees that no path is forgotten and
> + * that each path is probed at least occasionally.
> + */
> + if (!weight)
> + weight = 1;
> +
> + WRITE_ONCE(stat->weight, weight);
> + }
> +out:
> + srcu_read_unlock(&head->srcu, srcu_idx);
> + put_cpu();
> +}
> +
> +/*
> + * Formula to calculate the EWMA (Exponentially Weighted Moving Average):
> + * ewma = (old_ewma * (EWMA_SHIFT - 1) + (EWMA_SHIFT)) / EWMA_SHIFT
> + * For instance, with EWMA_SHIFT = 3, this assigns 7/8 (~87.5 %) weight to
> + * the existing/old ewma and 1/8 (~12.5%) weight to the new sample.
> + */
> +static inline u64 calc_ewma_update(u64 old, u64 new)
> +{
> + return (old * ((1 << NVME_DEFAULT_LATENCY_EWMA_SHIFT) - 1)
> + + new) >> NVME_DEFAULT_LATENCY_EWMA_SHIFT;
> +}
> +
> +static void nvme_mpath_add_sample(struct request *rq, struct nvme_ns *ns)
> +{
> + int cpu;
> + unsigned int op_type;
> + struct nvme_path_lat *path_lat;
> + struct nvme_path_lat_stat *stat;
> + u64 now, latency, slat_ns, avg_lat_ns;
> + struct nvme_ns_head *head = ns->head;
> +
> + if (list_is_singular(&head->list))
> + return;
> +
> + now = ktime_get_ns();
> + latency = now >= rq->io_start_time_ns ? now - rq->io_start_time_ns : 0;
> + if (!latency)
> + return;
> +
> + /*
> + * As completion code path is serialized(i.e. no same completion queue
> + * update code could run simultaneously on multiple cpu) we can safely
> + * access per cpu nvme path stat here from another cpu (in case the
> + * completion cpu is different from submission cpu).
> + * The only field which could be accessed simultaneously here is the
> + * path ->weight which may be accessed by this function as well as I/O
> + * submission path during path selection logic and we protect ->weight
> + * using READ_ONCE/WRITE_ONCE. Yes this may not be 100% accurate but
> + * we also don't need to be so accurate here as the path credit would
> + * be anyways refilled, based on path weight, once path consumes all
> + * its credits. And we limit path weight/credit max up to 64. Please
> + * also refer nvme_latency_path().
> + */
> + cpu = blk_mq_rq_cpu(rq);
> + op_type = nvme_data_dir(rq);
> + path_lat = &per_cpu_ptr(ns->path_lat, cpu)[op_type];
> + stat = &path_lat->stat;
> +
> + /*
> + * If latency > ~1s then ignore this sample to prevent EWMA from being
> + * skewed by pathological outliers (multi-second waits, controller
> + * timeouts etc.). This keeps path scores representative of normal
> + * performance and avoids instability from rare spikes. If such high
> + * latency is real, ANA state reporting or keep-alive error counters
> + * will mark the path unhealthy and remove it from the head node list,
> + * so we safely skip such sample here.
> + */
> + if (unlikely(latency > NSEC_PER_SEC)) {
> + stat->nr_ignored++;
> + dev_warn_ratelimited(ns->ctrl->device,
> + "ignoring sample with >1s latency (possible controller stall or timeout)\n");
> + return;
> + }
> +
> + /*
> + * Accumulate latency samples and increment the batch count for each
> + * ~15 second interval. When the interval expires, compute the simple
> + * average latency over that window, then update the smoothed (EWMA)
> + * latency. The path weight is recalculated based on this smoothed
> + * latency.
> + */
> + stat->batch += latency;
> + stat->batch_count++;
> + stat->nr_samples++;
> +
> + if (now > stat->last_batch_ts && ((now - stat->last_batch_ts) >=
> + NVME_DEFAULT_LATENCY_BATCH_TIMEOUT)) {
> +
> + /*
> + * Find simple average latency for the last epoch (~15 sec
> + * interval).
> + */
> + avg_lat_ns = div_u64(stat->batch, stat->batch_count);
> + stat->last_batch_ts = now;
> +
> + /*
> + * Calculate smooth/EWMA (Exponentially Weighted Moving Average)
> + * latency. EWMA is preferred over simple average latency
> + * because it smooths naturally, reduces jitter from sudden
> + * spikes, and adapts faster to changing conditions. It also
> + * avoids storing historical samples, and works well for both
> + * slow and fast I/O rates.
> + * Formula:
> + * slat_ns = (prev_slat_ns * (WEIGHT - 1) + (latency)) / WEIGHT
> + * With WEIGHT = 8, this assigns 7/8 (~87.5 %) weight to the
> + * existing latency and 1/8 (~12.5%) weight to the new latency.
> + */
> + if (unlikely(!stat->slat_ns))
> + WRITE_ONCE(stat->slat_ns, avg_lat_ns);
> + else {
> + slat_ns = calc_ewma_update(stat->slat_ns, avg_lat_ns);
> + WRITE_ONCE(stat->slat_ns, slat_ns);
> + }
> +
> + stat->batch = stat->batch_count = 0;
> +
> + /*
> + * Defer calculation of the path weight in per-cpu workqueue.
> + */
> + schedule_work_on(cpu, &path_lat->work.weight_work);
> + }
> +}
> +
> void nvme_mpath_end_request(struct request *rq)
> {
> struct nvme_ns *ns = rq->q->queuedata;
> @@ -206,6 +407,15 @@ void nvme_mpath_end_request(struct request *rq)
> if (nvme_req(rq)->flags & NVME_MPATH_CNT_ACTIVE)
> atomic_dec_if_positive(&ns->ctrl->nr_active);
>
> + if (test_bit(NVME_NS_PATH_STAT, &ns->flags)) {
> + int srcu_idx;
> +
> + srcu_idx = srcu_read_lock(&ns->head->srcu);
> + if (test_bit(NVME_NS_PATH_STAT, &ns->flags))
Some may ask why check NVME_NS_PATH_STAT twice.
> + nvme_mpath_add_sample(rq, ns);
> + srcu_read_unlock(&ns->head->srcu, srcu_idx);
> + }
> +
> if (!(nvme_req(rq)->flags & NVME_MPATH_IO_STATS))
> return;
> bdev_end_io_acct(ns->head->disk->part0, req_op(rq),
> @@ -239,6 +449,78 @@ static const char *nvme_ana_state_names[] = {
> [NVME_ANA_CHANGE] = "change",
> };
>
> +static void nvme_reset_ns_latency_stat(struct nvme_ns *ns)
> +{
> + int i, cpu;
> + struct nvme_path_lat_stat *stat;
reverse fir tree style ordering is nicer, in my opinion
> +
> + for_each_possible_cpu(cpu) {
> + for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
> + stat = &per_cpu_ptr(ns->path_lat, cpu)[i].stat;
> + memset(stat, 0, sizeof(struct nvme_path_lat_stat));
> + }
> + }
> +}
> +
> +void nvme_cancel_ns_latency_weight_work(struct nvme_ns *ns)
why not static? It seems to be only used in multipath.c
> +{
> + int i, cpu;
> + struct nvme_path_lat *path_lat;
> +
> + for_each_possible_cpu(cpu) {
> + for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
> + path_lat = &per_cpu_ptr(ns->path_lat, cpu)[i];
> + cancel_work_sync(&path_lat->work.weight_work);
> + }
> + }
> +}
> +
> +static bool nvme_enable_ns_latency_sampling(struct nvme_ns *ns)
return value never checked
> +{
> + struct nvme_ns_head *head = ns->head;
> +
> + if (!head->disk ||
> + READ_ONCE(head->subsys->iopolicy) != NVME_IOPOLICY_LATENCY)
> + return false;
> +
> + if (test_and_set_bit(NVME_NS_PATH_STAT, &ns->flags))
> + return false;
> +
> + blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, ns->queue);
Please explain why it is required. nvme_mpath_add_sample() looks to
mention this, but I think a brief explanation here would be good.
> + blk_stat_enable_accounting(ns->queue);
> + return true;
> +}
> +
> +static bool nvme_disable_ns_latency_sampling(struct nvme_ns *ns)
> +{
> + int cpu;
> + struct nvme_ns_head *head = ns->head;
> + bool changed = false;
> +
> + if (!test_and_clear_bit(NVME_NS_PATH_STAT, &ns->flags))
> + return false;
> +
> + for_each_possible_cpu(cpu) {
> + if (ns == READ_ONCE(*per_cpu_ptr(head->latency_path, cpu))) {
> + WRITE_ONCE(*per_cpu_ptr(head->latency_path, cpu), NULL);
> + changed = true;
> + }
> + }
> +
> + blk_stat_disable_accounting(ns->queue);
> + blk_queue_flag_clear(QUEUE_FLAG_SAME_FORCE, ns->queue);
eh, what if QUEUE_FLAG_SAME_FORCE was already enabled before
nvme_enable_ns_latency_sampling()?
> +
> + /*
> + * Ensure that we wait until completion side samplings (if any sneaked
> + * in after we clear NVME_NS_PATH_STAT) are all scheduled before we
> + * start cancelling those.
> + */
> + synchronize_srcu(&head->srcu);
> + nvme_cancel_ns_latency_weight_work(ns);
> + nvme_reset_ns_latency_stat(ns);
> + return changed;
> +}
> +
> bool nvme_mpath_clear_current_path(struct nvme_ns *ns)
> {
> struct nvme_ns_head *head = ns->head;
> @@ -251,6 +533,10 @@ bool nvme_mpath_clear_current_path(struct nvme_ns *ns)
> changed = true;
> }
> }
> +
> + if (nvme_disable_ns_latency_sampling(ns))
> + changed = true;
> +
> return changed;
> }
>
> @@ -268,6 +554,45 @@ void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl)
> srcu_read_unlock(&ctrl->srcu, srcu_idx);
> }
>
> +int nvme_alloc_ns_stat(struct nvme_ns *ns)
Surely "mpath" should be in the name, no? It seems that every other
public API in multpath.c has "mpath" in the name.
> +{
> + int i, cpu;
> + struct nvme_path_lat_work *work;
> + gfp_t gfp = GFP_KERNEL | __GFP_ZERO;
> +
> + if (!ns->head->disk)
> + return 0;
> +
> + ns->path_lat = __alloc_percpu_gfp(NVME_NUM_STAT_GROUPS *
> + sizeof(struct nvme_path_lat),
> + __alignof__(struct nvme_path_lat), gfp);
> + if (!ns->path_lat)
> + return -ENOMEM;
> +
> + for_each_possible_cpu(cpu) {
> + for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
> + work = &per_cpu_ptr(ns->path_lat, cpu)[i].work;
> + work->ns = ns;
> + work->op_type = i;
> + INIT_WORK(&work->weight_work, nvme_mpath_weight_work);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static void nvme_mpath_set_ctrl_paths(struct nvme_ctrl *ctrl)
what do you mean by "set" here?
> +{
> + struct nvme_ns *ns;
> + int srcu_idx;
> +
> + srcu_idx = srcu_read_lock(&ctrl->srcu);
> + list_for_each_entry_srcu(ns, &ctrl->namespaces, list,
> + srcu_read_lock_held(&ctrl->srcu))
> + nvme_enable_ns_latency_sampling(ns);
> + srcu_read_unlock(&ctrl->srcu, srcu_idx);
> +}
> +
> void nvme_mpath_revalidate_paths(struct nvme_ns_head *head)
> {
> sector_t capacity = get_capacity(head->disk);
> @@ -280,6 +605,8 @@ void nvme_mpath_revalidate_paths(struct nvme_ns_head *head)
> srcu_read_lock_held(&head->srcu)) {
> if (capacity != get_capacity(ns->disk))
> clear_bit(NVME_NS_READY, &ns->flags);
> +
> + nvme_reset_ns_latency_stat(ns);
> }
> srcu_read_unlock(&head->srcu, srcu_idx);
>
> @@ -404,6 +731,92 @@ static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head)
> return found;
> }
>
> +static inline bool nvme_state_is_live(enum nvme_ana_state state)
> +{
> + return state == NVME_ANA_OPTIMIZED || state == NVME_ANA_NONOPTIMIZED;
> +}
> +
> +static struct nvme_ns *nvme_latency_path(struct nvme_ns_head *head,
> + unsigned int op_type)
> +{
> + struct nvme_ns *ns, *start, *found = NULL;
> + struct nvme_path_lat_stat *stat;
> + u32 weight;
> + int cpu;
> +
> + cpu = get_cpu();
> + ns = READ_ONCE(*this_cpu_ptr(head->latency_path));
> + if (unlikely(!ns)) {
> + ns = list_first_or_null_rcu(&head->list,
> + struct nvme_ns, siblings);
> + if (unlikely(!ns))
> + goto out;
out: checks found, which is always NULL. You could add another label
after the found check at out: and goto that label to avoid the
unnecessary check.
> + }
> +found_ns:
> + start = ns;
> + while (nvme_path_is_disabled(ns) ||
> + !nvme_state_is_live(ns->ana_state)) {
> + ns = list_next_entry_circular(ns, &head->list, siblings);
> +
> + /*
> + * If we iterate through all paths in the list but find each
> + * path in list is either disabled or dead then bail out.
> + */
> + if (ns == start)
> + goto out;
> + }
> +
> + stat = &this_cpu_ptr(ns->path_lat)[op_type].stat;
> +
> + /*
> + * When the head path-list is singular we don't calculate the
> + * only path weight for optimization as we don't need to forward
> + * I/O to more than one path. The another possibility is when the
> + * path is newly added, we don't know its weight. So we go round
> + * -robin for each such path and forward I/O to it.Once we start
> + * getting response for such I/Os, the path weight calculation
> + * would kick in and then we start using path credit for
> + * forwarding I/O.
> + */
> + weight = READ_ONCE(stat->weight);
> + if (!weight) {
> + found = ns;
> + goto out;
> + }
> +
> + /*
> + * To keep path selection logic simple, we don't distinguish
> + * between ANA optimized and non-optimized states. The non-
> + * optimized path is expected to have a lower weight, and
> + * therefore fewer credits. As a result, only a small number of
> + * I/Os will be forwarded to paths in the non-optimized state.
> + */
> + if (stat->credit > 0) {
> + --stat->credit;
> + found = ns;
> + goto out;
this goto is superfluous
> + } else {
> + /*
> + * Refill credit from path weight and move to next path. The
> + * refilled credit of the current path will be used next when
> + * all remainng paths exhaust its credits.
> + */
> + weight = READ_ONCE(stat->weight);
> + stat->credit = weight;
> + ns = list_next_entry_circular(ns, &head->list, siblings);
> + if (likely(ns))
> + goto found_ns;
> + }
> +out:
> + if (found) {
> + stat->sel++;
> + WRITE_ONCE(*this_cpu_ptr(head->latency_path), found);
> + }
> +
> + put_cpu();
> + return found;
> +}
> +
> static struct nvme_ns *nvme_queue_depth_path(struct nvme_ns_head *head)
> {
> struct nvme_ns *best_opt = NULL, *best_nonopt = NULL, *ns;
> @@ -464,6 +877,8 @@ inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head,
> unsigned int op_type)
> {
> switch (READ_ONCE(head->subsys->iopolicy)) {
> + case NVME_IOPOLICY_LATENCY:
> + return nvme_latency_path(head, op_type);
> case NVME_IOPOLICY_QD:
> return nvme_queue_depth_path(head);
> case NVME_IOPOLICY_RR:
> @@ -754,6 +1169,10 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
> if (!nvme_is_unique_nsid(ctrl, head))
> return 0;
>
> + head->latency_path = alloc_percpu_gfp(struct nvme_ns*, GFP_KERNEL);
> + if (!head->latency_path)
> + return -ENOMEM;
> +
> blk_set_stacking_limits(&lim);
> lim.dma_alignment = 3;
> lim.features |= BLK_FEAT_IO_STAT | BLK_FEAT_NOWAIT |
> @@ -762,8 +1181,10 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
> lim.features |= BLK_FEAT_ZONED;
>
> head->disk = blk_alloc_disk(&lim, ctrl->numa_node);
> - if (IS_ERR(head->disk))
> + if (IS_ERR(head->disk)) {
> + free_percpu(head->latency_path);
> return PTR_ERR(head->disk);
> + }
> head->disk->fops = &nvme_ns_head_ops;
> head->disk->private_data = head;
>
> @@ -819,6 +1240,10 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
> }
> mutex_unlock(&head->lock);
>
> + mutex_lock(&nvme_subsystems_lock);
I am curious - why use the nvme_subsystems_lock?
> + nvme_enable_ns_latency_sampling(ns);
> + mutex_unlock(&nvme_subsystems_lock);
> +
> synchronize_srcu(&head->srcu);
> kblockd_schedule_work(&head->requeue_work);
> }
> @@ -867,11 +1292,6 @@ static int nvme_parse_ana_log(struct nvme_ctrl *ctrl, void *data,
> return 0;
> }
>
> -static inline bool nvme_state_is_live(enum nvme_ana_state state)
> -{
> - return state == NVME_ANA_OPTIMIZED || state == NVME_ANA_NONOPTIMIZED;
> -}
> -
> static void nvme_update_ns_ana_state(struct nvme_ana_group_desc *desc,
> struct nvme_ns *ns)
> {
> @@ -1049,10 +1469,12 @@ static void nvme_subsys_iopolicy_update(struct nvme_subsystem *subsys,
>
> WRITE_ONCE(subsys->iopolicy, iopolicy);
>
> - /* iopolicy changes clear the mpath by design */
> + /* iopolicy changes clear/reset the mpath by design */
> mutex_lock(&nvme_subsystems_lock);
> list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry)
> nvme_mpath_clear_ctrl_paths(ctrl);
> + list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry)
> + nvme_mpath_set_ctrl_paths(ctrl);
Do we need to have separate loops to call nvme_mpath_clear_ctrl_paths()
and nvme_mpath_set_ctrl_paths()?
> mutex_unlock(&nvme_subsystems_lock);
>
> pr_notice("subsysnqn %s iopolicy changed from %s to %s\n",
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 8a9ec502912d..3c82f67f8926 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -28,7 +28,9 @@ extern unsigned int nvme_io_timeout;
> extern unsigned int admin_timeout;
> #define NVME_ADMIN_TIMEOUT (admin_timeout * HZ)
>
> -#define NVME_DEFAULT_KATO 5
> +#define NVME_DEFAULT_KATO 5
> +#define NVME_DEFAULT_LATENCY_EWMA_SHIFT 3
> +#define NVME_DEFAULT_LATENCY_BATCH_TIMEOUT (15 * NSEC_PER_SEC)
>
> #ifdef CONFIG_ARCH_NO_SG_CHAIN
> #define NVME_INLINE_SG_CNT 0
> @@ -483,6 +485,7 @@ enum nvme_iopolicy {
> NVME_IOPOLICY_NUMA,
> NVME_IOPOLICY_RR,
> NVME_IOPOLICY_QD,
> + NVME_IOPOLICY_LATENCY,
> };
>
> struct nvme_subsystem {
> @@ -527,6 +530,30 @@ enum nvme_stat_group {
> NVME_NUM_STAT_GROUPS
> };
>
> +struct nvme_path_lat_stat {
> + u64 nr_samples; /* total num of samples processed */
why u64 and not unsigned long long?
> + u64 nr_ignored; /* num. of samples ignored */
> + u64 slat_ns; /* smoothed (ewma) latency in nanoseconds */
> + u64 score; /* score used for weight calculation */
> + u64 last_batch_ts; /* timestamp when last time avg. latency is calculated */
> + u64 sel; /* num of times this path is selcted for I/O */
> + u64 batch; /* accumulated latency sum for current window */
> + u32 batch_count; /* num of samples accumulated in current window */
> + u32 weight; /* path weight */
> + u32 credit; /* path credit for I/O forwarding */
> +};
> +
> +struct nvme_path_lat_work {
> + struct nvme_ns *ns; /* owning namespace */
> + struct work_struct weight_work; /* deferred work for weight calculation */
> + int op_type; /* op type : READ/WRITE/OTHER */
> +};
> +
> +struct nvme_path_lat {
> + struct nvme_path_lat_stat stat; /* path statistics */
> + struct nvme_path_lat_work work; /* background worker context */
> +};
> +
> /*
> * Anchor structure for namespaces. There is one for each namespace in a
> * NVMe subsystem that any of our controllers can see, and the namespace
> @@ -578,6 +605,8 @@ struct nvme_ns_head {
> unsigned int delayed_removal_secs;
> atomic_long_t io_requeue_no_usable_path_count;
> atomic_long_t io_fail_no_available_path_count;
> + struct nvme_ns * __percpu *latency_path;
> +
> #define NVME_NSHEAD_DISK_LIVE 0
> #define NVME_NSHEAD_QUEUE_IF_NO_PATH 1
> #define NVME_NSHEAD_CDEV_LIVE 2
> @@ -606,6 +635,7 @@ struct nvme_ns {
> enum nvme_ana_state ana_state;
> u32 ana_grpid;
> atomic_long_t failover;
> + struct nvme_path_lat __percpu *path_lat;
> #endif
> atomic_long_t retries;
> atomic_long_t errors;
> @@ -620,6 +650,7 @@ struct nvme_ns {
> #define NVME_NS_READY 4
> #define NVME_NS_SYSFS_ATTR_LINK 5
> #define NVME_NS_CDEV_LIVE 6
> +#define NVME_NS_PATH_STAT 7
>
> struct cdev cdev;
> struct device cdev_device;
> @@ -1100,6 +1131,8 @@ void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl);
> void nvme_mpath_remove_disk(struct nvme_ns_head *head);
> void nvme_mpath_start_request(struct request *rq);
> void nvme_mpath_end_request(struct request *rq);
> +int nvme_alloc_ns_stat(struct nvme_ns *ns);
> +void nvme_cancel_ns_latency_weight_work(struct nvme_ns *ns);
>
> static inline void nvme_trace_bio_complete(struct request *req)
> {
> @@ -1130,6 +1163,13 @@ static inline bool nvme_mpath_queue_if_no_path(struct nvme_ns_head *head)
> return true;
> return false;
> }
> +static inline void nvme_free_ns_stat(struct nvme_ns *ns)
> +{
> + if (!ns->head->disk)
> + return;
this check is not strictly required, as if ns->head->disk == NULL, the
ns->path_lat == NULL and free_percpu() can handle NULL.
> +
> + free_percpu(ns->path_lat);
> +}
> #else
> #define multipath false
> static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
> @@ -1221,6 +1261,16 @@ static inline bool nvme_mpath_queue_if_no_path(struct nvme_ns_head *head)
> {
> return false;
> }
> +static inline void nvme_cancel_ns_latency_weight_work(struct nvme_ns *ns)
> +{
> +}
> +static inline int nvme_alloc_ns_stat(struct nvme_ns *ns)
> +{
> + return 0;
> +}
> +static inline void nvme_free_ns_stat(struct nvme_ns *ns)
> +{
> +}
> #endif /* CONFIG_NVME_MULTIPATH */
>
> int nvme_ns_get_unique_id(struct nvme_ns *ns, u8 id[16],
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy
2026-08-10 10:46 ` John Garry
@ 2026-08-10 15:25 ` Nilay Shroff
2026-08-11 10:03 ` John Garry
0 siblings, 1 reply; 30+ messages in thread
From: Nilay Shroff @ 2026-08-10 15:25 UTC (permalink / raw)
To: John Garry, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
On 8/10/26 4:16 PM, John Garry wrote:
> On 09/08/2026 11:07, Nilay Shroff wrote:
>> This commit introduces a new I/O policy named "latency". Users can configure it
>> by writing "latency" to "/sys/class/nvme-subsystem/nvme- subsystemX/iopolicy"
>> The "latency" policy dynamically distributes I/O based on measured I/O
>> completion latency.
> >
>> This commit introduces a new I/O policy named "latency". Users can
>> configure it by writing "latency" to "/sys/class/nvme-subsystem/nvme-
>> subsystemX/iopolicy"
>>
>> The "latency" policy dynamically distributes I/O based on measured I/O
>> completion latency. The main idea is to calculate latency for each path,
>> derive a weight, and then proportionally forward I/O according to those
>> weights.
>>
>> To ensure scalability, path latency is measured per-CPU. Each CPU
>> maintains its own statistics, and I/O forwarding uses these per-CPU
>> values. Every ~15 seconds, a simple average latency of per-CPU batched
>> samples are computed and fed into an Exponentially Weighted Moving
>> Average (EWMA):
>>
>> avg_latency = div_u64(batch, batch_count);
>> new_ewma_latency = (prev_ewma_latency * (WEIGHT-1) + avg_latency)/WEIGHT
>>
>> With WEIGHT = 8, this assigns 7/8 (~87.5%) weight to the previous
>> latency value and 1/8 (~12.5%) to the most recent latency. This
>> smoothing reduces jitter, adapts quickly to changing conditions,
>> avoids storing historical samples, and works well for both low and
>> high I/O rates. Path weights are then derived from the smoothed (EWMA)
>> latency as follows (example with two paths A and B):
>>
>> path_A_score = NSEC_PER_SEC / path_A_ewma_latency
>> path_B_score = NSEC_PER_SEC / path_B_ewma_latency
>> total_score = path_A_score + path_B_score
>>
>> path_A_weight = (path_A_score * 64) / total_score
>> path_B_weight = (path_B_score * 64) / total_score
>>
>> where:
>> - path_X_ewma_latency is the smoothed latency of a path in nanoseconds
>> - NSEC_PER_SEC is used as a scaling factor since valid latencies
>> are < 1 second
>> - weights are normalized to a 0–64 scale across all paths.
>>
>> Path credits are refilled based on this weight, with one credit
>> consumed per I/O. When all credits are consumed, the credits are
>> refilled again based on the current weight. This ensures that I/O is
>> distributed across paths proportionally to their calculated weight.
>>
>> Reviewed-by: Hannes Reinecke <hare@suse.de>
>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>> ---
>> drivers/nvme/host/core.c | 15 +-
>> drivers/nvme/host/multipath.c | 444 +++++++++++++++++++++++++++++++++-
>> drivers/nvme/host/nvme.h | 52 +++-
>> 3 files changed, 496 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 453c1f0b2dd0..542e2ee036cc 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -690,6 +690,9 @@ static void nvme_free_ns_head(struct kref *ref)
>> cleanup_srcu_struct(&head->srcu);
>> nvme_put_subsystem(head->subsys);
>> kfree(head->plids);
>> +#ifdef CONFIG_NVME_MULTIPATH
>> + free_percpu(head->latency_path);
> > +#endif
>
> Since head->latency_path is allocated in nvme_mpath_alloc_disk(), can this free be done in nvme_mpath_put_disk()? I know that we would be doing more than a "put" of the disk, but we do other tidying tasks in nvme_mpath_put_disk() already.
Yes, that should be possible. It also matches the allocation/release pairing,
where the resource is allocated in nvme_mpath_alloc_disk() and released in
nvme_mpath_put_disk(). I'll make that change in the next version.
>
>> kfree(head);
>> }
>> @@ -707,6 +710,7 @@ static void nvme_free_ns(struct kref *kref)
>> {
>> struct nvme_ns *ns = container_of(kref, struct nvme_ns, kref);
>> + nvme_free_ns_stat(ns);
>> put_disk(ns->disk);
>> nvme_put_ns_head(ns->head);
>> nvme_put_ctrl(ns->ctrl);
>> @@ -4220,6 +4224,9 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
>> if (nvme_init_ns_head(ns, info))
>> goto out_cleanup_disk;
>> + if (nvme_alloc_ns_stat(ns))
>> + goto out_unlink_ns;
>> +
>> /*
>> * If multipathing is enabled, the device name for all disks and not
>> * just those that represent shared namespaces needs to be based on the
>> @@ -4244,7 +4251,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
>> }
>> if (nvme_update_ns_info(ns, info))
>> - goto out_unlink_ns;
>> + goto out_free_ns_stat;
>> mutex_lock(&ctrl->namespaces_lock);
>> /*
>> @@ -4253,7 +4260,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
>> */
>> if (test_bit(NVME_CTRL_FROZEN, &ctrl->flags)) {
>> mutex_unlock(&ctrl->namespaces_lock);
>> - goto out_unlink_ns;
>> + goto out_free_ns_stat;
>> }
>> blk_queue_rq_timeout(ns->queue, ctrl->io_timeout);
>> nvme_ns_add_to_ctrl_list(ns);
>> @@ -4278,6 +4285,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
>> list_del_rcu(&ns->list);
>> mutex_unlock(&ctrl->namespaces_lock);
>> synchronize_srcu(&ctrl->srcu);
>> +out_free_ns_stat:
>> + nvme_free_ns_stat(ns);
>> out_unlink_ns:
>> mutex_lock(&ctrl->subsys->lock);
>> list_del_rcu(&ns->siblings);
>> @@ -4317,7 +4326,7 @@ static void nvme_ns_remove(struct nvme_ns *ns)
>> /*
>> * Ensure that !NVME_NS_READY is seen by other threads to prevent
>> - * this ns going back into current_path.
>> + * this ns going back into current_path/latency_path.
>> */
>> synchronize_srcu(&ns->head->srcu);
>> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
>> index 8c20ff516e61..8086530b5350 100644
>> --- a/drivers/nvme/host/multipath.c
>> +++ b/drivers/nvme/host/multipath.c
>> @@ -6,6 +6,9 @@
>> #include <linux/backing-dev.h>
>> #include <linux/moduleparam.h>
>> #include <linux/vmalloc.h>
>> +#include <linux/blk-mq.h>
>> +#include <linux/math64.h>
>> +#include <linux/rculist.h>
>
> maintaining alpabetic ordering is nicer
Okay will update that in next version.
>
>> #include <trace/events/block.h>
>> #include "nvme.h"
>> @@ -66,9 +69,10 @@ MODULE_PARM_DESC(multipath_always_on,
>> "create multipath node always except for private namespace with non-unique nsid; note that this also implicitly enables native multipath support");
>> static const char *nvme_iopolicy_names[] = {
>> - [NVME_IOPOLICY_NUMA] = "numa",
>> - [NVME_IOPOLICY_RR] = "round-robin",
>> - [NVME_IOPOLICY_QD] = "queue-depth",
>> + [NVME_IOPOLICY_NUMA] = "numa",
>> + [NVME_IOPOLICY_RR] = "round-robin",
>> + [NVME_IOPOLICY_QD] = "queue-depth",
>> + [NVME_IOPOLICY_LATENCY] = "latency",
>> };
>> static int iopolicy = NVME_IOPOLICY_NUMA;
>> @@ -107,7 +111,7 @@ static int nvme_get_iopolicy(char *buf, const struct kernel_param *kp)
>> module_param_call(iopolicy, nvme_set_iopolicy, nvme_get_iopolicy,
>> &iopolicy, 0644);
>> MODULE_PARM_DESC(iopolicy,
>> - "Default multipath I/O policy; 'numa' (default), 'round-robin' or 'queue-depth'");
>> + "Default multipath I/O policy; 'numa' (default), 'round-robin' or 'queue-depth' or 'latency'");
>> void nvme_mpath_default_iopolicy(struct nvme_subsystem *subsys)
>> {
>> @@ -199,6 +203,203 @@ void nvme_mpath_start_request(struct request *rq)
>> }
>> EXPORT_SYMBOL_GPL(nvme_mpath_start_request);
>> +static void nvme_mpath_weight_work(struct work_struct *weight_work)
>> +{
>> + int cpu, srcu_idx;
>> + u32 weight;
>> + struct nvme_ns *ns;
>> + struct nvme_path_lat_stat *stat;
>> + struct nvme_path_lat_work *work = container_of(weight_work,
>> + struct nvme_path_lat_work, weight_work);
>> + struct nvme_ns_head *head = work->ns->head;
>> + int op_type = work->op_type;
>> + u64 total_score = 0;
>> +
>> + cpu = get_cpu();
>> +
>> + srcu_idx = srcu_read_lock(&head->srcu);
>> + list_for_each_entry_srcu(ns, &head->list, siblings,
>> + srcu_read_lock_held(&head->srcu)) {
>> +
>> + stat = &this_cpu_ptr(ns->path_lat)[op_type].stat;
>
> get_cpu() does a raw_smp_processor_id() call to get the cpu. So why not use the this_cpu_ptr() equivalent which is passed the cpu, which would save looking up the cpu again? I think that is per_cpu_ptr(), which is actually used elsewhere
Yes, that makes sense. Since get_cpu() already returns the CPU on which this work
is running, I can use per_cpu_ptr() and avoid looking it up again. I'll update
this in the next version.
>
>> + if (!READ_ONCE(stat->slat_ns)) {
>> + stat->score = 0;
>> + continue;
>> + }
>> + /*
>> + * Compute the path score as the inverse of smoothed
>> + * latency, scaled by NSEC_PER_SEC. Floating point
>> + * math is unavailable in the kernel, so fixed-point
>> + * scaling is used instead. NSEC_PER_SEC is chosen
>> + * because valid latencies are always < 1 second; longer
>> + * latencies are ignored.
>> + */
>> + stat->score = div_u64(NSEC_PER_SEC, READ_ONCE(stat->slat_ns));
>> +
>> + /* Compute total score. */
>> + total_score += stat->score;
>> + }
>> +
>> + if (!total_score)
>> + goto out;
>> +
>> + /*
>> + * After computing the total slatency, we derive per-path weight
>> + * (normalized to the range 0–64). The weight represents the
>> + * relative share of I/O the path should receive.
>> + *
>> + * - lower smoothed latency -> higher weight
>> + * - higher smoothed slatency -> lower weight
>> + *
>> + * Next, while forwarding I/O, we assign "credits" to each path
>> + * based on its weight (please also refer nvme_latency_path()):
>> + * - Initially, credits = weight.
>> + * - Each time an I/O is dispatched on a path, its credits are
>> + * decremented proportionally.
>> + * - When a path runs out of credits, it becomes temporarily
>> + * ineligible until credit is refilled.
>> + *
>> + * I/O distribution is therefore governed by available credits,
>> + * ensuring that over time the proportion of I/O sent to each
>> + * path matches its weight (and thus its performance).
>> + */
>> + list_for_each_entry_srcu(ns, &head->list, siblings,
>> + srcu_read_lock_held(&head->srcu)) {
>> +
>> + stat = &this_cpu_ptr(ns->path_lat)[op_type].stat;
>> + weight = div_u64(stat->score * 64, total_score);
>> +
>> + /*
>> + * Ensure the path weight never drops below 1. A weight
>> + * of 0 is used only for newly added paths. During
>> + * bootstrap, a few I/Os are sent to such paths to
>> + * establish an initial weight. Enforcing a minimum
>> + * weight of 1 guarantees that no path is forgotten and
>> + * that each path is probed at least occasionally.
>> + */
>> + if (!weight)
>> + weight = 1;
>> +
>> + WRITE_ONCE(stat->weight, weight);
>> + }
>> +out:
>> + srcu_read_unlock(&head->srcu, srcu_idx);
>> + put_cpu();
>> +}
>> +
>> +/*
>> + * Formula to calculate the EWMA (Exponentially Weighted Moving Average):
>> + * ewma = (old_ewma * (EWMA_SHIFT - 1) + (EWMA_SHIFT)) / EWMA_SHIFT
>> + * For instance, with EWMA_SHIFT = 3, this assigns 7/8 (~87.5 %) weight to
>> + * the existing/old ewma and 1/8 (~12.5%) weight to the new sample.
>> + */
>> +static inline u64 calc_ewma_update(u64 old, u64 new)
>> +{
>> + return (old * ((1 << NVME_DEFAULT_LATENCY_EWMA_SHIFT) - 1)
>> + + new) >> NVME_DEFAULT_LATENCY_EWMA_SHIFT;
>> +}
>> +
>> +static void nvme_mpath_add_sample(struct request *rq, struct nvme_ns *ns)
>> +{
>> + int cpu;
>> + unsigned int op_type;
>> + struct nvme_path_lat *path_lat;
>> + struct nvme_path_lat_stat *stat;
>> + u64 now, latency, slat_ns, avg_lat_ns;
>> + struct nvme_ns_head *head = ns->head;
>> +
>> + if (list_is_singular(&head->list))
>> + return;
>> +
>> + now = ktime_get_ns();
>> + latency = now >= rq->io_start_time_ns ? now - rq->io_start_time_ns : 0;
>> + if (!latency)
>> + return;
>> +
>> + /*
>> + * As completion code path is serialized(i.e. no same completion queue
>> + * update code could run simultaneously on multiple cpu) we can safely
>> + * access per cpu nvme path stat here from another cpu (in case the
>> + * completion cpu is different from submission cpu).
>> + * The only field which could be accessed simultaneously here is the
>> + * path ->weight which may be accessed by this function as well as I/O
>> + * submission path during path selection logic and we protect ->weight
>> + * using READ_ONCE/WRITE_ONCE. Yes this may not be 100% accurate but
>> + * we also don't need to be so accurate here as the path credit would
>> + * be anyways refilled, based on path weight, once path consumes all
>> + * its credits. And we limit path weight/credit max up to 64. Please
>> + * also refer nvme_latency_path().
>> + */
>> + cpu = blk_mq_rq_cpu(rq);
>> + op_type = nvme_data_dir(rq);
>> + path_lat = &per_cpu_ptr(ns->path_lat, cpu)[op_type];
>> + stat = &path_lat->stat;
>> +
>> + /*
>> + * If latency > ~1s then ignore this sample to prevent EWMA from being
>> + * skewed by pathological outliers (multi-second waits, controller
>> + * timeouts etc.). This keeps path scores representative of normal
>> + * performance and avoids instability from rare spikes. If such high
>> + * latency is real, ANA state reporting or keep-alive error counters
>> + * will mark the path unhealthy and remove it from the head node list,
>> + * so we safely skip such sample here.
>> + */
>> + if (unlikely(latency > NSEC_PER_SEC)) {
>> + stat->nr_ignored++;
>> + dev_warn_ratelimited(ns->ctrl->device,
>> + "ignoring sample with >1s latency (possible controller stall or timeout)\n");
>> + return;
>> + }
>> +
>> + /*
>> + * Accumulate latency samples and increment the batch count for each
>> + * ~15 second interval. When the interval expires, compute the simple
>> + * average latency over that window, then update the smoothed (EWMA)
>> + * latency. The path weight is recalculated based on this smoothed
>> + * latency.
>> + */
>> + stat->batch += latency;
>> + stat->batch_count++;
>> + stat->nr_samples++;
>> +
>> + if (now > stat->last_batch_ts && ((now - stat->last_batch_ts) >=
>> + NVME_DEFAULT_LATENCY_BATCH_TIMEOUT)) {
>> +
>> + /*
>> + * Find simple average latency for the last epoch (~15 sec
>> + * interval).
>> + */
>> + avg_lat_ns = div_u64(stat->batch, stat->batch_count);
>> + stat->last_batch_ts = now;
>> +
>> + /*
>> + * Calculate smooth/EWMA (Exponentially Weighted Moving Average)
>> + * latency. EWMA is preferred over simple average latency
>> + * because it smooths naturally, reduces jitter from sudden
>> + * spikes, and adapts faster to changing conditions. It also
>> + * avoids storing historical samples, and works well for both
>> + * slow and fast I/O rates.
>> + * Formula:
>> + * slat_ns = (prev_slat_ns * (WEIGHT - 1) + (latency)) / WEIGHT
>> + * With WEIGHT = 8, this assigns 7/8 (~87.5 %) weight to the
>> + * existing latency and 1/8 (~12.5%) weight to the new latency.
>> + */
>> + if (unlikely(!stat->slat_ns))
>> + WRITE_ONCE(stat->slat_ns, avg_lat_ns);
>> + else {
>> + slat_ns = calc_ewma_update(stat->slat_ns, avg_lat_ns);
>> + WRITE_ONCE(stat->slat_ns, slat_ns);
>> + }
>> +
>> + stat->batch = stat->batch_count = 0;
>> +
>> + /*
>> + * Defer calculation of the path weight in per-cpu workqueue.
>> + */
>> + schedule_work_on(cpu, &path_lat->work.weight_work);
>> + }
>> +}
>> +
>> void nvme_mpath_end_request(struct request *rq)
>> {
>> struct nvme_ns *ns = rq->q->queuedata;
>> @@ -206,6 +407,15 @@ void nvme_mpath_end_request(struct request *rq)
>> if (nvme_req(rq)->flags & NVME_MPATH_CNT_ACTIVE)
>> atomic_dec_if_positive(&ns->ctrl->nr_active);
>> + if (test_bit(NVME_NS_PATH_STAT, &ns->flags)) {
>> + int srcu_idx;
>> +
>> + srcu_idx = srcu_read_lock(&ns->head->srcu);
>> + if (test_bit(NVME_NS_PATH_STAT, &ns->flags))
>
> Some may ask why check NVME_NS_PATH_STAT twice.
The first check is a fast-path optimization to avoid taking the SRCU read
lock when latency sampling is disabled. The second check is needed because
NVME_NS_PATH_STAT could be cleared after the first test but before acquiring
the SRCU lock, so we revalidate it after entering the protected section.
>
>> + nvme_mpath_add_sample(rq, ns);
>> + srcu_read_unlock(&ns->head->srcu, srcu_idx);
>> + }
>> +
>> if (!(nvme_req(rq)->flags & NVME_MPATH_IO_STATS))
>> return;
>> bdev_end_io_acct(ns->head->disk->part0, req_op(rq),
>> @@ -239,6 +449,78 @@ static const char *nvme_ana_state_names[] = {
>> [NVME_ANA_CHANGE] = "change",
>> };
>> +static void nvme_reset_ns_latency_stat(struct nvme_ns *ns)
>> +{
>> + int i, cpu;
>> + struct nvme_path_lat_stat *stat;
>
> reverse fir tree style ordering is nicer, in my opinion
>
Okay I will redorder declarations.
>> +
>> + for_each_possible_cpu(cpu) {
>> + for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
>> + stat = &per_cpu_ptr(ns->path_lat, cpu)[i].stat;
>> + memset(stat, 0, sizeof(struct nvme_path_lat_stat));
>> + }
>> + }
>> +}
>> +
>> +void nvme_cancel_ns_latency_weight_work(struct nvme_ns *ns)
>
> why not static? It seems to be only used in multipath.c
Yes makes sense, will address it.
>
>> +{
>> + int i, cpu;
>> + struct nvme_path_lat *path_lat;
>> +
>> + for_each_possible_cpu(cpu) {
>> + for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
>> + path_lat = &per_cpu_ptr(ns->path_lat, cpu)[i];
>> + cancel_work_sync(&path_lat->work.weight_work);
>> + }
>> + }
>> +}
>> +
>> +static bool nvme_enable_ns_latency_sampling(struct nvme_ns *ns)
>
> return value never checked
Yeah, will update the function signature and mark return type as void.
>
>> +{
>> + struct nvme_ns_head *head = ns->head;
>> +
>> + if (!head->disk ||
>> + READ_ONCE(head->subsys->iopolicy) != NVME_IOPOLICY_LATENCY)
>> + return false;
>> +
>> + if (test_and_set_bit(NVME_NS_PATH_STAT, &ns->flags))
>> + return false;
>> +
>> + blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, ns->queue);
>
> Please explain why it is required. nvme_mpath_add_sample() looks to mention this, but I think a brief explanation here would be good.
Okay will add a brief explanation here.
>
>> + blk_stat_enable_accounting(ns->queue);
>> + return true;
>> +}
>> +
>> +static bool nvme_disable_ns_latency_sampling(struct nvme_ns *ns)
>> +{
>> + int cpu;
>> + struct nvme_ns_head *head = ns->head;
>> + bool changed = false;
>> +
>> + if (!test_and_clear_bit(NVME_NS_PATH_STAT, &ns->flags))
>> + return false;
>> +
>> + for_each_possible_cpu(cpu) {
>> + if (ns == READ_ONCE(*per_cpu_ptr(head->latency_path, cpu))) {
>> + WRITE_ONCE(*per_cpu_ptr(head->latency_path, cpu), NULL);
>> + changed = true;
>> + }
>> + }
>> +
>> + blk_stat_disable_accounting(ns->queue);
>> + blk_queue_flag_clear(QUEUE_FLAG_SAME_FORCE, ns->queue);
>
> eh, what if QUEUE_FLAG_SAME_FORCE was already enabled before nvme_enable_ns_latency_sampling()?
>
Good catch! It looks like we need a nested reference count for
QUEUE_FLAG_SAME_FORCE, similar to QUEUE_FLAG_STATS and
QUEUE_FLAG_QUIESCED.
>> +
>> + /*
>> + * Ensure that we wait until completion side samplings (if any sneaked
>> + * in after we clear NVME_NS_PATH_STAT) are all scheduled before we
>> + * start cancelling those.
>> + */
>> + synchronize_srcu(&head->srcu);
>> + nvme_cancel_ns_latency_weight_work(ns);
>> + nvme_reset_ns_latency_stat(ns);
>> + return changed;
>> +}
>> +
>> bool nvme_mpath_clear_current_path(struct nvme_ns *ns)
>> {
>> struct nvme_ns_head *head = ns->head;
>> @@ -251,6 +533,10 @@ bool nvme_mpath_clear_current_path(struct nvme_ns *ns)
>> changed = true;
>> }
>> }
>> +
>> + if (nvme_disable_ns_latency_sampling(ns))
>> + changed = true;
>> +
>> return changed;
>> }
>> @@ -268,6 +554,45 @@ void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl)
>> srcu_read_unlock(&ctrl->srcu, srcu_idx);
>> }
>> +int nvme_alloc_ns_stat(struct nvme_ns *ns)
>
> Surely "mpath" should be in the name, no? It seems that every other public API in multpath.c has "mpath" in the name.
Not all APIs have "mpath" in its name, such as nvme_failover_req(),
nvme_kick_requeue_lists() etc, but most other have. So I would
rename it to nvme_mpath_alloc_ns_stat().
>
>> +{
>> + int i, cpu;
>> + struct nvme_path_lat_work *work;
>> + gfp_t gfp = GFP_KERNEL | __GFP_ZERO;
>> +
>> + if (!ns->head->disk)
>> + return 0;
>> +
>> + ns->path_lat = __alloc_percpu_gfp(NVME_NUM_STAT_GROUPS *
>> + sizeof(struct nvme_path_lat),
>> + __alignof__(struct nvme_path_lat), gfp);
>> + if (!ns->path_lat)
>> + return -ENOMEM;
>> +
>> + for_each_possible_cpu(cpu) {
>> + for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
>> + work = &per_cpu_ptr(ns->path_lat, cpu)[i].work;
>> + work->ns = ns;
>> + work->op_type = i;
>> + INIT_WORK(&work->weight_work, nvme_mpath_weight_work);
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void nvme_mpath_set_ctrl_paths(struct nvme_ctrl *ctrl)
>
> what do you mean by "set" here?
It is intended as the counterpart of nvme_mpath_clear_ctrl_paths().
The former clears/disables the I/O policy state for the controller
namespaces, while this helper sets/enables it.
>
>> +{
>> + struct nvme_ns *ns;
>> + int srcu_idx;
>> +
>> + srcu_idx = srcu_read_lock(&ctrl->srcu);
>> + list_for_each_entry_srcu(ns, &ctrl->namespaces, list,
>> + srcu_read_lock_held(&ctrl->srcu))
>> + nvme_enable_ns_latency_sampling(ns);
>> + srcu_read_unlock(&ctrl->srcu, srcu_idx);
>> +}
>> +
>> void nvme_mpath_revalidate_paths(struct nvme_ns_head *head)
>> {
>> sector_t capacity = get_capacity(head->disk);
>> @@ -280,6 +605,8 @@ void nvme_mpath_revalidate_paths(struct nvme_ns_head *head)
>> srcu_read_lock_held(&head->srcu)) {
>> if (capacity != get_capacity(ns->disk))
>> clear_bit(NVME_NS_READY, &ns->flags);
>> +
>> + nvme_reset_ns_latency_stat(ns);
>> }
>> srcu_read_unlock(&head->srcu, srcu_idx);
>> @@ -404,6 +731,92 @@ static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head)
>> return found;
>> }
>> +static inline bool nvme_state_is_live(enum nvme_ana_state state)
>> +{
>> + return state == NVME_ANA_OPTIMIZED || state == NVME_ANA_NONOPTIMIZED;
>> +}
>> +
>> +static struct nvme_ns *nvme_latency_path(struct nvme_ns_head *head,
>> + unsigned int op_type)
>> +{
>> + struct nvme_ns *ns, *start, *found = NULL;
>> + struct nvme_path_lat_stat *stat;
>> + u32 weight;
>> + int cpu;
>> +
>> + cpu = get_cpu();
>> + ns = READ_ONCE(*this_cpu_ptr(head->latency_path));
>> + if (unlikely(!ns)) {
>> + ns = list_first_or_null_rcu(&head->list,
>> + struct nvme_ns, siblings);
>> + if (unlikely(!ns))
>> + goto out;
>
> out: checks found, which is always NULL. You could add another label after the found check at out: and goto that label to avoid the unnecessary check.
>
Okay will address this.
>> + }
>> +found_ns:
>> + start = ns;
>> + while (nvme_path_is_disabled(ns) ||
>> + !nvme_state_is_live(ns->ana_state)) {
>> + ns = list_next_entry_circular(ns, &head->list, siblings);
>> +
>> + /*
>> + * If we iterate through all paths in the list but find each
>> + * path in list is either disabled or dead then bail out.
>> + */
>> + if (ns == start)
>> + goto out;
>> + }
>> +
>> + stat = &this_cpu_ptr(ns->path_lat)[op_type].stat;
>> +
>> + /*
>> + * When the head path-list is singular we don't calculate the
>> + * only path weight for optimization as we don't need to forward
>> + * I/O to more than one path. The another possibility is when the
>> + * path is newly added, we don't know its weight. So we go round
>> + * -robin for each such path and forward I/O to it.Once we start
>> + * getting response for such I/Os, the path weight calculation
>> + * would kick in and then we start using path credit for
>> + * forwarding I/O.
>> + */
>> + weight = READ_ONCE(stat->weight);
>> + if (!weight) {
>> + found = ns;
>> + goto out;
>> + }
>> +
>> + /*
>> + * To keep path selection logic simple, we don't distinguish
>> + * between ANA optimized and non-optimized states. The non-
>> + * optimized path is expected to have a lower weight, and
>> + * therefore fewer credits. As a result, only a small number of
>> + * I/Os will be forwarded to paths in the non-optimized state.
>> + */
>> + if (stat->credit > 0) {
>> + --stat->credit;
>> + found = ns;
>> + goto out;
>
> this goto is superfluous
>
okay this goto could be removed.
>> + } else {
>> + /*
>> + * Refill credit from path weight and move to next path. The
>> + * refilled credit of the current path will be used next when
>> + * all remainng paths exhaust its credits.
>> + */
>> + weight = READ_ONCE(stat->weight);
>> + stat->credit = weight;
>> + ns = list_next_entry_circular(ns, &head->list, siblings);
>> + if (likely(ns))
>> + goto found_ns;
>> + }
>> +out:
>> + if (found) {
>> + stat->sel++;
>> + WRITE_ONCE(*this_cpu_ptr(head->latency_path), found);
>> + }
>> +
>> + put_cpu();
>> + return found;
>> +}
>> +
>> static struct nvme_ns *nvme_queue_depth_path(struct nvme_ns_head *head)
>> {
>> struct nvme_ns *best_opt = NULL, *best_nonopt = NULL, *ns;
>> @@ -464,6 +877,8 @@ inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head,
>> unsigned int op_type)
>> {
>> switch (READ_ONCE(head->subsys->iopolicy)) {
>> + case NVME_IOPOLICY_LATENCY:
>> + return nvme_latency_path(head, op_type);
>> case NVME_IOPOLICY_QD:
>> return nvme_queue_depth_path(head);
>> case NVME_IOPOLICY_RR:
>> @@ -754,6 +1169,10 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
>> if (!nvme_is_unique_nsid(ctrl, head))
>> return 0;
>> + head->latency_path = alloc_percpu_gfp(struct nvme_ns*, GFP_KERNEL);
>> + if (!head->latency_path)
>> + return -ENOMEM;
>> +
>> blk_set_stacking_limits(&lim);
>> lim.dma_alignment = 3;
>> lim.features |= BLK_FEAT_IO_STAT | BLK_FEAT_NOWAIT |
>> @@ -762,8 +1181,10 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
>> lim.features |= BLK_FEAT_ZONED;
>> head->disk = blk_alloc_disk(&lim, ctrl->numa_node);
>> - if (IS_ERR(head->disk))
>> + if (IS_ERR(head->disk)) {
>> + free_percpu(head->latency_path);
>> return PTR_ERR(head->disk);
>> + }
>> head->disk->fops = &nvme_ns_head_ops;
>> head->disk->private_data = head;
>> @@ -819,6 +1240,10 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
>> }
>> mutex_unlock(&head->lock);
>> + mutex_lock(&nvme_subsystems_lock);
>
> I am curious - why use the nvme_subsystems_lock?
>
nvme_subsys_iopolicy_update() and nvme_mpath_set_live() can run concurrently.
nvme_subsystems_lock serializes these paths so that latency sampling is
enabled consistently with the subsystem I/O policy.
>> + nvme_enable_ns_latency_sampling(ns);
>> + mutex_unlock(&nvme_subsystems_lock);
>> +
>> synchronize_srcu(&head->srcu);
>> kblockd_schedule_work(&head->requeue_work);
>> }
>> @@ -867,11 +1292,6 @@ static int nvme_parse_ana_log(struct nvme_ctrl *ctrl, void *data,
>> return 0;
>> }
>> -static inline bool nvme_state_is_live(enum nvme_ana_state state)
>> -{
>> - return state == NVME_ANA_OPTIMIZED || state == NVME_ANA_NONOPTIMIZED;
>> -}
>> -
>> static void nvme_update_ns_ana_state(struct nvme_ana_group_desc *desc,
>> struct nvme_ns *ns)
>> {
>> @@ -1049,10 +1469,12 @@ static void nvme_subsys_iopolicy_update(struct nvme_subsystem *subsys,
>> WRITE_ONCE(subsys->iopolicy, iopolicy);
>> - /* iopolicy changes clear the mpath by design */
>> + /* iopolicy changes clear/reset the mpath by design */
>> mutex_lock(&nvme_subsystems_lock);
>> list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry)
>> nvme_mpath_clear_ctrl_paths(ctrl);
>> + list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry)
>> + nvme_mpath_set_ctrl_paths(ctrl);
>
> Do we need to have separate loops to call nvme_mpath_clear_ctrl_paths() and nvme_mpath_set_ctrl_paths()?
>
I think so. Since a shared namespace (and its nvme_ns_head) may be
referenced by multiple controllers, combining the clear and set
operations into a single loop could cause them to interfere with
each other. Keeping them as separate passes ensures all state is
cleared before any state is re-enabled.
>> mutex_unlock(&nvme_subsystems_lock);
>> pr_notice("subsysnqn %s iopolicy changed from %s to %s\n",
>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>> index 8a9ec502912d..3c82f67f8926 100644
>> --- a/drivers/nvme/host/nvme.h
>> +++ b/drivers/nvme/host/nvme.h
>> @@ -28,7 +28,9 @@ extern unsigned int nvme_io_timeout;
>> extern unsigned int admin_timeout;
>> #define NVME_ADMIN_TIMEOUT (admin_timeout * HZ)
>> -#define NVME_DEFAULT_KATO 5
>> +#define NVME_DEFAULT_KATO 5
>> +#define NVME_DEFAULT_LATENCY_EWMA_SHIFT 3
>> +#define NVME_DEFAULT_LATENCY_BATCH_TIMEOUT (15 * NSEC_PER_SEC)
>> #ifdef CONFIG_ARCH_NO_SG_CHAIN
>> #define NVME_INLINE_SG_CNT 0
>> @@ -483,6 +485,7 @@ enum nvme_iopolicy {
>> NVME_IOPOLICY_NUMA,
>> NVME_IOPOLICY_RR,
>> NVME_IOPOLICY_QD,
>> + NVME_IOPOLICY_LATENCY,
>> };
>> struct nvme_subsystem {
>> @@ -527,6 +530,30 @@ enum nvme_stat_group {
>> NVME_NUM_STAT_GROUPS
>> };
>> +struct nvme_path_lat_stat {
>> + u64 nr_samples; /* total num of samples processed */
>
> why u64 and not unsigned long long?
>
I used u64 intentionally because this is a monotonically increasing
sample counter, and I wanted a fixed-width 64-bit type. I didn't see
any particular advantage in using unsigned long long here. If there's
a reason to prefer it in this context, I'm happy to change it.
>> + u64 nr_ignored; /* num. of samples ignored */
>> + u64 slat_ns; /* smoothed (ewma) latency in nanoseconds */
>> + u64 score; /* score used for weight calculation */
>> + u64 last_batch_ts; /* timestamp when last time avg. latency is calculated */
>> + u64 sel; /* num of times this path is selcted for I/O */
>> + u64 batch; /* accumulated latency sum for current window */
>> + u32 batch_count; /* num of samples accumulated in current window */
>> + u32 weight; /* path weight */
>> + u32 credit; /* path credit for I/O forwarding */
>> +};
>> +
>> +struct nvme_path_lat_work {
>> + struct nvme_ns *ns; /* owning namespace */
>> + struct work_struct weight_work; /* deferred work for weight calculation */
>> + int op_type; /* op type : READ/WRITE/OTHER */
>> +};
>> +
>> +struct nvme_path_lat {
>> + struct nvme_path_lat_stat stat; /* path statistics */
>> + struct nvme_path_lat_work work; /* background worker context */
>> +};
>> +
>> /*
>> * Anchor structure for namespaces. There is one for each namespace in a
>> * NVMe subsystem that any of our controllers can see, and the namespace
>> @@ -578,6 +605,8 @@ struct nvme_ns_head {
>> unsigned int delayed_removal_secs;
>> atomic_long_t io_requeue_no_usable_path_count;
>> atomic_long_t io_fail_no_available_path_count;
>> + struct nvme_ns * __percpu *latency_path;
>> +
>> #define NVME_NSHEAD_DISK_LIVE 0
>> #define NVME_NSHEAD_QUEUE_IF_NO_PATH 1
>> #define NVME_NSHEAD_CDEV_LIVE 2
>> @@ -606,6 +635,7 @@ struct nvme_ns {
>> enum nvme_ana_state ana_state;
>> u32 ana_grpid;
>> atomic_long_t failover;
>> + struct nvme_path_lat __percpu *path_lat;
>> #endif
>> atomic_long_t retries;
>> atomic_long_t errors;
>> @@ -620,6 +650,7 @@ struct nvme_ns {
>> #define NVME_NS_READY 4
>> #define NVME_NS_SYSFS_ATTR_LINK 5
>> #define NVME_NS_CDEV_LIVE 6
>> +#define NVME_NS_PATH_STAT 7
>> struct cdev cdev;
>> struct device cdev_device;
>> @@ -1100,6 +1131,8 @@ void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl);
>> void nvme_mpath_remove_disk(struct nvme_ns_head *head);
>> void nvme_mpath_start_request(struct request *rq);
>> void nvme_mpath_end_request(struct request *rq);
>> +int nvme_alloc_ns_stat(struct nvme_ns *ns);
>> +void nvme_cancel_ns_latency_weight_work(struct nvme_ns *ns);
>> static inline void nvme_trace_bio_complete(struct request *req)
>> {
>> @@ -1130,6 +1163,13 @@ static inline bool nvme_mpath_queue_if_no_path(struct nvme_ns_head *head)
>> return true;
>> return false;
>> }
>> +static inline void nvme_free_ns_stat(struct nvme_ns *ns)
>> +{
>> + if (!ns->head->disk)
>> + return;
>
> this check is not strictly required, as if ns->head->disk == NULL, the ns->path_lat == NULL and free_percpu() can handle NULL.
>
Yes this check can be avoided, will remove it.
>> +
>> + free_percpu(ns->path_lat);
>> +}
>> #else
>> #define multipath false
>> static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
>> @@ -1221,6 +1261,16 @@ static inline bool nvme_mpath_queue_if_no_path(struct nvme_ns_head *head)
>> {
>> return false;
>> }
>> +static inline void nvme_cancel_ns_latency_weight_work(struct nvme_ns *ns)
>> +{
>> +}
>> +static inline int nvme_alloc_ns_stat(struct nvme_ns *ns)
>> +{
>> + return 0;
>> +}
>> +static inline void nvme_free_ns_stat(struct nvme_ns *ns)
>> +{
>> +}
>> #endif /* CONFIG_NVME_MULTIPATH */
>> int nvme_ns_get_unique_id(struct nvme_ns *ns, u8 id[16],
>> --
>> 2.53.0
>>
>
Thanks for your detailed review and feedback!
--Nilay
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy
2026-08-10 15:25 ` Nilay Shroff
@ 2026-08-11 10:03 ` John Garry
2026-08-12 7:55 ` Nilay Shroff
0 siblings, 1 reply; 30+ messages in thread
From: John Garry @ 2026-08-11 10:03 UTC (permalink / raw)
To: Nilay Shroff, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
>>> +}
>>> +
>>> +/*
>>> + * Formula to calculate the EWMA (Exponentially Weighted Moving Average):
>>> + * ewma = (old_ewma * (EWMA_SHIFT - 1) + (EWMA_SHIFT)) / EWMA_SHIFT
>>> + * For instance, with EWMA_SHIFT = 3, this assigns 7/8 (~87.5 %) weight to
>>> + * the existing/old ewma and 1/8 (~12.5%) weight to the new sample.
>>> + */
>>> +static inline u64 calc_ewma_update(u64 old, u64 new)
>>> +{
>>> + return (old * ((1 << NVME_DEFAULT_LATENCY_EWMA_SHIFT) - 1)
>>> + + new) >> NVME_DEFAULT_LATENCY_EWMA_SHIFT;
side note: I have to admit that I did not check all the mathematics of
these ewma calculations ...
>>> +}
>>> +
>>> +static void nvme_mpath_add_sample(struct request *rq, struct nvme_ns *ns)
Could the context analysis annotation be added here eventually to
declare that the srcu read lock is held?
>>> +{
>>> + int cpu;
>>> + unsigned int op_type;
>>> + struct nvme_path_lat *path_lat;
>>> + struct nvme_path_lat_stat *stat;
>>> + u64 now, latency, slat_ns, avg_lat_ns;
>>> + struct nvme_ns_head *head = ns->head;
>>> +
>>> + if (list_is_singular(&head->list))
>>> + return;
>>> +
>>> + now = ktime_get_ns();
>>> + latency = now >= rq->io_start_time_ns ? now - rq->io_start_time_ns : 0;
>>> + if (!latency)
>>> + return;
>>> +
>>> + /*
>>> + * As completion code path is serialized(i.e. no same completion queue
>>> + * update code could run simultaneously on multiple cpu) we can safely
>>> + * access per cpu nvme path stat here from another cpu (in case the
>>> + * completion cpu is different from submission cpu).
>>> + * The only field which could be accessed simultaneously here is the
>>> + * path ->weight which may be accessed by this function as well as I/O
>>> + * submission path during path selection logic and we protect ->weight
>>> + * using READ_ONCE/WRITE_ONCE. Yes this may not be 100% accurate but
>>> + * we also don't need to be so accurate here as the path credit would
>>> + * be anyways refilled, based on path weight, once path consumes all
>>> + * its credits. And we limit path weight/credit max up to 64. Please
>>> + * also refer nvme_latency_path().
>>> + */
...
>>> void nvme_mpath_end_request(struct request *rq)
>>> {
>>> struct nvme_ns *ns = rq->q->queuedata;
>>> @@ -206,6 +407,15 @@ void nvme_mpath_end_request(struct request *rq)
>>> if (nvme_req(rq)->flags & NVME_MPATH_CNT_ACTIVE)
>>> atomic_dec_if_positive(&ns->ctrl->nr_active);
>>> + if (test_bit(NVME_NS_PATH_STAT, &ns->flags)) {
>>> + int srcu_idx;
>>> +
>>> + srcu_idx = srcu_read_lock(&ns->head->srcu);
>>> + if (test_bit(NVME_NS_PATH_STAT, &ns->flags))
>>
>> Some may ask why check NVME_NS_PATH_STAT twice.
>
> The first check is a fast-path optimization to avoid taking the SRCU read
> lock when latency sampling is disabled. The second check is needed because
> NVME_NS_PATH_STAT could be cleared after the first test but before acquiring
> the SRCU lock, so we revalidate it after entering the protected section.
It is probably worth a brief comment on that. A similar trick is done in
__blk_mq_tag_busy() and every so often someone asks about it. Or maybe
it is another function. I don't remember.
>>
>>> + blk_stat_enable_accounting(ns->queue);
>>> + return true;
>>> +}
>>> +
>>> +static bool nvme_disable_ns_latency_sampling(struct nvme_ns *ns)
>>> +{
>>> + int cpu;
>>> + struct nvme_ns_head *head = ns->head;
>>> + bool changed = false;
>>> +
>>> + if (!test_and_clear_bit(NVME_NS_PATH_STAT, &ns->flags))
>>> + return false;
>>> +
>>> + for_each_possible_cpu(cpu) {
>>> + if (ns == READ_ONCE(*per_cpu_ptr(head->latency_path, cpu))) {
>>> + WRITE_ONCE(*per_cpu_ptr(head->latency_path, cpu), NULL);
>>> + changed = true;
>>> + }
>>> + }
>>> +
>>> + blk_stat_disable_accounting(ns->queue);
>>> + blk_queue_flag_clear(QUEUE_FLAG_SAME_FORCE, ns->queue);
>>
>> eh, what if QUEUE_FLAG_SAME_FORCE was already enabled before nvme_enable_ns_latency_sampling()?
>>
> Good catch! It looks like we need a nested reference count for
> QUEUE_FLAG_SAME_FORCE, similar to QUEUE_FLAG_STATS and
> QUEUE_FLAG_QUIESCED.
Furthermore, I think that userspace can change this via sysfs, no? I
think that the file is rq_affinity. If so, could that break things (if
userspace did change this flag)?
>>> }
>>> @@ -268,6 +554,45 @@ void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl)
>>> srcu_read_unlock(&ctrl->srcu, srcu_idx);
>>> }
>>> +int nvme_alloc_ns_stat(struct nvme_ns *ns)
>>
>> Surely "mpath" should be in the name, no? It seems that every other public API in multpath.c has "mpath" in the name.
>
> Not all APIs have "mpath" in its name, such as nvme_failover_req(),
> nvme_kick_requeue_lists() etc, but most other have. So I would
> rename it to nvme_mpath_alloc_ns_stat().
nvme_failover_req() would obviously be a multipath function from the
name. Anyway, "mpath" in the name just seem better.
>>
>>> +{
>>> + int i, cpu;
>>> + struct nvme_path_lat_work *work;
>>> + gfp_t gfp = GFP_KERNEL | __GFP_ZERO;
>>> +
>>> + if (!ns->head->disk)
>>> + return 0;
>>> +
>>> + ns->path_lat = __alloc_percpu_gfp(NVME_NUM_STAT_GROUPS *
>>> + sizeof(struct nvme_path_lat),
>>> + __alignof__(struct nvme_path_lat), gfp);
>>> + if (!ns->path_lat)
>>> + return -ENOMEM;
>>> +
>>> + for_each_possible_cpu(cpu) {
>>> + for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
>>> + work = &per_cpu_ptr(ns->path_lat, cpu)[i].work;
>>> + work->ns = ns;
>>> + work->op_type = i;
>>> + INIT_WORK(&work->weight_work, nvme_mpath_weight_work);
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void nvme_mpath_set_ctrl_paths(struct nvme_ctrl *ctrl)
>>
>> what do you mean by "set" here?
>
> It is intended as the counterpart of nvme_mpath_clear_ctrl_paths().
> The former clears/disables the I/O policy state for the controller
> namespaces, while this helper sets/enables it.
To me, clear paths meaning is obvious, in that any per-NUMA node paths
are cleared for all the paths associated with the controller.
nvme_mpath_set_ctrl_paths() does not really do the opposite - it instead
just enables the IO latency sampling per path.
Anyway, I don't feel too strongly about this, but it just seems that the
naming could be improved.
>>
>>> +{
>>> + struct nvme_ns *ns;
>>> + int srcu_idx;
>>> +
>>> + srcu_idx = srcu_read_lock(&ctrl->srcu);
>>> + list_for_each_entry_srcu(ns, &ctrl->namespaces, list,
>>> + srcu_read_lock_held(&ctrl->srcu))
>>> + nvme_enable_ns_latency_sampling(ns);
>>> + srcu_read_unlock(&ctrl->srcu, srcu_idx);
>>> +}
>>> +
>>> void nvme_mpath_revalidate_paths(struct nvme_ns_head *head)
>>> {
>>> sector_t capacity = get_capacity(head->disk);
>>> @@ -280,6 +605,8 @@ void nvme_mpath_revalidate_paths(struct nvme_ns_head *head)
>>> srcu_read_lock_held(&head->srcu)) {
>>> if (capacity != get_capacity(ns->disk))
>>> clear_bit(NVME_NS_READY, &ns->flags);
>>> +
>>> + nvme_reset_ns_latency_stat(ns);
>>> }
>>> srcu_read_unlock(&head->srcu, srcu_idx);
>>> @@ -404,6 +731,92 @@ static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head)
>>> return found;
>>> }
>>> +static inline bool nvme_state_is_live(enum nvme_ana_state state)
...
>>> }
>>> mutex_unlock(&head->lock);
>>> + mutex_lock(&nvme_subsystems_lock);
>>
>> I am curious - why use the nvme_subsystems_lock?
>>
> nvme_subsys_iopolicy_update() and nvme_mpath_set_live() can run concurrently.
> nvme_subsystems_lock serializes these paths so that latency sampling is
> enabled consistently with the subsystem I/O policy.
ok, maybe then please consider a comment. It can be useful.
>>> @@ -527,6 +530,30 @@ enum nvme_stat_group {
>>> NVME_NUM_STAT_GROUPS
>>> };
>>> +struct nvme_path_lat_stat {
>>> + u64 nr_samples; /* total num of samples processed */
>>
>> why u64 and not unsigned long long?
>>
> I used u64 intentionally because this is a monotonically increasing
> sample counter, and I wanted a fixed-width 64-bit type. I didn't see
> any particular advantage in using unsigned long long here. If there's
> a reason to prefer it in this context, I'm happy to change it.
hmmm... I thought that in general we only should use a fixed width type
when it is required, e.g. reading from a 32b register, then use u32.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy
2026-08-11 10:03 ` John Garry
@ 2026-08-12 7:55 ` Nilay Shroff
0 siblings, 0 replies; 30+ messages in thread
From: Nilay Shroff @ 2026-08-12 7:55 UTC (permalink / raw)
To: John Garry, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
On 8/11/26 3:33 PM, John Garry wrote:
>
>>>> +}
>>>> +
>>>> +/*
>>>> + * Formula to calculate the EWMA (Exponentially Weighted Moving Average):
>>>> + * ewma = (old_ewma * (EWMA_SHIFT - 1) + (EWMA_SHIFT)) / EWMA_SHIFT
>>>> + * For instance, with EWMA_SHIFT = 3, this assigns 7/8 (~87.5 %) weight to
>>>> + * the existing/old ewma and 1/8 (~12.5%) weight to the new sample.
>>>> + */
>>>> +static inline u64 calc_ewma_update(u64 old, u64 new)
>>>> +{
>>>> + return (old * ((1 << NVME_DEFAULT_LATENCY_EWMA_SHIFT) - 1)
>>>> + + new) >> NVME_DEFAULT_LATENCY_EWMA_SHIFT;
>
> side note: I have to admit that I did not check all the mathematics of these ewma calculations ...
>
>>>> +}
>>>> +
>>>> +static void nvme_mpath_add_sample(struct request *rq, struct nvme_ns *ns)
>
> Could the context analysis annotation be added here eventually to declare that the srcu read lock is held?
>
Yes it will be added when I resend series based off nvme-7.3 as
support of clang context annotation is added in nvme-7.3.
>>>> +{
>>>> + int cpu;
>>>> + unsigned int op_type;
>>>> + struct nvme_path_lat *path_lat;
>>>> + struct nvme_path_lat_stat *stat;
>>>> + u64 now, latency, slat_ns, avg_lat_ns;
>>>> + struct nvme_ns_head *head = ns->head;
>>>> +
>>>> + if (list_is_singular(&head->list))
>>>> + return;
>>>> +
>>>> + now = ktime_get_ns();
>>>> + latency = now >= rq->io_start_time_ns ? now - rq->io_start_time_ns : 0;
>>>> + if (!latency)
>>>> + return;
>>>> +
>>>> + /*
>>>> + * As completion code path is serialized(i.e. no same completion queue
>>>> + * update code could run simultaneously on multiple cpu) we can safely
>>>> + * access per cpu nvme path stat here from another cpu (in case the
>>>> + * completion cpu is different from submission cpu).
>>>> + * The only field which could be accessed simultaneously here is the
>>>> + * path ->weight which may be accessed by this function as well as I/O
>>>> + * submission path during path selection logic and we protect ->weight
>>>> + * using READ_ONCE/WRITE_ONCE. Yes this may not be 100% accurate but
>>>> + * we also don't need to be so accurate here as the path credit would
>>>> + * be anyways refilled, based on path weight, once path consumes all
>>>> + * its credits. And we limit path weight/credit max up to 64. Please
>>>> + * also refer nvme_latency_path().
>>>> + */
>
> ...
>
>>>> void nvme_mpath_end_request(struct request *rq)
>>>> {
>>>> struct nvme_ns *ns = rq->q->queuedata;
>>>> @@ -206,6 +407,15 @@ void nvme_mpath_end_request(struct request *rq)
>>>> if (nvme_req(rq)->flags & NVME_MPATH_CNT_ACTIVE)
>>>> atomic_dec_if_positive(&ns->ctrl->nr_active);
>>>> + if (test_bit(NVME_NS_PATH_STAT, &ns->flags)) {
>>>> + int srcu_idx;
>>>> +
>>>> + srcu_idx = srcu_read_lock(&ns->head->srcu);
>>>> + if (test_bit(NVME_NS_PATH_STAT, &ns->flags))
>>>
>>> Some may ask why check NVME_NS_PATH_STAT twice.
>>
>> The first check is a fast-path optimization to avoid taking the SRCU read
>> lock when latency sampling is disabled. The second check is needed because
>> NVME_NS_PATH_STAT could be cleared after the first test but before acquiring
>> the SRCU lock, so we revalidate it after entering the protected section.
>
> It is probably worth a brief comment on that. A similar trick is done in __blk_mq_tag_busy() and every so often someone asks about it. Or maybe it is another function. I don't remember.
>
Yeah okay will add comment in the code.
>>>
>>>> + blk_stat_enable_accounting(ns->queue);
>>>> + return true;
>>>> +}
>>>> +
>>>> +static bool nvme_disable_ns_latency_sampling(struct nvme_ns *ns)
>>>> +{
>>>> + int cpu;
>>>> + struct nvme_ns_head *head = ns->head;
>>>> + bool changed = false;
>>>> +
>>>> + if (!test_and_clear_bit(NVME_NS_PATH_STAT, &ns->flags))
>>>> + return false;
>>>> +
>>>> + for_each_possible_cpu(cpu) {
>>>> + if (ns == READ_ONCE(*per_cpu_ptr(head->latency_path, cpu))) {
>>>> + WRITE_ONCE(*per_cpu_ptr(head->latency_path, cpu), NULL);
>>>> + changed = true;
>>>> + }
>>>> + }
>>>> +
>>>> + blk_stat_disable_accounting(ns->queue);
>>>> + blk_queue_flag_clear(QUEUE_FLAG_SAME_FORCE, ns->queue);
>>>
>>> eh, what if QUEUE_FLAG_SAME_FORCE was already enabled before nvme_enable_ns_latency_sampling()?
>>>
>> Good catch! It looks like we need a nested reference count for
>> QUEUE_FLAG_SAME_FORCE, similar to QUEUE_FLAG_STATS and
>> QUEUE_FLAG_QUIESCED.
>
> Furthermore, I think that userspace can change this via sysfs, no? I think that the file is rq_affinity. If so, could that break things (if userspace did change this flag)?
>
So that's where I suggested using a nested ref count. I'd
add an helper similar to blk_stat_{enable|dsiable}_accounting() and
that new helper would be then used in both sysfs path as well
latency policy enable/disable path.
>>>> }
>>>> @@ -268,6 +554,45 @@ void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl)
>>>> srcu_read_unlock(&ctrl->srcu, srcu_idx);
>>>> }
>>>> +int nvme_alloc_ns_stat(struct nvme_ns *ns)
>>>
>>> Surely "mpath" should be in the name, no? It seems that every other public API in multpath.c has "mpath" in the name.
>>
>> Not all APIs have "mpath" in its name, such as nvme_failover_req(),
>> nvme_kick_requeue_lists() etc, but most other have. So I would
>> rename it to nvme_mpath_alloc_ns_stat().
>
> nvme_failover_req() would obviously be a multipath function from the name. Anyway, "mpath" in the name just seem better.
>
>>>
>>>> +{
>>>> + int i, cpu;
>>>> + struct nvme_path_lat_work *work;
>>>> + gfp_t gfp = GFP_KERNEL | __GFP_ZERO;
>>>> +
>>>> + if (!ns->head->disk)
>>>> + return 0;
>>>> +
>>>> + ns->path_lat = __alloc_percpu_gfp(NVME_NUM_STAT_GROUPS *
>>>> + sizeof(struct nvme_path_lat),
>>>> + __alignof__(struct nvme_path_lat), gfp);
>>>> + if (!ns->path_lat)
>>>> + return -ENOMEM;
>>>> +
>>>> + for_each_possible_cpu(cpu) {
>>>> + for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
>>>> + work = &per_cpu_ptr(ns->path_lat, cpu)[i].work;
>>>> + work->ns = ns;
>>>> + work->op_type = i;
>>>> + INIT_WORK(&work->weight_work, nvme_mpath_weight_work);
>>>> + }
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void nvme_mpath_set_ctrl_paths(struct nvme_ctrl *ctrl)
>>>
>>> what do you mean by "set" here?
>>
>> It is intended as the counterpart of nvme_mpath_clear_ctrl_paths().
>> The former clears/disables the I/O policy state for the controller
>> namespaces, while this helper sets/enables it.
>
> To me, clear paths meaning is obvious, in that any per-NUMA node paths are cleared for all the paths associated with the controller.
>
> nvme_mpath_set_ctrl_paths() does not really do the opposite - it instead just enables the IO latency sampling per path.
>
> Anyway, I don't feel too strongly about this, but it just seems that the naming could be improved.
>
>>>
>>>> +{
>>>> + struct nvme_ns *ns;
>>>> + int srcu_idx;
>>>> +
>>>> + srcu_idx = srcu_read_lock(&ctrl->srcu);
>>>> + list_for_each_entry_srcu(ns, &ctrl->namespaces, list,
>>>> + srcu_read_lock_held(&ctrl->srcu))
>>>> + nvme_enable_ns_latency_sampling(ns);
>>>> + srcu_read_unlock(&ctrl->srcu, srcu_idx);
>>>> +}
>>>> +
>>>> void nvme_mpath_revalidate_paths(struct nvme_ns_head *head)
>>>> {
>>>> sector_t capacity = get_capacity(head->disk);
>>>> @@ -280,6 +605,8 @@ void nvme_mpath_revalidate_paths(struct nvme_ns_head *head)
>>>> srcu_read_lock_held(&head->srcu)) {
>>>> if (capacity != get_capacity(ns->disk))
>>>> clear_bit(NVME_NS_READY, &ns->flags);
>>>> +
>>>> + nvme_reset_ns_latency_stat(ns);
>>>> }
>>>> srcu_read_unlock(&head->srcu, srcu_idx);
>>>> @@ -404,6 +731,92 @@ static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head)
>>>> return found;
>>>> }
>>>> +static inline bool nvme_state_is_live(enum nvme_ana_state state)
> ...
>
>>>> }
>>>> mutex_unlock(&head->lock);
>>>> + mutex_lock(&nvme_subsystems_lock);
>>>
>>> I am curious - why use the nvme_subsystems_lock?
>>>
>> nvme_subsys_iopolicy_update() and nvme_mpath_set_live() can run concurrently.
>> nvme_subsystems_lock serializes these paths so that latency sampling is
>> enabled consistently with the subsystem I/O policy.
>
> ok, maybe then please consider a comment. It can be useful.
>
sure, will add one.
>
>>>> @@ -527,6 +530,30 @@ enum nvme_stat_group {
>>>> NVME_NUM_STAT_GROUPS
>>>> };
>>>> +struct nvme_path_lat_stat {
>>>> + u64 nr_samples; /* total num of samples processed */
>>>
>>> why u64 and not unsigned long long?
>>>
>> I used u64 intentionally because this is a monotonically increasing
>> sample counter, and I wanted a fixed-width 64-bit type. I didn't see
>> any particular advantage in using unsigned long long here. If there's
>> a reason to prefer it in this context, I'm happy to change it.
>
> hmmm... I thought that in general we only should use a fixed width type when it is required, e.g. reading from a 32b register, then use u32.
>
The sizeof unsigned long long counter would be 8 bytes (or 64 bit) on both
32-bit and 64-but system, isn't it? So, it seems, using u64 makes the
intended width clearer than unsigned long long.
Thanks,
--Nilay
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v7 5/9] nvme: add generic debugfs support
2026-08-09 10:07 [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy Nilay Shroff
` (3 preceding siblings ...)
2026-08-09 10:07 ` [PATCH v7 4/9] nvme-multipath: add support for latency I/O policy Nilay Shroff
@ 2026-08-09 10:08 ` Nilay Shroff
2026-08-09 10:08 ` [PATCH v7 6/9] nvme-multipath: add debugfs attribute latency_ewma_shift Nilay Shroff
` (5 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Nilay Shroff @ 2026-08-09 10:08 UTC (permalink / raw)
To: linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, john.g.garry, gjoyce, Nilay Shroff
Add generic infrastructure for creating and managing debugfs files in
the NVMe module. This introduces helper APIs that allow NVMe drivers to
register and unregister debugfs entries, along with a reusable attribute
structure for defining new debugfs files.
The implementation uses seq_file interfaces to safely expose per-NS and
per-NS-head statistics, while supporting both simple show callbacks and
full seq_operations.
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
drivers/nvme/host/Makefile | 2 +-
drivers/nvme/host/core.c | 3 +
drivers/nvme/host/debugfs.c | 138 ++++++++++++++++++++++++++++++++++
drivers/nvme/host/multipath.c | 2 +
drivers/nvme/host/nvme.h | 10 +++
5 files changed, 154 insertions(+), 1 deletion(-)
create mode 100644 drivers/nvme/host/debugfs.c
diff --git a/drivers/nvme/host/Makefile b/drivers/nvme/host/Makefile
index 6414ec968f99..7962dfc3b2ad 100644
--- a/drivers/nvme/host/Makefile
+++ b/drivers/nvme/host/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_NVME_FC) += nvme-fc.o
obj-$(CONFIG_NVME_TCP) += nvme-tcp.o
obj-$(CONFIG_NVME_APPLE) += nvme-apple.o
-nvme-core-y += core.o ioctl.o sysfs.o pr.o
+nvme-core-y += core.o ioctl.o sysfs.o pr.o debugfs.o
nvme-core-$(CONFIG_NVME_VERBOSE_ERRORS) += constants.o
nvme-core-$(CONFIG_TRACING) += trace.o
nvme-core-$(CONFIG_NVME_MULTIPATH) += multipath.o
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 542e2ee036cc..21ed0ba9ac49 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4271,6 +4271,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
if (device_add_disk(ctrl->device, ns->disk, nvme_ns_attr_groups))
goto out_cleanup_ns_from_list;
+ nvme_debugfs_register(ns->disk);
+
if (!nvme_ns_head_multipath(ns->head))
nvme_add_ns_cdev(ns);
@@ -4353,6 +4355,7 @@ static void nvme_ns_remove(struct nvme_ns *ns)
nvme_mpath_remove_sysfs_link(ns);
+ nvme_debugfs_unregister(ns->disk);
del_gendisk(ns->disk);
mutex_lock(&ns->ctrl->namespaces_lock);
diff --git a/drivers/nvme/host/debugfs.c b/drivers/nvme/host/debugfs.c
new file mode 100644
index 000000000000..26a50566e4a1
--- /dev/null
+++ b/drivers/nvme/host/debugfs.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 IBM Corporation
+ * Nilay Shroff <nilay@linux.ibm.com>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+
+#include "nvme.h"
+
+struct nvme_debugfs_attr {
+ const char *name;
+ umode_t mode;
+ int (*show)(void *data, struct seq_file *m);
+ ssize_t (*write)(void *data, const char __user *buf, size_t count,
+ loff_t *ppos);
+ const struct seq_operations *seq_ops;
+};
+
+struct nvme_debugfs_ctx {
+ void *data;
+ struct nvme_debugfs_attr *attr;
+ int srcu_idx;
+};
+
+static int nvme_debugfs_show(struct seq_file *m, void *v)
+{
+ struct nvme_debugfs_ctx *ctx = m->private;
+ void *data = ctx->data;
+ struct nvme_debugfs_attr *attr = ctx->attr;
+
+ return attr->show(data, m);
+}
+
+static int nvme_debugfs_open(struct inode *inode, struct file *file)
+{
+ void *data = inode->i_private;
+ struct nvme_debugfs_attr *attr = debugfs_get_aux(file);
+ struct nvme_debugfs_ctx *ctx;
+ struct seq_file *m;
+ int ret;
+
+ ctx = kzalloc_obj(struct nvme_debugfs_ctx);
+ if (WARN_ON_ONCE(!ctx))
+ return -ENOMEM;
+
+ ctx->data = data;
+ ctx->attr = attr;
+
+ if (attr->seq_ops) {
+ ret = seq_open(file, attr->seq_ops);
+ if (ret) {
+ kfree(ctx);
+ return ret;
+ }
+ m = file->private_data;
+ m->private = ctx;
+ return ret;
+ }
+
+ if (WARN_ON_ONCE(!attr->show)) {
+ kfree(ctx);
+ return -EPERM;
+ }
+
+ return single_open(file, nvme_debugfs_show, ctx);
+}
+
+static ssize_t nvme_debugfs_write(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct seq_file *m = file->private_data;
+ struct nvme_debugfs_ctx *ctx = m->private;
+ struct nvme_debugfs_attr *attr = ctx->attr;
+
+ if (!attr->write)
+ return -EPERM;
+
+ return attr->write(ctx->data, buf, count, ppos);
+}
+
+static int nvme_debugfs_release(struct inode *inode, struct file *file)
+{
+ struct seq_file *m = file->private_data;
+ struct nvme_debugfs_ctx *ctx = m->private;
+ struct nvme_debugfs_attr *attr = ctx->attr;
+ int ret;
+
+ if (attr->seq_ops)
+ ret = seq_release(inode, file);
+ else
+ ret = single_release(inode, file);
+
+ kfree(ctx);
+ return ret;
+}
+
+static const struct file_operations nvme_debugfs_fops = {
+ .owner = THIS_MODULE,
+ .open = nvme_debugfs_open,
+ .read = seq_read,
+ .write = nvme_debugfs_write,
+ .llseek = seq_lseek,
+ .release = nvme_debugfs_release,
+};
+
+
+static const struct nvme_debugfs_attr nvme_mpath_debugfs_attrs[] = {
+ {},
+};
+
+static const struct nvme_debugfs_attr nvme_ns_debugfs_attrs[] = {
+ {},
+};
+
+static void nvme_debugfs_create_files(struct request_queue *q,
+ const struct nvme_debugfs_attr *attr, void *data)
+{
+ if (WARN_ON_ONCE(!q->debugfs_dir))
+ return;
+
+ for (; attr->name; attr++)
+ debugfs_create_file_aux(attr->name, attr->mode, q->debugfs_dir,
+ data, (void *)attr, &nvme_debugfs_fops);
+}
+
+void nvme_debugfs_register(struct gendisk *disk)
+{
+ const struct nvme_debugfs_attr *attr;
+
+ if (nvme_disk_is_ns_head(disk))
+ attr = nvme_mpath_debugfs_attrs;
+ else
+ attr = nvme_ns_debugfs_attrs;
+
+ nvme_debugfs_create_files(disk->queue, attr, disk->private_data);
+}
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 8086530b5350..b893df83d821 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -1116,6 +1116,7 @@ static void nvme_remove_head(struct nvme_ns_head *head)
if (test_and_clear_bit(NVME_NSHEAD_CDEV_LIVE, &head->flags))
nvme_cdev_del(&head->cdev, &head->cdev_device);
synchronize_srcu(&head->srcu);
+ nvme_debugfs_unregister(head->disk);
del_gendisk(head->disk);
}
nvme_put_ns_head(head);
@@ -1225,6 +1226,7 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
}
nvme_add_ns_head_cdev(head);
queue_work(nvme_wq, &head->partition_scan_work);
+ nvme_debugfs_register(head->disk);
}
nvme_mpath_add_sysfs_link(ns->head);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 3c82f67f8926..cd0d4565ebeb 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -1103,6 +1103,16 @@ static inline int nvme_data_dir(struct request *req)
return __nvme_data_dir(req_op(req));
}
+void nvme_debugfs_register(struct gendisk *disk);
+static inline void nvme_debugfs_unregister(struct gendisk *disk)
+{
+ /*
+ * Nothing to do for now. When the request queue is unregistered,
+ * all files under q->debugfs_dir are recursively deleted.
+ * This is just a placeholder; the compiler will optimize it out.
+ */
+}
+
#ifdef CONFIG_NVME_MULTIPATH
static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
{
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v7 6/9] nvme-multipath: add debugfs attribute latency_ewma_shift
2026-08-09 10:07 [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy Nilay Shroff
` (4 preceding siblings ...)
2026-08-09 10:08 ` [PATCH v7 5/9] nvme: add generic debugfs support Nilay Shroff
@ 2026-08-09 10:08 ` Nilay Shroff
2026-08-09 10:08 ` [PATCH v7 7/9] nvme-multipath: add debugfs attribute latency_batch_timeout Nilay Shroff
` (4 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Nilay Shroff @ 2026-08-09 10:08 UTC (permalink / raw)
To: linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, john.g.garry, gjoyce, Nilay Shroff
By default, the EWMA (Exponentially Weighted Moving Average) shift
value, used for storing latency samples for latency iopolicy, is set
to 3. The EWMA is calculated using the following formula:
ewma = (old * ((1 << ewma_shift) - 1) + new) >> ewma_shift;
The default value of 3 assigns ~87.5% weight to the existing EWMA value
and ~12.5% weight to the new latency sample. This provides a stable
average that smooths out short-term variations.
However, different workloads may require faster or slower adaptation to
changing conditions. This commit introduces a new debugfs attribute,
latency_ewma_shift, allowing users to tune the weighting factor.
For example:
- latency_ewma_shift = 2 => 75% old, 25% new
- latency_ewma_shift = 1 => 50% old, 50% new
- latency_ewma_shift = 0 => 0% old, 100% new
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
drivers/nvme/host/debugfs.c | 46 +++++++++++++++++++++++++++++++++++
drivers/nvme/host/multipath.c | 9 ++++---
drivers/nvme/host/nvme.h | 1 +
3 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/drivers/nvme/host/debugfs.c b/drivers/nvme/host/debugfs.c
index 26a50566e4a1..4371d7aafae8 100644
--- a/drivers/nvme/host/debugfs.c
+++ b/drivers/nvme/host/debugfs.c
@@ -105,8 +105,54 @@ static const struct file_operations nvme_debugfs_fops = {
.release = nvme_debugfs_release,
};
+#ifdef CONFIG_NVME_MULTIPATH
+static int nvme_latency_ewma_shift_show(void *data, struct seq_file *m)
+{
+ struct nvme_ns_head *head = data;
+
+ seq_printf(m, "%u\n", READ_ONCE(head->latency_ewma_shift));
+ return 0;
+}
+
+static ssize_t nvme_latency_ewma_shift_store(void *data,
+ const char __user *ubuf, size_t count, loff_t *ppos)
+{
+ struct nvme_ns_head *head = data;
+ char kbuf[8];
+ u32 res;
+ int ret;
+ size_t len;
+ char *arg;
+
+ len = min(sizeof(kbuf) - 1, count);
+
+ if (copy_from_user(kbuf, ubuf, len))
+ return -EFAULT;
+
+ kbuf[len] = '\0';
+ arg = strstrip(kbuf);
+
+ ret = kstrtou32(arg, 0, &res);
+ if (ret)
+ return ret;
+
+ /*
+ * Values greater than 8 are nonsensical, as they effectively assign
+ * zero weight to new samples.
+ */
+ if (res > 8)
+ return -EINVAL;
+
+ WRITE_ONCE(head->latency_ewma_shift, res);
+ return count;
+}
+#endif
static const struct nvme_debugfs_attr nvme_mpath_debugfs_attrs[] = {
+#ifdef CONFIG_NVME_MULTIPATH
+ {"latency_ewma_shift", 0600, nvme_latency_ewma_shift_show,
+ nvme_latency_ewma_shift_store},
+#endif
{},
};
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index b893df83d821..70a72c1a1602 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -293,10 +293,9 @@ static void nvme_mpath_weight_work(struct work_struct *weight_work)
* For instance, with EWMA_SHIFT = 3, this assigns 7/8 (~87.5 %) weight to
* the existing/old ewma and 1/8 (~12.5%) weight to the new sample.
*/
-static inline u64 calc_ewma_update(u64 old, u64 new)
+static inline u64 calc_ewma_update(u64 old, u64 new, u32 ewma_shift)
{
- return (old * ((1 << NVME_DEFAULT_LATENCY_EWMA_SHIFT) - 1)
- + new) >> NVME_DEFAULT_LATENCY_EWMA_SHIFT;
+ return (old * ((1 << ewma_shift) - 1) + new) >> ewma_shift;
}
static void nvme_mpath_add_sample(struct request *rq, struct nvme_ns *ns)
@@ -387,7 +386,8 @@ static void nvme_mpath_add_sample(struct request *rq, struct nvme_ns *ns)
if (unlikely(!stat->slat_ns))
WRITE_ONCE(stat->slat_ns, avg_lat_ns);
else {
- slat_ns = calc_ewma_update(stat->slat_ns, avg_lat_ns);
+ slat_ns = calc_ewma_update(stat->slat_ns, avg_lat_ns,
+ READ_ONCE(head->latency_ewma_shift));
WRITE_ONCE(stat->slat_ns, slat_ns);
}
@@ -1151,6 +1151,7 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
INIT_WORK(&head->partition_scan_work, nvme_partition_scan_work);
INIT_DELAYED_WORK(&head->remove_work, nvme_remove_head_work);
head->delayed_removal_secs = 0;
+ head->latency_ewma_shift = NVME_DEFAULT_LATENCY_EWMA_SHIFT;
/*
* If "multipath_always_on" is enabled, a multipath node is added
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index cd0d4565ebeb..842b9eace5dd 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -606,6 +606,7 @@ struct nvme_ns_head {
atomic_long_t io_requeue_no_usable_path_count;
atomic_long_t io_fail_no_available_path_count;
struct nvme_ns * __percpu *latency_path;
+ u32 latency_ewma_shift;
#define NVME_NSHEAD_DISK_LIVE 0
#define NVME_NSHEAD_QUEUE_IF_NO_PATH 1
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v7 7/9] nvme-multipath: add debugfs attribute latency_batch_timeout
2026-08-09 10:07 [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy Nilay Shroff
` (5 preceding siblings ...)
2026-08-09 10:08 ` [PATCH v7 6/9] nvme-multipath: add debugfs attribute latency_ewma_shift Nilay Shroff
@ 2026-08-09 10:08 ` Nilay Shroff
2026-08-09 10:08 ` [PATCH v7 8/9] nvme-multipath: add debugfs attribute latency_stat Nilay Shroff
` (3 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Nilay Shroff @ 2026-08-09 10:08 UTC (permalink / raw)
To: linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, john.g.garry, gjoyce, Nilay Shroff
By default, the latency I/O policy accumulates latency samples over a
15-second window. When this window expires, the driver computes the
average latency and updates the smoothed (EWMA) latency value. The
path weight is then recalculated based on this data.
A 15-second window provides a good balance for most workloads, as it
helps smooth out transient latency spikes and produces a more stable
path weight profile. However, some workloads may benefit from faster
or slower adaptation to changing latency conditions.
This commit introduces a new debugfs attribute, latency_batch_timeout,
which allows users to configure the latency batch window and thus path
weight calculation interval based on their workload requirements.
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
drivers/nvme/host/debugfs.c | 37 +++++++++++++++++++++++++++++++++++
drivers/nvme/host/multipath.c | 8 ++++++--
drivers/nvme/host/nvme.h | 1 +
3 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/debugfs.c b/drivers/nvme/host/debugfs.c
index 4371d7aafae8..63b0ad5d105b 100644
--- a/drivers/nvme/host/debugfs.c
+++ b/drivers/nvme/host/debugfs.c
@@ -146,12 +146,49 @@ static ssize_t nvme_latency_ewma_shift_store(void *data,
WRITE_ONCE(head->latency_ewma_shift, res);
return count;
}
+
+static int nvme_latency_batch_timeout_show(void *data, struct seq_file *m)
+{
+ struct nvme_ns_head *head = data;
+
+ seq_printf(m, "%llu\n",
+ div_u64(READ_ONCE(head->latency_batch_timeout), NSEC_PER_SEC));
+ return 0;
+}
+
+static ssize_t nvme_latency_batch_timeout_store(void *data,
+ const char __user *ubuf, size_t count, loff_t *ppos)
+{
+ struct nvme_ns_head *head = data;
+ char kbuf[8];
+ u32 res;
+ int ret;
+ size_t len;
+ char *arg;
+
+ len = min(sizeof(kbuf) - 1, count);
+
+ if (copy_from_user(kbuf, ubuf, len))
+ return -EFAULT;
+
+ kbuf[len] = '\0';
+ arg = strstrip(kbuf);
+
+ ret = kstrtou32(arg, 0, &res);
+ if (ret)
+ return ret;
+
+ WRITE_ONCE(head->latency_batch_timeout, res * NSEC_PER_SEC);
+ return count;
+}
#endif
static const struct nvme_debugfs_attr nvme_mpath_debugfs_attrs[] = {
#ifdef CONFIG_NVME_MULTIPATH
{"latency_ewma_shift", 0600, nvme_latency_ewma_shift_show,
nvme_latency_ewma_shift_store},
+ {"latency_batch_timeout", 0600, nvme_latency_batch_timeout_show,
+ nvme_latency_batch_timeout_store},
#endif
{},
};
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 70a72c1a1602..82a1bedd093c 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -361,8 +361,11 @@ static void nvme_mpath_add_sample(struct request *rq, struct nvme_ns *ns)
stat->batch_count++;
stat->nr_samples++;
- if (now > stat->last_batch_ts && ((now - stat->last_batch_ts) >=
- NVME_DEFAULT_LATENCY_BATCH_TIMEOUT)) {
+ if (now > stat->last_batch_ts) {
+ u64 timeout = READ_ONCE(head->latency_batch_timeout);
+
+ if ((now - stat->last_batch_ts) < timeout)
+ return;
/*
* Find simple average latency for the last epoch (~15 sec
@@ -1152,6 +1155,7 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
INIT_DELAYED_WORK(&head->remove_work, nvme_remove_head_work);
head->delayed_removal_secs = 0;
head->latency_ewma_shift = NVME_DEFAULT_LATENCY_EWMA_SHIFT;
+ head->latency_batch_timeout = NVME_DEFAULT_LATENCY_BATCH_TIMEOUT;
/*
* If "multipath_always_on" is enabled, a multipath node is added
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 842b9eace5dd..0f350bd52272 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -607,6 +607,7 @@ struct nvme_ns_head {
atomic_long_t io_fail_no_available_path_count;
struct nvme_ns * __percpu *latency_path;
u32 latency_ewma_shift;
+ u64 latency_batch_timeout;
#define NVME_NSHEAD_DISK_LIVE 0
#define NVME_NSHEAD_QUEUE_IF_NO_PATH 1
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v7 8/9] nvme-multipath: add debugfs attribute latency_stat
2026-08-09 10:07 [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy Nilay Shroff
` (6 preceding siblings ...)
2026-08-09 10:08 ` [PATCH v7 7/9] nvme-multipath: add debugfs attribute latency_batch_timeout Nilay Shroff
@ 2026-08-09 10:08 ` Nilay Shroff
2026-08-09 10:08 ` [PATCH v7 9/9] nvme-multipath: add documentation for latency I/O policy Nilay Shroff
` (2 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Nilay Shroff @ 2026-08-09 10:08 UTC (permalink / raw)
To: linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, john.g.garry, gjoyce, Nilay Shroff
This commit introduces a new debugfs attribute, "latency_stat", under
both per-path and head debugfs directories (defined under /sys/kernel/
debug/block/). This attribute provides visibility into the internal
state of the latency I/O policy to aid in debugging and performance
analysis.
For per-path entries, "latency_stat" reports the corresponding path
statistics such as I/O weight, selection count, processed samples, and
ignored samples.
For head entries, it reports per-CPU statistics for each reachable path,
including I/O weight, path score, smoothed (EWMA) latency, selection
count, processed samples, and ignored samples.
These additions enhance observability of the I/O path selection behavior
and help diagnose imbalance or instability in multipath performance.
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
drivers/nvme/host/debugfs.c | 124 ++++++++++++++++++++++++++++++++++++
1 file changed, 124 insertions(+)
diff --git a/drivers/nvme/host/debugfs.c b/drivers/nvme/host/debugfs.c
index 63b0ad5d105b..b4bc7c710050 100644
--- a/drivers/nvme/host/debugfs.c
+++ b/drivers/nvme/host/debugfs.c
@@ -181,6 +181,126 @@ static ssize_t nvme_latency_batch_timeout_store(void *data,
WRITE_ONCE(head->latency_batch_timeout, res * NSEC_PER_SEC);
return count;
}
+
+static void *nvme_mpath_latency_stat_start(struct seq_file *m, loff_t *pos)
+{
+ struct nvme_ns *ns;
+ struct nvme_debugfs_ctx *ctx = m->private;
+ struct nvme_ns_head *head = ctx->data;
+
+ if (!head->disk)
+ return NULL;
+
+ /* Remember srcu index, so we can unlock later. */
+ ctx->srcu_idx = srcu_read_lock(&head->srcu);
+ ns = list_first_or_null_rcu(&head->list, struct nvme_ns, siblings);
+
+ while (*pos && ns) {
+ ns = list_next_or_null_rcu(&head->list, &ns->siblings,
+ struct nvme_ns, siblings);
+ (*pos)--;
+ }
+
+ return ns;
+}
+
+static void *nvme_mpath_latency_stat_next(struct seq_file *m, void *v,
+ loff_t *pos)
+{
+ struct nvme_ns *ns = v;
+ struct nvme_debugfs_ctx *ctx = m->private;
+ struct nvme_ns_head *head = ctx->data;
+
+ (*pos)++;
+
+ return list_next_or_null_rcu(&head->list, &ns->siblings,
+ struct nvme_ns, siblings);
+}
+
+static void nvme_mpath_latency_stat_stop(struct seq_file *m, void *v)
+{
+ struct nvme_debugfs_ctx *ctx = m->private;
+ struct nvme_ns_head *head = ctx->data;
+ int srcu_idx = ctx->srcu_idx;
+
+ if (!head->disk)
+ return;
+
+ srcu_read_unlock(&head->srcu, srcu_idx);
+}
+
+static int nvme_mpath_latency_stat_show(struct seq_file *m, void *v)
+{
+ int i, cpu;
+ struct nvme_path_lat_stat *stat;
+ struct nvme_ns *ns = v;
+
+ seq_printf(m, "%s:\n", ns->disk->disk_name);
+ for_each_online_cpu(cpu) {
+ seq_printf(m, "cpu %d : ", cpu);
+ for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
+ stat = &per_cpu_ptr(ns->path_lat, cpu)[i].stat;
+ seq_printf(m, "%u %u %llu %llu %llu %llu %llu ",
+ stat->weight, stat->credit, stat->score,
+ stat->slat_ns, stat->sel,
+ stat->nr_samples, stat->nr_ignored);
+ }
+ seq_putc(m, '\n');
+ }
+ return 0;
+}
+
+static const struct seq_operations nvme_mpath_latency_stat_seq_ops = {
+ .start = nvme_mpath_latency_stat_start,
+ .next = nvme_mpath_latency_stat_next,
+ .stop = nvme_mpath_latency_stat_stop,
+ .show = nvme_mpath_latency_stat_show
+};
+
+static void nvme_latency_stat_read_all(struct nvme_ns *ns,
+ struct nvme_path_lat_stat *batch)
+{
+ int i, cpu;
+ u32 ncpu[NVME_NUM_STAT_GROUPS] = {0};
+ struct nvme_path_lat_stat *stat;
+
+ for_each_online_cpu(cpu) {
+ for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
+ stat = &per_cpu_ptr(ns->path_lat, cpu)[i].stat;
+ batch[i].sel += stat->sel;
+ batch[i].nr_samples += stat->nr_samples;
+ batch[i].nr_ignored += stat->nr_ignored;
+ batch[i].weight += stat->weight;
+ if (stat->weight)
+ ncpu[i]++;
+ }
+ }
+
+ for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
+ if (!ncpu[i])
+ continue;
+ batch[i].weight = DIV_U64_ROUND_CLOSEST(batch[i].weight,
+ ncpu[i]);
+ }
+}
+
+static int nvme_ns_latency_stat_show(void *data, struct seq_file *m)
+{
+ int i;
+ struct nvme_path_lat_stat stat[NVME_NUM_STAT_GROUPS] = {0};
+ struct nvme_ns *ns = (struct nvme_ns *)data;
+
+ if (!ns->head->disk)
+ return 0;
+
+ nvme_latency_stat_read_all(ns, stat);
+ for (i = 0; i < NVME_NUM_STAT_GROUPS; i++) {
+ seq_printf(m, "%u %llu %llu %llu ",
+ stat[i].weight, stat[i].sel,
+ stat[i].nr_samples, stat[i].nr_ignored);
+ }
+ return 0;
+}
#endif
static const struct nvme_debugfs_attr nvme_mpath_debugfs_attrs[] = {
@@ -189,11 +309,15 @@ static const struct nvme_debugfs_attr nvme_mpath_debugfs_attrs[] = {
nvme_latency_ewma_shift_store},
{"latency_batch_timeout", 0600, nvme_latency_batch_timeout_show,
nvme_latency_batch_timeout_store},
+ {"latency_stat", 0400, .seq_ops = &nvme_mpath_latency_stat_seq_ops},
#endif
{},
};
static const struct nvme_debugfs_attr nvme_ns_debugfs_attrs[] = {
+#ifdef CONFIG_NVME_MULTIPATH
+ {"latency_stat", 0400, nvme_ns_latency_stat_show},
+#endif
{},
};
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v7 9/9] nvme-multipath: add documentation for latency I/O policy
2026-08-09 10:07 [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy Nilay Shroff
` (7 preceding siblings ...)
2026-08-09 10:08 ` [PATCH v7 8/9] nvme-multipath: add debugfs attribute latency_stat Nilay Shroff
@ 2026-08-09 10:08 ` Nilay Shroff
2026-08-10 6:45 ` Hannes Reinecke
2026-08-10 7:50 ` [PATCH v7 0/9] nvme-multipath: introduce " John Garry
2026-08-10 9:43 ` Guixin Liu
10 siblings, 1 reply; 30+ messages in thread
From: Nilay Shroff @ 2026-08-09 10:08 UTC (permalink / raw)
To: linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, john.g.garry, gjoyce, Nilay Shroff
Update the nvme-multipath documentation to describe the latency I/O
policy, its behavior, and when it is suitable for use.
Suggested-by: Guixin Liu <kanie@linux.alibaba.com>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
Documentation/admin-guide/nvme-multipath.rst | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/Documentation/admin-guide/nvme-multipath.rst b/Documentation/admin-guide/nvme-multipath.rst
index 97ca1ccef459..41a8054638ff 100644
--- a/Documentation/admin-guide/nvme-multipath.rst
+++ b/Documentation/admin-guide/nvme-multipath.rst
@@ -70,3 +70,22 @@ When to use the queue-depth policy:
1. High load with small I/Os: Effectively balances load across paths when
the load is high, and I/O operations consist of small, relatively
fixed-sized requests.
+
+Latency
+--------
+
+The latency policy manages I/O requests based on path latency. It periodically
+calculates a weight for each path and distributes I/O accordingly. Paths with
+higher latency receive lower weights, resulting in fewer I/O requests being sent
+to them, while paths with lower latency handle a proportionally larger share of
+the I/O load.
+
+When to use the latency policy
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+1. Homogeneous Path Performance: Utilizes all available paths efficiently when
+ their performance characteristics (e.g., latency, bandwidth) are similar.
+
+2. Heterogeneous Path Performance: Dynamically distributes I/O based on per-path
+ performance characteristics. Paths with lower latency receive a higher share
+ of I/O compared to those with higher latency.
--
2.53.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v7 9/9] nvme-multipath: add documentation for latency I/O policy
2026-08-09 10:08 ` [PATCH v7 9/9] nvme-multipath: add documentation for latency I/O policy Nilay Shroff
@ 2026-08-10 6:45 ` Hannes Reinecke
0 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2026-08-10 6:45 UTC (permalink / raw)
To: Nilay Shroff, linux-nvme
Cc: kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, john.g.garry, gjoyce
On 8/9/26 12:08 PM, Nilay Shroff wrote:
> Update the nvme-multipath documentation to describe the latency I/O
> policy, its behavior, and when it is suitable for use.
>
> Suggested-by: Guixin Liu <kanie@linux.alibaba.com>
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
> ---
> Documentation/admin-guide/nvme-multipath.rst | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy
2026-08-09 10:07 [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy Nilay Shroff
` (8 preceding siblings ...)
2026-08-09 10:08 ` [PATCH v7 9/9] nvme-multipath: add documentation for latency I/O policy Nilay Shroff
@ 2026-08-10 7:50 ` John Garry
2026-08-10 17:27 ` Nilay Shroff
2026-08-10 9:43 ` Guixin Liu
10 siblings, 1 reply; 30+ messages in thread
From: John Garry @ 2026-08-10 7:50 UTC (permalink / raw)
To: Nilay Shroff, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
This does not apply (cleanly) to the nvme-7.3 branch. It applies to
v7.2-rc7. I would think that it should apply to the nvme-7.3 branch. Or
am I missing something?
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy
2026-08-10 7:50 ` [PATCH v7 0/9] nvme-multipath: introduce " John Garry
@ 2026-08-10 17:27 ` Nilay Shroff
2026-08-11 9:37 ` John Garry
0 siblings, 1 reply; 30+ messages in thread
From: Nilay Shroff @ 2026-08-10 17:27 UTC (permalink / raw)
To: John Garry, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
On 8/10/26 1:20 PM, John Garry wrote:
> This does not apply (cleanly) to the nvme-7.3 branch. It applies to v7.2-rc7. I would think that it should apply to the nvme-7.3 branch. Or am I missing something?
Yes, you're right. It should have been rebased onto the current nvme-7.3 branch.
I'll rebase it and send the updated version in the next revision.
Thanks,
--Nilay
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy
2026-08-10 17:27 ` Nilay Shroff
@ 2026-08-11 9:37 ` John Garry
2026-08-12 8:07 ` Nilay Shroff
0 siblings, 1 reply; 30+ messages in thread
From: John Garry @ 2026-08-11 9:37 UTC (permalink / raw)
To: Nilay Shroff, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
JFYI, on another topic, I did a quick port of this to libmultipath as a PoC:
https://github.com/johnpgarry/linux/commits/scsi-multipath-v7.2-v7-latency-v7/
It's really just a direct symbol-for-symbol change.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy
2026-08-11 9:37 ` John Garry
@ 2026-08-12 8:07 ` Nilay Shroff
0 siblings, 0 replies; 30+ messages in thread
From: Nilay Shroff @ 2026-08-12 8:07 UTC (permalink / raw)
To: John Garry, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, kanie, jmeneghi, randyj,
martin.petersen, gjoyce
On 8/11/26 3:07 PM, John Garry wrote:
> JFYI, on another topic, I did a quick port of this to libmultipath as a PoC:
>
> https://github.com/johnpgarry/linux/commits/scsi-multipath-v7.2-v7-latency-v7/
>
> It's really just a direct symbol-for-symbol change.
This is great!
Thanks,
--Nilay
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy
2026-08-09 10:07 [PATCH v7 0/9] nvme-multipath: introduce latency I/O policy Nilay Shroff
` (9 preceding siblings ...)
2026-08-10 7:50 ` [PATCH v7 0/9] nvme-multipath: introduce " John Garry
@ 2026-08-10 9:43 ` Guixin Liu
10 siblings, 0 replies; 30+ messages in thread
From: Guixin Liu @ 2026-08-10 9:43 UTC (permalink / raw)
To: Nilay Shroff, linux-nvme
Cc: hare, kbusch, hch, sagi, dwagner, jmeneghi, randyj,
martin.petersen, john.g.garry, gjoyce
Looks good to me.
Reviewed-by: Guixin Liu <kanie@linux.alibaba.com>
^ permalink raw reply [flat|nested] 30+ messages in thread