diff mbox series

[v2,3/3] usb: roles: Fix a false positive recursive locking complaint

Message ID 20240905204709.556577-4-bvanassche@acm.org
State New
Headers show
Series Fix a lockdep complaint related to USB role switching | expand

Commit Message

Bart Van Assche Sept. 5, 2024, 8:47 p.m. UTC
Suppress the following lockdep complaint by giving each sw->lock
a unique lockdep key instead of using the same lockdep key for all
sw->lock instances:

INFO: trying to register non-static key.
The code is fine but needs lockdep annotation, or maybe
you didn't initialize this object before use?
turning off the locking correctness validator.

Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Badhri Jagan Sridharan <badhri@google.com>
Cc: stable@vger.kernel.org
Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/usb/roles/class.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Andy Shevchenko Sept. 5, 2024, 8:54 p.m. UTC | #1
On Thu, Sep 5, 2024 at 11:47 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> Suppress the following lockdep complaint by giving each sw->lock
> a unique lockdep key instead of using the same lockdep key for all
> sw->lock instances:
>
> INFO: trying to register non-static key.
> The code is fine but needs lockdep annotation, or maybe
> you didn't initialize this object before use?
> turning off the locking correctness validator.


> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Badhri Jagan Sridharan <badhri@google.com>
> Cc: stable@vger.kernel.org

If you put these Cc:s after --- line it will reduce the commit message
while having the same effect (assuming use of `git send-email`).
lore.kernel.org archive will keep it.

> Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>

Co-developed-by ?

> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Bart Van Assche Sept. 5, 2024, 9:18 p.m. UTC | #2
On 9/5/24 1:54 PM, Andy Shevchenko wrote:
> On Thu, Sep 5, 2024 at 11:47 PM Bart Van Assche <bvanassche@acm.org> wrote:
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Badhri Jagan Sridharan <badhri@google.com>
>> Cc: stable@vger.kernel.org
> 
> If you put these Cc:s after --- line it will reduce the commit message
> while having the same effect (assuming use of `git send-email`).
> lore.kernel.org archive will keep it.
> 
>> Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> 
> Co-developed-by ?

Thanks for the quick follow-up. I will make these changes if I have to
repost this patch series.

Thanks,

Bart.
diff mbox series

Patch

diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index 7aca1ef7f44c..37556aa0eeee 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;
@@ -329,6 +330,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 +370,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;