LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
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
Date: Thu, 4 Apr 2019 21:18:24 +0900	[thread overview]
Message-ID: <20190404121823.GA3793@localhost.localdomain> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1904011040570.1506-100000@iolanthe.rowland.org>

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

  reply	other threads:[~2019-04-04 12:18 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-01 14:16 Suwan Kim
2019-04-01 14:43 ` Alan Stern
2019-04-04 12:18   ` Suwan Kim [this message]
2019-04-04 20:54     ` Alan Stern
2019-04-08 14:26       ` Suwan Kim
2019-05-07 16:02 ` Suwan Kim
2019-05-08 12:04   ` Mathias Nyman
2019-05-08 14:40     ` Suwan Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190404121823.GA3793@localhost.localdomain \
    --to=suwan.kim027@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@linux.intel.com \
    --cc=stern@rowland.harvard.edu \
    --subject='Re: [PATCH v2] usb: host: xhci: Support running urb giveback in tasklet context' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).