lkml.org 
[lkml]   [2021]   [Oct]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2021-10-06 14:31    [W:0.348 / U:0.932 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site