From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933361AbbKMKOO (ORCPT ); Fri, 13 Nov 2015 05:14:14 -0500 Received: from mx2.suse.de ([195.135.220.15]:53239 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932597AbbKMKOK (ORCPT ); Fri, 13 Nov 2015 05:14:10 -0500 Date: Fri, 13 Nov 2015 11:14:08 +0100 (CET) From: Miroslav Benes To: Chris J Arges cc: live-patching@vger.kernel.org, jeyu@redhat.com, Josh Poimboeuf , Seth Jennings , Jiri Kosina , Vojtech Pavlik , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/4 v5] livepatch: add old_sympos as disambiguator field to klp_func In-Reply-To: <1447347595-30728-2-git-send-email-chris.j.arges@canonical.com> Message-ID: References: <1447259366-7055-1-git-send-email-chris.j.arges@canonical.com> <1447347595-30728-1-git-send-email-chris.j.arges@canonical.com> <1447347595-30728-2-git-send-email-chris.j.arges@canonical.com> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 12 Nov 2015, Chris J Arges wrote: > diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h > index 31db7a0..3d18dff 100644 > --- a/include/linux/livepatch.h > +++ b/include/linux/livepatch.h > @@ -37,8 +37,9 @@ enum klp_state { > * struct klp_func - function structure for live patching > * @old_name: name of the function to be patched > * @new_func: pointer to the patched function code > - * @old_addr: a hint conveying at what address the old function > - * can be found (optional, vmlinux patches only) > + * @old_sympos: a hint indicating which symbol position the old function > + * can be found (optional) > + * @old_addr: the address of the function being patched > * @kobj: kobject for sysfs resources > * @state: tracks function-level patch application state > * @stack_node: list node for klp_ops func_stack list > @@ -47,17 +48,18 @@ struct klp_func { > /* external */ > const char *old_name; > void *new_func; > + A nit, but this new empty line is not necessary. Let's keep all the external stuff in one pack. > /* > - * The old_addr field is optional and can be used to resolve > - * duplicate symbol names in the vmlinux object. If this > - * information is not present, the symbol is located by name > - * with kallsyms. If the name is not unique and old_addr is > - * not provided, the patch application fails as there is no > - * way to resolve the ambiguity. > + * The old_sympos field is optional and can be used to resolve > + * duplicate symbol names in livepatch objects. If this field is zero, > + * it is expected the symbol is unique, otherwise patching fails. If > + * this value is greater than zero then that occurrence of the symbol > + * in kallsyms for the given object is used. > */ > @@ -749,8 +733,13 @@ static int klp_init_object_loaded(struct klp_patch *patch, > return ret; > } > > + /* > + * Verify the symbol, find old_addr, and write it to the structure. > + */ It is not verification anymore. I think a comment here is not necessary. It is quite clear what klp_find_object_symbol does. > klp_for_each_func(obj, func) { > - ret = klp_find_verify_func_addr(obj, func); > + ret = klp_find_object_symbol(obj->name, func->old_name, > + &func->old_addr, > + func->old_sympos); > if (ret) > return ret; > } Thanks, Miroslav