Re: [PATCH 05/17] nvme: wire-up support for async-passthru on char-device.

Sagi Grimberg
2022-03-16T10:57:01Z

On 3/16/22 11:21, Kanchan Joshi wrote:
> On Tue, Mar 15, 2022 at 11:02:30AM +0200, Sagi Grimberg wrote:
>>
>>>>> +int nvme_ns_head_chr_async_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);
>>>>> +     int ret = -EWOULDBLOCK;
>>>>> +
>>>>> +     if (ns)
>>>>> +             ret = nvme_ns_async_ioctl(ns, ioucmd);
>>>>> +     srcu_read_unlock(&head->srcu, srcu_idx);
>>>>> +     return ret;
>>>>> +}
>>>>
>>>> No one cares that this has no multipathing capabilities what-so-ever?
>>>> despite being issued on the mpath device node?
>>>>
>>>> I know we are not doing multipathing for userspace today, but this
>>>> feels like an alternative I/O interface for nvme, seems a bit cripled
>>>> with zero multipathing capabilities...
>>>
>>> Multipathing is on the radar. Either in the first cut or in
>>> subsequent. Thanks for bringing this up.
>>
>> Good to know...
>>
>>> So the char-node (/dev/ngX) will be exposed to the host if we enable
>>> controller passthru on the target side. And then the host can send
>>> commands using uring-passthru in the same way.
>>
>> Not sure I follow...
>
> Doing this on target side:
> echo -n /dev/nvme0 >
> /sys/kernel/config/nvmet/subsystems/testnqn/passthru/device_path
> echo 1 > /sys/kernel/config/nvmet/subsystems/testnqn/passthru/enable

Cool, what does that have to do with what I asked?

>>> May I know what are the other requirements here.
>>
>> Again, not sure I follow... The fundamental capability is to
>> requeue/failover I/O if there is no I/O capable path available...
>
> That is covered I think, with nvme_find_path() at places including the
> one you highlighted above.

No it isn't. nvme_find_path is a simple function that retrieves an I/O
capable path which is not guaranteed to exist, it has nothing to do with
I/O requeue/failover.

Please see nvme_ns_head_submit_bio, nvme_failover_req,
nvme_requeue_work.