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

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

[Andy Shevchenko]

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(-)

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

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

[David Laight]

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)

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

['Andy Shevchenko']

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?

-- 
With Best Regards,
Andy Shevchenko