netdev - Re: [RFT net-next 1/6] devlink: expose instance locking and add locked port registering

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: <YisTMpcWif02S1VC@nanopsycho>
Date:   Fri, 11 Mar 2022 10:15:30 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     idosch@...dia.com, petrm@...dia.com, simon.horman@...igine.com,
        netdev@...r.kernel.org, leonro@...dia.com
Subject: Re: [RFT net-next 1/6] devlink: expose instance locking and add
 locked port registering

Thu, Mar 10, 2022 at 09:06:24PM CET, kuba@...nel.org wrote:
>On Thu, 10 Mar 2022 10:14:26 +0100 Jiri Pirko wrote:
>> It is kind of confusing to have:
>> devlink_* - locked api
>> devl_* - unlocked api
>> 
>> And not really, because by this division, devl_lock() should be called
>> devlink_lock(). So it is oddly mixed..
>> 
>> I believe that "_" or "__" prefix is prefered here and everyone knows
>> with away what it it is good for.
>> 
>> If you find "__devlink_port_register" as "too much typing" (I don't),
>> why don't we have all devlink api shortened to:
>> devl_*
>> and then the unlocked api could be called:
>> __devl_*
>> ?
>
>The goal is for that API to be the main one, we can rename the devlink_
>to something else at the end. The parts of it which are not completely
>removed.

Okay. So please have it as:
devl_* - normal
__devl_* - unlocked

Thanks!


>
>> >+bool devl_lock_is_held(struct devlink *devlink)
>> >+{
>> >+	/* We have to check this at runtime because struct devlink
>> >+	 * is now private. Normally lock_is_held() should be eliminated  
>> 
>> "is now private" belong more to the patch description, not to the actual
>> code I believe.
>
>Alright. The comment started as a warning not to use this for anything
>but lockdep but I couldn't resist taking a dig at hiding the structure.