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

[Sagi Grimberg <sagi@grimberg.me>]

>>>> +static void nvme_rdma_complete_timed_out(struct request *rq)
>>>> +{
>>>> +    struct nvme_rdma_request *req = blk_mq_rq_to_pdu(rq);
>>>> +    struct nvme_rdma_queue *queue = req->queue;
>>>> +    struct nvme_rdma_ctrl *ctrl = queue->ctrl;
>>>> +
>>>> +    /* fence other contexts that may complete the command */
>>>> +    flush_work(&ctrl->err_work);
>>>> +    nvme_rdma_stop_queue(queue);
>>> There maybe concurrent with error recovery, may cause abnormal because
>>> nvme_rdma_stop_queue will return but the queue is not stoped,
>>> maybe is stopping by the error recovery.
>>
>> err_work flush used to fence, once we did queue stop, it should be safe
>> to complete the command from the timeout handler.
> 
> Flush work just can avoid trigger error recovery by nvme_rdma_timeout or
> reduce concurrent probalibity trigger error recovery by other progress,
> but can not avoid.

The point is that we can complete the command because err_work
was flushed and the queue was stopped, which means we shouldn't have
any context completing the request.

> if nvme_rdma_cm_handler or other progress call
> nvme_rdma_error_recovery, between change state to queue_work may
> interrupt by hard interrupt, and then timeout happen, thus flush work
> can not avoid concurrent.
> Like this:
> 
> static void nvme_rdma_error_recovery(struct nvme_rdma_ctrl *ctrl)
> {
>      if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_RESETTING))
>          return;
> --------------------------------

If we are in RESETTING/CONNECTING state already, this won't do anything.

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

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