Re: [RFC PATCH v2 11/22] devlink: add functions to take snapshot while locked

[Jacob Keller <jacob.e.keller@intel.com>]

On 3/2/2020 9:43 AM, Jiri Pirko wrote:
> Sat, Feb 15, 2020 at 12:22:10AM CET, jacob.e.keller@intel.com wrote:
>> A future change is going to add a new devlink command to request
>> a snapshot on demand. This function will want to call the
>> devlink_region_snapshot_id_get and devlink_region_snapshot_create
>> functions while already holding the devlink instance lock.
>>
>> Extract the logic of these two functions into static functions prefixed
>> by `__` to indicate they are internal helper functions. Modify the
>> original functions to be implemented in terms of the new locked
>> functions.
>>
>> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> 
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> 
> 
>> ---
>> net/core/devlink.c | 93 ++++++++++++++++++++++++++++++----------------
>> 1 file changed, 61 insertions(+), 32 deletions(-)
>>
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index fef93f48028c..0e94887713f4 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -3760,6 +3760,65 @@ static void devlink_nl_region_notify(struct devlink_region *region,
>> 	nlmsg_free(msg);
>> }
>>
>> +/**
>> + *	__devlink_region_snapshot_id_get - get snapshot ID
>> + *	@devlink: devlink instance
>> + *
>> + *	Returns a new snapshot id. Must be called while holding the
>> + *	devlink instance lock.
>> + */
> 
> You don't need this docu comment for static functions.
> 
> 

I like having these for all functions. I'll remove it if you feel
strongly, though.

Thanks,
Jake