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@xxxxxxxx>
Best Regards,
Petr
