Re: [PATCH] usb: gadget: u_ether: enable qmult on SuperSpeed Plus as well

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

 



btw. it looks like irq throttling in the same file:

@@ -598,9 +599,10 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
-       /* throttle highspeed IRQ rate back slightly */
+       /* throttle high/super speed IRQ rate back slightly */
        if (gadget_is_dualspeed(dev->gadget))
-               req->no_interrupt = (dev->gadget->speed == USB_SPEED_HIGH)
+               req->no_interrupt = (dev->gadget->speed == USB_SPEED_HIGH ||
+                                    dev->gadget->speed == USB_SPEED_SUPER)

should also be fixed to be >= SUPER and not ==.

On Thu, Aug 6, 2020 at 10:29 AM Maciej Żenczykowski
<zenczykowski@xxxxxxxxx> wrote:
>
> On Thu, Aug 6, 2020 at 9:17 AM Lorenzo Colitti <lorenzo@xxxxxxxxxx> wrote:
> >
> > The u_ether driver has a qmult setting that multiplies the
> > transmit queue length (which by default is 2).
> >
> > The intent is that it should be enabled at high/super speed, but
> > because the code does not explicitly check for USB_SUPER_PLUS,
> > it is disabled at that speed.
> >
> > Fix this by ensuring that the queue multiplier is enabled for any
> > wired link at high speed or above. Using >= for USB_SPEED_*
> > constants seems correct because it is what the gadget_is_xxxspeed
> > functions do.
> >
> > The queue multiplier substantially helps performance at higher
> > speeds. On a direct SuperSpeed Plus link to a Linux laptop,
> > iperf3 single TCP stream:
> >
> > Before (qmult=1): 1.3 Gbps
> > After  (qmult=5): 3.2 Gbps
> >
> > Signed-off-by: Lorenzo Colitti <lorenzo@xxxxxxxxxx>
> > ---
> >  drivers/usb/gadget/function/u_ether.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
> > index c3cc6bd14e..31ea76adcc 100644
> > --- a/drivers/usb/gadget/function/u_ether.c
> > +++ b/drivers/usb/gadget/function/u_ether.c
> > @@ -93,7 +93,7 @@ struct eth_dev {
> >  static inline int qlen(struct usb_gadget *gadget, unsigned qmult)
> >  {
> >         if (gadget_is_dualspeed(gadget) && (gadget->speed == USB_SPEED_HIGH ||
> > -                                           gadget->speed == USB_SPEED_SUPER))
> > +                                           gadget->speed >= USB_SPEED_SUPER))
> >                 return qmult * DEFAULT_QLEN;
> >         else
> >                 return DEFAULT_QLEN;
> > --
> > 2.28.0.163.g6104cc2f0b6-goog
> >
>
> Reviewed-by: Maciej Żenczykowski <maze@xxxxxxxxxx>
>
> Though I think this probably deserves a fixes tag of some sort.
> Probably:
>
> Fixes: 04617db7aa68 ("usb: gadget: add SS descriptors to Ethernet gadget")
>
> since that's the commit that did:
>
> -MODULE_PARM_DESC(qmult, "queue length multiplier at high speed");
> +MODULE_PARM_DESC(qmult, "queue length multiplier at high/super speed");
>
> ...
>
> -/* for dual-speed hardware, use deeper queues at highspeed */
> +/* for dual-speed hardware, use deeper queues at high/super speed */
>  static inline int qlen(struct usb_gadget *gadget)
>  {
> -       if (gadget_is_dualspeed(gadget) && gadget->speed == USB_SPEED_HIGH)
> +       if (gadget_is_dualspeed(gadget) && (gadget->speed == USB_SPEED_HIGH ||
> +                                           gadget->speed == USB_SPEED_SUPER))
>                 return qmult * DEFAULT_QLEN;
>         else
>                 return DEFAULT_QLEN;




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

  Powered by Linux