Re: [PATCH v7 3/7] livepatch: Initial support for dynamic structures

[Miroslav Benes]

On Tue, 6 Feb 2018, Petr Mladek wrote:

> From: Jason Baron <jbaron@xxxxxxxxxx>
> 
> We are going to add a feature called atomic replace. It will allow to
> create a patch that would replace all already registered patches.
> For this, we will need to dynamically create funcs and objects
> for functions that are not longer patched.
> 
> This patch adds basic framework to handle such dynamic structures.
> 
> It adds enum klp_func_type that allows to distinguish the dynamically
> allocated funcs' structures. Note that objects' structures do not have
> a clear type. Namely the static objects' structures might list both static
> and dynamic funcs' structures.
> 
> The function type is then used to limit klp_free() functions. We will
> want to free the dynamic structures separately when they are not
> longer needed. At the same time, we also want to make our life easier,
> and introduce _any_ type that will allow to process all existing
> structures in one go.
> 
> We need to be careful here. First, objects' structures must be freed
> only when all listed funcs' structures are freed. Also we must avoid
> double free. Both problems are solved by removing the freed structures
> from the list.
> 
> Also note that klp_free*() functions are called also in klp_init_patch()
> error path when only some kobjects have been initialized. The other
> dynamic structures must be freed immediately by calling the respective
> klp_free_*_dynamic() functions.
> 
> Finally, the dynamic objects' structures are generic. The respective
> klp_allocate_object_dynamic() and klp_free_object_dynamic() can
> be implemented here. On the other hand, klp_free_func_dynamic()
> is empty. It must be updated when a particular dynamic
> klp_func_type is introduced.
> 
> Signed-off-by: Jason Baron <jbaron@xxxxxxxxxx>
> [pmladek@xxxxxxxx: Converted into a generic API]
> Signed-off-by: Petr Mladek <pmladek@xxxxxxxx>
> Cc: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Cc: Jessica Yu <jeyu@xxxxxxxxxx>
> Cc: Jiri Kosina <jikos@xxxxxxxxxx>
> Cc: Miroslav Benes <mbenes@xxxxxxx>

This patch introduces klp_get_or_add_object(), but it is not used here. 
One of the later patches adds its call site. GCC warns about this. I like 
it being introduced here, because it belongs to the patch. Someone may 
complain though.

Also klp_free_objects() is not declared as static and sparse complains 
about it. Petr, you can make it static here and drop the keyword later.

Otherwise it looks good.

Acked-by: Miroslav Benes <mbenes@xxxxxxx>

Miroslav