Re: [PATCH] CIFS: Wait for credits if at least one request is in flight

[Tom Talpey <tom@talpey.com>]

It's reasonable to fail a request that can never have sufficient
credits to send, but EOPNOTSUPP is a really strange error to return.
The operation might work if the payload were smaller, right? So,
would a resource error such as EDEADLK be more meaningful, and allow
the caller to recover, even?

Also, can you elaborate on why this is only triggered when no
requests at all are in flight? Or is this some kind of corner
case for requests that need every credit the server currently
is offering?

Tom.

On 2/1/2021 8:01 PM, Pavel Shilovsky wrote:
> Currently we try to guess if a compound request is going to succeed
> waiting for credits or not based on the number of requests in flight.
> This approach doesn't work correctly all the time because there may
> be only one request in flight which is going to bring multiple credits
> satisfying the compound request.
> 
> Change the behavior to fail a request only if there are no requests
> in flight at all and proceed waiting for credits otherwise.
> 
> Cc: <stable@vger.kernel.org> # 5.1+
> Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
> ---
>   fs/cifs/transport.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 4ffbf8f965814..84f33fdd1f4e0 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -659,10 +659,10 @@ wait_for_compound_request(struct TCP_Server_Info *server, int num,
>   	spin_lock(&server->req_lock);
>   	if (*credits < num) {
>   		/*
> -		 * Return immediately if not too many requests in flight since
> -		 * we will likely be stuck on waiting for credits.
> +		 * Return immediately if no requests in flight since
> +		 * we will be stuck on waiting for credits.
>   		 */
> -		if (server->in_flight < num - *credits) {
> +		if (server->in_flight == 0) {
>   			spin_unlock(&server->req_lock);
>   			return -ENOTSUPP;
>   		}
>