[RFC PATCH] nvme-tcp: rerun io_work if req_list is not empty

[Keith Busch]

On Mon, May 17, 2021 at 08:02:41PM -0700, Sagi Grimberg wrote:
> > nvme_tcp_send_all() breaks out of the loop if nvme_tcp_fetch_request()
> > returns NULL. If that happens just before io_work calls
> > nvme_tcp_handle_r2t() to enqueue the H2C request, nvme_tcp_send_all()
> > will not see that request, but send_mutex is still held. We're counting
> > on io_work to run again to handle sending the H2C data in that case.
> > Unlikely as it sounds, if the same nvme_tcp_send_all() context is still
> > holding the send_mutex when io_work gets back to trying to take it, how
> > will the data get sent?
> 
> Yes you are correct, overlooked this race. I guess this is all coming
> from having less queues than cores (where rx really competes with tx)
> which is not as common as a non default.
> 
> This is enough to convince me that this is needed:
> Reviewed-by: Sagi Grimberg <sagi at grimberg.me>

Great! I thought the scenario seemed possible, but wasn't completely
sure, so thank you for confirming.

Christoph, can we pick this up for the next rc? For stable, we can add

Fixes: db5ad6b7f8cdd ("nvme-tcp: try to send request in queue_rq context")
 
> > > Can we maybe try to catch if that is the case?
> > 
> > Do you have a better idea on how we can catch this? I think there was
> > only one occurance of this sighting so far, and it looks like it took a
> > long time to encounter it, but we will try again if you have a proposal.
> 
> We can continue to test with the patch and hunt for another occurance,
> given the argument above, this patch is needed regardless...

Sounds good, we'll run with the patch and see what happens. If the tests
are successful, I'm not sure if we can conclude this definitely fixes
the timeout or if we just got lucky. If a timeout is observed, though, I
will try to work in a debug patch.