netdev - Re: [patch net-next 2/3] net: devlink: call lockdep_assert

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YsBrDhZuV4j3uCk3@nanopsycho>
Date:   Sat, 2 Jul 2022 17:58:06 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, pabeni@...hat.com,
        edumazet@...gle.com, mlxsw@...dia.com, saeedm@...dia.com,
        moshe@...dia.com
Subject: Re: [patch net-next 2/3] net: devlink: call lockdep_assert_held()
 for devlink->lock directly

Fri, Jul 01, 2022 at 06:33:16PM CEST, kuba@...nel.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.