[kernel/f14] Add patches to fix RHBZ #663186

Josh Boyer jwboyer at fedoraproject.org
Fri Oct 14 17:29:32 UTC 2011


commit c6232b615c97b19ca86dd6db6c69f64b921b3179
Author: Josh Boyer <jwboyer at redhat.com>
Date:   Fri Oct 14 13:23:15 2011 -0400

    Add patches to fix RHBZ #663186

 ...HCI-don-t-rescan-interrupt-QHs-needlessly.patch |   95 +++++++++
 ...-back-to-using-the-system-clock-for-QH-un.patch |  217 ++++++++++++++++++++
 kernel.spec                                        |   13 +-
 3 files changed, 324 insertions(+), 1 deletions(-)
---
diff --git a/0001-EHCI-don-t-rescan-interrupt-QHs-needlessly.patch b/0001-EHCI-don-t-rescan-interrupt-QHs-needlessly.patch
new file mode 100644
index 0000000..499c31a
--- /dev/null
+++ b/0001-EHCI-don-t-rescan-interrupt-QHs-needlessly.patch
@@ -0,0 +1,95 @@
+From 1729d20333739e6cc23023e405fe8668f08117a2 Mon Sep 17 00:00:00 2001
+From: Alan Stern <stern at rowland.harvard.edu>
+Date: Tue, 17 May 2011 10:40:51 -0400
+Subject: [PATCH 1/2] EHCI: don't rescan interrupt QHs needlessly
+
+This patch (as1466) speeds up processing of ehci-hcd's periodic list.
+The existing code will pointlessly rescan an interrupt endpoint queue
+each time it encounters the queue's QH in the periodic list, which can
+happen quite a few times if the endpoint's period is low.  On some
+embedded systems, this useless overhead can waste so much time that
+the driver falls hopelessly behind and loses events.
+
+The patch introduces a "periodic_stamp" variable, which gets
+incremented each time scan_periodic() runs and each time the scan
+advances to a new frame.  If the corresponding stamp in an interrupt
+QH is equal to the current periodic_stamp, we assume the QH has
+already been scanned and skip over it.  Otherwise we scan the QH as
+usual, and if none of its URBs have completed then we store the
+current periodic_stamp in the QH's stamp, preventing it from being
+scanned again.
+
+Signed-off-by: Alan Stern <stern at rowland.harvard.edu>
+Signed-off-by: Greg Kroah-Hartman <gregkh at suse.de>
+---
+ drivers/usb/host/ehci-q.c     |    1 +
+ drivers/usb/host/ehci-sched.c |   14 ++++++++++----
+ drivers/usb/host/ehci.h       |    1 +
+ 3 files changed, 12 insertions(+), 4 deletions(-)
+
+diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
+index ed8db6a..0079610 100644
+--- a/drivers/usb/host/ehci-q.c
++++ b/drivers/usb/host/ehci-q.c
+@@ -826,6 +826,7 @@ qh_make (
+ 				is_input, 0,
+ 				hb_mult(maxp) * max_packet(maxp)));
+ 		qh->start = NO_FRAME;
++		qh->stamp = ehci->periodic_stamp;
+ 
+ 		if (urb->dev->speed == USB_SPEED_HIGH) {
+ 			qh->c_usecs = 0;
+diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
+index a530856..9e54e85 100644
+--- a/drivers/usb/host/ehci-sched.c
++++ b/drivers/usb/host/ehci-sched.c
+@@ -2351,6 +2351,7 @@ scan_periodic (struct ehci_hcd *ehci)
+ 	}
+ 	clock %= mod;
+ 	clock_frame = clock >> 3;
++	++ehci->periodic_stamp;
+ 
+ 	for (;;) {
+ 		union ehci_shadow	q, *q_p;
+@@ -2379,10 +2380,14 @@ restart:
+ 				temp.qh = qh_get (q.qh);
+ 				type = Q_NEXT_TYPE(ehci, q.qh->hw->hw_next);
+ 				q = q.qh->qh_next;
+-				modified = qh_completions (ehci, temp.qh);
+-				if (unlikely(list_empty(&temp.qh->qtd_list) ||
+-						temp.qh->needs_rescan))
+-					intr_deschedule (ehci, temp.qh);
++				if (temp.qh->stamp != ehci->periodic_stamp) {
++					modified = qh_completions(ehci, temp.qh);
++					if (!modified)
++						temp.qh->stamp = ehci->periodic_stamp;
++					if (unlikely(list_empty(&temp.qh->qtd_list) ||
++							temp.qh->needs_rescan))
++						intr_deschedule(ehci, temp.qh);
++				}
+ 				qh_put (temp.qh);
+ 				break;
+ 			case Q_TYPE_FSTN:
+@@ -2515,6 +2520,7 @@ restart:
+ 			if (ehci->clock_frame != clock_frame) {
+ 				free_cached_lists(ehci);
+ 				ehci->clock_frame = clock_frame;
++				++ehci->periodic_stamp;
+ 			}
+ 		} else {
+ 			now_uframe++;
+diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
+index c702e4b..c2c2f9d 100644
+--- a/drivers/usb/host/ehci.h
++++ b/drivers/usb/host/ehci.h
+@@ -117,6 +117,7 @@ struct ehci_hcd {			/* one per controller */
+ 	struct timer_list	watchdog;
+ 	unsigned long		actions;
+ 	unsigned		stamp;
++	unsigned		periodic_stamp;
+ 	unsigned		random_frame;
+ 	unsigned long		next_statechange;
+ 	ktime_t			last_periodic_enable;
+-- 
+1.7.6
+
diff --git a/0002-USB-EHCI-go-back-to-using-the-system-clock-for-QH-un.patch b/0002-USB-EHCI-go-back-to-using-the-system-clock-for-QH-un.patch
new file mode 100644
index 0000000..404ece1
--- /dev/null
+++ b/0002-USB-EHCI-go-back-to-using-the-system-clock-for-QH-un.patch
@@ -0,0 +1,217 @@
+From e8b80056d07bc849777d032bc699a59cc28f9ddb Mon Sep 17 00:00:00 2001
+From: Alan Stern <stern at rowland.harvard.edu>
+Date: Tue, 5 Jul 2011 12:34:05 -0400
+Subject: [PATCH 2/2] USB: EHCI: go back to using the system clock for QH
+ unlinks
+
+This patch (as1477) fixes a problem affecting a few types of EHCI
+controller.  Contrary to what one might expect, these controllers
+automatically stop their internal frame counter when no ports are
+enabled.  Since ehci-hcd currently relies on the frame counter for
+determining when it should unlink QHs from the async schedule, those
+controllers run into trouble: The frame counter stops and the QHs
+never get unlinked.
+
+Some systems have also experienced other problems traced back to
+commit b963801164618e25fbdc0cd452ce49c3628b46c8 (USB: ehci-hcd unlink
+speedups), which made the original switch from using the system clock
+to using the frame counter.  It never became clear what the reason was
+for these problems, but evidently it is related to use of the frame
+counter.
+
+To fix all these problems, this patch more or less reverts that commit
+and goes back to using the system clock.  But this can't be done
+cleanly because other changes have since been made to the scan_async()
+subroutine.  One of these changes involved the tricky logic that tries
+to avoid rescanning QHs that have already been seen when the scanning
+loop is restarted, which happens whenever an URB is given back.
+Switching back to clock-based unlinks would make this logic even more
+complicated.
+
+Therefore the new code doesn't rescan the entire async list whenever a
+giveback occurs.  Instead it rescans only the current QH and continues
+on from there.  This requires the use of a separate pointer to keep
+track of the next QH to scan, since the current QH may be unlinked
+while the scanning is in progress.  That new pointer must be global,
+so that it can be adjusted forward whenever the _next_ QH gets
+unlinked.  (uhci-hcd uses this same trick.)
+
+Simplification of the scanning loop removes a level of indentation,
+which accounts for the size of the patch.  The amount of code changed
+is relatively small, and it isn't exactly a reversion of the
+b963801164 commit.
+
+This fixes Bugzilla #32432.
+
+Signed-off-by: Alan Stern <stern at rowland.harvard.edu>
+CC: <stable at kernel.org>
+Tested-by: Matej Kenda <matejken at gmail.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh at suse.de>
+---
+ drivers/usb/host/ehci-hcd.c |    8 ++---
+ drivers/usb/host/ehci-q.c   |   82 +++++++++++++++++++++----------------------
+ drivers/usb/host/ehci.h     |    3 +-
+ 3 files changed, 45 insertions(+), 48 deletions(-)
+
+diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
+index 761bbb3..8e5b995 100644
+--- a/drivers/usb/host/ehci-hcd.c
++++ b/drivers/usb/host/ehci-hcd.c
+@@ -83,7 +83,8 @@ static const char	hcd_name [] = "ehci_hcd";
+ #define EHCI_IAA_MSECS		10		/* arbitrary */
+ #define EHCI_IO_JIFFIES		(HZ/10)		/* io watchdog > irq_thresh */
+ #define EHCI_ASYNC_JIFFIES	(HZ/20)		/* async idle timeout */
+-#define EHCI_SHRINK_FRAMES	5		/* async qh unlink delay */
++#define EHCI_SHRINK_JIFFIES	(DIV_ROUND_UP(HZ, 200) + 1)
++						/* 200-ms async qh unlink delay */
+ 
+ /* Initial IRQ latency:  faster than hw default */
+ static int log2_irq_thresh = 0;		// 0 to 6
+@@ -138,10 +139,7 @@ timer_action(struct ehci_hcd *ehci, enum ehci_timer_action action)
+ 			break;
+ 		/* case TIMER_ASYNC_SHRINK: */
+ 		default:
+-			/* add a jiffie since we synch against the
+-			 * 8 KHz uframe counter.
+-			 */
+-			t = DIV_ROUND_UP(EHCI_SHRINK_FRAMES * HZ, 1000) + 1;
++			t = EHCI_SHRINK_JIFFIES;
+ 			break;
+ 		}
+ 		mod_timer(&ehci->watchdog, t + jiffies);
+diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
+index 0079610..a664203 100644
+--- a/drivers/usb/host/ehci-q.c
++++ b/drivers/usb/host/ehci-q.c
+@@ -1226,6 +1226,8 @@ static void start_unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
+ 
+ 	prev->hw->hw_next = qh->hw->hw_next;
+ 	prev->qh_next = qh->qh_next;
++	if (ehci->qh_scan_next == qh)
++		ehci->qh_scan_next = qh->qh_next.qh;
+ 	wmb ();
+ 
+ 	/* If the controller isn't running, we don't have to wait for it */
+@@ -1251,53 +1253,49 @@ static void scan_async (struct ehci_hcd *ehci)
+ 	struct ehci_qh		*qh;
+ 	enum ehci_timer_action	action = TIMER_IO_WATCHDOG;
+ 
+-	ehci->stamp = ehci_readl(ehci, &ehci->regs->frame_index);
+ 	timer_action_done (ehci, TIMER_ASYNC_SHRINK);
+-rescan:
+ 	stopped = !HC_IS_RUNNING(ehci_to_hcd(ehci)->state);
+-	qh = ehci->async->qh_next.qh;
+-	if (likely (qh != NULL)) {
+-		do {
+-			/* clean any finished work for this qh */
+-			if (!list_empty(&qh->qtd_list) && (stopped ||
+-					qh->stamp != ehci->stamp)) {
+-				int temp;
+-
+-				/* unlinks could happen here; completion
+-				 * reporting drops the lock.  rescan using
+-				 * the latest schedule, but don't rescan
+-				 * qhs we already finished (no looping)
+-				 * unless the controller is stopped.
+-				 */
+-				qh = qh_get (qh);
+-				qh->stamp = ehci->stamp;
+-				temp = qh_completions (ehci, qh);
+-				if (qh->needs_rescan)
+-					unlink_async(ehci, qh);
+-				qh_put (qh);
+-				if (temp != 0) {
+-					goto rescan;
+-				}
+-			}
+ 
+-			/* unlink idle entries, reducing DMA usage as well
+-			 * as HCD schedule-scanning costs.  delay for any qh
+-			 * we just scanned, there's a not-unusual case that it
+-			 * doesn't stay idle for long.
+-			 * (plus, avoids some kind of re-activation race.)
++	ehci->qh_scan_next = ehci->async->qh_next.qh;
++	while (ehci->qh_scan_next) {
++		qh = ehci->qh_scan_next;
++		ehci->qh_scan_next = qh->qh_next.qh;
++ rescan:
++		/* clean any finished work for this qh */
++		if (!list_empty(&qh->qtd_list)) {
++			int temp;
++
++			/*
++			 * Unlinks could happen here; completion reporting
++			 * drops the lock.  That's why ehci->qh_scan_next
++			 * always holds the next qh to scan; if the next qh
++			 * gets unlinked then ehci->qh_scan_next is adjusted
++			 * in start_unlink_async().
+ 			 */
+-			if (list_empty(&qh->qtd_list)
+-					&& qh->qh_state == QH_STATE_LINKED) {
+-				if (!ehci->reclaim && (stopped ||
+-					((ehci->stamp - qh->stamp) & 0x1fff)
+-						>= EHCI_SHRINK_FRAMES * 8))
+-					start_unlink_async(ehci, qh);
+-				else
+-					action = TIMER_ASYNC_SHRINK;
+-			}
++			qh = qh_get(qh);
++			temp = qh_completions(ehci, qh);
++			if (qh->needs_rescan)
++				unlink_async(ehci, qh);
++			qh->unlink_time = jiffies + EHCI_SHRINK_JIFFIES;
++			qh_put(qh);
++			if (temp != 0)
++				goto rescan;
++		}
+ 
+-			qh = qh->qh_next.qh;
+-		} while (qh);
++		/* unlink idle entries, reducing DMA usage as well
++		 * as HCD schedule-scanning costs.  delay for any qh
++		 * we just scanned, there's a not-unusual case that it
++		 * doesn't stay idle for long.
++		 * (plus, avoids some kind of re-activation race.)
++		 */
++		if (list_empty(&qh->qtd_list)
++				&& qh->qh_state == QH_STATE_LINKED) {
++			if (!ehci->reclaim && (stopped ||
++					time_after_eq(jiffies, qh->unlink_time)))
++				start_unlink_async(ehci, qh);
++			else
++				action = TIMER_ASYNC_SHRINK;
++		}
+ 	}
+ 	if (action == TIMER_ASYNC_SHRINK)
+ 		timer_action (ehci, TIMER_ASYNC_SHRINK);
+diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
+index c2c2f9d..23d9a5e 100644
+--- a/drivers/usb/host/ehci.h
++++ b/drivers/usb/host/ehci.h
+@@ -74,6 +74,7 @@ struct ehci_hcd {			/* one per controller */
+ 	/* async schedule support */
+ 	struct ehci_qh		*async;
+ 	struct ehci_qh		*reclaim;
++	struct ehci_qh		*qh_scan_next;
+ 	unsigned		scanning : 1;
+ 
+ 	/* periodic schedule support */
+@@ -116,7 +117,6 @@ struct ehci_hcd {			/* one per controller */
+ 	struct timer_list	iaa_watchdog;
+ 	struct timer_list	watchdog;
+ 	unsigned long		actions;
+-	unsigned		stamp;
+ 	unsigned		periodic_stamp;
+ 	unsigned		random_frame;
+ 	unsigned long		next_statechange;
+@@ -337,6 +337,7 @@ struct ehci_qh {
+ 	struct ehci_qh		*reclaim;	/* next to reclaim */
+ 
+ 	struct ehci_hcd		*ehci;
++	unsigned long		unlink_time;
+ 
+ 	/*
+ 	 * Do NOT use atomic operations for QH refcounting. On some CPUs
+-- 
+1.7.6
+
diff --git a/kernel.spec b/kernel.spec
index fc0ff89..534676b 100644
--- a/kernel.spec
+++ b/kernel.spec
@@ -48,7 +48,7 @@ Summary: The Linux kernel
 # reset this by hand to 1 (or to 0 and then use rpmdev-bumpspec).
 # scripts/rebase.sh should be made to do that for you, actually.
 #
-%global baserelease 98
+%global baserelease 99
 %global fedora_build %{baserelease}
 
 # base_sublevel is the kernel version we're starting with and patching
@@ -892,6 +892,10 @@ Patch14059: fuse-check-size-of-FUSE_NOTIFY_INVAL_ENTRY-message.patch
 Patch14060: TPM-Call-tpm_transmit-with-correct-size.patch
 Patch14061: TPM-Zero-buffer-after-copying-to-userspace.patch
 
+# RHBZ #663186
+Patch14062: 0001-EHCI-don-t-rescan-interrupt-QHs-needlessly.patch
+Patch14063: 0002-USB-EHCI-go-back-to-using-the-system-clock-for-QH-un.patch
+
 %endif
 
 BuildRoot: %{_tmppath}/kernel-%{KVERREL}-root
@@ -1687,6 +1691,10 @@ ApplyPatch fuse-check-size-of-FUSE_NOTIFY_INVAL_ENTRY-message.patch
 ApplyPatch TPM-Call-tpm_transmit-with-correct-size.patch
 ApplyPatch TPM-Zero-buffer-after-copying-to-userspace.patch
 
+# RHBZ #663186
+ApplyPatch 0001-EHCI-don-t-rescan-interrupt-QHs-needlessly.patch
+ApplyPatch 0002-USB-EHCI-go-back-to-using-the-system-clock-for-QH-un.patch
+
 # END OF PATCH APPLICATIONS
 
 %endif
@@ -2273,6 +2281,9 @@ fi
 # and build.
 
 %changelog
+* Fri Oct 14 2011 Josh Boyer <jwboyer at redhat.com>
+- Add patches to fix RHBZ #663186
+
 * Tue Oct 11 2011 Dave Jones <davej at redhat.com>
 - acer-wmi: Fix capitalisation of GUID in module alias (rhbz 661322)
 


More information about the scm-commits mailing list