Re: [PATCH 2/3] nvme/pci: Remove cq_vector check in IO path

Sagi Grimberg
2017-12-31T12:30:09Z

>>>> Awesome!
>>>>
>>>> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
>>>
>>> Wait, aren't we unquiesce queues also in nvme_dev_disable?
>>>
>>> Doesn't that rely that the queues are suspended and queue_rq
>>> will fail there?
>>
>> We don't seem to have any other check as far as I can tell.
>
> Ok, we currently do need this check in the submission side to flush
> entered requests on invalidated hw contexts. We didn't have a way to
> back out entered requests before, so that's why this request killing
> check still exists. We can steal bio's to back out requests now, so I
> think we should do that instead of failing them.
>
> I'll do rework the series a bit to make that possible, plus add the
> removal of the submission side nvme_process_cq, and resend.

Not sure if stealing bios from requests is a better design. Note that
we do exactly this in other transport (nvme_[rdma|loop|fc]_is_ready).

I think it would be better to stick to a coherent behavior across
the nvme subsystem. If this condition statement is really something
that is buying us measurable performance gain, I think we should apply
it for other transports as well (although in fabrics we're a bit
different because we have a dedicated connect that enters .queue_rq)