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

Sagi Grimberg sagi at grimberg.me
Thu Aug 20 18:13:33 EDT 2020


>>> 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?



More information about the Linux-nvme mailing list