Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 4 Apr 2019 21:18:24 +0900
From:   Suwan Kim <suwan.kim027@gmail.com>
To:     Alan Stern <stern@rowland.harvard.edu>
Cc:     mathias.nyman@linux.intel.com, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org
Subject: Re: [PATCH v2] usb: host: xhci: Support running urb giveback in
 tasklet context
Message-ID: <20190404121823.GA3793@localhost.localdomain>
References: <20190401141611.10087-1-suwan.kim027@gmail.com>
 <Pine.LNX.4.44L0.1904011040570.1506-100000@iolanthe.rowland.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Pine.LNX.4.44L0.1904011040570.1506-100000@iolanthe.rowland.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Alan,

On Mon, Apr 01, 2019 at 10:43:24AM -0400, Alan Stern wrote:
> On Mon, 1 Apr 2019, Suwan Kim wrote:
> 
> > Patch "USB: HCD: support giveback of URB in tasklet context"[1]
> > introduced giveback of urb in tasklet context. [1] This patch was
> > applied to ehci but not xhci. [2] This patch significantly reduces
> > the hard irq time of xhci. Especially for uvc driver, the hard irq
> > including the uvc completion function runs quite long but applying
> > this patch reduces the hard irq time of xhci.
> 
> Please read the kerneldoc for usb_submit_urb() and usb_kill_urb(), in 
> particular, the parts that describe how isochronous URBs are treated.  
> Can you guarantee that with this patch applied, xhci-hcd will continue 
> to work as the kerneldoc describes?
> 

I read the description of usb_submit_urb() and usb_kill_urb() and i
think that xhci-hcd with which this patch is applied works as the
description of usb_submit_urb() and usb_kill_urb().

In the case of usb_submit_urb(), xhci spec 4.10.3.1 "Ring Overrun and
Underrun" describes the behavior of xhci when an isochronous ring is
empty due to the late URB submission in driver. (In this patch, empty
isochronous ring can happen due to tasklet scheduling delay in URB
complete function which calls the next usb_submit_urb())

According to the xhci spec, xHC deals with a late isochronous URB
according to the SIA(Start Isoch ASAP) flag of TRB and SIA flag is
set according to URB_ISO_ASAP flag in xhci_queue_isoc_tx().

If the SIA flag is set, xHC will schedule the late isochronous URB in
the next "Endpoint Service Interval Time" (next available frame) and
transmits ischronous URB in that frame.

If the SIA flag is cleared (URB_ISO_ASAP flag is cleared), xHC generates
"Missed Service Error" event and skips the late isochronous URB and
doen't transmit it. When the interrupt handler (xhci_irq) receives
"Missed Service Error" event, it returns the late isochronous URB to
the driver calling usb_hcd_giveback_urb() with -EXDEV error code in
usb_iso_packet_descriptor->status at skip_isoc_td(). So xhci behavior
about the late isochronous URB in spec and implementation is same
with the description of usb_submit_urb().

In the case of usb_kill_urb(), description says that it waits until
the URB complete function finishes and the important point is that
whether the USB complete function is called early or late doesn't
affect the behavior of usb_kill_urb() because __usb_hcd_giveback_urb()
wakes up usb_kill_urb() after calling URB complete function.
So, pending a URB complete function in tasklet doesn't affect the
behavior of xhci in usb_kill_urb().

Regards

Suwan Kim