Messages in this thread Patch in this message | | | Date | Wed, 6 Oct 2021 14:30:35 +0200 | From | Petr Mladek <> | Subject | Re: [RFC][PATCH v2 10/11] livepatch: Remove klp_synchronize_transition() |
| |
On Wed 2021-09-29 17:17:33, Peter Zijlstra wrote: > The whole premise is broken, any trace usage outside of RCU is a BUG > and should be fixed. Notable all code prior (and a fair bit after) > ct_user_exit() is noinstr and explicitly doesn't allow any tracing.
I see. Is the situation the same with idle threads? I see that some functions, called inside rcu_idle_enter()/exit() are still visible for ftrace. For example, arch_cpu_idle() or tick_check_broadcast_expired().
That said, I am not sure if anyone would ever want to livepatch this code. And there is still a workaround by livepatching the callers.
Also it should be easy to catch eventual problems if we check rcu_is_watching() in klp_ftrace_handler(). Most affected scheduler and idle code paths will likely be called even during the simple boot test.
> Use regular RCU, which has the benefit of actually respecting > NOHZ_FULL.
Good to know.
After all, the patch looks good to me. I would just add something like:
--- a/kernel/livepatch/patch.c +++ b/kernel/livepatch/patch.c @@ -69,6 +69,12 @@ static void notrace klp_ftrace_handler(unsigned long ip, if (WARN_ON_ONCE(!func)) goto unlock; + if (!rcu_is_watching()) { + printk_deferred_once(KERN_WARNING + "warning: called \"%s\" without RCU watching. The function is not guarded by the consistency model. Also beware when removing the livepatch module.\n", + func->old_name); + } + /* * In the enable path, enforce the order of the ops->func_stack and * func->transition reads. The corresponding write barrier is in But we could do this in a separate patch later.
Feel free to use:
Reviewed-by: Petr Mladek <pmladek@suse.com>
Best Regards, Petr
| |