Re: [patch net-next 2/3] net: devlink: call lockdep_assert_held() for devlink->lock directly

[Jiri Pirko <jiri@resnulli.us>]

Fri, Jul 01, 2022 at 06:33:16PM CEST, kuba@kernel.org wrote:
>On Fri,  1 Jul 2022 11:59:25 +0200 Jiri Pirko wrote:
>> In devlink.c there is direct access to whole struct devlink so there is
>> no need to use helper. So obey the customs and work with lock directly
>> avoiding helpers which might obfuscate things a bit.
>
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index 25b481dd1709..a7477addbd59 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -10185,7 +10185,7 @@ int devl_rate_leaf_create(struct devlink_port *devlink_port, void *priv)
>>  	struct devlink *devlink = devlink_port->devlink;
>>  	struct devlink_rate *devlink_rate;
>>  
>> -	devl_assert_locked(devlink_port->devlink);
>> +	lockdep_assert_held(&devlink_port->devlink->lock);
>
>I don't understand why. Do we use lockdep asserts directly on rtnl_mutex
>in rtnetlink.c?

Well:

1) it's been a long time policy not to use helpers for locks if not
   needed. There reason is that the reader has easier job in seeing what
   the code is doing. And here, it is not needed to use helper (we can
   access the containing struct)
2) lock/unlock for devlink->lock is done here w/o helpers as well
3) there is really no gain of using helper here.
4) rtnl_mutex is probably not good example, it has a lot of ancient
   history behind it.