Re: [PATCH 2/3] nvme-tcp: Check for write space before queueing requests

[Sagi Grimberg <sagi@grimberg.me>]

> The current model of always queue incoming requests lead to
> write stalls as we easily overload the network device under
> high I/O load.
> To avoid unlimited queueing we should rather check if write
> space is available before accepting new requests.

I'm somewhat on the fence with this one... On one end, we
are checking the sock write space, but don't check the queued
requests. And, this is purely advisory and not really a check
we rely on.

The merit of doing something like this is that we don't start
the request timer, but we can just as easily queue the request
and have it later queued for long due to sock being overloaded.

Can you explain your thoughts to why this is a good solution?

> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   drivers/nvme/host/tcp.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index ede76a0719a0..606565a4c708 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -2464,6 +2464,9 @@ static blk_status_t nvme_tcp_queue_rq(struct blk_mq_hw_ctx *hctx,
>   	if (unlikely(ret))
>   		return ret;
>   
> +	if (!sk_stream_is_writeable(queue->sock->sk))
> +		return BLK_STS_RESOURCE;
> +
>   	blk_mq_start_request(rq);
>   
>   	nvme_tcp_queue_request(req, true, bd->last);