RE: [PATCH] cifs: avoid a kmalloc in smb2_send_recv/SendReceive2 for the common case

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

 




2017-11-20 20:08 GMT-08:00 Ronnie Sahlberg <lsahlber@xxxxxxxxxx>:
> In both functions, use an array of 8 (arbitrary but should be big enough
> for all current uses) iov and avoid having to kmalloc the array
> for the common case.
>
> If 8 is too small, then fall back to the original behaviour and use
> kmalloc/kfree.
>
> This should not change any behaviour but should save us a tiny amount of
> cpu cycles.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> ---
>  fs/cifs/transport.c | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index e678307bb7a0..510f41a435c8 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -38,6 +38,9 @@
>  #include "cifsproto.h"
>  #include "cifs_debug.h"
>
> +/* Max number of iovectors we can use off the stack when sending requests. */
> +#define CIFS_MAX_IOV_SIZE 8
> +
>  void
>  cifs_wake_up_task(struct mid_q_entry *mid)
>  {
> @@ -803,12 +806,16 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>              const int flags, struct kvec *resp_iov)
>  {
>         struct smb_rqst rqst;
> -       struct kvec *new_iov;
> +       struct kvec s_iov[CIFS_MAX_IOV_SIZE], *new_iov;
>         int rc;
>
> -       new_iov = kmalloc(sizeof(struct kvec) * (n_vec + 1), GFP_KERNEL);
> -       if (!new_iov)
> -               return -ENOMEM;
> +       if (n_vec + 1 > CIFS_MAX_IOV_SIZE) {
> +               new_iov = kmalloc(sizeof(struct kvec) * (n_vec + 1),
> +                                 GFP_KERNEL);
> +               if (!new_iov)
> +                       return -ENOMEM;
> +       } else
> +               new_iov = s_iov;
>
>         /* 1st iov is a RFC1001 length followed by the rest of the packet */
>         memcpy(new_iov + 1, iov, (sizeof(struct kvec) * n_vec));
> @@ -823,7 +830,8 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>         rqst.rq_nvec = n_vec + 1;
>
>         rc = cifs_send_recv(xid, ses, &rqst, resp_buf_type, flags, resp_iov);
> -       kfree(new_iov);
> +       if (n_vec + 1 > CIFS_MAX_IOV_SIZE)
> +               kfree(new_iov);
>         return rc;
>  }
>
> @@ -834,15 +842,19 @@ smb2_send_recv(const unsigned int xid, struct cifs_ses *ses,
>                const int flags, struct kvec *resp_iov)
>  {
>         struct smb_rqst rqst;
> -       struct kvec *new_iov;
> +       struct kvec s_iov[CIFS_MAX_IOV_SIZE], *new_iov;
>         int rc;
>         int i;
>         __u32 count;
>         __be32 rfc1002_marker;
>
> -       new_iov = kmalloc(sizeof(struct kvec) * (n_vec + 1), GFP_KERNEL);
> -       if (!new_iov)
> -               return -ENOMEM;
> +       if (n_vec + 1 > CIFS_MAX_IOV_SIZE) {
> +               new_iov = kmalloc(sizeof(struct kvec) * (n_vec + 1),
> +                                 GFP_KERNEL);
> +               if (!new_iov)
> +                       return -ENOMEM;
> +       } else
> +               new_iov = s_iov;
>
>         /* 1st iov is an RFC1002 Session Message length */
>         memcpy(new_iov + 1, iov, (sizeof(struct kvec) * n_vec));
> @@ -861,7 +873,8 @@ smb2_send_recv(const unsigned int xid, struct cifs_ses *ses,
>         rqst.rq_nvec = n_vec + 1;
>
>         rc = cifs_send_recv(xid, ses, &rqst, resp_buf_type, flags, resp_iov);
> -       kfree(new_iov);
> +       if (n_vec + 1 > CIFS_MAX_IOV_SIZE)
> +               kfree(new_iov);
>         return rc;
>  }
>
> --
> 2.13.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks for the patch!

Should we split it into two and mark the 1st part for stable? This would address possible performance degradation in previous kernels because SendReceive2() was changed to use kmalloc() several kernels ago. I didn't measure a performance difference, so not sure if it is worth the effort.

In both cases (as one patch or two), you can add my

Reviewed-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx>

--
Best regards,
Pavel Shilovsky
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




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

  Powered by Linux