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

[Jiri Pirko <jiri@resnulli.us>]

Thu, Mar 10, 2022 at 09:06:24PM CET, kuba@kernel.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.