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

Jiri Pirko
2022-07-02T16:00:00Z

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.