Re: [PATCH for-next v8 3/5] nvme: refactor nvme_alloc_user_request

Kanchan Joshi
2022-09-27T17:08:05Z

[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]

On Mon, Sep 26, 2022 at 04:51:59PM +0200, Christoph Hellwig wrote:
>On Mon, Sep 26, 2022 at 01:13:54AM +0530, Kanchan Joshi wrote:
>>>> + if (ret)
>>>> + goto out;
>>>> + bio = req->bio;
>>>
>>> I think we can also do away with this bio local variable now.
>>>
>>>> + if (bdev)
>>>> + bio_set_dev(bio, bdev);
>>>
>>> We don't need the bio_set_dev here as mentioned last time, so I think
>>> we should remove it in a prep patch.
>>
>> we miss completing polled io with this change.
>> bdev needs to be put in bio to complete polled passthrough IO.
>> nvme_ns_chr_uring_cmd_iopoll uses bio_poll and that in turn makes use of
>> this.
>
>Oh, indeed - polling is another and someone unexpected user in
>addition to the I/O accounting that does not apply to passthrough
>requests. That also means we can't poll admin commands at all.

Yes. That falls back to IRQ completions.

I think it should be possible to support if we use request-only
interface. Most of the information in bio-poll interface comes from
request.
But I doubt if polling for admin command is a useful thing.

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]