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: <20220310120624.4c445129@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Thu, 10 Mar 2022 12:06:24 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jiri Pirko <jiri@...nulli.us>
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

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.

> >+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.