Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 21 Apr 2022 18:48:40 +0200
From:   Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To:     Tao Wang <quic_wat@quicinc.com>
Cc:     Mathias Nyman <mathias.nyman@intel.com>,
        "open list:USB SUBSYSTEM" <linux-usb@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        quic_linyyuan@quicinc.com
Subject: Re: [PATCH] usb: hcd: add a flag for whether using shared_hcd priv
Message-ID: <YmGK6DXDdM5vx3Qe@kroah.com>
References: <1649299514-64014-1-git-send-email-quic_wat@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1649299514-64014-1-git-send-email-quic_wat@quicinc.com>
Precedence: bulk

On Thu, Apr 07, 2022 at 10:45:14AM +0800, Tao Wang wrote:
> The shared_hcd->hcd_priv is not used in xhci, so not
> need to malloc hcd priv memory for shared_hcd.

What is the problem with allocating that memory?  Do you have systems
with thousands of USB host controllers where this memory is wasted?

> This change add a shared_hcd_no_priv flag to indicate
> if shared_hcd use priv, and set the flag of xhci to 1.

1 is not a boolean :(

> 
> Signed-off-by: Tao Wang <quic_wat@quicinc.com>
> ---
>  drivers/usb/core/hcd.c       | 6 +++++-
>  drivers/usb/host/xhci-plat.c | 1 +
>  include/linux/usb/hcd.h      | 1 +
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 99908d8..f339c3e 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2419,7 +2419,11 @@ struct usb_hcd *__usb_create_hcd(const struct hc_driver *driver,
>  {
>  	struct usb_hcd *hcd;
>  
> -	hcd = kzalloc(sizeof(*hcd) + driver->hcd_priv_size, GFP_KERNEL);
> +	if (primary_hcd && driver->shared_hcd_no_priv)
> +		hcd = kzalloc(sizeof(*hcd), GFP_KERNEL);
> +	else
> +		hcd = kzalloc(sizeof(*hcd) + driver->hcd_priv_size, GFP_KERNEL);
> +
>  	if (!hcd)
>  		return NULL;
>  	if (primary_hcd == NULL) {
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 21280a6..223e508 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -236,6 +236,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  
>  	driver = &xhci_plat_hc_driver;
> +	driver->shared_hcd_no_priv = 1;

true?

>  
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0)
> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> index 2ffafbd..03ac312 100644
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -256,6 +256,7 @@ struct hc_driver {
>  	const char	*description;	/* "ehci-hcd" etc */
>  	const char	*product_desc;	/* product/vendor string */
>  	size_t		hcd_priv_size;	/* size of private data */
> +	bool		shared_hcd_no_priv;	/* 0 use priv, 1 not use priv*/

Did you check to see how much extra padding you just added to the
structure?  When writing a change to try to save memory, do not cause
extra memory to be wasted for everyone.

thanks,

greg k-h