netdev - Re: [RFC net-next 04/10] devlink: always check if the devlink instance is registered

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: <20230103184959.621f4b9c@kernel.org>
Date:   Tue, 3 Jan 2023 18:49:59 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     jacob.e.keller@...el.com, leon@...nel.org, netdev@...r.kernel.org
Subject: Re: [RFC net-next 04/10] devlink: always check if the devlink
 instance is registered

On Tue, 3 Jan 2023 10:26:12 +0100 Jiri Pirko wrote:
> >> Why "alive"? To be consistent with the existing terminology, how about
> >> to name it devl_is_registered()?  
> >
> >I dislike the similarity to device_is_registered() which has very
> >different semantics. I prefer alive.  
> 
> Interesting. Didn't occur to me to look into device.h when reading
> devlink.c code. I mean, is device_register() behaviour in sync with
> devlink_register?
> 
> Your alive() helper is checking "register mark". It's an odd and unneded
> inconsistency in newly added code :/

Alright.

> >> Also, "devl_" implicates that it should be called with devlink instance
> >> lock held, so probably devlink_is_registered() would be better.  
> >
> >I'm guessing you realized this isn't correct later on.  
> 
> From what I see, no need to hold instance mutex for xa mark checking,
> alhough I understand why you want the helper to be called with the lock.
> Perhaps assert and a little comment would make this clear?

I'll add the comment. The assert would have to OR holding the subobject
locks. Is that what you had in mind?