mbox series

[v2,0/3] Fix a lockdep complaint related to USB role switching

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

Message

Bart Van Assche Sept. 5, 2024, 8:47 p.m. UTC 
Hi Greg,

This patch series suppresses a lockdep complaint about recursive locking
that is triggered by switching USB roles. Please consider this patch series
for the next merge window.

Thanks,

Bart.

Changes compared to v1:
 - Added two patches. One that combines the two mutex_init() definitions and
   another patch that introduces mutex_init_with_key().
 - Changed patch 3/3 such that it uses mutex_init_with_key(). Added Amit's
   Signed-off-by.

Bart Van Assche (3):
  locking/mutex: Define mutex_init() once
  locking/mutex: Introduce mutex_init_with_key()
  usb: roles: Fix a false positive recursive locking complaint

 drivers/usb/roles/class.c |  6 +++++-
 include/linux/mutex.h     | 19 ++++++++++++-------
 2 files changed, 17 insertions(+), 8 deletions(-)

Comments

gregkh@linuxfoundation.org Sept. 11, 2024, 1:41 p.m. UTC | #1 
On Thu, Sep 05, 2024 at 01:47:06PM -0700, Bart Van Assche wrote:
> Hi Greg,
> 
> This patch series suppresses a lockdep complaint about recursive locking
> that is triggered by switching USB roles. Please consider this patch series
> for the next merge window.

I already took this commit into my tree for -rc1:
	https://lore.kernel.org/r/20240822223717.253433-1-amitsd@google.com

It's almost identical to yours, but you are messing with mutex states
here, why?  I'll be glad to take a series on top of that one if you can
describe why this one is "more correct" that that one.

thanks,

greg k-h
Bart Van Assche Sept. 11, 2024, 6:03 p.m. UTC | #2 
On 9/11/24 6:41 AM, Greg Kroah-Hartman wrote:
> On Thu, Sep 05, 2024 at 01:47:06PM -0700, Bart Van Assche wrote:
>> This patch series suppresses a lockdep complaint about recursive locking
>> that is triggered by switching USB roles. Please consider this patch series
>> for the next merge window.
> 
> I already took this commit into my tree for -rc1:
> 	https://lore.kernel.org/r/20240822223717.253433-1-amitsd@google.com
> 
> It's almost identical to yours, but you are messing with mutex states
> here, why?

What does "messing with mutex states" mean in this context? My patch 3/3
does not modify the mutex lock class key at runtime. Instead, it
sets the mutex lock class key when the mutex is initialized. I think
this is a cleaner approach than modifying the lock class key at runtime.

> I'll be glad to take a series on top of that one if you can
> describe why this one is "more correct" that that one.

I will rebase my patch series on top of your for-next branch. I think
there is agreement that the approach of patch 3/3 in this series is
slightly better than the approach of the patch that has already been
queued.

Thanks,

Bart.