[PATCH v7 3/9] nvme-multipath: pass I/O type to nvme_find_path()
Nilay Shroff
nilay at linux.ibm.com
Wed Aug 12 01:05:35 PDT 2026
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
More information about the Linux-nvme
mailing list