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

Jiri Pirko
2023-01-04T16:15:18Z

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