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

[Alan Stern]

On Tue, Jun 30, 2026 at 01:27:08PM +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, make kfree() calls unconditional since kfree() can be passed a
> NULL pointer.

That is not a good way of describing the last change.  The 
"kfree(buf->page)" call may indeed pass a NULL pointer, but the fact 
that it doesn't dereference a NULL pointer (i.e., buf) is not obvious.

> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@xxxxxxxxxx
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> ---
>  drivers/usb/host/ohci-dbg.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-dbg.c b/drivers/usb/host/ohci-dbg.c
> index 9e0e06bbc570..0a648a7d9136 100644
> --- a/drivers/usb/host/ohci-dbg.c
> +++ b/drivers/usb/host/ohci-dbg.c
> @@ -1,4 +1,5 @@
>  // SPDX-License-Identifier: GPL-1.0+
> +#include <linux/slab.h>

Thanks to the odd way in which ohci-hcd is built, this line is 
completely unnecessary.  It should not be part of the patch.

Alan Stern

>  /*
>   * OHCI HCD (Host Controller Driver) for USB.
>   *
> @@ -683,7 +684,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 +730,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;
>  }
> @@ -782,4 +780,3 @@ static inline void remove_debug_files (struct ohci_hcd *ohci)
>  }
>  
>  /*-------------------------------------------------------------------------*/
> -
> 
> -- 
> 2.53.0
>