linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] xhci fixes for usb-linus
@ 2020-10-28 20:31 Mathias Nyman
  2020-10-28 20:31 ` [PATCH 1/3] xhci: Fix sizeof() mismatch Mathias Nyman
                   ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Mathias Nyman @ 2020-10-28 20:31 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few xhci patches for usb-linus, including a fix for the xhci lockdep
oops in 5.10-rc1

-Mathias

Colin Ian King (1):
  xhci: Fix sizeof() mismatch

Mathias Nyman (1):
  xhci: Don't create stream debugfs files with spinlock held.

Sandeep Singh (1):
  usb: xhci: Workaround for S3 issue on AMD SNPS 3.0 xHC

 drivers/usb/host/xhci-mem.c |  4 ++--
 drivers/usb/host/xhci-pci.c | 17 +++++++++++++++++
 drivers/usb/host/xhci.c     |  5 ++++-
 drivers/usb/host/xhci.h     |  1 +
 4 files changed, 24 insertions(+), 3 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/3] xhci: Fix sizeof() mismatch
  2020-10-28 20:31 [PATCH 0/3] xhci fixes for usb-linus Mathias Nyman
@ 2020-10-28 20:31 ` Mathias Nyman
  2020-10-28 20:31 ` [PATCH 2/3] usb: xhci: Workaround for S3 issue on AMD SNPS 3.0 xHC Mathias Nyman
  2020-10-28 20:31 ` [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held Mathias Nyman
  2 siblings, 0 replies; 24+ messages in thread
From: Mathias Nyman @ 2020-10-28 20:31 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Colin Ian King, Mathias Nyman

From: Colin Ian King <colin.king@canonical.com>

An incorrect sizeof() is being used, sizeof(rhub->ports) is not
correct, it should be sizeof(*rhub->ports).  This bug did not
cause any issues because it just so happens the sizes are the same.

Addresses-Coverity: ("Sizeof not portable (SIZEOF_MISMATCH)")
Fixes: bcaa9d5c5900 ("xhci: Create new structures to store xhci port information")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index fe405cd38dbc..138ba4528dd3 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2252,8 +2252,8 @@ static void xhci_create_rhub_port_array(struct xhci_hcd *xhci,
 
 	if (!rhub->num_ports)
 		return;
-	rhub->ports = kcalloc_node(rhub->num_ports, sizeof(rhub->ports), flags,
-			dev_to_node(dev));
+	rhub->ports = kcalloc_node(rhub->num_ports, sizeof(*rhub->ports),
+			flags, dev_to_node(dev));
 	for (i = 0; i < HCS_MAX_PORTS(xhci->hcs_params1); i++) {
 		if (xhci->hw_ports[i].rhub != rhub ||
 		    xhci->hw_ports[i].hcd_portnum == DUPLICATE_ENTRY)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/3] usb: xhci: Workaround for S3 issue on AMD SNPS 3.0 xHC
  2020-10-28 20:31 [PATCH 0/3] xhci fixes for usb-linus Mathias Nyman
  2020-10-28 20:31 ` [PATCH 1/3] xhci: Fix sizeof() mismatch Mathias Nyman
@ 2020-10-28 20:31 ` Mathias Nyman
  2020-10-28 20:31 ` [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held Mathias Nyman
  2 siblings, 0 replies; 24+ messages in thread
From: Mathias Nyman @ 2020-10-28 20:31 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Sandeep Singh, Sanket Goswami, stable, Mathias Nyman

From: Sandeep Singh <sandeep.singh@amd.com>

On some platform of AMD, S3 fails with HCE and SRE errors. To fix this,
need to disable a bit which is enable in sparse controller.

Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Sandeep Singh <sandeep.singh@amd.com>
Cc: stable@vger.kernel.org#v4.19+
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-pci.c | 17 +++++++++++++++++
 drivers/usb/host/xhci.h     |  1 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index c26c06e5c88c..bf89172c43ca 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -23,6 +23,8 @@
 #define SSIC_PORT_CFG2_OFFSET	0x30
 #define PROG_DONE		(1 << 30)
 #define SSIC_PORT_UNUSED	(1 << 31)
+#define SPARSE_DISABLE_BIT	17
+#define SPARSE_CNTL_ENABLE	0xC12C
 
 /* Device for a quirk */
 #define PCI_VENDOR_ID_FRESCO_LOGIC	0x1b73
@@ -161,6 +163,9 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 	    (pdev->device == 0x15e0 || pdev->device == 0x15e1))
 		xhci->quirks |= XHCI_SNPS_BROKEN_SUSPEND;
 
+	if (pdev->vendor == PCI_VENDOR_ID_AMD && pdev->device == 0x15e5)
+		xhci->quirks |= XHCI_DISABLE_SPARSE;
+
 	if (pdev->vendor == PCI_VENDOR_ID_AMD)
 		xhci->quirks |= XHCI_TRUST_TX_LENGTH;
 
@@ -498,6 +503,15 @@ static void xhci_pme_quirk(struct usb_hcd *hcd)
 	readl(reg);
 }
 
+static void xhci_sparse_control_quirk(struct usb_hcd *hcd)
+{
+	u32 reg;
+
+	reg = readl(hcd->regs + SPARSE_CNTL_ENABLE);
+	reg &= ~BIT(SPARSE_DISABLE_BIT);
+	writel(reg, hcd->regs + SPARSE_CNTL_ENABLE);
+}
+
 static int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
 {
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
@@ -517,6 +531,9 @@ static int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
 	if (xhci->quirks & XHCI_SSIC_PORT_UNUSED)
 		xhci_ssic_port_unused_quirk(hcd, true);
 
+	if (xhci->quirks & XHCI_DISABLE_SPARSE)
+		xhci_sparse_control_quirk(hcd);
+
 	ret = xhci_suspend(xhci, do_wakeup);
 	if (ret && (xhci->quirks & XHCI_SSIC_PORT_UNUSED))
 		xhci_ssic_port_unused_quirk(hcd, false);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 8be88379c0fb..ebb359ebb261 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1877,6 +1877,7 @@ struct xhci_hcd {
 #define XHCI_SNPS_BROKEN_SUSPEND    BIT_ULL(35)
 #define XHCI_RENESAS_FW_QUIRK	BIT_ULL(36)
 #define XHCI_SKIP_PHY_INIT	BIT_ULL(37)
+#define XHCI_DISABLE_SPARSE	BIT_ULL(38)
 
 	unsigned int		num_active_eps;
 	unsigned int		limit_active_eps;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-28 20:31 [PATCH 0/3] xhci fixes for usb-linus Mathias Nyman
  2020-10-28 20:31 ` [PATCH 1/3] xhci: Fix sizeof() mismatch Mathias Nyman
  2020-10-28 20:31 ` [PATCH 2/3] usb: xhci: Workaround for S3 issue on AMD SNPS 3.0 xHC Mathias Nyman
@ 2020-10-28 20:31 ` Mathias Nyman
  2020-10-29  6:03   ` Mike Galbraith
  2020-10-30  8:01   ` Jun Li
  2 siblings, 2 replies; 24+ messages in thread
From: Mathias Nyman @ 2020-10-28 20:31 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman, Hans de Goede, Mike Galbraith, Li Jun

Creating debugfs files while loding the spin_lock_irqsave(xhci->lock)
creates a lock dependecy that could possibly deadlock.

Lockdep warns:

=====================================================
WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
5.10.0-rc1pdx86+ #8 Not tainted
-----------------------------------------------------
systemd-udevd/386 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
ffffffffb1a94038 (pin_fs_lock){+.+.}-{2:2}, at: simple_pin_fs+0x22/0xa0

and this task is already holding:
ffff9e7b87fbc430 (&xhci->lock){-.-.}-{2:2}, at: xhci_alloc_streams+0x5f9/0x810
which would create a new lock dependency:
(&xhci->lock){-.-.}-{2:2} -> (pin_fs_lock){+.+.}-{2:2}

Create the files a bit later after lock is released.

Reported-by: Hans de Goede <hdegoede@redhat.com>
Reported-by: Mike Galbraith <efault@gmx.de>
Tested-by: Hans de Goede <hdegoede@redhat.com>
CC: Li Jun <jun.li@nxp.com>
Fixes: 673d74683627 ("usb: xhci: add debugfs support for ep with stream")
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 482fe8c5e3b4..d4a8d0efbbc4 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3533,11 +3533,14 @@ static int xhci_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev,
 		xhci_dbg(xhci, "Slot %u ep ctx %u now has streams.\n",
 			 udev->slot_id, ep_index);
 		vdev->eps[ep_index].ep_state |= EP_HAS_STREAMS;
-		xhci_debugfs_create_stream_files(xhci, vdev, ep_index);
 	}
 	xhci_free_command(xhci, config_cmd);
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
+	for (i = 0; i < num_eps; i++) {
+		ep_index = xhci_get_endpoint_index(&eps[i]->desc);
+		xhci_debugfs_create_stream_files(xhci, vdev, ep_index);
+	}
 	/* Subtract 1 for stream 0, which drivers can't use */
 	return num_streams - 1;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-28 20:31 ` [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held Mathias Nyman
@ 2020-10-29  6:03   ` Mike Galbraith
  2020-10-29  9:41     ` Mathias Nyman
  2020-10-30  8:01   ` Jun Li
  1 sibling, 1 reply; 24+ messages in thread
From: Mike Galbraith @ 2020-10-29  6:03 UTC (permalink / raw)
  To: Mathias Nyman, gregkh
  Cc: linux-usb, Hans de Goede, Li Jun, Sebastian Andrzej Siewior

Greetings,

Testing $subject in shiny new 5.10-rt induced a lockdep grumble, so I
plugged it into master and booted lappy with threadirqs.  When I then
fired up my android-x86 vm toy, that kernel grumbled as well.

[   81.184240] tun: Universal TUN/TAP device driver, 1.6
[   81.187185] virbr0: port 1(virbr0-nic) entered blocking state
[   81.187230] virbr0: port 1(virbr0-nic) entered disabled state
[   81.187529] device virbr0-nic entered promiscuous mode
[   81.867218] virbr0: port 1(virbr0-nic) entered blocking state
[   81.867326] virbr0: port 1(virbr0-nic) entered listening state
[   82.038707] virbr0: port 1(virbr0-nic) entered disabled state
[   93.259770] virbr0: port 2(vnet0) entered blocking state
[   93.259818] virbr0: port 2(vnet0) entered disabled state
[   93.260118] device vnet0 entered promiscuous mode
[   93.260888] virbr0: port 2(vnet0) entered blocking state
[   93.260935] virbr0: port 2(vnet0) entered listening state
[   93.601718] cgroup: cgroup: disabling cgroup2 socket matching due to net_prio or net_cls activation
[   95.278373] virbr0: port 2(vnet0) entered learning state
[   97.293928] virbr0: port 2(vnet0) entered forwarding state
[   97.293984] virbr0: topology change detected, propagating

[  107.611645] =====================================================
[  107.611650] WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
[  107.611656] 5.10.0.g23859ae-master #17 Tainted: G S        I E
[  107.611660] -----------------------------------------------------
[  107.611666] worker/4060 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
[  107.611671] ffff90bba6d2c680 (lock#2){+.+.}-{2:2}, at: radix_tree_maybe_preload+0x8/0xb0
[  107.611684]
               and this task is already holding:
[  107.611689] ffff90ba501a8430 (&xhci->lock){..-.}-{2:2}, at: xhci_urb_enqueue+0x20a/0x6f0 [xhci_hcd]
[  107.611706] which would create a new lock dependency:
[  107.611710]  (&xhci->lock){..-.}-{2:2} -> (lock#2){+.+.}-{2:2}
[  107.611719]
               but this new dependency connects a SOFTIRQ-irq-safe lock:
[  107.611725]  (&xhci->lock){..-.}-{2:2}
[  107.611727]
               ... which became SOFTIRQ-irq-safe at:
[  107.611741]   lock_acquire+0x1a7/0x3b0
[  107.611748]   _raw_spin_lock_irqsave+0x3d/0x50
[  107.611760]   xhci_hub_status_data+0xc2/0x390 [xhci_hcd]
[  107.611776]   usb_hcd_poll_rh_status+0x41/0x180 [usbcore]
[  107.611783]   call_timer_fn+0x9d/0x2b0
[  107.611788]   run_timer_softirq+0x48d/0x570
[  107.611793]   __do_softirq+0xeb/0x4a4
[  107.611799]   run_ksoftirqd+0x32/0x60
[  107.611805]   smpboot_thread_fn+0x1d8/0x280
[  107.611810]   kthread+0x14c/0x170
[  107.611816]   ret_from_fork+0x1f/0x30
[  107.611819]
               to a SOFTIRQ-irq-unsafe lock:
[  107.611825]  (lock#2){+.+.}-{2:2}
[  107.611828]
               ... which became SOFTIRQ-irq-unsafe at:
[  107.611837] ...
[  107.611839]   lock_acquire+0x1a7/0x3b0
[  107.611846]   __radix_tree_preload+0x5c/0x2a0
[  107.611851]   idr_preload+0xf/0xb0
[  107.611857]   __kernfs_new_node+0x66/0x270
[  107.611862]   kernfs_create_root+0xab/0x140
[  107.611868]   sysfs_init+0x14/0x53
[  107.611873]   mnt_init+0xc9/0x1f1
[  107.611878]   vfs_caches_init+0xca/0xd4
[  107.611884]   start_kernel+0x734/0x78a
[  107.611889]   secondary_startup_64_no_verify+0xb8/0xbb
[  107.611893]
               other info that might help us debug this:

[  107.611901]  Possible interrupt unsafe locking scenario:

[  107.611908]        CPU0                    CPU1
[  107.611912]        ----                    ----
[  107.611916]   lock(lock#2);
[  107.611921]                                local_irq_disable();
[  107.611926]                                lock(&xhci->lock);
[  107.611932]                                lock(lock#2);
[  107.611938]   <Interrupt>
[  107.611942]     lock(&xhci->lock);
[  107.611947]
                *** DEADLOCK ***

[  107.611954] 4 locks held by worker/4060:
[  107.611958]  #0: ffffffffb056a9e0 (rcu_read_lock){....}-{1:2}, at: hctx_lock+0x17/0xe0
[  107.611969]  #1: ffff90ba45d58048 (shost->host_lock){....}-{2:2}, at: uas_queuecommand+0x28/0x260 [uas]
[  107.611981]  #2: ffff90ba45d59848 (&devinfo->lock){..-.}-{2:2}, at: uas_queuecommand+0xd9/0x260 [uas]
[  107.611992]  #3: ffff90ba501a8430 (&xhci->lock){..-.}-{2:2}, at: xhci_urb_enqueue+0x20a/0x6f0 [xhci_hcd]
[  107.612008]
               the dependencies between SOFTIRQ-irq-safe lock and the holding lock:
[  107.612035] -> (&xhci->lock){..-.}-{2:2} {
[  107.612043]    IN-SOFTIRQ-W at:
[  107.612048]                     lock_acquire+0x1a7/0x3b0
[  107.612054]                     _raw_spin_lock_irqsave+0x3d/0x50
[  107.612066]                     xhci_hub_status_data+0xc2/0x390 [xhci_hcd]
[  107.612079]                     usb_hcd_poll_rh_status+0x41/0x180 [usbcore]
[  107.612085]                     call_timer_fn+0x9d/0x2b0
[  107.612091]                     run_timer_softirq+0x48d/0x570
[  107.612096]                     __do_softirq+0xeb/0x4a4
[  107.612102]                     run_ksoftirqd+0x32/0x60
[  107.612108]                     smpboot_thread_fn+0x1d8/0x280
[  107.612113]                     kthread+0x14c/0x170
[  107.612119]                     ret_from_fork+0x1f/0x30
[  107.612123]    INITIAL USE at:
[  107.612129]                    lock_acquire+0x1a7/0x3b0
[  107.612134]                    _raw_spin_lock_irqsave+0x3d/0x50
[  107.612146]                    xhci_hub_control+0xa5/0x2440 [xhci_hcd]
[  107.612158]                    usb_hcd_submit_urb+0x743/0xb00 [usbcore]
[  107.612171]                    usb_start_wait_urb+0x55/0x140 [usbcore]
[  107.612184]                    usb_control_msg+0xc3/0x110 [usbcore]
[  107.612195]                    hub_configure+0x103/0xc00 [usbcore]
[  107.612207]                    hub_probe+0x29c/0x350 [usbcore]
[  107.612220]                    usb_probe_interface+0x105/0x300 [usbcore]
[  107.612227]                    really_probe+0xf7/0x4d0
[  107.612232]                    driver_probe_device+0x5d/0x140
[  107.612238]                    bus_for_each_drv+0x5e/0x90
[  107.612244]                    __device_attach+0xbe/0x140
[  107.612249]                    bus_probe_device+0x9c/0xb0
[  107.612255]                    device_add+0x3fa/0x820
[  107.612267]                    usb_set_configuration+0x469/0x960 [usbcore]
[  107.612280]                    usb_generic_driver_probe+0x28/0x70 [usbcore]
[  107.612293]                    usb_probe_device+0x3b/0xc0 [usbcore]
[  107.612298]                    really_probe+0xf7/0x4d0
[  107.612304]                    driver_probe_device+0x5d/0x140
[  107.612309]                    bus_for_each_drv+0x5e/0x90
[  107.612315]                    __device_attach+0xbe/0x140
[  107.612320]                    bus_probe_device+0x9c/0xb0
[  107.612325]                    device_add+0x3fa/0x820
[  107.612337]                    usb_new_device+0x21d/0x4a0 [usbcore]
[  107.612349]                    usb_add_hcd+0x60b/0x7f0 [usbcore]
[  107.612361]                    usb_hcd_pci_probe+0x237/0x480 [usbcore]
[  107.612367]                    xhci_pci_probe+0x6d/0x1df [xhci_pci]
[  107.612374]                    local_pci_probe+0x42/0x90
[  107.612380]                    pci_device_probe+0xe7/0x1a0
[  107.612385]                    really_probe+0xf7/0x4d0
[  107.612391]                    driver_probe_device+0x5d/0x140
[  107.612397]                    device_driver_attach+0x4f/0x60
[  107.612402]                    __driver_attach+0xa4/0x140
[  107.612408]                    bus_for_each_dev+0x67/0x90
[  107.612414]                    bus_add_driver+0x18c/0x230
[  107.612419]                    driver_register+0x5b/0xf0
[  107.612425]                    do_one_initcall+0x54/0x2f0
[  107.612430]                    do_init_module+0x5b/0x21b
[  107.612436]                    load_module+0x1e40/0x2370
[  107.612442]                    __do_sys_finit_module+0x98/0xe0
[  107.612448]                    do_syscall_64+0x33/0x40
[  107.612453]                    entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  107.612458]  }
[  107.612467]  ... key      at: [<ffffffffc06044e0>] __key.49448+0x0/0xfffffffffffe5b20 [xhci_hcd]
[  107.612473]  ... acquired at:
[  107.612478]    lock_acquire+0x1a7/0x3b0
[  107.612483]    radix_tree_maybe_preload+0x49/0xb0
[  107.612493]    xhci_update_stream_segment_mapping+0x63/0x160 [xhci_hcd]
[  107.612503]    xhci_ring_expansion+0x19d/0x320 [xhci_hcd]
[  107.612514]    prepare_ring+0x8b/0x1f0 [xhci_hcd]
[  107.612523]    prepare_transfer+0x64/0x140 [xhci_hcd]
[  107.612534]    xhci_queue_bulk_tx+0xe7/0xb70 [xhci_hcd]
[  107.612544]    xhci_urb_enqueue+0x615/0x6f0 [xhci_hcd]
[  107.612556]    usb_hcd_submit_urb+0xa7/0xb00 [usbcore]
[  107.612561]    uas_submit_urbs+0x35b/0x500 [uas]
[  107.612566]    uas_queuecommand+0x1db/0x260 [uas]
[  107.612579]    scsi_queue_rq+0x5ca/0xbe0 [scsi_mod]
[  107.612585]    blk_mq_dispatch_rq_list+0x285/0x6d0
[  107.612590]    __blk_mq_do_dispatch_sched+0x210/0x260
[  107.612596]    __blk_mq_sched_dispatch_requests+0x133/0x170
[  107.612602]    blk_mq_sched_dispatch_requests+0x30/0x60
[  107.612607]    __blk_mq_run_hw_queue+0x6f/0xd0
[  107.612612]    __blk_mq_delay_run_hw_queue+0xda/0x1f0
[  107.612617]    blk_mq_run_hw_queue+0x6a/0xd0
[  107.612623]    blk_mq_sched_insert_requests+0xdd/0x2a0
[  107.612628]    blk_mq_flush_plug_list+0x12d/0x240
[  107.612633]    blk_flush_plug_list+0xb4/0xd0
[  107.612638]    blk_mq_submit_bio+0x365/0x770
[  107.612643]    submit_bio_noacct+0x41e/0x4b0
[  107.612648]    submit_bio+0x33/0x160
[  107.612677]    ext4_mpage_readpages+0x1ad/0x980 [ext4]
[  107.612683]    read_pages+0x7d/0x230
[  107.612688]    page_cache_ra_unbounded+0x138/0x1f0
[  107.612695]    generic_file_buffered_read+0x144/0xa10
[  107.612701]    do_iter_readv_writev+0x17c/0x1b0
[  107.612706]    do_iter_read+0xca/0x1c0
[  107.612711]    vfs_readv+0x6f/0xa0
[  107.612716]    do_preadv+0x9c/0xe0
[  107.612720]    do_syscall_64+0x33/0x40
[  107.612726]    entry_SYSCALL_64_after_hwframe+0x44/0xa9

[  107.612734]
               the dependencies between the lock to be acquired
[  107.612734]  and SOFTIRQ-irq-unsafe lock:
[  107.612761] -> (lock#2){+.+.}-{2:2} {
[  107.612769]    HARDIRQ-ON-W at:
[  107.612775]                     lock_acquire+0x1a7/0x3b0
[  107.612780]                     __radix_tree_preload+0x5c/0x2a0
[  107.612786]                     idr_preload+0xf/0xb0
[  107.612792]                     __kernfs_new_node+0x66/0x270
[  107.612798]                     kernfs_create_root+0xab/0x140
[  107.612804]                     sysfs_init+0x14/0x53
[  107.612809]                     mnt_init+0xc9/0x1f1
[  107.612814]                     vfs_caches_init+0xca/0xd4
[  107.612820]                     start_kernel+0x734/0x78a
[  107.612826]                     secondary_startup_64_no_verify+0xb8/0xbb
[  107.612831]    SOFTIRQ-ON-W at:
[  107.612836]                     lock_acquire+0x1a7/0x3b0
[  107.612841]                     __radix_tree_preload+0x5c/0x2a0
[  107.612847]                     idr_preload+0xf/0xb0
[  107.612853]                     __kernfs_new_node+0x66/0x270
[  107.612858]                     kernfs_create_root+0xab/0x140
[  107.612864]                     sysfs_init+0x14/0x53
[  107.612869]                     mnt_init+0xc9/0x1f1
[  107.612875]                     vfs_caches_init+0xca/0xd4
[  107.612880]                     start_kernel+0x734/0x78a
[  107.612885]                     secondary_startup_64_no_verify+0xb8/0xbb
[  107.612891]    INITIAL USE at:
[  107.612896]                    lock_acquire+0x1a7/0x3b0
[  107.612901]                    __radix_tree_preload+0x5c/0x2a0
[  107.612906]                    idr_preload+0xf/0xb0
[  107.612912]                    __kernfs_new_node+0x66/0x270
[  107.612918]                    kernfs_create_root+0xab/0x140
[  107.612923]                    sysfs_init+0x14/0x53
[  107.612928]                    mnt_init+0xc9/0x1f1
[  107.612933]                    vfs_caches_init+0xca/0xd4
[  107.612939]                    start_kernel+0x734/0x78a
[  107.612944]                    secondary_startup_64_no_verify+0xb8/0xbb
[  107.612949]  }
[  107.612953]  ... key      at: [<ffff90bba6c2c680>] 0xffff90bba6c2c680
[  107.612958]  ... acquired at:
[  107.612963]    lock_acquire+0x1a7/0x3b0
[  107.612968]    radix_tree_maybe_preload+0x49/0xb0
[  107.612978]    xhci_update_stream_segment_mapping+0x63/0x160 [xhci_hcd]
[  107.612989]    xhci_ring_expansion+0x19d/0x320 [xhci_hcd]
[  107.612999]    prepare_ring+0x8b/0x1f0 [xhci_hcd]
[  107.613008]    prepare_transfer+0x64/0x140 [xhci_hcd]
[  107.613019]    xhci_queue_bulk_tx+0xe7/0xb70 [xhci_hcd]
[  107.613029]    xhci_urb_enqueue+0x615/0x6f0 [xhci_hcd]
[  107.613041]    usb_hcd_submit_urb+0xa7/0xb00 [usbcore]
[  107.613047]    uas_submit_urbs+0x35b/0x500 [uas]
[  107.613052]    uas_queuecommand+0x1db/0x260 [uas]
[  107.613063]    scsi_queue_rq+0x5ca/0xbe0 [scsi_mod]
[  107.613068]    blk_mq_dispatch_rq_list+0x285/0x6d0
[  107.613073]    __blk_mq_do_dispatch_sched+0x210/0x260
[  107.613079]    __blk_mq_sched_dispatch_requests+0x133/0x170
[  107.613085]    blk_mq_sched_dispatch_requests+0x30/0x60
[  107.613090]    __blk_mq_run_hw_queue+0x6f/0xd0
[  107.613095]    __blk_mq_delay_run_hw_queue+0xda/0x1f0
[  107.613100]    blk_mq_run_hw_queue+0x6a/0xd0
[  107.613106]    blk_mq_sched_insert_requests+0xdd/0x2a0
[  107.613111]    blk_mq_flush_plug_list+0x12d/0x240
[  107.613116]    blk_flush_plug_list+0xb4/0xd0
[  107.613121]    blk_mq_submit_bio+0x365/0x770
[  107.613126]    submit_bio_noacct+0x41e/0x4b0
[  107.613131]    submit_bio+0x33/0x160
[  107.613153]    ext4_mpage_readpages+0x1ad/0x980 [ext4]
[  107.613158]    read_pages+0x7d/0x230
[  107.613163]    page_cache_ra_unbounded+0x138/0x1f0
[  107.613169]    generic_file_buffered_read+0x144/0xa10
[  107.613175]    do_iter_readv_writev+0x17c/0x1b0
[  107.613180]    do_iter_read+0xca/0x1c0
[  107.613185]    vfs_readv+0x6f/0xa0
[  107.613189]    do_preadv+0x9c/0xe0
[  107.613194]    do_syscall_64+0x33/0x40
[  107.613199]    entry_SYSCALL_64_after_hwframe+0x44/0xa9

[  107.613207]
               stack backtrace:
[  107.613214] CPU: 2 PID: 4060 Comm: worker Tainted: G S        I E     5.10.0.g23859ae-master #17
[  107.613220] Hardware name: HP HP Spectre x360 Convertible/804F, BIOS F.47 11/22/2017
[  107.613226] Call Trace:
[  107.613233]  dump_stack+0x77/0x97
[  107.613239]  check_irq_usage+0x7a8/0x7b0
[  107.613246]  ? check_path.constprop.54+0x22/0x40
[  107.613255]  ? __lock_acquire+0x14af/0x1a70
[  107.613260]  __lock_acquire+0x14af/0x1a70
[  107.613268]  lock_acquire+0x1a7/0x3b0
[  107.613273]  ? radix_tree_maybe_preload+0x8/0xb0
[  107.613283]  ? xhci_segment_alloc+0xd8/0x100 [xhci_hcd]
[  107.613290]  radix_tree_maybe_preload+0x49/0xb0
[  107.613295]  ? radix_tree_maybe_preload+0x8/0xb0
[  107.613305]  xhci_update_stream_segment_mapping+0x63/0x160 [xhci_hcd]
[  107.613316]  xhci_ring_expansion+0x19d/0x320 [xhci_hcd]
[  107.613328]  prepare_ring+0x8b/0x1f0 [xhci_hcd]
[  107.613338]  prepare_transfer+0x64/0x140 [xhci_hcd]
[  107.613350]  xhci_queue_bulk_tx+0xe7/0xb70 [xhci_hcd]
[  107.613363]  xhci_urb_enqueue+0x615/0x6f0 [xhci_hcd]
[  107.613377]  usb_hcd_submit_urb+0xa7/0xb00 [usbcore]
[  107.613383]  ? find_held_lock+0x2d/0x90
[  107.613389]  ? uas_submit_urbs+0x34a/0x500 [uas]
[  107.613396]  uas_submit_urbs+0x35b/0x500 [uas]
[  107.613403]  uas_queuecommand+0x1db/0x260 [uas]
[  107.613415]  scsi_queue_rq+0x5ca/0xbe0 [scsi_mod]
[  107.613421]  blk_mq_dispatch_rq_list+0x285/0x6d0
[  107.613430]  __blk_mq_do_dispatch_sched+0x210/0x260
[  107.613437]  __blk_mq_sched_dispatch_requests+0x133/0x170
[  107.613444]  blk_mq_sched_dispatch_requests+0x30/0x60
[  107.613449]  __blk_mq_run_hw_queue+0x6f/0xd0
[  107.613455]  __blk_mq_delay_run_hw_queue+0xda/0x1f0
[  107.613461]  blk_mq_run_hw_queue+0x6a/0xd0
[  107.613467]  blk_mq_sched_insert_requests+0xdd/0x2a0
[  107.613473]  blk_mq_flush_plug_list+0x12d/0x240
[  107.613480]  blk_flush_plug_list+0xb4/0xd0
[  107.613486]  blk_mq_submit_bio+0x365/0x770
[  107.613493]  submit_bio_noacct+0x41e/0x4b0
[  107.613500]  ? submit_bio+0x33/0x160
[  107.613505]  ? xa_load+0xb6/0x130
[  107.613509]  submit_bio+0x33/0x160
[  107.613514]  ? bio_add_page+0x29/0x70
[  107.613536]  ext4_mpage_readpages+0x1ad/0x980 [ext4]
[  107.613544]  ? find_held_lock+0x2d/0x90
[  107.613550]  read_pages+0x7d/0x230
[  107.613556]  ? lru_cache_add+0x113/0x1b0
[  107.613562]  ? page_cache_ra_unbounded+0x138/0x1f0
[  107.613567]  page_cache_ra_unbounded+0x138/0x1f0
[  107.613575]  generic_file_buffered_read+0x144/0xa10
[  107.613583]  do_iter_readv_writev+0x17c/0x1b0
[  107.613589]  do_iter_read+0xca/0x1c0
[  107.613595]  vfs_readv+0x6f/0xa0
[  107.613602]  ? __fget_files+0xea/0x1f0
[  107.613608]  ? __fget_files+0x102/0x1f0
[  107.613614]  ? do_preadv+0x9c/0xe0
[  107.613618]  do_preadv+0x9c/0xe0
[  107.613624]  do_syscall_64+0x33/0x40
[  107.613629]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  107.613637] RIP: 0033:0x7fd344b8cc83
[  107.613647] Code: 49 89 ca b8 27 01 00 00 0f 05 48 3d 01 f0 ff ff 73 34 c3 48 83 ec 08 e8 fb 64 01 00 48 89 04 24 49 89 ca b8 27 01 00 00 0f 05 <48> 8b 3c 24 48 89 c2 e8 41 65 01 00 48 89 d0 48 83 c4 08 48 3d 01
[  107.613657] RSP: 002b:00007fd33d39a850 EFLAGS: 00000293 ORIG_RAX: 0000000000000127
[  107.613664] RAX: ffffffffffffffda RBX: 00007fd20e0a46f0 RCX: 00007fd344b8cc83
[  107.613670] RDX: 0000000000000002 RSI: 00007fd3242644b0 RDI: 000000000000000b
[  107.613675] RBP: 0000563a9eb21140 R08: 0000000000000000 R09: 00000000ffffffff
[  107.613681] R10: 00000000da0c9000 R11: 0000000000000293 R12: 0000563a9cdc36b0
[  107.613686] R13: 0000563a9eb211a8 R14: 00007fd324269590 R15: 00007ffeab138280


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-29  6:03   ` Mike Galbraith
@ 2020-10-29  9:41     ` Mathias Nyman
  2020-10-29  9:42       ` Sebastian Andrzej Siewior
  2020-10-29 11:11       ` Mike Galbraith
  0 siblings, 2 replies; 24+ messages in thread
From: Mathias Nyman @ 2020-10-29  9:41 UTC (permalink / raw)
  To: Mike Galbraith, gregkh
  Cc: linux-usb, Hans de Goede, Li Jun, Sebastian Andrzej Siewior

Hi

On 29.10.2020 8.03, Mike Galbraith wrote:
> Greetings,
> 
> Testing $subject in shiny new 5.10-rt induced a lockdep grumble, so I
> plugged it into master and booted lappy with threadirqs.  When I then
> fired up my android-x86 vm toy, that kernel grumbled as well.
> 
> 
> [  107.611645] =====================================================
> [  107.611650] WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
> [  107.611656] 5.10.0.g23859ae-master #17 Tainted: G S        I E
> [  107.611660] -----------------------------------------------------
> [  107.611666] worker/4060 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
> [  107.611671] ffff90bba6d2c680 (lock#2){+.+.}-{2:2}, at: radix_tree_maybe_preload+0x8/0xb0
> [  107.611684]
>                and this task is already holding:
> [  107.611689] ffff90ba501a8430 (&xhci->lock){..-.}-{2:2}, at: xhci_urb_enqueue+0x20a/0x6f0 [xhci_hcd]
> [  107.611706] which would create a new lock dependency:
> [  107.611710]  (&xhci->lock){..-.}-{2:2} -> (lock#2){+.+.}-{2:2}
> [  107.611719]

This looks like a separate locking issue related to stream ring buffer expansion and radix tree usage.
This has probably been there for a while. Can you check if this can be reproduced with 5.9 kernel?

I don't have a quick fix for this. I need to look into it more.
Anyway, this shouldn't prevent PATCH 3/3 from fixing the streams debugfs issue introduced in 5.10-rc1

-Mathias

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-29  9:41     ` Mathias Nyman
@ 2020-10-29  9:42       ` Sebastian Andrzej Siewior
  2020-10-29 10:23         ` Mike Galbraith
  2020-10-29 11:11       ` Mike Galbraith
  1 sibling, 1 reply; 24+ messages in thread
From: Sebastian Andrzej Siewior @ 2020-10-29  9:42 UTC (permalink / raw)
  To: Mathias Nyman; +Cc: Mike Galbraith, gregkh, linux-usb, Hans de Goede, Li Jun

On 2020-10-29 11:41:41 [+0200], Mathias Nyman wrote:
> This looks like a separate locking issue related to stream ring buffer expansion and radix tree usage.
> This has probably been there for a while. Can you check if this can be reproduced with 5.9 kernel?
> 
> I don't have a quick fix for this. I need to look into it more.
> Anyway, this shouldn't prevent PATCH 3/3 from fixing the streams debugfs issue introduced in 5.10-rc1

let me look into this.

> -Mathias

Sebastian

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-29  9:42       ` Sebastian Andrzej Siewior
@ 2020-10-29 10:23         ` Mike Galbraith
  2020-10-29 10:37           ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 24+ messages in thread
From: Mike Galbraith @ 2020-10-29 10:23 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior, Mathias Nyman
  Cc: gregkh, linux-usb, Hans de Goede, Li Jun

On Thu, 2020-10-29 at 10:42 +0100, Sebastian Andrzej Siewior wrote:
> On 2020-10-29 11:41:41 [+0200], Mathias Nyman wrote:
> > This looks like a separate locking issue related to stream ring buffer expansion and radix tree usage.
> > This has probably been there for a while. Can you check if this can be reproduced with 5.9 kernel?
> >
> > I don't have a quick fix for this. I need to look into it more.
> > Anyway, this shouldn't prevent PATCH 3/3 from fixing the streams debugfs issue introduced in 5.10-rc1
>
> let me look into this.

Below, the aforementioned 5.10-rt splat in case that helps.

[  155.051661] ======================================================
[  155.051661] WARNING: possible circular locking dependency detected
[  155.051662] 5.10.0.g23859ae-rt1-rt #11 Tainted: G S        I E
[  155.051663] ------------------------------------------------------
[  155.051663] worker/4086 is trying to acquire lock:
[  155.051664] ffff8b6666daa090 ((lock).lock){+.+.}-{2:2}, at: radix_tree_maybe_preload+0x42/0x110
[  155.051672]
               but task is already holding lock:
[  155.051673] ffff8b6520c945b8 (&xhci->lock){+.+.}-{0:0}, at: xhci_urb_enqueue+0x217/0x740 [xhci_hcd]
[  155.051683]
               which lock already depends on the new lock.

[  155.051683]
               the existing dependency chain (in reverse order) is:
[  155.051683]
               -> #2 (&xhci->lock){+.+.}-{0:0}:
[  155.051685]        rt_spin_lock+0x2b/0xc0
[  155.051687]        xhci_irq+0x3f/0x540 [xhci_hcd]
[  155.051695]        irq_forced_thread_fn+0x30/0x80
[  155.051697]        irq_thread+0xf7/0x1a0
[  155.051698]        kthread+0x16c/0x190
[  155.051700]        ret_from_fork+0x1f/0x30
[  155.051702]
               -> #1 ((softirq_ctrl.lock).lock){+.+.}-{2:2}:
[  155.051703]        rt_spin_lock+0x2b/0xc0
[  155.051704]        __local_bh_disable_ip+0x127/0x2c0
[  155.051706]        __do_sys_bpf+0x1959/0x1f50
[  155.051708]        do_syscall_64+0x33/0x40
[  155.051710]        entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  155.051711]
               -> #0 ((lock).lock){+.+.}-{2:2}:
[  155.051712]        __lock_acquire+0x149d/0x1a70
[  155.051714]        lock_acquire+0x1a7/0x410
[  155.051716]        rt_spin_lock+0x2b/0xc0
[  155.051716]        radix_tree_maybe_preload+0x42/0x110
[  155.051718]        xhci_update_stream_segment_mapping+0x63/0x190 [xhci_hcd]
[  155.051723]        xhci_ring_expansion+0x19d/0x390 [xhci_hcd]
[  155.051728]        prepare_ring+0x8b/0x1f0 [xhci_hcd]
[  155.051734]        prepare_transfer+0x64/0x140 [xhci_hcd]
[  155.051739]        xhci_queue_bulk_tx+0xe7/0xc50 [xhci_hcd]
[  155.051745]        xhci_urb_enqueue+0x627/0x740 [xhci_hcd]
[  155.051750]        usb_hcd_submit_urb+0xa7/0xac0 [usbcore]
[  155.051760]        uas_submit_urbs+0x35b/0x500 [uas]
[  155.051762]        uas_queuecommand+0x1d3/0x240 [uas]
[  155.051763]        scsi_queue_rq+0x5e5/0xce0 [scsi_mod]
[  155.051772]        blk_mq_dispatch_rq_list+0x291/0x6e0
[  155.051774]        __blk_mq_do_dispatch_sched+0x219/0x260
[  155.051775]        __blk_mq_sched_dispatch_requests+0x154/0x190
[  155.051776]        blk_mq_sched_dispatch_requests+0x33/0x60
[  155.051777]        __blk_mq_run_hw_queue+0x86/0xf0
[  155.051779]        __blk_mq_delay_run_hw_queue+0x135/0x180
[  155.051781]        blk_mq_run_hw_queue+0x70/0xe0
[  155.051782]        blk_mq_sched_insert_requests+0xdd/0x2a0
[  155.051783]        blk_mq_flush_plug_list+0x12d/0x2b0
[  155.051784]        blk_flush_plug_list+0xb4/0xd0
[  155.051785]        blk_mq_submit_bio+0x365/0x930
[  155.051786]        submit_bio_noacct+0x41e/0x4b0
[  155.051786]        submit_bio+0x33/0x160
[  155.051787]        ext4_mpage_readpages+0x1ad/0x930 [ext4]
[  155.051811]        read_pages+0x7d/0x230
[  155.051813]        page_cache_ra_unbounded+0x13b/0x1f0
[  155.051814]        generic_file_buffered_read+0x144/0xa10
[  155.051815]        do_iter_readv_writev+0x182/0x1b0
[  155.051817]        do_iter_read+0xca/0x1c0
[  155.051818]        vfs_readv+0x6f/0xa0
[  155.051819]        do_preadv+0x9c/0xe0
[  155.051820]        do_syscall_64+0x33/0x40
[  155.051822]        entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  155.051823]
               other info that might help us debug this:

[  155.051824] Chain exists of:
                 (lock).lock --> (softirq_ctrl.lock).lock --> &xhci->lock

[  155.051825]  Possible unsafe locking scenario:

[  155.051826]        CPU0                    CPU1
[  155.051826]        ----                    ----
[  155.051826]   lock(&xhci->lock);
[  155.051827]                                lock((softirq_ctrl.lock).lock);
[  155.051828]                                lock(&xhci->lock);
[  155.051828]   lock((lock).lock);
[  155.051829]
                *** DEADLOCK ***

[  155.051829] 7 locks held by worker/4086:
[  155.051830]  #0: ffffffffbd59c8e0 (rcu_read_lock){....}-{1:2}, at: hctx_lock+0x17/0xe0
[  155.051833]  #1: ffff8b65201340c0 (shost->host_lock){+.+.}-{0:0}, at: uas_queuecommand+0x2b/0x240 [uas]
[  155.051837]  #2: ffffffffbd59c8e0 (rcu_read_lock){....}-{1:2}, at: rt_spin_lock+0x5/0xc0
[  155.051839]  #3: ffff8b6520136010 (&devinfo->lock){+.+.}-{0:0}, at: uas_queuecommand+0xd7/0x240 [uas]
[  155.051841]  #4: ffffffffbd59c8e0 (rcu_read_lock){....}-{1:2}, at: rt_spin_lock+0x5/0xc0
[  155.051843]  #5: ffff8b6520c945b8 (&xhci->lock){+.+.}-{0:0}, at: xhci_urb_enqueue+0x217/0x740 [xhci_hcd]
[  155.051850]  #6: ffffffffbd59c8e0 (rcu_read_lock){....}-{1:2}, at: rt_spin_lock+0x5/0xc0
[  155.051853]
               stack backtrace:
[  155.051853] CPU: 3 PID: 4086 Comm: worker Tainted: G S        I E     5.10.0.g23859ae-rt1-rt #11
[  155.051855] Hardware name: HP HP Spectre x360 Convertible/804F, BIOS F.47 11/22/2017
[  155.051856] Call Trace:
[  155.051858]  dump_stack+0x77/0x97
[  155.051861]  check_noncircular+0xe7/0x100
[  155.051865]  ? __lock_acquire+0x149d/0x1a70
[  155.051867]  __lock_acquire+0x149d/0x1a70
[  155.051870]  lock_acquire+0x1a7/0x410
[  155.051872]  ? radix_tree_maybe_preload+0x42/0x110
[  155.051875]  ? xhci_segment_alloc+0xd8/0x100 [xhci_hcd]
[  155.051880]  rt_spin_lock+0x2b/0xc0
[  155.051882]  ? radix_tree_maybe_preload+0x42/0x110
[  155.051883]  radix_tree_maybe_preload+0x42/0x110
[  155.051885]  xhci_update_stream_segment_mapping+0x63/0x190 [xhci_hcd]
[  155.051891]  xhci_ring_expansion+0x19d/0x390 [xhci_hcd]
[  155.051897]  prepare_ring+0x8b/0x1f0 [xhci_hcd]
[  155.051903]  prepare_transfer+0x64/0x140 [xhci_hcd]
[  155.051909]  xhci_queue_bulk_tx+0xe7/0xc50 [xhci_hcd]
[  155.051919]  xhci_urb_enqueue+0x627/0x740 [xhci_hcd]
[  155.051924]  ? dma_map_sg_attrs+0x30/0x50
[  155.051926]  ? usb_hcd_map_urb_for_dma+0xc2/0x460 [usbcore]
[  155.051935]  usb_hcd_submit_urb+0xa7/0xac0 [usbcore]
[  155.051942]  ? mark_held_locks+0x49/0x70
[  155.051944]  ? _raw_spin_unlock_irqrestore+0x70/0x80
[  155.051945]  ? lockdep_hardirqs_on+0x85/0x110
[  155.051947]  ? _raw_spin_unlock_irqrestore+0x34/0x80
[  155.051948]  ? rt_mutex_futex_unlock+0x56/0x80
[  155.051950]  uas_submit_urbs+0x35b/0x500 [uas]
[  155.051952]  ? sdev_evt_alloc+0x60/0x60 [scsi_mod]
[  155.051959]  uas_queuecommand+0x1d3/0x240 [uas]
[  155.051962]  scsi_queue_rq+0x5e5/0xce0 [scsi_mod]
[  155.051969]  blk_mq_dispatch_rq_list+0x291/0x6e0
[  155.051970]  ? rt_mutex_futex_unlock+0x56/0x80
[  155.051973]  __blk_mq_do_dispatch_sched+0x219/0x260
[  155.051976]  __blk_mq_sched_dispatch_requests+0x154/0x190
[  155.051978]  blk_mq_sched_dispatch_requests+0x33/0x60
[  155.051979]  __blk_mq_run_hw_queue+0x86/0xf0
[  155.051981]  __blk_mq_delay_run_hw_queue+0x135/0x180
[  155.051983]  blk_mq_run_hw_queue+0x70/0xe0
[  155.051985]  blk_mq_sched_insert_requests+0xdd/0x2a0
[  155.051987]  blk_mq_flush_plug_list+0x12d/0x2b0
[  155.051990]  blk_flush_plug_list+0xb4/0xd0
[  155.051992]  blk_mq_submit_bio+0x365/0x930
[  155.051994]  submit_bio_noacct+0x41e/0x4b0
[  155.051996]  ? submit_bio+0x33/0x160
[  155.051997]  ? xa_load+0xb6/0x130
[  155.051999]  submit_bio+0x33/0x160
[  155.052000]  ? bio_add_page+0x29/0x70
[  155.052002]  ext4_mpage_readpages+0x1ad/0x930 [ext4]
[  155.052021]  ? _raw_spin_unlock_irqrestore+0x70/0x80
[  155.052022]  ? lockdep_hardirqs_on+0x85/0x110
[  155.052024]  read_pages+0x7d/0x230
[  155.052026]  ? rcu_read_lock_sched_held+0x32/0x60
[  155.052029]  ? page_cache_ra_unbounded+0x13b/0x1f0
[  155.052030]  page_cache_ra_unbounded+0x13b/0x1f0
[  155.052032]  generic_file_buffered_read+0x144/0xa10
[  155.052036]  do_iter_readv_writev+0x182/0x1b0
[  155.052038]  do_iter_read+0xca/0x1c0
[  155.052040]  vfs_readv+0x6f/0xa0
[  155.052043]  ? __fget_files+0xf6/0x1f0
[  155.052045]  ? __fget_files+0x10e/0x1f0
[  155.052047]  ? do_preadv+0x9c/0xe0
[  155.052048]  do_preadv+0x9c/0xe0
[  155.052049]  do_syscall_64+0x33/0x40
[  155.052051]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  155.052053] RIP: 0033:0x7f70da5adc83
[  155.052055] Code: 49 89 ca b8 27 01 00 00 0f 05 48 3d 01 f0 ff ff 73 34 c3 48 83 ec 08 e8 fb 64 01 00 48 89 04 24 49 89 ca b8 27 01 00 00 0f 05 <48> 8b 3c 24 48 89 c2 e8 41 65 01 00 48 89 d0 48 83 c4 08 48 3d 01
[  155.052056] RSP: 002b:00007f70d2dbb850 EFLAGS: 00000293 ORIG_RAX: 0000000000000127
[  155.052058] RAX: ffffffffffffffda RBX: 00007f6fa6ded6f0 RCX: 00007f70da5adc83
[  155.052058] RDX: 0000000000000002 RSI: 00007f70b80f45d0 RDI: 000000000000000b
[  155.052059] RBP: 000055752e62e140 R08: 0000000000000000 R09: 00000000ffffffff
[  155.052059] R10: 000000015fd7e000 R11: 0000000000000293 R12: 000055752bfc36b0
[  155.052060] R13: 000055752e62e1a8 R14: 00007f70b81a4540 R15: 00007ffe9e730b10


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-29 10:23         ` Mike Galbraith
@ 2020-10-29 10:37           ` Sebastian Andrzej Siewior
  0 siblings, 0 replies; 24+ messages in thread
From: Sebastian Andrzej Siewior @ 2020-10-29 10:37 UTC (permalink / raw)
  To: Mike Galbraith; +Cc: Mathias Nyman, gregkh, linux-usb, Hans de Goede, Li Jun

On 2020-10-29 11:23:25 [+0100], Mike Galbraith wrote:
> Below, the aforementioned 5.10-rt splat in case that helps.
> 
> [  155.051826]        CPU0                    CPU1
> [  155.051826]        ----                    ----
> [  155.051826]   lock(&xhci->lock);
> [  155.051827]                                lock((softirq_ctrl.lock).lock);
> [  155.051828]                                lock(&xhci->lock);
> [  155.051828]   lock((lock).lock);
> [  155.051829]

no, that is yet again different. Based on these lines the reason is most
likely the same as in net/TCP you reported a few releases ago (RT
specific).
I need to consult core & locking overlords to figure out how we deal
with this in future since is seems this cases pop up in packs.

If you have more lockdep reports with softirq_ctrl.lock involved, please
throw them at me.

Sebastian

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-29  9:41     ` Mathias Nyman
  2020-10-29  9:42       ` Sebastian Andrzej Siewior
@ 2020-10-29 11:11       ` Mike Galbraith
  2020-10-29 11:22         ` Mathias Nyman
  1 sibling, 1 reply; 24+ messages in thread
From: Mike Galbraith @ 2020-10-29 11:11 UTC (permalink / raw)
  To: Mathias Nyman, gregkh
  Cc: linux-usb, Hans de Goede, Li Jun, Sebastian Andrzej Siewior

On Thu, 2020-10-29 at 11:41 +0200, Mathias Nyman wrote:
> Can you check if this can be reproduced with 5.9 kernel?

Nope, 5.9.2 didn't reproduce.

	-Mike


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-29 11:11       ` Mike Galbraith
@ 2020-10-29 11:22         ` Mathias Nyman
  2020-10-29 11:38           ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 24+ messages in thread
From: Mathias Nyman @ 2020-10-29 11:22 UTC (permalink / raw)
  To: Mike Galbraith, gregkh
  Cc: linux-usb, Hans de Goede, Li Jun, Sebastian Andrzej Siewior

On 29.10.2020 13.11, Mike Galbraith wrote:
> On Thu, 2020-10-29 at 11:41 +0200, Mathias Nyman wrote:
>> Can you check if this can be reproduced with 5.9 kernel?
> 
> Nope, 5.9.2 didn't reproduce.
> 

Very odd.
It might be hard to reproduce as it requires xhci ring expansion to trigger it,
meaning at some point there is so much data queued to a device the current ring buffer
can't fit it.

Other possibility is that there were some radix tree changes in 5.10-rc1, haven't looked into those.

-Mathias


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-29 11:22         ` Mathias Nyman
@ 2020-10-29 11:38           ` Sebastian Andrzej Siewior
  2020-10-29 12:44             ` Jun Li
  2020-10-29 13:08             ` Mathias Nyman
  0 siblings, 2 replies; 24+ messages in thread
From: Sebastian Andrzej Siewior @ 2020-10-29 11:38 UTC (permalink / raw)
  To: Mathias Nyman; +Cc: Mike Galbraith, gregkh, linux-usb, Hans de Goede, Li Jun

On 2020-10-29 13:22:20 [+0200], Mathias Nyman wrote:
> On 29.10.2020 13.11, Mike Galbraith wrote:
> > On Thu, 2020-10-29 at 11:41 +0200, Mathias Nyman wrote:
> >> Can you check if this can be reproduced with 5.9 kernel?
> > 
> > Nope, 5.9.2 didn't reproduce.
> > 
> 
> Very odd.
> It might be hard to reproduce as it requires xhci ring expansion to trigger it,
> meaning at some point there is so much data queued to a device the current ring buffer
> can't fit it.
> 
> Other possibility is that there were some radix tree changes in 5.10-rc1, haven't looked into those.

This came with commit
   673d74683627b ("usb: xhci: add debugfs support for ep with stream")

which is appeared in v5.10-rc1. This hunk below works around it:

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 482fe8c5e3b47..699777fb523b6 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3533,11 +3533,13 @@ static int xhci_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev,
 		xhci_dbg(xhci, "Slot %u ep ctx %u now has streams.\n",
 			 udev->slot_id, ep_index);
 		vdev->eps[ep_index].ep_state |= EP_HAS_STREAMS;
-		xhci_debugfs_create_stream_files(xhci, vdev, ep_index);
 	}
 	xhci_free_command(xhci, config_cmd);
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
+	for (i = 0; i < num_eps; i++)
+		xhci_debugfs_create_stream_files(xhci, vdev, ep_index);
+
 	/* Subtract 1 for stream 0, which drivers can't use */
 	return num_streams - 1;
 
This makes the warning go away here, I'm not 100% sure that this okay.
One thing I noticed: free_streams() does not remove the debugfs file so:
| $ cd /sys/bus/usb/drivers/uas
| $ echo 2-1:1.0 > bind
| $ echo 2-1:1.0 > unbind

and the kernel log shows:
|sd 7:0:0:0: [sdb] Synchronizing SCSI cache
|sd 7:0:0:0: [sdb] Synchronize Cache(10) failed: Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK
|debugfs: File 'stream_id' in directory 'ep07' already present!
|debugfs: File 'stream_context_array' in directory 'ep07' already present!
|debugfs: File 'stream_id' in directory 'ep07' already present!
|debugfs: File 'stream_context_array' in directory 'ep07' already present!
|scsi host7: uas
|scsi 7:0:0:0: Direct-Access     QEMU     QEMU HARDDISK    2.5+ PQ: 0 ANSI: 5
|sd 7:0:0:0: Attached scsi generic sg2 type 0
|sd 7:0:0:0: [sdb] 262144 512-byte logical blocks: (134 MB/128 MiB)
|sd 7:0:0:0: [sdb] Write Protect is off
|sd 7:0:0:0: [sdb] Mode Sense: 63 00 00 08
|sd 7:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
|sd 7:0:0:0: [sdb] Attached SCSI disk

> -Mathias

Sebastian

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* RE: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-29 11:38           ` Sebastian Andrzej Siewior
@ 2020-10-29 12:44             ` Jun Li
  2020-10-29 13:08             ` Mathias Nyman
  1 sibling, 0 replies; 24+ messages in thread
From: Jun Li @ 2020-10-29 12:44 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior, Mathias Nyman
  Cc: Mike Galbraith, gregkh, linux-usb, Hans de Goede



> -----Original Message-----
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Sent: Thursday, October 29, 2020 7:38 PM
> To: Mathias Nyman <mathias.nyman@linux.intel.com>
> Cc: Mike Galbraith <efault@gmx.de>; gregkh@linuxfoundation.org;
> linux-usb@vger.kernel.org; Hans de Goede <hdegoede@redhat.com>; Jun Li
> <jun.li@nxp.com>
> Subject: Re: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock
> held.
> 
> On 2020-10-29 13:22:20 [+0200], Mathias Nyman wrote:
> > On 29.10.2020 13.11, Mike Galbraith wrote:
> > > On Thu, 2020-10-29 at 11:41 +0200, Mathias Nyman wrote:
> > >> Can you check if this can be reproduced with 5.9 kernel?
> > >
> > > Nope, 5.9.2 didn't reproduce.
> > >
> >
> > Very odd.
> > It might be hard to reproduce as it requires xhci ring expansion to
> > trigger it, meaning at some point there is so much data queued to a
> > device the current ring buffer can't fit it.
> >
> > Other possibility is that there were some radix tree changes in 5.10-rc1,
> haven't looked into those.
> 
> This came with commit
>    673d74683627b ("usb: xhci: add debugfs support for ep with stream")
> 
> which is appeared in v5.10-rc1. This hunk below works around it:
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index
> 482fe8c5e3b47..699777fb523b6 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -3533,11 +3533,13 @@ static int xhci_alloc_streams(struct usb_hcd *hcd,
> struct usb_device *udev,
>  		xhci_dbg(xhci, "Slot %u ep ctx %u now has streams.\n",
>  			 udev->slot_id, ep_index);
>  		vdev->eps[ep_index].ep_state |= EP_HAS_STREAMS;
> -		xhci_debugfs_create_stream_files(xhci, vdev, ep_index);
>  	}
>  	xhci_free_command(xhci, config_cmd);
>  	spin_unlock_irqrestore(&xhci->lock, flags);
> 
> +	for (i = 0; i < num_eps; i++)
> +		xhci_debugfs_create_stream_files(xhci, vdev, ep_index);
> +
>  	/* Subtract 1 for stream 0, which drivers can't use */
>  	return num_streams - 1;
> 
> This makes the warning go away here, I'm not 100% sure that this okay.
> One thing I noticed: free_streams() does not remove the debugfs file so:
> | $ cd /sys/bus/usb/drivers/uas
> | $ echo 2-1:1.0 > bind
> | $ echo 2-1:1.0 > unbind
> 
> and the kernel log shows:
> |sd 7:0:0:0: [sdb] Synchronizing SCSI cache sd 7:0:0:0: [sdb]
> |Synchronize Cache(10) failed: Result: hostbyte=DID_ERROR
> |driverbyte=DRIVER_OK
> |debugfs: File 'stream_id' in directory 'ep07' already present!
> |debugfs: File 'stream_context_array' in directory 'ep07' already present!
> |debugfs: File 'stream_id' in directory 'ep07' already present!
> |debugfs: File 'stream_context_array' in directory 'ep07' already present!
> |scsi host7: uas
> |scsi 7:0:0:0: Direct-Access     QEMU     QEMU HARDDISK    2.5+ PQ: 0 ANSI:
> 5
> |sd 7:0:0:0: Attached scsi generic sg2 type 0 sd 7:0:0:0: [sdb] 262144
> |512-byte logical blocks: (134 MB/128 MiB) sd 7:0:0:0: [sdb] Write
> |Protect is off sd 7:0:0:0: [sdb] Mode Sense: 63 00 00 08 sd 7:0:0:0:
> |[sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or
> |FUA sd 7:0:0:0: [sdb] Attached SCSI disk

This is because your change is using wrong *ep_index* when create
stream files, if you use the below patch from Mathias, you will not
have this problem.

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 482fe8c5e3b4..d4a8d0efbbc4 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3533,11 +3533,14 @@ static int xhci_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev,
 		xhci_dbg(xhci, "Slot %u ep ctx %u now has streams.\n",
 			 udev->slot_id, ep_index);
 		vdev->eps[ep_index].ep_state |= EP_HAS_STREAMS;
-		xhci_debugfs_create_stream_files(xhci, vdev, ep_index);
 	}
 	xhci_free_command(xhci, config_cmd);
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
+	for (i = 0; i < num_eps; i++) {
+		ep_index = xhci_get_endpoint_index(&eps[i]->desc);
+		xhci_debugfs_create_stream_files(xhci, vdev, ep_index);
+	}
 	/* Subtract 1 for stream 0, which drivers can't use */
 	return num_streams - 1;

Li Jun
> 
> > -Mathias
> 
> Sebastian

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-29 11:38           ` Sebastian Andrzej Siewior
  2020-10-29 12:44             ` Jun Li
@ 2020-10-29 13:08             ` Mathias Nyman
  2020-10-29 15:10               ` Sebastian Andrzej Siewior
                                 ` (2 more replies)
  1 sibling, 3 replies; 24+ messages in thread
From: Mathias Nyman @ 2020-10-29 13:08 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Mike Galbraith, gregkh, linux-usb, Hans de Goede, Li Jun

On 29.10.2020 13.38, Sebastian Andrzej Siewior wrote:
> On 2020-10-29 13:22:20 [+0200], Mathias Nyman wrote:
>> On 29.10.2020 13.11, Mike Galbraith wrote:
>>> On Thu, 2020-10-29 at 11:41 +0200, Mathias Nyman wrote:
>>>> Can you check if this can be reproduced with 5.9 kernel?
>>>
>>> Nope, 5.9.2 didn't reproduce.
>>>
>>
>> Very odd.
>> It might be hard to reproduce as it requires xhci ring expansion to trigger it,
>> meaning at some point there is so much data queued to a device the current ring buffer
>> can't fit it.
>>
>> Other possibility is that there were some radix tree changes in 5.10-rc1, haven't looked into those.
> 
> This came with commit
>    673d74683627b ("usb: xhci: add debugfs support for ep with stream")
> 
> which is appeared in v5.10-rc1. This hunk below works around it:
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 482fe8c5e3b47..699777fb523b6 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -3533,11 +3533,13 @@ static int xhci_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev,
>  		xhci_dbg(xhci, "Slot %u ep ctx %u now has streams.\n",
>  			 udev->slot_id, ep_index);
>  		vdev->eps[ep_index].ep_state |= EP_HAS_STREAMS;
> -		xhci_debugfs_create_stream_files(xhci, vdev, ep_index);
>  	}
>  	xhci_free_command(xhci, config_cmd);
>  	spin_unlock_irqrestore(&xhci->lock, flags);
>  
> +	for (i = 0; i < num_eps; i++)
> +		xhci_debugfs_create_stream_files(xhci, vdev, ep_index);
> +
>  	/* Subtract 1 for stream 0, which drivers can't use */
>  	return num_streams - 1;
>  

Sebastian, I think you might be missing the first mail in this thread. 

https://lore.kernel.org/linux-usb/20201028203124.375344-4-mathias.nyman@linux.intel.com/

Let me do a quick recap to avoid confusion here. So far we have 2 issues.

1. Lockdep issue #1, stream debugfs entry created with local interrupts disabled and xhci->lock held
   Cause: 673d74683627b ("usb: xhci: add debugfs support for ep with stream") in v5.10-rc1
   Fix:  [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held. (i.e. the mail we are replying to)
   Comment: easily reproduced, enable lockdep and uas, mount debugfs and connect a UAS device.
            discovered by both Hans and Mike
.
2. Lockdep issue #2, adding entries to radix tree during (stream) ring expansion with interrupts disabled and xhci->lock held.
   Cause: unknown, probably a patch since we started using radix trees for finding streams
   Fix: unknown.
   Comment: Discovered by Mike when testing fix for issue#1. I suspect it can be reproduced on 5.9 but is 
   probably really hard as it involves ring expansion.

-Mathias



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-29 13:08             ` Mathias Nyman
@ 2020-10-29 15:10               ` Sebastian Andrzej Siewior
  2020-10-29 16:26               ` David Laight
  2020-10-29 17:43               ` Sebastian Andrzej Siewior
  2 siblings, 0 replies; 24+ messages in thread
From: Sebastian Andrzej Siewior @ 2020-10-29 15:10 UTC (permalink / raw)
  To: Mathias Nyman; +Cc: Mike Galbraith, gregkh, linux-usb, Hans de Goede, Li Jun

On 2020-10-29 15:08:06 [+0200], Mathias Nyman wrote:
> Sebastian, I think you might be missing the first mail in this thread. 

thank you for clearing that up.

> https://lore.kernel.org/linux-usb/20201028203124.375344-4-mathias.nyman@linux.intel.com/
> 
> Let me do a quick recap to avoid confusion here. So far we have 2 issues.
> 
> 1. Lockdep issue #1, stream debugfs entry created with local interrupts disabled and xhci->lock held
>    Cause: 673d74683627b ("usb: xhci: add debugfs support for ep with stream") in v5.10-rc1
>    Fix:  [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held. (i.e. the mail we are replying to)
>    Comment: easily reproduced, enable lockdep and uas, mount debugfs and connect a UAS device.
>             discovered by both Hans and Mike
> .
> 2. Lockdep issue #2, adding entries to radix tree during (stream) ring expansion with interrupts disabled and xhci->lock held.
>    Cause: unknown, probably a patch since we started using radix trees for finding streams
>    Fix: unknown.
>    Comment: Discovered by Mike when testing fix for issue#1. I suspect it can be reproduced on 5.9 but is 
>    probably really hard as it involves ring expansion.
> 
> -Mathias

Sebastian

^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-29 13:08             ` Mathias Nyman
  2020-10-29 15:10               ` Sebastian Andrzej Siewior
@ 2020-10-29 16:26               ` David Laight
  2020-10-29 17:43               ` Sebastian Andrzej Siewior
  2 siblings, 0 replies; 24+ messages in thread
From: David Laight @ 2020-10-29 16:26 UTC (permalink / raw)
  To: 'Mathias Nyman', Sebastian Andrzej Siewior
  Cc: Mike Galbraith, gregkh, linux-usb, Hans de Goede, Li Jun

From: Mathias Nyman
> Sent: 29 October 2020 13:08
...
> 2. Lockdep issue #2, adding entries to radix tree during (stream) ring expansion with interrupts
> disabled and xhci->lock held.
>    Cause: unknown, probably a patch since we started using radix trees for finding streams
>    Fix: unknown.
>    Comment: Discovered by Mike when testing fix for issue#1.
>    I suspect it can be reproduced on 5.9 but is
>    probably really hard as it involves ring expansion.

ISTR thinking that code was allocating 'memory pages' in a place
where (IIRC) vmalloc() wouldn't be allowed.

I'd sort of assumed the allocation function was allowed.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-29 13:08             ` Mathias Nyman
  2020-10-29 15:10               ` Sebastian Andrzej Siewior
  2020-10-29 16:26               ` David Laight
@ 2020-10-29 17:43               ` Sebastian Andrzej Siewior
  2 siblings, 0 replies; 24+ messages in thread
From: Sebastian Andrzej Siewior @ 2020-10-29 17:43 UTC (permalink / raw)
  To: Mathias Nyman
  Cc: Mike Galbraith, gregkh, linux-usb, Hans de Goede, Li Jun,
	Peter Zijlstra, Thomas Gleixner

On 2020-10-29 15:08:06 [+0200], Mathias Nyman wrote:
> 2. Lockdep issue #2, adding entries to radix tree during (stream) ring expansion with interrupts disabled and xhci->lock held.
>    Cause: unknown, probably a patch since we started using radix trees for finding streams
>    Fix: unknown.
>    Comment: Discovered by Mike when testing fix for issue#1. I suspect it can be reproduced on 5.9 but is 
>    probably really hard as it involves ring expansion.

So lockdep complains with this:
      CPU0                    CPU1
      ----                    ----
 lock(lock#2);
                              local_irq_disable();
                              lock(&xhci->lock);
                              lock(lock#2);
 <Interrupt>
   lock(&xhci->lock);

(https://lore.kernel.org/linux-usb/1cbb8b7defb1db1d4747995c11ebebb3dd9a66ec.camel@gmx.de/)

lockdep does not like that preload is used with disabled interrupts (in
non-interrupt context) and acquires lock#2 (the local_lock) which could
lead to a dead-lock as described above.
The flaw in the logic above is that lock#2 on CPU0 != CPU1.

We could make lockdep happy and remove what it observes on CPU1 by doing
this:

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 138ba4528dd3a..2cb8874c1c51f 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -197,12 +197,15 @@ static int xhci_insert_segment_mapping(struct radix_tree_root *trb_address_map,
 	if (radix_tree_lookup(trb_address_map, key))
 		return 0;
 
-	ret = radix_tree_maybe_preload(mem_flags);
-	if (ret)
-		return ret;
-	ret = radix_tree_insert(trb_address_map,
-			key, ring);
-	radix_tree_preload_end();
+	if (gfpflags_allow_blocking(mem_flags)) {
+		ret = radix_tree_maybe_preload(mem_flags);
+		if (ret)
+			return ret;
+	}
+	ret = radix_tree_insert(trb_address_map, key, ring);
+
+	if (gfpflags_allow_blocking(mem_flags))
+		radix_tree_preload_end();
 	return ret;
 }
 

There is no pre-load in GFP_ATOMIC case but it still acquires the
local_lock.
We might be able to drop radix_tree_maybe_preload(). I saw GFP_KERNEL
only during enumeration from xhci_alloc_streams(). Later, while
transfers were pending it came always from the scsi stack with disabled
interrupts and xhci->lock acquired.

> -Mathias

Sebastian

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* RE: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held.
  2020-10-28 20:31 ` [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held Mathias Nyman
  2020-10-29  6:03   ` Mike Galbraith
@ 2020-10-30  8:01   ` Jun Li
  1 sibling, 0 replies; 24+ messages in thread
From: Jun Li @ 2020-10-30  8:01 UTC (permalink / raw)
  To: Mathias Nyman, gregkh; +Cc: linux-usb, Hans de Goede, Mike Galbraith



> -----Original Message-----
> From: Mathias Nyman <mathias.nyman@linux.intel.com>
> Sent: Thursday, October 29, 2020 4:31 AM
> To: gregkh@linuxfoundation.org
> Cc: linux-usb@vger.kernel.org; Mathias Nyman
> <mathias.nyman@linux.intel.com>; Hans de Goede <hdegoede@redhat.com>; Mike
> Galbraith <efault@gmx.de>; Jun Li <jun.li@nxp.com>
> Subject: [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock
> held.
> 
> Creating debugfs files while loding the spin_lock_irqsave(xhci->lock)
> creates a lock dependecy that could possibly deadlock.
> 
> Lockdep warns:
> 
> =====================================================
> WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
> 5.10.0-rc1pdx86+ #8 Not tainted
> -----------------------------------------------------
> systemd-udevd/386 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
> ffffffffb1a94038 (pin_fs_lock){+.+.}-{2:2}, at: simple_pin_fs+0x22/0xa0
> 
> and this task is already holding:
> ffff9e7b87fbc430 (&xhci->lock){-.-.}-{2:2}, at:
> xhci_alloc_streams+0x5f9/0x810 which would create a new lock dependency:
> (&xhci->lock){-.-.}-{2:2} -> (pin_fs_lock){+.+.}-{2:2}
> 
> Create the files a bit later after lock is released.
> 
> Reported-by: Hans de Goede <hdegoede@redhat.com>
> Reported-by: Mike Galbraith <efault@gmx.de>
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> CC: Li Jun <jun.li@nxp.com>
> Fixes: 673d74683627 ("usb: xhci: add debugfs support for ep with stream")
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>

Reviewed-and-tested-by: Li Jun <jun.li@nxp.com>

> ---
>  drivers/usb/host/xhci.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index
> 482fe8c5e3b4..d4a8d0efbbc4 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -3533,11 +3533,14 @@ static int xhci_alloc_streams(struct usb_hcd *hcd,
> struct usb_device *udev,
>  		xhci_dbg(xhci, "Slot %u ep ctx %u now has streams.\n",
>  			 udev->slot_id, ep_index);
>  		vdev->eps[ep_index].ep_state |= EP_HAS_STREAMS;
> -		xhci_debugfs_create_stream_files(xhci, vdev, ep_index);
>  	}
>  	xhci_free_command(xhci, config_cmd);
>  	spin_unlock_irqrestore(&xhci->lock, flags);
> 
> +	for (i = 0; i < num_eps; i++) {
> +		ep_index = xhci_get_endpoint_index(&eps[i]->desc);
> +		xhci_debugfs_create_stream_files(xhci, vdev, ep_index);
> +	}
>  	/* Subtract 1 for stream 0, which drivers can't use */
>  	return num_streams - 1;
> 
> --
> 2.25.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/3] xhci fixes for usb-linus
@ 2023-03-30 14:30 Mathias Nyman
  0 siblings, 0 replies; 24+ messages in thread
From: Mathias Nyman @ 2023-03-30 14:30 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few small patches for usb-linus.
Reverting xhci-pci asynchronous probe due to Renesas host regression, and
fixing a memory leak.

Thanks
Mathias

D Scott Phillips (1):
  xhci: also avoid the XHCI_ZERO_64B_REGS quirk with a passthrough iommu

Mathias Nyman (2):
  Revert "usb: xhci-pci: Set PROBE_PREFER_ASYNCHRONOUS"
  xhci: Free the command allocated for setting LPM if we return early

 drivers/usb/host/xhci-pci.c | 7 +++----
 drivers/usb/host/xhci.c     | 7 ++++++-
 2 files changed, 9 insertions(+), 5 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/3] xhci fixes for usb-linus
@ 2022-08-25 15:08 Mathias Nyman
  0 siblings, 0 replies; 24+ messages in thread
From: Mathias Nyman @ 2022-08-25 15:08 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few xhci fixes for usb-linus.

USB3 devices aren't visible immediately after xHC reset, so don't
stop polling the roothub and suspend too early after xHC reset.

Also Revert the port poweroff patch due to regression,
and fix a null pointer issue for xHC hosts with just one roothub.

-Mathias

Mathias Nyman (3):
  xhci: Fix null pointer dereference in remove if xHC has only one
    roothub
  xhci: Add grace period after xHC start to prevent premature runtime
    suspend.
  Revert "xhci: turn off port power in shutdown"

 drivers/usb/host/xhci-hub.c  | 13 ++++++++++++-
 drivers/usb/host/xhci-plat.c | 11 ++++++++---
 drivers/usb/host/xhci.c      | 19 +++++--------------
 drivers/usb/host/xhci.h      |  4 +---
 4 files changed, 26 insertions(+), 21 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/3] xhci fixes for usb-linus
@ 2022-04-08 13:48 Mathias Nyman
  0 siblings, 0 replies; 24+ messages in thread
From: Mathias Nyman @ 2022-04-08 13:48 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few small xhci fixes for usb-linus

Fixes USB 3 link resume for tunneled USB 3 devices, timers left running
after shutdown, and a bug in how we handle a full event ring.

Thanks
-Mathias


Henry Lin (1):
  xhci: stop polling roothubs after shutdown

Mathias Nyman (1):
  xhci: increase usb U3 -> U0 link resume timeout from 100ms to 500ms

Weitao Wang (1):
  USB: Fix xhci event ring dequeue pointer ERDP update issue

 drivers/usb/host/xhci-hub.c  |  2 +-
 drivers/usb/host/xhci-ring.c |  1 +
 drivers/usb/host/xhci.c      | 11 +++++++++++
 3 files changed, 13 insertions(+), 1 deletion(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/3] xhci fixes for usb-linus
@ 2020-08-21  9:15 Mathias Nyman
  0 siblings, 0 replies; 24+ messages in thread
From: Mathias Nyman @ 2020-08-21  9:15 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few xhci fixes for usb-linus.
This series makes sure we don't ignore devices attached during suspend that
are stuck in a resume "cold attach status" state, and makes sure xhci driver
doesn't prevent queuing urbs to a endpoint only because driver previously
refused to manually clear the data toggle of a non-empty endpoint.

-Mathias

Ding Hui (1):
  xhci: Always restore EP_SOFT_CLEAR_TOGGLE even if ep reset failed

Kai-Heng Feng (1):
  xhci: Do warm-reset when both CAS and XDEV_RESUME are set

Li Jun (1):
  usb: host: xhci: fix ep context print mismatch in debugfs

 drivers/usb/host/xhci-debugfs.c |  8 ++++----
 drivers/usb/host/xhci-hub.c     | 19 ++++++++++---------
 drivers/usb/host/xhci.c         |  3 ++-
 3 files changed, 16 insertions(+), 14 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/3] xhci fixes for usb-linus
@ 2020-04-21 14:08 Mathias Nyman
  0 siblings, 0 replies; 24+ messages in thread
From: Mathias Nyman @ 2020-04-21 14:08 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

These xhci fixes for usb-linus sort out halted empty endpoint handling,
protocol stall handling, and overcurrent handling.

Theses issues have caused usb devices to appear as if they stop working
suddenly, failed to enumerate LS/FS devices behind HS hubs (seen on some
audio devices), and host from freezing in case of suspend on over-current.

-Mathias 

Mathias Nyman (3):
  xhci: Fix handling halted endpoint even if endpoint ring appears empty
  xhci: prevent bus suspend if a roothub port detected a over-current
    condition
  xhci: Don't clear hub TT buffer on ep0 protocol stall

 drivers/usb/host/xhci-hub.c  |  9 +++++++
 drivers/usb/host/xhci-ring.c | 46 +++++++++++++++++++++++++++++++-----
 drivers/usb/host/xhci.c      | 14 +++++------
 drivers/usb/host/xhci.h      |  5 ++--
 4 files changed, 59 insertions(+), 15 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/3] xhci fixes for usb-linus
@ 2019-10-25 14:30 Mathias Nyman
  0 siblings, 0 replies; 24+ messages in thread
From: Mathias Nyman @ 2019-10-25 14:30 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few xhci fixes for usb-linus, solving a couple endianness issues, and a
use-after-free regression reported by Johan Hovold

-Mathias

Ben Dooks (Codethink) (1):
  usb: xhci: fix __le32/__le64 accessors in debugfs code

Mathias Nyman (1):
  xhci: Fix use-after-free regression in xhci clear hub TT
    implementation

Samuel Holland (1):
  usb: xhci: fix Immediate Data Transfer endianness

 drivers/usb/host/xhci-debugfs.c | 24 +++++++++---------
 drivers/usb/host/xhci-ring.c    |  2 ++
 drivers/usb/host/xhci.c         | 54 ++++++++++++++++++++++++++++++++++-------
 3 files changed, 59 insertions(+), 21 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2023-03-30 14:30 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-28 20:31 [PATCH 0/3] xhci fixes for usb-linus Mathias Nyman
2020-10-28 20:31 ` [PATCH 1/3] xhci: Fix sizeof() mismatch Mathias Nyman
2020-10-28 20:31 ` [PATCH 2/3] usb: xhci: Workaround for S3 issue on AMD SNPS 3.0 xHC Mathias Nyman
2020-10-28 20:31 ` [PATCH 3/3] xhci: Don't create stream debugfs files with spinlock held Mathias Nyman
2020-10-29  6:03   ` Mike Galbraith
2020-10-29  9:41     ` Mathias Nyman
2020-10-29  9:42       ` Sebastian Andrzej Siewior
2020-10-29 10:23         ` Mike Galbraith
2020-10-29 10:37           ` Sebastian Andrzej Siewior
2020-10-29 11:11       ` Mike Galbraith
2020-10-29 11:22         ` Mathias Nyman
2020-10-29 11:38           ` Sebastian Andrzej Siewior
2020-10-29 12:44             ` Jun Li
2020-10-29 13:08             ` Mathias Nyman
2020-10-29 15:10               ` Sebastian Andrzej Siewior
2020-10-29 16:26               ` David Laight
2020-10-29 17:43               ` Sebastian Andrzej Siewior
2020-10-30  8:01   ` Jun Li
  -- strict thread matches above, loose matches on Subject: below --
2023-03-30 14:30 [PATCH 0/3] xhci fixes for usb-linus Mathias Nyman
2022-08-25 15:08 Mathias Nyman
2022-04-08 13:48 Mathias Nyman
2020-08-21  9:15 Mathias Nyman
2020-04-21 14:08 Mathias Nyman
2019-10-25 14:30 Mathias Nyman

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).