Re: [PATCH v2 1/2] usb: host: ohci-dbg: use kmalloc() for print buffer

[Alan Stern]

On Wed, Jul 01, 2026 at 12:41:50PM +0300, Mike Rapoport (Microsoft) wrote:
> ochi-dbg allocates buffers for formatting of various dump outputs.
> 
> These buffers can be allocated with kmalloc() as there's nothing special
> about them 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_zeroed_page() with kzalloc() and free_page() with
> kfree().
> 
> While on it, drop the NULL checks in debug_close(). buf is never NULL
> here because all the open handlers return -ENOMEM when alloc_buffer()
> fails, and kfree() can handle a NULL buf->page.
> 
> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@xxxxxxxxxx
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> ---

Reviewed-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

Incidentally, I filtered it out of my reply this time, but on this and 
previous submissions you have a CC: entry that says <rpppt@xxxxxxxxxx>, 
which is an invalid address (three 'p's instead of two).  Is there a 
typo in one of your scripts or settings?  It's a little surprising that 
you aren't constantly getting email error messages every time you send 
something out.

Alan Stern

>  drivers/usb/host/ohci-dbg.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-dbg.c b/drivers/usb/host/ohci-dbg.c
> index 9e0e06bbc570..23dc9eddc06c 100644
> --- a/drivers/usb/host/ohci-dbg.c
> +++ b/drivers/usb/host/ohci-dbg.c
> @@ -683,7 +683,7 @@ static int fill_buffer(struct debug_buffer *buf)
>  	int ret;
>  
>  	if (!buf->page)
> -		buf->page = (char *)get_zeroed_page(GFP_KERNEL);
> +		buf->page = kzalloc(PAGE_SIZE, GFP_KERNEL);
>  
>  	if (!buf->page) {
>  		ret = -ENOMEM;
> @@ -729,11 +729,8 @@ static int debug_close(struct inode *inode, struct file *file)
>  {
>  	struct debug_buffer *buf = file->private_data;
>  
> -	if (buf) {
> -		if (buf->page)
> -			free_page((unsigned long)buf->page);
> -		kfree(buf);
> -	}
> +	kfree(buf->page);
> +	kfree(buf);
>  
>  	return 0;
>  }
> 
> -- 
> 2.53.0
>