diff mbox series

[v1,1/1] usb: gadget: configfs: Use memcpy_and_pad()

Message ID 20230202151736.64552-1-andriy.shevchenko@linux.intel.com
State New
Headers
Series [v1,1/1] usb: gadget: configfs: Use memcpy_and_pad() |

Commit Message

Andy Shevchenko Feb. 2, 2023, 3:17 p.m. UTC 
  Instead of zeroing some memory and then copying data in part or all of it,
use memcpy_and_pad().
This avoids writing some memory twice and should save a few cycles.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/usb/gadget/configfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

David Laight Feb. 2, 2023, 10:21 p.m. UTC | #1 
From: Andy Shevchenko
> Sent: 02 February 2023 15:18
> 
> Instead of zeroing some memory and then copying data in part or all of it,
> use memcpy_and_pad().
> This avoids writing some memory twice and should save a few cycles.

Maybe, maybe not.
It rather depends on the lengths involved (the code doesn't seem to be in the
main tree).

The cost of the conditionals and the misaligned length/start for the
memset() could easily overwhelm any apparent saving.

A memset() of a constant whole number of words is going to be significantly
faster than the partial one.

	David

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/usb/gadget/configfs.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
> index 1346b330b358..0ee47e4c22cb 100644
> --- a/drivers/usb/gadget/configfs.c
> +++ b/drivers/usb/gadget/configfs.c
> @@ -909,8 +909,7 @@ static ssize_t webusb_landingPage_store(struct config_item *item, const char *pa
> 
>  	mutex_lock(&gi->lock);
>  	// ensure 0 bytes are set, in case the new landing page is shorter then the old one.
> -	memset(gi->landing_page, 0, sizeof(gi->landing_page));
> -	memcpy(gi->landing_page, page, l);
> +	memcpy_and_pad(gi->landing_page, sizeof(gi->landing_page), page, l, 0);
>  	mutex_unlock(&gi->lock);
> 
>  	return len;
> --
> 2.39.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Andy Shevchenko Feb. 3, 2023, 11:27 a.m. UTC | #2 
On Thu, Feb 02, 2023 at 10:21:09PM +0000, David Laight wrote:
> From: Andy Shevchenko
> > Sent: 02 February 2023 15:18
> > 
> > Instead of zeroing some memory and then copying data in part or all of it,
> > use memcpy_and_pad().
> > This avoids writing some memory twice and should save a few cycles.
> 
> Maybe, maybe not.
> It rather depends on the lengths involved (the code doesn't seem to be in the
> main tree).
> 
> The cost of the conditionals and the misaligned length/start for the
> memset() could easily overwhelm any apparent saving.
> 
> A memset() of a constant whole number of words is going to be significantly
> faster than the partial one.


Then you can put some (little I suppose) efforts in optimizing memcpy_and_pad()
once for all, no?
diff mbox series

Patch

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 1346b330b358..0ee47e4c22cb 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -909,8 +909,7 @@  static ssize_t webusb_landingPage_store(struct config_item *item, const char *pa
 
 	mutex_lock(&gi->lock);
 	// ensure 0 bytes are set, in case the new landing page is shorter then the old one.
-	memset(gi->landing_page, 0, sizeof(gi->landing_page));
-	memcpy(gi->landing_page, page, l);
+	memcpy_and_pad(gi->landing_page, sizeof(gi->landing_page), page, l, 0);
 	mutex_unlock(&gi->lock);
 
 	return len;