[PATCH] nvme-tcp: handle failing req immediately for REQ_FAILFAST_DRIVER
Sagi Grimberg
sagi at grimberg.me
Thu Apr 7 06:04:44 PDT 2022
>>> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
>>> index 10fc45d95b86..f714d7ad38c0 100644
>>> --- a/drivers/nvme/host/tcp.c
>>> +++ b/drivers/nvme/host/tcp.c
>>> @@ -1145,9 +1145,11 @@ static int nvme_tcp_try_send(struct nvme_tcp_queue *queue)
>>> if (ret == -EAGAIN) {
>>> ret = 0;
>>> } else if (ret < 0) {
>>> + struct request *rq = blk_mq_rq_from_pdu(queue->request);
>>> dev_err(queue->ctrl->ctrl.device,
>>> "failed to send request %d\n", ret);
>>> - if (ret != -EPIPE && ret != -ECONNRESET)
>>> + if ((ret != -EPIPE && ret != -ECONNRESET) ||
>>> + rq->cmd_flags & REQ_FAILFAST_DRIVER)
>>
>> I'm wandering if this will race with nvme_cancel_admin_tagset which
>> can also complete the command... The whole assumption is that the
>> cancel iterator will be running as the socket is shutdown by the peer
>> and error_recovery was triggered.
>>
>> I think these shouldn't race because we shutdown the queue (and flush
>> the io_work) and only then cancel. But I've seen races of that
>> sort in the past...
>
> This particular request is issued by nvme_shutdown_ctrl() so we know
> exactly where we are. But this might not be the case for any other
> REQ_FAILFAST_DRIVER flagged request. So this might introduce the race
> you are pointing out.
>
> The option is see is what Margin suggested in
>
> http://lists.infradead.org/pipermail/linux-nvme/2021-September/027867.html
Don't know if I like the state check. Why this does not apply in a ctrl
reset?
More information about the Linux-nvme
mailing list