Re: [PATCH] usb: roles: Fix a false positive recursive locking complaint

[Amit Sunil Dhamne <amitsd@google.com>]

Hi Bart,

On 9/4/24 2:15 PM, Bart Van Assche wrote:
> On 9/4/24 2:00 PM, Badhri Jagan Sridharan wrote:
>> https://lore.kernel.org/all/ZsiYRAJST%2F2hAju1@kuha.fi.intel.com/ was
>> already accepted
>
> Thanks, I hadn't noticed this yet.
>
>> and is perhaps better than what you are suggesting as
>> it does not use the internal methods of mutex_init().
>
> Although I do not have a strong opinion about which patch is sent to
> Linus, I think my patch has multiple advantages compared to the patch
> mentioned above:
> - Cleaner. lockdep_set_class() is not used. Hence, it is not possible
>   that the wrong lockdep key is used (the one assigned by
>   mutex_init()).
> - The lock_class_key declaration occurs close to the sw->lock
>   declaration.
> - The lockdep_register_key() call occurs close to __mutex_init() call
>   that uses the registered key.
> - Needs less memory in debug kernels. The advantage of __mutex_init()
>   compared to mutex_init() is that it does not allocate (static) memory
>   for a lockdep key.
>
Thanks for the patch.

While I agree on (1) & (4), *may* be a good reason to reconsider.
However, I have seen almost 30+ instances of the prior
method 
(https://lore.kernel.org/all/20240822223717.253433-1-amitsd@google.com/)
of registering lockdep key, which is what I followed.
However, if that's is not the right way, it brings into question the purpose
of lockdep_set_class() considering I would always and unconditionally use
__mutex_init()  if I want to manage the lockdep class keys myself or
mutex_init() if I didn't.


Thanks,

Amit

> Thanks,
>
> Bart.
>