Re: [PATCH v2] xhci: Allocate the td array and urb_priv together.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 18, 2013 at 3:24 AM, David Laight <David.Laight@xxxxxxxxxx> wrote:
>
> This saves a kzalloc() call on every transfer and some memory
> indirections.
>
> The only possible downside is for isochronous tranfers with 64 td
> when the allocate is 8+4096 bytes (on 64bit systems) so requires
> an additional page.
>
> Signed-off-by: David Laight <david.laight@xxxxxxxxxx>
> ---
> <snip>


>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 6e0d886..0969f74 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1248,12 +1248,11 @@ static int xhci_check_maxpacket(struct xhci_hcd *xhci, unsigned int slot_id,
>  int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
>  {
>         struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> -       struct xhci_td *buffer;
>         unsigned long flags;
>         int ret = 0;
>         unsigned int slot_id, ep_index;
>         struct urb_priv *urb_priv;
> -       int size, i;
> +       int size;
>
>         if (!urb || xhci_check_args(hcd, urb->dev, urb->ep,
>                                         true, true, __func__) <= 0)
> @@ -1275,21 +1274,10 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
>                 size = 1;
>
>         urb_priv = kzalloc(sizeof(struct urb_priv) +
> -                                 size * sizeof(struct xhci_td *), mem_flags);
> +                          size * sizeof(struct xhci_td), mem_flags);
>         if (!urb_priv)
>                 return -ENOMEM;
>
> -       buffer = kzalloc(size * sizeof(struct xhci_td), mem_flags);
> -       if (!buffer) {


Hi David,

Are you sure this will not cause weird bugs on non-cache coherent
systems? You are smashing two memory allocations into one. This does
not insure that the memory in each allocation is cache line aligned
does it? Arm and Mips systems don't like having two i/o actions or cpu
and i/o activity on data sharing a cache line.

Regards, Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux