Re: [PATCH v4 2/3] livepatch: shuffle core.c function order

[Petr Mladek]

On Thu 2017-10-12 17:12:28, Jason Baron wrote:
> In preparation for __klp_enable_patch() to call a number of 'static'
> functions, in a subsequent patch, move them earlier in core.c. This patch
> should be a nop from a functional pov.
> 
> Signed-off-by: Jason Baron <jbaron@xxxxxxxxxx>
> Cc: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Cc: Jessica Yu <jeyu@xxxxxxxxxx>
> Cc: Jiri Kosina <jikos@xxxxxxxxxx>
> Cc: Miroslav Benes <mbenes@xxxxxxx>
> Cc: Petr Mladek <pmladek@xxxxxxxx>
> ---
>  kernel/livepatch/core.c | 349 ++++++++++++++++++++++++------------------------
>  1 file changed, 173 insertions(+), 176 deletions(-)
> 
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index b7f77be..f53eed5 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -283,6 +283,179 @@ static int klp_write_object_relocations(struct module *pmod,
> +static int klp_init_func(struct klp_object *obj, struct klp_func *func)
> +{
> +	if (!func->old_name || !func->new_func)
> +		return -EINVAL;
> +
> +	INIT_LIST_HEAD(&func->stack_node);
> +	func->patched = false;
> +	func->transition = false;

You lost the change from the 1st patch:

	list_add(&func->func_entry, &obj->func_list);

> +
> +	/* The format for the sysfs directory is <function,sympos> where sympos
> +	 * is the nth occurrence of this symbol in kallsyms for the patched
> +	 * object. If the user selects 0 for old_sympos, then 1 will be used
> +	 * since a unique symbol will be the first occurrence.
> +	 */
> +	return kobject_init_and_add(&func->kobj, &klp_ktype_func,
> +				    &obj->kobj, "%s,%lu", func->old_name,
> +				    func->old_sympos ? func->old_sympos : 1);
> +}

[...]

> +static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
> +{
> +	struct klp_func *func;
> +	int ret;
> +	const char *name;
> +
> +	if (!obj->funcs)
> +		return -EINVAL;
> +
> +	obj->patched = false;
> +	obj->mod = NULL;
> +
> +	klp_find_object_module(obj);
> +
> +	name = klp_is_module(obj) ? obj->name : "vmlinux";
> +	ret = kobject_init_and_add(&obj->kobj, &klp_ktype_object,
> +				   &patch->kobj, "%s", name);
> +	if (ret)
> +		return ret;
> +

Same here:

	list_add(&obj->obj_entry, &patch->obj_list);
	INIT_LIST_HEAD(&obj->func_list);
	klp_for_each_func_static(obj, func) {

> +	klp_for_each_func(obj, func) {
> +		ret = klp_init_func(obj, func);
> +		if (ret)
> +			goto free;
> +	}
> +
> +	if (klp_is_object_loaded(obj)) {
> +		ret = klp_init_object_loaded(patch, obj);
> +		if (ret)
> +			goto free;
> +	}
> +
> +	return 0;
> +
> +free:
> +	klp_free_funcs_limited(obj, func);
> +	kobject_put(&obj->kobj);
> +	return ret;
> +}

Best Regards,
Petr