On Tue, Jan 20, 2015 at 04:37:59PM +0100, Jiri Slaby wrote:
> On 01/20/2015, 04:26 PM, Josh Poimboeuf wrote:
> > Add support for patching a function multiple times. If multiple patches
> > affect a function, the function in the most recently enabled patch
> > "wins". This enables a cumulative patch upgrade path, where each patch
> > is a superset of previous patches.
> >
> > This requires restructuring the data a little bit. With the current
> > design, where each klp_func struct has its own ftrace_ops, we'd have to
> > unregister the old ops and then register the new ops, because
> > FTRACE_OPS_FL_IPMODIFY prevents us from having two ops registered for
> > the same function at the same time. That would leave a regression
> > window where the function isn't patched at all (not good for a patch
> > upgrade path).
> >
> > This patch replaces the per-klp_func ftrace_ops with a global klp_ops
> > list, with one ftrace_ops per original function. A single ftrace_ops is
> > shared between all klp_funcs which have the same old_addr. This allows
> > the switch between function versions to happen instantaneously by
> > updating the klp_ops struct's func_stack list. The winner is the
> > klp_func at the top of the func_stack (front of the list).
> >
> > Signed-off-by: Josh Poimboeuf <jpoim...@redhat.com>
>
> Reviewed-by: Jiri Slaby <jsl...@suse.cz>
Thanks for the review!
> But...
>
> > @@ -267,16 +303,28 @@ static int klp_write_object_relocations(struct module
> > *pmod,
> >
> > static void notrace klp_ftrace_handler(unsigned long ip,
> > unsigned long parent_ip,
> > - struct ftrace_ops *ops,
> > + struct ftrace_ops *fops,
> > struct pt_regs *regs)
> > {
> > - struct klp_func *func = ops->private;
> > + struct klp_ops *ops;
> > + struct klp_func *func;
> > +
> > + ops = container_of(fops, struct klp_ops, fops);
> > +
> > + rcu_read_lock();
> > + func = list_first_or_null_rcu(&ops->func_stack, struct klp_func,
> > + stack_node);
> > + rcu_read_unlock();
> > +
> > + if (WARN_ON(!func))
> > + return;
>
> If it ever happens, the warn will drown the machine in the output splash.
Yeah, maybe, depending on the nature of the bug.
> WARN_ON_RATELIMIT?
Since this warning should never happen unless there's a code bug, I
think WARN_ON_ONCE should be sufficient?
--
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
