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

[Jason Baron]

On 10/16/2017 10:42 AM, Petr Mladek wrote:
> 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) {

Yes, thanks. I noticed that too when insertion didn't match deletion :(
The final patch ends up adding it back, instead of this one, so the
states of things after patch 3 is ok.

Thanks,

-Jason