Re: [PATCH 2/2] usb: core: devices: use kmalloc() to allocate dump buffer

[Alan Stern <stern@rowland.harvard.edu>]

On Tue, Jun 30, 2026 at 01:27:09PM +0300, Mike Rapoport (Microsoft) wrote:
> usb_device_dump() allocates a buffer for formatting
> /proc/bus/usb/devices output text.

You mean /sys/kernel/debug/usb/devices.  There is no /proc/bus/usb/ 
directory.

Aside from that mistake:

Reviewed-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern

> 
> This buffer can be allocated with kmalloc() as there's nothing special
> about it to go directly to the page allocator.
> 
> kmalloc() provides a better API that does not require ugly casts and
> kfree() does not need to know the size of the freed object.
> 
> Performance difference between kmalloc() and __get_free_pages() is not
> measurable as both allocators take an object/page from a per-CPU list for
> fast path allocations.
> 
> For the slow path the performance is anyway determined by the amount of
> reclaim involved rather than by what allocator is used.
> 
> Replace use of __get_free_pages() with kmalloc() and free_pages() with
> kfree().
> 
> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>  drivers/usb/core/devices.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
> index a247da73f34d..6f0354aba38b 100644
> --- a/drivers/usb/core/devices.c
> +++ b/drivers/usb/core/devices.c
> @@ -37,6 +37,7 @@
>   */
>  
>  #include <linux/fs.h>
> +#include <linux/slab.h>
>  #include <linux/mm.h>
>  #include <linux/gfp.h>
>  #include <linux/usb.h>
> @@ -408,7 +409,7 @@ static ssize_t usb_device_dump(char __user **buffer, size_t *nbytes,
>  		return 0;
>  	/* allocate 2^1 pages = 8K (on i386);
>  	 * should be more than enough for one device */
> -	pages_start = (char *)__get_free_pages(GFP_NOIO, 1);
> +	pages_start = kmalloc(PAGE_SIZE << 1, GFP_NOIO);
>  	if (!pages_start)
>  		return -ENOMEM;
>  
> @@ -479,7 +480,7 @@ static ssize_t usb_device_dump(char __user **buffer, size_t *nbytes,
>  		if (length > *nbytes)
>  			length = *nbytes;
>  		if (copy_to_user(*buffer, pages_start + *skip_bytes, length)) {
> -			free_pages((unsigned long)pages_start, 1);
> +			kfree(pages_start);
>  			return -EFAULT;
>  		}
>  		*nbytes -= length;
> @@ -490,7 +491,7 @@ static ssize_t usb_device_dump(char __user **buffer, size_t *nbytes,
>  	} else
>  		*skip_bytes -= length;
>  
> -	free_pages((unsigned long)pages_start, 1);
> +	kfree(pages_start);
>  
>  	/* Now look at all of this device's children. */
>  	usb_hub_for_each_child(usbdev, chix, childdev) {
> 
> -- 
> 2.53.0
>