Re: [PATCH] CIFS: Reset current mid to next value in sequence on send failures

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Awesome!

Reviewed-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>

On Thu, Feb 21, 2019 at 9:38 AM Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>
> When we hit failures during constructing MIDs or sending PDUs
> through the network, we end up not using the message IDs assigned
> to the packet. The next SMB packet will skip those message IDs and
> continue with the next one. This behavior may lead to a server not
> granting us credits until we use the skipped IDs. Fix this by
> reverting the current ID to the original value if any errors occur
> before we push the packet through the network stack.
>
> This patch fixes the generic/310 test from the xfs-tests.
>
> Signed-off-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx>
> ---
>  fs/cifs/cifsglob.h      | 15 +++++++++++++++
>  fs/cifs/smb2ops.c       | 12 ++++++++++++
>  fs/cifs/smb2transport.c | 13 +++++++++++--
>  fs/cifs/transport.c     |  6 +++++-
>  4 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 5bf463c..10be31e 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -238,6 +238,8 @@ struct smb_version_operations {
>         int * (*get_credits_field)(struct TCP_Server_Info *, const int);
>         unsigned int (*get_credits)(struct mid_q_entry *);
>         __u64 (*get_next_mid)(struct TCP_Server_Info *);
> +       void (*revert_current_mid)(struct TCP_Server_Info *server,
> +                                  const unsigned int val);
>         /* data offset from read response message */
>         unsigned int (*read_data_offset)(char *);
>         /*
> @@ -789,6 +791,19 @@ get_next_mid(struct TCP_Server_Info *server)
>         return cpu_to_le16(mid);
>  }
>
> +static inline void
> +revert_current_mid(struct TCP_Server_Info *server, const unsigned int val)
> +{
> +       if (server->ops->revert_current_mid)
> +               server->ops->revert_current_mid(server, val);
> +}
> +
> +static inline void
> +dec_current_mid(struct TCP_Server_Info *server)
> +{
> +       return revert_current_mid(server, 1);
> +}
> +
>  static inline __u16
>  get_mid(const struct smb_hdr *smb)
>  {
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 080929a..1243407 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -265,6 +265,14 @@ smb2_get_next_mid(struct TCP_Server_Info *server)
>         return mid;
>  }
>
> +static void
> +smb2_revert_current_mid(struct TCP_Server_Info *server, const unsigned int val)
> +{
> +       spin_lock(&GlobalMid_Lock);
> +       server->CurrentMid -= val;
> +       spin_unlock(&GlobalMid_Lock);
> +}
> +
>  static struct mid_q_entry *
>  smb2_find_mid(struct TCP_Server_Info *server, char *buf)
>  {
> @@ -3606,6 +3614,7 @@ struct smb_version_operations smb20_operations = {
>         .get_credits = smb2_get_credits,
>         .wait_mtu_credits = cifs_wait_mtu_credits,
>         .get_next_mid = smb2_get_next_mid,
> +       .revert_current_mid = smb2_revert_current_mid,
>         .read_data_offset = smb2_read_data_offset,
>         .read_data_length = smb2_read_data_length,
>         .map_error = map_smb2_to_linux_error,
> @@ -3702,6 +3711,7 @@ struct smb_version_operations smb21_operations = {
>         .wait_mtu_credits = smb2_wait_mtu_credits,
>         .adjust_credits = smb2_adjust_credits,
>         .get_next_mid = smb2_get_next_mid,
> +       .revert_current_mid = smb2_revert_current_mid,
>         .read_data_offset = smb2_read_data_offset,
>         .read_data_length = smb2_read_data_length,
>         .map_error = map_smb2_to_linux_error,
> @@ -3799,6 +3809,7 @@ struct smb_version_operations smb30_operations = {
>         .wait_mtu_credits = smb2_wait_mtu_credits,
>         .adjust_credits = smb2_adjust_credits,
>         .get_next_mid = smb2_get_next_mid,
> +       .revert_current_mid = smb2_revert_current_mid,
>         .read_data_offset = smb2_read_data_offset,
>         .read_data_length = smb2_read_data_length,
>         .map_error = map_smb2_to_linux_error,
> @@ -3905,6 +3916,7 @@ struct smb_version_operations smb311_operations = {
>         .wait_mtu_credits = smb2_wait_mtu_credits,
>         .adjust_credits = smb2_adjust_credits,
>         .get_next_mid = smb2_get_next_mid,
> +       .revert_current_mid = smb2_revert_current_mid,
>         .read_data_offset = smb2_read_data_offset,
>         .read_data_length = smb2_read_data_length,
>         .map_error = map_smb2_to_linux_error,
> diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
> index 7b351c6..36baf1b 100644
> --- a/fs/cifs/smb2transport.c
> +++ b/fs/cifs/smb2transport.c
> @@ -674,13 +674,18 @@ smb2_setup_request(struct cifs_ses *ses, struct smb_rqst *rqst)
>         smb2_seq_num_into_buf(ses->server, shdr);
>
>         rc = smb2_get_mid_entry(ses, shdr, &mid);
> -       if (rc)
> +       if (rc) {
> +               dec_current_mid(ses->server);
>                 return ERR_PTR(rc);
> +       }
> +
>         rc = smb2_sign_rqst(rqst, ses->server);
>         if (rc) {
> +               dec_current_mid(ses->server);
>                 cifs_delete_mid(mid);
>                 return ERR_PTR(rc);
>         }
> +
>         return mid;
>  }
>
> @@ -690,16 +695,20 @@ smb2_setup_async_request(struct TCP_Server_Info *server, struct smb_rqst *rqst)
>         int rc;
>         struct smb2_sync_hdr *shdr =
>                         (struct smb2_sync_hdr *)rqst->rq_iov[0].iov_base;
> +       unsigned int num = le16_to_cpu(shdr->CreditCharge);
>         struct mid_q_entry *mid;
>
>         smb2_seq_num_into_buf(server, shdr);
>
>         mid = smb2_mid_entry_alloc(shdr, server);
> -       if (mid == NULL)
> +       if (mid == NULL) {
> +               revert_current_mid(server, num);
>                 return ERR_PTR(-ENOMEM);
> +       }
>
>         rc = smb2_sign_rqst(rqst, server);
>         if (rc) {
> +               revert_current_mid(server, num);
>                 DeleteMidQEntry(mid);
>                 return ERR_PTR(rc);
>         }
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index d4f1224f..849a45d 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -695,6 +695,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst,
>         cifs_in_send_dec(server);
>
>         if (rc < 0) {
> +               revert_current_mid(server, credits.value);
>                 server->sequence_number -= 2;
>                 cifs_delete_mid(mid);
>         }
> @@ -988,6 +989,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>         for (i = 0; i < num_rqst; i++) {
>                 midQ[i] = ses->server->ops->setup_request(ses, &rqst[i]);
>                 if (IS_ERR(midQ[i])) {
> +                       revert_current_mid(ses->server, i);
>                         for (j = 0; j < i; j++)
>                                 cifs_delete_mid(midQ[j]);
>                         mutex_unlock(&ses->server->srv_mutex);
> @@ -1017,8 +1019,10 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>         for (i = 0; i < num_rqst; i++)
>                 cifs_save_when_sent(midQ[i]);
>
> -       if (rc < 0)
> +       if (rc < 0) {
> +               revert_current_mid(ses->server, num_rqst);
>                 ses->server->sequence_number -= 2;
> +       }
>
>         mutex_unlock(&ses->server->srv_mutex);
>
> --
> 2.7.4
>



[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux