Hi, Peter Chen <[email protected]> writes: > On Tue, Nov 01, 2016 at 01:29:59PM +0200, Felipe Balbi wrote: >> According to Dave Miller "the networking stack has a >> hard requirement that all SKBs which are transmitted >> must have their completion signalled in a fininte >> amount of time. This is because, until the SKB is >> freed by the driver, it holds onto socket, >> netfilter, and other subsystem resources." >> >> In summary, this means that using TX IRQ throttling >> for the networking gadgets is, at least, complex and >> we should avoid it for the time being. >> >> Cc: <[email protected]> >> Reported-by: Ville Syrjälä <[email protected]> >> Suggested-by: David Miller <[email protected]> >> Signed-off-by: Felipe Balbi <[email protected]> >> --- >> drivers/usb/gadget/function/u_ether.c | 8 -------- >> 1 file changed, 8 deletions(-) >> >> diff --git a/drivers/usb/gadget/function/u_ether.c >> b/drivers/usb/gadget/function/u_ether.c >> index f4a640216913..119a2e5848e8 100644 >> --- a/drivers/usb/gadget/function/u_ether.c >> +++ b/drivers/usb/gadget/function/u_ether.c >> @@ -589,14 +589,6 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb, >> >> req->length = length; >> >> - /* throttle high/super speed IRQ rate back slightly */ >> - if (gadget_is_dualspeed(dev->gadget)) >> - req->no_interrupt = (((dev->gadget->speed == USB_SPEED_HIGH || >> - dev->gadget->speed == USB_SPEED_SUPER)) >> && >> - !list_empty(&dev->tx_reqs)) >> - ? ((atomic_read(&dev->tx_qlen) % dev->qmult) != 0) >> - : 0; >> - >> retval = usb_ep_queue(in, req, GFP_ATOMIC); >> switch (retval) { >> default: >> -- > > Felipe, it may increase cpu utilization since more interrupts will be there, > it may affect the SoC which has lower cpu frequency. This code existed > many years, why this problem has only reported at dwc3 recently?
No idea, but at least for networking gadgets we shouldn't throttle. This
has been a bug since the beginning. Read Dave Miller's explanation at
[1]
moreover, dwc3 seems to be the only one actually throttling IRQ. Here's
a rundown of a few of the UDCs:
- chipidea: uses TD_IOC conditionally, but always sets TD_TERMINATE
lastnode->ptr->next = cpu_to_le32(TD_TERMINATE);
if (!hwreq->req.no_interrupt)
lastnode->ptr->token |= cpu_to_le32(TD_IOC);
I'm guessing TD_TERMINATE works similar to dwc3's LST bit. If
it's set, it will force an interrupt.
- musb: no_interrupt only used for tracing
- atmel_usba_udc: no_interrupt only used for tracing
- mv_u3d_core: probably throttles interrupt, but probably exhibits same
behavior. It's just that it hasn't been reported.
- fsl_udc_core: probably throttles interrupt, but probably exhibits same
behavior. It's just that it hasn't been reported.
and there are the only uses for the no_interrupt flag.
[1] https://marc.info/?l=linux-usb&m=147793992922064&w=2
--
balbi
signature.asc
Description: PGP signature
