linux-kernel - [PATCH] usb: don't offload isochronous urb completions to ksoftirq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LRH.2.02.1806121020130.10769@file01.intranet.prod.int.rdu2.redhat.com>
Date:   Tue, 12 Jun 2018 10:29:04 -0400 (EDT)
From:   Mikulas Patocka <mpatocka@...hat.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Ming Lei <ming.lei@...onical.com>
cc:     linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] usb: don't offload isochronous urb completions to ksoftirq

I have a single-core machine with usb2 soundcard. When I increase mplayer
priority (to real-time or high non-realtime priority), the sound is
stuttering. The reason for the stuttering is that mplayer at high priority
preempts the softirq thread, preventing URBs from being completed. It was
caused by the patch 428aac8a81058 that offloads URB completion to softirq.

This patch prevents offloading isochronous URBs to softirq to fix the
stuttering.

Fixes: c04ee4b1136e ("Revert "Revert "USB: EHCI: support running URB giveback in tasklet context""")
Cc: stable@...r.kernel.org

---
 drivers/usb/core/hcd.c    |   10 ++++++++++
 drivers/usb/host/ehci-q.c |   11 ++++++++++-
 include/linux/usb/hcd.h   |    2 ++
 3 files changed, 22 insertions(+), 1 deletion(-)

Index: linux-4.17/drivers/usb/core/hcd.c
===================================================================
--- linux-4.17.orig/drivers/usb/core/hcd.c	2018-06-12 16:06:23.000000000 +0200
+++ linux-4.17/drivers/usb/core/hcd.c	2018-06-12 16:07:51.000000000 +0200
@@ -1858,6 +1858,16 @@ void usb_hcd_giveback_urb(struct usb_hcd
 }
 EXPORT_SYMBOL_GPL(usb_hcd_giveback_urb);
 
+void _usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb, int status)
+{
+	/* pass status to tasklet via unlinked */
+	if (likely(!urb->unlinked))
+		urb->unlinked = status;
+
+	__usb_hcd_giveback_urb(urb);
+}
+EXPORT_SYMBOL_GPL(_usb_hcd_giveback_urb);
+
 /*-------------------------------------------------------------------------*/
 
 /* Cancel all URBs pending on this endpoint and wait for the endpoint's
Index: linux-4.17/drivers/usb/host/ehci-q.c
===================================================================
--- linux-4.17.orig/drivers/usb/host/ehci-q.c	2018-06-12 16:06:23.000000000 +0200
+++ linux-4.17/drivers/usb/host/ehci-q.c	2018-06-12 16:09:28.000000000 +0200
@@ -238,6 +238,8 @@ static int qtd_copy_status (
 
 static void
 ehci_urb_done(struct ehci_hcd *ehci, struct urb *urb, int status)
+__releases(ehci->lock)
+__acquires(ehci->lock)
 {
 	if (usb_pipetype(urb->pipe) == PIPE_INTERRUPT) {
 		/* ... update hc-wide periodic stats */
@@ -264,7 +266,14 @@ ehci_urb_done(struct ehci_hcd *ehci, str
 #endif
 
 	usb_hcd_unlink_urb_from_ep(ehci_to_hcd(ehci), urb);
-	usb_hcd_giveback_urb(ehci_to_hcd(ehci), urb, status);
+	if (usb_pipeisoc(urb->pipe)) {
+		/* complete() can reenter this HCD */
+		spin_unlock(&ehci->lock);
+		_usb_hcd_giveback_urb(ehci_to_hcd(ehci), urb, status);
+		spin_lock(&ehci->lock);
+	} else {
+		usb_hcd_giveback_urb(ehci_to_hcd(ehci), urb, status);
+	}
 }
 
 static int qh_schedule (struct ehci_hcd *ehci, struct ehci_qh *qh);
Index: linux-4.17/include/linux/usb/hcd.h
===================================================================
--- linux-4.17.orig/include/linux/usb/hcd.h	2018-06-05 21:07:27.000000000 +0200
+++ linux-4.17/include/linux/usb/hcd.h	2018-06-12 16:07:11.000000000 +0200
@@ -428,6 +428,8 @@ extern int usb_hcd_submit_urb(struct urb
 extern int usb_hcd_unlink_urb(struct urb *urb, int status);
 extern void usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb,
 		int status);
+extern void _usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb,
+		int status);
 extern int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
 		gfp_t mem_flags);
 extern void usb_hcd_unmap_urb_setup_for_dma(struct usb_hcd *, struct urb *);