[PATCH v3 2/9] nvme-fabrics: allow to queue requests for live queues

[James Smart]

On 8/20/2020 3:13 PM, Sagi Grimberg wrote:
>
>>>> I'm still rather bothered with the admin queue exception.  And 
>>>> given that
>>>> the q_usage_counter problem should only really be an issue for file 
>>>> system
>>>> requests, as passthrough requests do not automatically get retried why
>>>> can't we just reject all user command to be symetric and straight 
>>>> forward?
>>>> The callers in userspace need to be able to cope with retryable errors
>>>> anyway.
>>>
>>> Looking at the code again, I think we can kill it as well.
>>>
>>> The concern is we may issue user generated admin commands before
>>> the controller is enabled (generating an unforced error just because
>>> we queued to early). That used to be the case when the admin connect
>>> used the admin_q which meant we needed to unquiesce before, but
>>> now that the admin connect uses the fabrics_q, that should no
>>> longer be an issue.
>>>
>>> in nvme-tcp we unquiesce after we enable the ctrl:
>>> ...
>>>
>>> James, can you please have a look if this is still an issue?
>>
>> I still dislike any random ioctls coming in while we're still 
>> initializing the controller.  Looking at the flow - I wouldn't want 
>> them to be allowed until after nvme_init_identify() is complete. 
>> Especially if the ioctls are doing subsystem or controller dumping or 
>> using commands that should be capped by values set by 
>> nvme_queue_limits(). But, if we're going to allow nvme_init_identify 
>> the admin_q needs to be unquiesced.
>>
>> So I'm still voting for the admin queue exception.
>
> I think that if the controller is enabled, it should be able to accept
> commands. For your specific concern, I don't think that the right place
> to protect against it is __nvmf_check_ready, because from that
> standpoint the controller _is_ ready.
>
> Perhaps if this is a real concern we should have passthru commands
> to check the controller state before executing?
>
> Christoph, Keith, WDYT?

there is always a window between when the cmd is submitted after passing 
the check vs when it comes off the q for execution and now the state of 
the controller has changed

-- james