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] [day] [month] [year] [list]

Message-ID: <Y7Wl021n/jSJYrWJ@nanopsycho>
Date:   Wed, 4 Jan 2023 17:14:11 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jakub Kicinski <kuba@...nel.org>
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

Wed, Jan 04, 2023 at 03:49:59AM CET, kuba@...nel.org wrote:
>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?

Ah right, the subobject locks. That will go away I'm sure. Yes, that is
what I had in mind. After that, we can put assert here.