On 10/4/22 2:19 AM, Sagi Grimberg wrote:
>
>
> 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@xxxxxxxxxxx>
>>> ---
>>> 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.
I prefer it the way that you have it, avoids a function call for
the hot path of not being multipath.
--
Jens Axboe
