[PATCH] block: re-introduce blk_mq_complete_request_sync

[Ming Lei]

On Mon, Oct 12, 2020 at 11:59:21AM +0800, Chao Leng wrote:
> 
> 
> On 2020/10/10 14:08, Yi Zhang wrote:
> > 
> > 
> > On 10/10/20 2:29 AM, Sagi Grimberg wrote:
> > > 
> > > 
> > > On 10/9/20 6:55 AM, Yi Zhang wrote:
> > > > Hi Sagi
> > > > 
> > > > On 10/9/20 4:09 PM, Sagi Grimberg wrote:
> > > > > > Hi Sagi
> > > > > > 
> > > > > > I applied this patch on block origin/for-next and still can reproduce it.
> > > > > 
> > > > > That's unexpected, can you try this patch?
> > > > > -- 
> > > > > diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> > > > > index 629b025685d1..46428ff0b0fc 100644
> > > > > --- a/drivers/nvme/host/tcp.c
> > > > > +++ b/drivers/nvme/host/tcp.c
> > > > > @@ -2175,7 +2175,7 @@ static void nvme_tcp_complete_timed_out(struct request *rq)
> > > > >         /* fence other contexts that may complete the command */
> > > > >         mutex_lock(&to_tcp_ctrl(ctrl)->teardown_lock);
> > > > >         nvme_tcp_stop_queue(ctrl, nvme_tcp_queue_id(req->queue));
> > > > > -       if (!blk_mq_request_completed(rq)) {
> > > > > +       if (blk_mq_request_started(rq) && !blk_mq_request_completed(rq)) {
> > > > >                 nvme_req(rq)->status = NVME_SC_HOST_ABORTED_CMD;
> > > > >                 blk_mq_complete_request_sync(rq);
> > > > >         }
> This may just reduce the probability. The concurrency of timeout and teardown will cause the same request
> be treated repeatly, this is not we expected.

That is right, not like SCSI, NVME doesn't apply atomic request completion, so
request may be completed/freed from both timeout & nvme_cancel_request().

.teardown_lock still may cover the race with Sagi's patch because teardown
actually cancels requests in sync style.

> In the teardown process, after quiesced queues delete the timer and cancel the timeout work maybe a better option.

Seems better solution, given it is aligned with NVME PCI's reset
handling. nvme_sync_queues() may be called in nvme_tcp_teardown_io_queues() to
avoid this race.


Thanks, 
Ming