[PATCH v2 2/2] nvme: support io stats on the mpath device

[Sagi Grimberg]

On 10/4/22 09:11, Hannes Reinecke wrote:
> On 10/3/22 11:43, Sagi Grimberg wrote:
>> Our mpath stack device is just a shim that selects a bottom namespace
>> and submits the bio to it without any fancy splitting. This also means
>> that we don't clone the bio or have any context to the bio beyond
>> submission. However it really sucks that we don't see the mpath device
>> io stats.
>>
>> Given that the mpath device can't do that without adding some context
>> to it, we let the bottom device do it on its behalf (somewhat similar
>> to the approach taken in nvme_trace_bio_complete).
>>
>> When the IO starts, we account the request for multipath IO stats using
>> REQ_NVME_MPATH_IO_STATS nvme_request flag to avoid queue io stats disable
>> in the middle of the request.
>>
>> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
>> ---
>>   drivers/nvme/host/core.c      |  4 ++++
>>   drivers/nvme/host/multipath.c | 25 +++++++++++++++++++++++++
>>   drivers/nvme/host/nvme.h      | 12 ++++++++++++
>>   3 files changed, 41 insertions(+)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 64fd772de817..d5a54ddf73f2 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -384,6 +384,8 @@ static inline void nvme_end_req(struct request *req)
>>           nvme_log_error(req);
>>       nvme_end_req_zoned(req);
>>       nvme_trace_bio_complete(req);
>> +    if (req->cmd_flags & REQ_NVME_MPATH)
>> +        nvme_mpath_end_request(req);
>>       blk_mq_end_request(req, status);
>>   }
>> @@ -421,6 +423,8 @@ EXPORT_SYMBOL_GPL(nvme_complete_rq);
>>   void nvme_start_request(struct request *rq)
>>   {
>> +    if (rq->cmd_flags & REQ_NVME_MPATH)
>> +        nvme_mpath_start_request(rq);
>>       blk_mq_start_request(rq);
>>   }
>>   EXPORT_SYMBOL_GPL(nvme_start_request);
> 
> Why don't you move the check for REQ_NVME_MPATH into 
> nvme_mpath_{start,end}_request?

I'm less fond of calling a function that may or may not
do anything...

But it is a pattern that exists in the code, if people prefer
it I can change it.