cifs: cancel all remaining requests if one failed in a compound

Message ID 20181221031820.30070-1-lsahlber@redhat.com
State New
Headers show
Series
  • cifs: cancel all remaining requests if one failed in a compound
Related show

Commit Message

Ronnie Sahlberg Dec. 21, 2018, 3:18 a.m.
If we failed to get a reply to a request in a compound we should cancel
the current request and also all further requests in the chain.

Reported-by: Pavel Shilovsky <piastryyy@gmail.com>
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/transport.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Steve French Dec. 21, 2018, 3:19 a.m. | #1
shouldn't this be cc:stable

On Thu, Dec 20, 2018 at 9:18 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> If we failed to get a reply to a request in a compound we should cancel
> the current request and also all further requests in the chain.
>
> Reported-by: Pavel Shilovsky <piastryyy@gmail.com>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/transport.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 83ff0c25710d..ed6cd5066396 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -873,7 +873,11 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>
>         for (i = 0; i < num_rqst; i++) {
>                 rc = wait_for_response(ses->server, midQ[i]);
> -               if (rc != 0) {
> +               if (rc != 0)
> +                       break;
> +       }
> +       if (rc != 0) {
> +               for (; i < num_rqst; i++) {
>                         cifs_dbg(VFS, "Cancelling wait for mid %llu cmd: %d\n",
>                                  midQ[i]->mid, le16_to_cpu(midQ[i]->command));
>                         send_cancel(ses->server, &rqst[i], midQ[i]);
> @@ -881,9 +885,6 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>                         if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED) {
>                                 midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
>                                 midQ[i]->callback = DeleteMidQEntry;
> -                               spin_unlock(&GlobalMid_Lock);
> -                               add_credits(ses->server, 1, optype);
> -                               return rc;
>                         }
>                         spin_unlock(&GlobalMid_Lock);
>                 }
> @@ -892,8 +893,6 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>         for (i = 0; i < num_rqst; i++)
>                 if (midQ[i]->resp_buf)
>                         credits += ses->server->ops->get_credits(midQ[i]);
> -       if (!credits)
> -               credits = 1;
>
>         for (i = 0; i < num_rqst; i++) {
>                 if (rc < 0)
> --
> 2.13.6
>
Ronnie Sahlberg Dec. 21, 2018, 3:48 a.m. | #2
----- Original Message -----
> From: "Steve French" <smfrench@gmail.com>
> To: "Ronnie Sahlberg" <lsahlber@redhat.com>
> Cc: "Linux CIFS mailing list" <linux-cifs@vger.kernel.org>
> Sent: Friday, 21 December, 2018 1:19:01 PM
> Subject: Re: [PATCH] cifs: cancel all remaining requests if one failed in a compound
> 
> shouldn't this be cc:stable

Probably.
I would like Pavel to bless it first.


> 
> On Thu, Dec 20, 2018 at 9:18 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
> >
> > If we failed to get a reply to a request in a compound we should cancel
> > the current request and also all further requests in the chain.
> >
> > Reported-by: Pavel Shilovsky <piastryyy@gmail.com>
> > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > ---
> >  fs/cifs/transport.c | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> > index 83ff0c25710d..ed6cd5066396 100644
> > --- a/fs/cifs/transport.c
> > +++ b/fs/cifs/transport.c
> > @@ -873,7 +873,11 @@ compound_send_recv(const unsigned int xid, struct
> > cifs_ses *ses,
> >
> >         for (i = 0; i < num_rqst; i++) {
> >                 rc = wait_for_response(ses->server, midQ[i]);
> > -               if (rc != 0) {
> > +               if (rc != 0)
> > +                       break;
> > +       }
> > +       if (rc != 0) {
> > +               for (; i < num_rqst; i++) {
> >                         cifs_dbg(VFS, "Cancelling wait for mid %llu cmd:
> >                         %d\n",
> >                                  midQ[i]->mid,
> >                                  le16_to_cpu(midQ[i]->command));
> >                         send_cancel(ses->server, &rqst[i], midQ[i]);
> > @@ -881,9 +885,6 @@ compound_send_recv(const unsigned int xid, struct
> > cifs_ses *ses,
> >                         if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED) {
> >                                 midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
> >                                 midQ[i]->callback = DeleteMidQEntry;
> > -                               spin_unlock(&GlobalMid_Lock);
> > -                               add_credits(ses->server, 1, optype);
> > -                               return rc;
> >                         }
> >                         spin_unlock(&GlobalMid_Lock);
> >                 }
> > @@ -892,8 +893,6 @@ compound_send_recv(const unsigned int xid, struct
> > cifs_ses *ses,
> >         for (i = 0; i < num_rqst; i++)
> >                 if (midQ[i]->resp_buf)
> >                         credits += ses->server->ops->get_credits(midQ[i]);
> > -       if (!credits)
> > -               credits = 1;
> >
> >         for (i = 0; i < num_rqst; i++) {
> >                 if (rc < 0)
> > --
> > 2.13.6
> >
> 
> 
> --
> Thanks,
> 
> Steve
>
Pavel Shilovsky Dec. 21, 2018, 7:54 p.m. | #3
Once we undo the add_credits removal, I think it is good to go to stable.

--
Best regards,
Pavel Shilovsky

пт, 21 дек. 2018 г. в 00:53, Ronnie Sahlberg <lsahlber@redhat.com>:
>
>
>
>
>
> ----- Original Message -----
> > From: "Steve French" <smfrench@gmail.com>
> > To: "Ronnie Sahlberg" <lsahlber@redhat.com>
> > Cc: "Linux CIFS mailing list" <linux-cifs@vger.kernel.org>
> > Sent: Friday, 21 December, 2018 1:19:01 PM
> > Subject: Re: [PATCH] cifs: cancel all remaining requests if one failed in a compound
> >
> > shouldn't this be cc:stable
>
> Probably.
> I would like Pavel to bless it first.
>
>
> >
> > On Thu, Dec 20, 2018 at 9:18 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
> > >
> > > If we failed to get a reply to a request in a compound we should cancel
> > > the current request and also all further requests in the chain.
> > >
> > > Reported-by: Pavel Shilovsky <piastryyy@gmail.com>
> > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > > ---
> > >  fs/cifs/transport.c | 11 +++++------
> > >  1 file changed, 5 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> > > index 83ff0c25710d..ed6cd5066396 100644
> > > --- a/fs/cifs/transport.c
> > > +++ b/fs/cifs/transport.c
> > > @@ -873,7 +873,11 @@ compound_send_recv(const unsigned int xid, struct
> > > cifs_ses *ses,
> > >
> > >         for (i = 0; i < num_rqst; i++) {
> > >                 rc = wait_for_response(ses->server, midQ[i]);
> > > -               if (rc != 0) {
> > > +               if (rc != 0)
> > > +                       break;
> > > +       }
> > > +       if (rc != 0) {
> > > +               for (; i < num_rqst; i++) {
> > >                         cifs_dbg(VFS, "Cancelling wait for mid %llu cmd:
> > >                         %d\n",
> > >                                  midQ[i]->mid,
> > >                                  le16_to_cpu(midQ[i]->command));
> > >                         send_cancel(ses->server, &rqst[i], midQ[i]);
> > > @@ -881,9 +885,6 @@ compound_send_recv(const unsigned int xid, struct
> > > cifs_ses *ses,
> > >                         if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED) {
> > >                                 midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
> > >                                 midQ[i]->callback = DeleteMidQEntry;
> > > -                               spin_unlock(&GlobalMid_Lock);
> > > -                               add_credits(ses->server, 1, optype);
> > > -                               return rc;
> > >                         }
> > >                         spin_unlock(&GlobalMid_Lock);
> > >                 }
> > > @@ -892,8 +893,6 @@ compound_send_recv(const unsigned int xid, struct
> > > cifs_ses *ses,
> > >         for (i = 0; i < num_rqst; i++)
> > >                 if (midQ[i]->resp_buf)
> > >                         credits += ses->server->ops->get_credits(midQ[i]);
> > > -       if (!credits)
> > > -               credits = 1;
> > >
> > >         for (i = 0; i < num_rqst; i++) {
> > >                 if (rc < 0)
> > > --
> > > 2.13.6
> > >
> >
> >
> > --
> > Thanks,
> >
> > Steve
> >

Patch

diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 83ff0c25710d..ed6cd5066396 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -873,7 +873,11 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 
 	for (i = 0; i < num_rqst; i++) {
 		rc = wait_for_response(ses->server, midQ[i]);
-		if (rc != 0) {
+		if (rc != 0)
+			break;
+	}
+	if (rc != 0) {
+		for (; i < num_rqst; i++) {
 			cifs_dbg(VFS, "Cancelling wait for mid %llu cmd: %d\n",
 				 midQ[i]->mid, le16_to_cpu(midQ[i]->command));
 			send_cancel(ses->server, &rqst[i], midQ[i]);
@@ -881,9 +885,6 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 			if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED) {
 				midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
 				midQ[i]->callback = DeleteMidQEntry;
-				spin_unlock(&GlobalMid_Lock);
-				add_credits(ses->server, 1, optype);
-				return rc;
 			}
 			spin_unlock(&GlobalMid_Lock);
 		}
@@ -892,8 +893,6 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 	for (i = 0; i < num_rqst; i++)
 		if (midQ[i]->resp_buf)
 			credits += ses->server->ops->get_credits(midQ[i]);
-	if (!credits)
-		credits = 1;
 
 	for (i = 0; i < num_rqst; i++) {
 		if (rc < 0)