| Message ID | 20210202195538.243256-1-pshilov@microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] CIFS: Wait for credits if at least one request is in flight | expand |
Updated comment looks good! Reviewed-By: Tom Talpey <tom@talpey.com> On 2/2/2021 2:55 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 | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c > index 4ffbf8f965814..eab7940bfebef 100644 > --- a/fs/cifs/transport.c > +++ b/fs/cifs/transport.c > @@ -659,10 +659,22 @@ 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. > + * If the server is tight on resources or just gives us less > + * credits for other reasons (e.g. requests are coming out of > + * order and the server delays granting more credits until it > + * processes a missing mid) and we exhausted most available > + * credits there may be situations when we try to send > + * a compound request but we don't have enough credits. At this > + * point the client needs to decide if it should wait for > + * additional credits or fail the request. If at least one > + * request is in flight there is a high probability that the > + * server will return enough credits to satisfy this compound > + * request. > + * > + * 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; > } >
updated patch (had minor merge conflict) and added the two reviewed-by and tentatively merged into cifs-2.6.git On Tue, Feb 2, 2021 at 3:37 PM Tom Talpey <tom@talpey.com> wrote: > > Updated comment looks good! > > Reviewed-By: Tom Talpey <tom@talpey.com> > > On 2/2/2021 2:55 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 | 18 +++++++++++++++--- > > 1 file changed, 15 insertions(+), 3 deletions(-) > > > > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c > > index 4ffbf8f965814..eab7940bfebef 100644 > > --- a/fs/cifs/transport.c > > +++ b/fs/cifs/transport.c > > @@ -659,10 +659,22 @@ 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. > > + * If the server is tight on resources or just gives us less > > + * credits for other reasons (e.g. requests are coming out of > > + * order and the server delays granting more credits until it > > + * processes a missing mid) and we exhausted most available > > + * credits there may be situations when we try to send > > + * a compound request but we don't have enough credits. At this > > + * point the client needs to decide if it should wait for > > + * additional credits or fail the request. If at least one > > + * request is in flight there is a high probability that the > > + * server will return enough credits to satisfy this compound > > + * request. > > + * > > + * 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; > > } > >
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index 4ffbf8f965814..eab7940bfebef 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -659,10 +659,22 @@ 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. + * If the server is tight on resources or just gives us less + * credits for other reasons (e.g. requests are coming out of + * order and the server delays granting more credits until it + * processes a missing mid) and we exhausted most available + * credits there may be situations when we try to send + * a compound request but we don't have enough credits. At this + * point the client needs to decide if it should wait for + * additional credits or fail the request. If at least one + * request is in flight there is a high probability that the + * server will return enough credits to satisfy this compound + * request. + * + * 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; }
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 | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)