Re: [PATCH net-next 05/10] dpll: convert dpll_lock to rwsem

[Ivan Vecera <ivecera@redhat.com>]

On 11/10/26 18:53, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
> 
> Low priority / nit picks for your consideration:
> 
> - [Low] This patch turns dpll_lock from a mutex into an rwsem.
> 
> --
> 
> dpll: convert dpll_lock to rwsem
> 
> This commit converts dpll_lock from a mutex to an rwsem. Core functions
> that change the topology take it for write. Netlink requests, dump parts
> and change notifications take it for read, so callbacks of different
> domains can run concurrently.
> 
>> diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
>> index dd09f52817099..8eb7468234315 100644
>> --- a/drivers/dpll/dpll_core.c
>> +++ b/drivers/dpll/dpll_core.c
>> @@ -19,8 +19,12 @@
>>   #include "dpll_core.h"
>>   #include "dpll_netlink.h"
>>   
>> -/* Mutex lock to protect DPLL subsystem devices and pins */
>> -DEFINE_MUTEX(dpll_lock);
>> +/* Protects the topology and lifetime of DPLL devices and pins. Taken for write
>> + * to change them and for read for the whole netlink request, part of a dump or
>> + * change notification that uses them. Driver callbacks are serialized by domain
>> + * locks, which are only taken under this lock.
>> + */
>> +DECLARE_RWSEM(dpll_lock);
> 
> [Severity: Low]
> Should the assertions in dpll_domain_get() and dpll_domain_put() be changed
> to lockdep_assert_held_write() along with this conversion?

[...]
> call_dpll_notifiers() can keep the plain lockdep_assert_held(), because it
> legitimately runs under the read lock.
Yes, v2 uses lockdep_assert_held_write() in both and keeps
lockdep_assert_held() in call_dpll_notifiers().

Thanks,
Ivan