mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Badhri Jagan Sridharan <badhri@google.com>
To: gregkh@linuxfoundation.org, stern@rowland.harvard.edu,
	colin.i.king@gmail.com, xuetao09@huawei.com,
	quic_eserrao@quicinc.com, water.zhangjiantao@huawei.com,
	peter.chen@freescale.com, balbi@ti.com, francesco@dolcini.it,
	alistair@alistair23.me, stephan@gerhold.net,
	bagasdotme@gmail.com, luca@z3ntu.xyz
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org,
	Badhri Jagan Sridharan <badhri@google.com>,
	Francesco Dolcini <francesco.dolcini@toradex.com>
Subject: [PATCH v1] usb: gadget: udc: core: Offload usb_udc_vbus_handler processing
Date: Wed, 17 May 2023 11:59:55 +0000	[thread overview]
Message-ID: <20230517115955.1078339-1-badhri@google.com> (raw)

chipidea udc calls usb_udc_vbus_handler from udc_start gadget
ops causing a deadlock. Avoid this by offloading usb_udc_vbus_handler
processing.

============================================
WARNING: possible recursive locking detected
640-rc1-000-devel-00005-gcda3c69ebc14 #1 Not tainted
-------------------------------------------

CPU0
----
lock(&udc->connect_lock);
lock(&udc->connect_lock);

 DEADLOCK

stack backtrace:
  CPU: 1 PID: 566 Comm: echo Not tainted 640-rc1-000-devel-00005-gcda3c69ebc14 #1
  Hardware name: Freescale iMX7 Dual (Device Tree)
  unwind_backtrace from show_stack+0x10/0x14
  show_stack from dump_stack_lvl+0x70/0xb0
  dump_stack_lvl from __lock_acquire+0x924/0x22c4
  __lock_acquire from lock_acquire+0x100/0x370
  lock_acquire from __mutex_lock+0xa8/0xfb4
  __mutex_lock from mutex_lock_nested+0x1c/0x24
  mutex_lock_nested from usb_udc_vbus_handler+0x1c/0x60
  usb_udc_vbus_handler from ci_udc_start+0x74/0x9c
  ci_udc_start from gadget_bind_driver+0x130/0x230
  gadget_bind_driver from really_probe+0xd8/0x3fc
  really_probe from __driver_probe_device+0x94/0x1f0
  __driver_probe_device from driver_probe_device+0x2c/0xc4
  driver_probe_device from __driver_attach+0x114/0x1cc
  __driver_attach from bus_for_each_dev+0x7c/0xcc
  bus_for_each_dev from bus_add_driver+0xd4/0x200
  bus_add_driver from driver_register+0x7c/0x114
  driver_register from usb_gadget_register_driver_owner+0x40/0xe0
  usb_gadget_register_driver_owner from gadget_dev_desc_UDC_store+0xd4/0x110
  gadget_dev_desc_UDC_store from configfs_write_iter+0xac/0x118
  configfs_write_iter from vfs_write+0x1b4/0x40c
  vfs_write from ksys_write+0x70/0xf8
  ksys_write from ret_fast_syscall+0x0/0x1c

Fixes: 0db213ea8eed ("usb: gadget: udc: core: Invoke usb_gadget_connect only when started")
Cc: stable@vger.kernel.org
Reported-by: Stephan Gerhold <stephan@gerhold.net>
Closes: https://lore.kernel.org/all/ZF4bMptC3Lf2Hnee@gerhold.net/
Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Closes: https://lore.kernel.org/all/ZF4BvgsOyoKxdPFF@francesco-nb.int.toradex.com/
Reported-by: Alistair <alistair@alistair23.me>
Closes: https://lore.kernel.org/lkml/0cf8c588b701d7cf25ffe1a9217b81716e6a5c51.camel@alistair23.me/
Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
 drivers/usb/gadget/udc/core.c | 55 +++++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 69041cca5d24..bbcfab50f657 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -41,6 +41,9 @@ static const struct bus_type gadget_bus_type;
  * functions. usb_gadget_connect_locked, usb_gadget_disconnect_locked,
  * usb_udc_connect_control_locked, usb_gadget_udc_start_locked, usb_gadget_udc_stop_locked are
  * called with this lock held.
+ * @vbus_events: list head for processing vbus updates on usb_udc_vbus_handler.
+ * @vbus_events_lock: protects vbus_events list
+ * @vbus_work: work item that invokes usb_udc_connect_control_locked.
  *
  * This represents the internal data structure which is used by the UDC-class
  * to hold information about udc driver and gadget together.
