[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