lkml.org 
[lkml]   [2019]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 7/8] nvme: use blk_mq_queue_tag_busy_iter
From
Date
On 3/15/19 5:39 PM, James Smart wrote:
>
>
> On 3/15/2019 9:33 AM, James Smart wrote:
>> On 3/15/2019 1:57 AM, Jianchao Wang wrote:
>>> blk_mq_tagset_busy_iter is not safe that it could get stale request
>>> in tags->rqs[]. Use blk_mq_queue_tag_busy_iter here. A new helper
>>> interface nvme_iterate_inflight_rqs is introduced to iterate
>>> all of the ns under a ctrl.
>>>
>>> Signed-off-by: Jianchao Wang <jianchao.w.wang@oracle.com>
>>> ---
>>>   drivers/nvme/host/core.c   | 12 ++++++++++++
>>>   drivers/nvme/host/fc.c     | 12 ++++++------
>>>   drivers/nvme/host/nvme.h   |  2 ++
>>>   drivers/nvme/host/pci.c    |  5 +++--
>>>   drivers/nvme/host/rdma.c   |  6 +++---
>>>   drivers/nvme/host/tcp.c    |  5 +++--
>>>   drivers/nvme/target/loop.c |  6 +++---
>>>   7 files changed, 32 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>>> index 6a9dd68..5760fad 100644
>>> --- a/drivers/nvme/host/core.c
>>> +++ b/drivers/nvme/host/core.c
>>> @@ -3816,6 +3816,18 @@ void nvme_start_queues(struct nvme_ctrl *ctrl)
>>>   }
>>>   EXPORT_SYMBOL_GPL(nvme_start_queues);
>>>   +void nvme_iterate_inflight_rqs(struct nvme_ctrl *ctrl,
>>> +        busy_iter_fn *fn, void *data)
>>> +{
>>> +    struct nvme_ns *ns;
>>> +
>>> +    down_read(&ctrl->namespaces_rwsem);
>>> +    list_for_each_entry(ns, &ctrl->namespaces, list)
>>> +        blk_mq_queue_tag_busy_iter(ns->queue, fn, data, true);
>>> +    up_read(&ctrl->namespaces_rwsem);
>>> +}
>>> +EXPORT_SYMBOL_GPL(nvme_iterate_inflight_rqs);
>>
>> Hmm... so this aborts ios for namespaces. How about ios that aren't
>> for a namespace but rather for the controller itself.  For example,
>> anything on the admin queue ?   Rather critical for those ios be
>> terminated as well.
> NVM - didn't look at which tag set.  but it does highlight - doesn't
> cover anything issued by core/transport on the io queues.
>
Actually, I would consider that an error.
_All_ commands on io queues (internal or block-layer generated) whould
be tracked sbitmap and hence the tag iterator.
That's what we have the 'private' tags for, and why the iter callback
has this ominous 'bool' argument...

Cheers,

Hannes

\
 
 \ /
  Last update: 2019-03-15 17:50    [W:0.044 / U:0.332 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site