From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f195.google.com ([209.85.192.195]:34738 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752547AbcKCAcc (ORCPT ); Wed, 2 Nov 2016 20:32:32 -0400 Date: Thu, 3 Nov 2016 08:32:26 +0800 From: Peter Chen To: Felipe Balbi Cc: Linux USB , David Miller , Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , stable@vger.kernel.org Subject: Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling Message-ID: <20161103003226.GB894@b29397-desktop> References: <20161101112959.19640-1-felipe.balbi@linux.intel.com> <20161102060202.GD28525@b29397-desktop> <87vaw6nwn3.fsf@linux.intel.com> <20161102083632.GA22181@b29397-desktop> <87lgx2nnz0.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87lgx2nnz0.fsf@linux.intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Wed, Nov 02, 2016 at 01:02:59PM +0200, Felipe Balbi wrote: > > Hi, > > Peter Chen writes: > >> >> 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. > > > > No, TD_TERMINATE just stands for it is the last TD, and this pointer will > > be updated when the new request is added. The interrupt is only triggered > > by IOC (Interrupt On Complete) bit at TD. > > > > I am not sure if dwc3 supports ITC (Interrupt Threshold Control) > > software control, it is an EHCI compliant register entry, and > > the device mode is supported for chipidea too. It is a timeout > > mechanism from controller side for pending requests. > > > > The interrupt will be triggered either the request has completed for TD > > which IOC bit is set or the ITC is fired (125us currently) and the > > request has completed, so the problem David described should not exist, > > at least for chipidea. > > In other words, you don't *really* throttle interrupt as they'll fire > after the micro-frame expires :-p No, even in one uFrame, there are at most ~10 packets for bulk at USB2. At least, you can throttle interrupt within SoF, it is useful for high throughout use case. > > > If DWC3 has similar ITC bits, would you try to tune it? The default ITC > > value for chipidea is not enough, and we tuned it before. > > there's no such thing in dwc3 > So, how about add another parameter to support throttling interrupt separately. Current parameter 'mult' combined user request number and throttle interrupt together. -- Best Regards, Peter Chen