Re: [PATCH v7 3/9] nvme-multipath: pass I/O type to nvme_find_path()

[John Garry <john.g.garry@oracle.com>]

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
>>