From 7d01ef789bdcdd02ac42b98ae5b7f98310a0e3d2 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 12 Sep 2024 15:39:56 -0700 Subject: [PATCH] usb: roles: Improve the fix for a false positive recursive locking complaint Improve commit fc88bb116179 ("usb: roles: add lockdep class key to struct usb_role_switch") as follows: * Move the lock class key declaration just above the mutex declaration such that the declaration order of these objects matches their initialization order. * Destroy the mutex and lock class key just before these objects are freed. This makes it easier to verify that the destruction calls happen after the last use of these objects. * Instead of switching the mutex key to the dynamic lock class key after initialization of the mutex has completed, initialize the mutex with the dynamic lock class key. Cc: Amit Sunil Dhamne Cc: Badhri Jagan Sridharan Cc: Hans de Goede Cc: Andy Shevchenko Cc: Heikki Krogerus Cc: Greg Kroah-Hartman Signed-off-by: Bart Van Assche Link: https://lore.kernel.org/r/20240912223956.3554086-4-bvanassche@acm.org Signed-off-by: Greg Kroah-Hartman --- drivers/usb/roles/class.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c index 7aca1ef7f44c..c58a12c147f4 100644 --- a/drivers/usb/roles/class.c +++ b/drivers/usb/roles/class.c @@ -22,6 +22,7 @@ static const struct class role_class = { struct usb_role_switch { struct device dev; + struct lock_class_key key; struct mutex lock; /* device lock*/ struct module *module; /* the module this device depends on */ enum usb_role role; @@ -34,8 +35,6 @@ struct usb_role_switch { usb_role_switch_set_t set; usb_role_switch_get_t get; bool allow_userspace_control; - - struct lock_class_key key; }; #define to_role_switch(d) container_of(d, struct usb_role_switch, dev) @@ -329,6 +328,8 @@ static void usb_role_switch_release(struct device *dev) { struct usb_role_switch *sw = to_role_switch(dev); + mutex_destroy(&sw->lock); + lockdep_unregister_key(&sw->key); kfree(sw); } @@ -367,7 +368,8 @@ usb_role_switch_register(struct device *parent, if (!sw) return ERR_PTR(-ENOMEM); - mutex_init(&sw->lock); + lockdep_register_key(&sw->key); + mutex_init_with_key(&sw->lock, &sw->key); sw->allow_userspace_control = desc->allow_userspace_control; sw->usb2_port = desc->usb2_port; @@ -399,9 +401,6 @@ usb_role_switch_register(struct device *parent, sw->registered = true; - lockdep_register_key(&sw->key); - lockdep_set_class(&sw->lock, &sw->key); - /* TODO: Symlinks for the host port and the device controller. */ return sw; @@ -418,9 +417,6 @@ void usb_role_switch_unregister(struct usb_role_switch *sw) { if (IS_ERR_OR_NULL(sw)) return; - - lockdep_unregister_key(&sw->key); - sw->registered = false; if (dev_fwnode(&sw->dev)) component_del(&sw->dev, &connector_ops);