Re: [PATCH 5/6] nvme-rdma: fix timeout handler

From: Chao Leng <lengchao@huawei.com>
To: Sagi Grimberg <sagi@grimberg.me>,
	<linux-nvme@lists.infradead.org>, Christoph Hellwig <hch@lst.de>,
	Keith Busch <kbusch@kernel.org>,
	James Smart <james.smart@broadcom.com>
Subject: Re: [PATCH 5/6] nvme-rdma: fix timeout handler
Date: Wed, 5 Aug 2020 15:14:50 +0800	[thread overview]
Message-ID: <537c387f-61e2-77f4-a7ee-55689270bdfd@huawei.com> (raw)
In-Reply-To: <ae787cff-f1a3-b464-f471-18b3d736c381@grimberg.me>


On 2020/8/5 15:00, Sagi Grimberg wrote:
> 
>>>>>> may interrupt by hard interrupt, and then timeout progress flush work
>>>>>> at this time. Thus error recovery and nvme_rdma_complete_timed_out may
>>>>>> concurrent to stop queue. will cause: error recovery may cancel request
>>>>>> or nvme_rdma_complete_timed_out may complete request, but the queue may
>>>>>> not be stoped. Thus will cause abnormal.
>>>>>
>>>>> We should be fine and safe to complete the I/O.
>>>>
>>>> Complete request in nvme_rdma_timeout or cancel request in
>>>> nvme_rdma_error_recovery_work or nvme_rdma_reset_ctrl_work is not safe.
>>>> Because the queue may be not really stoped, it may just cleard the flag:
>>>> NVME_RDMA_Q_ALLOCATED for the queue. Thus one request may concurrent
>>>> treat by two progress, it is not allowed.
>>>
>>> The request being timed out cannot be completed after the queue is
>>> stopped, that is the point of nvme_rdma_stop_queue. if it is only
>>> ALLOCATED, we did not yet connect hence there is zero chance for
>>> any command to complete.
>> The request may already complete before stop queue, it is in the cq, but
>> is not treated by software.
> 
> Not possible, ib_drain_cq completion guarantees that all cqes were
> reaped and handled by SW.
> 
>> If nvme_rdma_stop_queue concurrent
> 
> Before we complete we make sure the queue is stopped (and drained and
> reaped).
> 
> , for
>> example:
>> The error recovery run first, it will clear the flag:NVME_RDMA_Q_LIVE,
>> and then wait drain cq. At the same time nvme_rdma_timeout
>> call nvme_rdma_stop_queue will return immediately, and then may call
>> blk_mq_complete_request. but error recovery may drain cq at the same
>> time, and may also treat the same request.
> 
> We flush the err_work before running nvme_rdma_stop_queue exactly
> because of that. your example cannot happen.
Flush work is not safe. See my previous email.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme