Re: [PATCH v5 2/2] nvme: use blk_mq_[un]quiesce_tagset

[Chao Leng <lengchao@huawei.com>]

On 2020/7/28 11:34, Sagi Grimberg wrote:
> 
>>>> All controller namespaces share the same tagset, so we
>>>> can use this interface which does the optimal operation
>>>> for parallel quiesce based on the tagset type (e.g.
>>>> blocking tagsets and non-blocking tagsets).
>>>>
>>>> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
>>>> ---
>>>>   drivers/nvme/host/core.c | 14 ++------------
>>>>   1 file changed, 2 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>>>> index 05aa568a60af..c41df20996d7 100644
>>>> --- a/drivers/nvme/host/core.c
>>>> +++ b/drivers/nvme/host/core.c
>>>> @@ -4557,23 +4557,13 @@ EXPORT_SYMBOL_GPL(nvme_start_freeze);
>>>>   void nvme_stop_queues(struct nvme_ctrl *ctrl)
>>>>   {
>>>> -    struct nvme_ns *ns;
>>>> -
>>>> -    down_read(&ctrl->namespaces_rwsem);
>>>> -    list_for_each_entry(ns, &ctrl->namespaces, list)
>>>> -        blk_mq_quiesce_queue(ns->queue);
>>>> -    up_read(&ctrl->namespaces_rwsem);
>>>> +    blk_mq_quiesce_tagset(ctrl->tagset);
>>>
>>> Rrr.. this one is slightly annoying. We have the connect_q in
>>> fabrics that we use to issue the connect command which is now
>>> quiesced too...
>>>
>>> If we will use this interface, we can unquiesce it right away,
>>> but that seems kinda backwards..Io queue and admin queue has different treat mechanism, introduce
>> blk_mq_quiesce_tagset may make the mechanism unclear. So this is
>> probably not a good choice.
> 
> The meaning of blk_mq_quiesce_tagset is clear, the awkward thing is
> that we need to unquiesce the connect_q after blk_mq_quiesce_tagset
> quiesced it.
Io queue and admin queue has different treat mechanism. If we switch to
blk_mq_quiesce_tagset, maybe we need do more extras such as unquiesce
the connect_q, thus the mechanism maybe unclear. Surely if we introduce
blk_mq_quiesce_tagset for new feature, the abstraction is great.
> 
> I'm thinking of resorting from this abstraction...
> .