On Mon, Nov 02, 2015 at 01:52:44PM -0600, Josh Poimboeuf wrote: > On Mon, Nov 02, 2015 at 11:58:47AM -0600, Chris J Arges wrote: > > The following directory structure will allow for cases when the same > > function name exists in a single object. > > /sys/kernel/livepatch/<patch>/<object>/<function.number> > > > > The number is incremented on each known initialized func kobj thus creating > > unique names in this case. > > > > An example of this issue is documented here: > > https://github.com/dynup/kpatch/issues/493 > > > > Signed-off-by: Chris J Arges <chris.j.arges@xxxxxxxxxxxxx> > > --- > > Documentation/ABI/testing/sysfs-kernel-livepatch | 2 +- > > kernel/livepatch/core.c | 20 ++++++++++++++++++-- > > 2 files changed, 19 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch b/Documentation/ABI/testing/sysfs-kernel-livepatch > > index 5bf42a8..dcd36db 100644 > > --- a/Documentation/ABI/testing/sysfs-kernel-livepatch > > +++ b/Documentation/ABI/testing/sysfs-kernel-livepatch > > @@ -33,7 +33,7 @@ Description: > > The object directory contains subdirectories for each function > > that is patched within the object. > > > > -What: /sys/kernel/livepatch/<patch>/<object>/<function> > > +What: /sys/kernel/livepatch/<patch>/<object>/<function.number> > > Date: Nov 2014 > > KernelVersion: 3.19.0 > > Contact: live-patching@xxxxxxxxxxxxxxx > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > > index 6e53441..ecacf65 100644 > > --- a/kernel/livepatch/core.c > > +++ b/kernel/livepatch/core.c > > @@ -587,7 +587,7 @@ EXPORT_SYMBOL_GPL(klp_enable_patch); > > * /sys/kernel/livepatch/<patch> > > * /sys/kernel/livepatch/<patch>/enabled > > * /sys/kernel/livepatch/<patch>/<object> > > - * /sys/kernel/livepatch/<patch>/<object>/<func> > > + * /sys/kernel/livepatch/<patch>/<object>/<func.number> > > */ > > > > static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr, > > @@ -727,13 +727,29 @@ static void klp_free_patch(struct klp_patch *patch) > > kobject_put(&patch->kobj); > > } > > > > +static int klp_count_sysfs_funcs(struct klp_object *obj, const char *name) > > +{ > > + struct klp_func *func; > > + int n = 0; > > + > > + /* count the times a function name occurs and is initialized */ > > + klp_for_each_func(obj, func) { > > + if ((!strcmp(func->old_name, name) && > > + func->kobj.state_initialized)) > > + n++; > > + } > > + > > + return n; > > +} > > + > > static int klp_init_func(struct klp_object *obj, struct klp_func *func) > > { > > INIT_LIST_HEAD(&func->stack_node); > > func->state = KLP_DISABLED; > > > > return kobject_init_and_add(&func->kobj, &klp_ktype_func, > > - &obj->kobj, "%s", func->old_name); > > + &obj->kobj, "%s.%d", func->old_name, > > + klp_count_sysfs_funcs(obj, func->old_name)); > > } > > > > /* parts of the initialization that is done only when the object is loaded */ > > -- > > 1.9.1 > > I'd prefer something other than a period for the string separator > because some symbols have a period in their name. How about a space? > Perhaps a '-' would be better? /t_next-0 /t_next-1 > Also, this shows the nth occurrence of the symbol name in the patch > module. But I think it would be better to instead display the nth > occurrence of the symbol name in the kallsyms for the patched object. > That way user space can deterministically detect which function was > patched. > > For example: > > $ grep " t_next" /proc/kallsyms > ffffffff811597d0 t t_next > ffffffff81163bb0 t t_next > ... > > In my kernel there are 6 functions named t_next in vmlinux. "t_next 0" > would refer to the function at 0xffffffff811597d0. "t_next 1" would > refer to the one at 0xffffffff81163bb0. > This makes sense to me. > While we're at it, should we also encode the replacement function name > (func->new_func)? e.g.: > > "t_next 0 t_next__patched". > > > -- > Josh > Since we are creating a directory for this function, at some point would we add this as a file in that func directory? I think encoding the func name with old_name + occurrence should accomplish uniqueness and consistency. However, another approach would be: /<old_func>-<new_func> Which presumably would be unique and consistent (depending on how the patch was authored). --chris -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