@@ -53,6 +56,19 @@ struct usb_udc {
 	bool				vbus;
 	bool				started;
 	struct mutex			connect_lock;
+	struct list_head		vbus_events;
+	struct mutex			vbus_events_lock;
+	struct work_struct		vbus_work;
+};
+
+/**
+ * struct vbus_event - used to notify vbus updates posted through usb_udc_vbus_handler.
+ * @vbus_on: true when vbus is on. false other wise.
+ * @node: list node for maintaining a list of pending updates to be processed.
+ */
+struct vbus_event {
+	bool vbus_on;
+	struct list_head node;
 };
 
 static struct class *udc_class;
@@ -1134,6 +1150,23 @@ static int usb_udc_connect_control_locked(struct usb_udc *udc) __must_hold(&udc-
 	return ret;
 }
 
+static void vbus_event_work(struct work_struct *work)
+{
+	struct vbus_event *event, *n;
+	struct usb_udc *udc = container_of(work, struct usb_udc, vbus_work);
+
+	mutex_lock(&udc->vbus_events_lock);
+	list_for_each_entry_safe(event, n, &udc->vbus_events, node) {
+		mutex_lock(&udc->connect_lock);
+		udc->vbus = event->vbus_on;
+		usb_udc_connect_control_locked(udc);
+		list_del(&event->node);
+		kfree(event);
+		mutex_unlock(&udc->connect_lock);
+	}
+	mutex_unlock(&udc->vbus_events_lock);
+}
+
 /**
  * usb_udc_vbus_handler - updates the udc core vbus status, and try to
  * connect or disconnect gadget
@@ -1146,13 +1179,20 @@ static int usb_udc_connect_control_locked(struct usb_udc *udc) __must_hold(&udc-
 void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)
 {
 	struct usb_udc *udc = gadget->udc;
+	struct vbus_event *vbus_event;
 
-	mutex_lock(&udc->connect_lock);
-	if (udc) {
-		udc->vbus = status;
-		usb_udc_connect_control_locked(udc);
-	}
-	mutex_unlock(&udc->connect_lock);
+	if (!udc)
+		return;
+
+	vbus_event = kzalloc(sizeof(*vbus_event), GFP_KERNEL);
+	if (!vbus_event)
+		return;
+
+	mutex_lock(&udc->vbus_events_lock);
+	vbus_event->vbus_on = status;
+	list_add_tail(&vbus_event->node, &udc->vbus_events);
+	mutex_unlock(&udc->vbus_events_lock);
+	schedule_work(&udc->vbus_work);
 }
 EXPORT_SYMBOL_GPL(usb_udc_vbus_handler);
 
@@ -1379,6 +1419,9 @@ int usb_add_gadget(struct usb_gadget *gadget)
 	udc->gadget = gadget;
 	gadget->udc = udc;
 	mutex_init(&udc->connect_lock);
+	INIT_LIST_HEAD(&udc->vbus_events);
+	mutex_init(&udc->vbus_events_lock);
+	INIT_WORK(&udc->vbus_work, vbus_event_work);
 
 	udc->started = false;
 

base-commit: 6bae03b0484b54f699d69339fbec5658e885c224
-- 
2.40.1.606.ga4b1b128d6-goog


             reply	other threads:[~2023-05-17 12:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-17 11:59 Badhri Jagan Sridharan [this message]
2023-05-17 12:04 ` Badhri Jagan Sridharan
2023-05-17 14:44 ` Alan Stern
2023-05-17 17:19   ` Badhri Jagan Sridharan
2023-05-17 20:01     ` Alan Stern
2023-05-19  4:29       ` Badhri Jagan Sridharan

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=20230517115955.1078339-1-badhri@google.com \
    --to=badhri@google.com \
    --cc=alistair@alistair23.me \
    --cc=bagasdotme@gmail.com \
    --cc=balbi@ti.com \
    --cc=colin.i.king@gmail.com \
    --cc=francesco.dolcini@toradex.com \
    --cc=francesco@dolcini.it \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=luca@z3ntu.xyz \
    --cc=peter.chen@freescale.com \
    --cc=quic_eserrao@quicinc.com \
    --cc=stable@vger.kernel.org \
    --cc=stephan@gerhold.net \
    --cc=stern@rowland.harvard.edu \
    --cc=water.zhangjiantao@huawei.com \
    --cc=xuetao09@huawei.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®