| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20170526101619.57d2c23de9ba10e31ea190e8@kernel.org>
Date: Fri, 26 May 2017 10:16:19 +0900
From: Masami Hiramatsu <mhiramat@...nel.org>
To: paulmck@...ux.vnet.ibm.com
Cc: Ingo Molnar <mingo@...nel.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Steven Rostedt <rostedt@...dmis.org>,
linux-kernel@...r.kernel.org,
Peter Zijlstra <peterz@...radead.org>,
Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
Thomas Gleixner <tglx@...utronix.de>,
"H . Peter Anvin" <hpa@...or.com>
Subject: Re: [RFC PATCH tip/master] kprobes: Use synchronize_rcu_tasks() for
optprobe wit CONFIG_PREEMPT
On Thu, 25 May 2017 08:14:01 -0700
"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com> wrote:
> On Thu, May 25, 2017 at 08:15:55AM +0200, Ingo Molnar wrote:
> >
> > * Masami Hiramatsu <mhiramat@...nel.org> wrote:
> >
> > > --- a/kernel/kprobes.c
> > > +++ b/kernel/kprobes.c
> > > @@ -377,6 +377,23 @@ static inline void copy_kprobe(struct kprobe *ap, struct kprobe *p)
> > > static bool kprobes_allow_optimization;
> > >
> > > /*
> > > + * Synchronizing wait on trampline code for interrupted tasks/threads.
> > > + * Since the threads running on dynamically allocated trampline code
> > > + * can be interrupted, kprobes has to wait for those tasks back on
> > > + * track and scheduled. If the kernel is preemptive, the thread can be
> > > + * preempted by other tasks on the trampoline too. For such case, this
> > > + * calls synchronize_rcu_tasks() to wait for those tasks back on track.
> > > + */
> > > +static void synchronize_on_trampoline(void)
> > > +{
> > > +#ifdef CONFIG_PREEMPT
> > > + synchronize_rcu_tasks();
> > > +#else
> > > + synchronize_sched();
> > > +#endif
> > > +}
> >
> > So that's really unacceptably ugly.
> >
> > Paul, I still question the need to have tasks-RCU as a Kconfig distinction,
> > _especially_ if its API usage results in such ugly secondary #ifdefs...
> >
> > Why isn't there a single synchronize_rcu_tasks() API function, which does what is
> > expected, where the _RCU_ code figures out how to implement it?
> >
> > I.e.:
> >
> > - There should be no user configurable TASKS_RCU Kconfig setting - at most a
> > helper Kconfig that is automatically selected by the RCU code itself.
> >
> > - Both ftrace andkprobes would use a simple synchronize_rcu_tasks() call.
> >
> > Thanks,
>
> How about the following (untested) patch?
This patch is good to me for kprobes at least.
Reviewed-by: Masami Hiramatsu <mhiramat@...nel.org>
Steve, how about ftrace usage?
Thank you,
>
> This is against -rcu's rcu/dev branch, FWIW.
>
> And I am also queuing patches with other cleanups, including do_exit(),
> enabled by this approach.
>
> Thanx, Paul
>
> ------------------------------------------------------------------------
>
> commit 9ab47fba45cea06e223e524d392621b64c174720
> Author: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> Date: Thu May 25 08:05:00 2017 -0700
>
> rcu: Drive TASKS_RCU directly off of PREEMPT
>
> The actual use of TASKS_RCU is only when PREEMPT, otherwise RCU-sched
> is used instead. This commit therefore makes synchronize_rcu_tasks()
> and call_rcu_tasks() available always, but mapped to synchronize_sched()
> and call_rcu_sched(), respectively, when !PREEMPT. This approach also
> allows some #ifdefs to be removed from rcutorture.
>
> Reported-by: Ingo Molnar <mingo@...nel.org>
> Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
>
> ---
>
> include/linux/rcupdate.h | 6 ++++--
> kernel/rcu/Kconfig | 3 +--
> kernel/rcu/rcutorture.c | 17 +----------------
> 3 files changed, 6 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index f816fc72b51e..c3f380befdd7 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -58,8 +58,6 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func);
> void call_rcu_bh(struct rcu_head *head, rcu_callback_t func);
> void call_rcu_sched(struct rcu_head *head, rcu_callback_t func);
> void synchronize_sched(void);
> -void call_rcu_tasks(struct rcu_head *head, rcu_callback_t func);
> -void synchronize_rcu_tasks(void);
> void rcu_barrier_tasks(void);
>
> #ifdef CONFIG_PREEMPT_RCU
> @@ -176,10 +174,14 @@ extern struct srcu_struct tasks_rcu_exit_srcu;
> rcu_all_qs(); \
> rcu_note_voluntary_context_switch_lite(t); \
> } while (0)
> +void call_rcu_tasks(struct rcu_head *head, rcu_callback_t func);
> +void synchronize_rcu_tasks(void);
> #else /* #ifdef CONFIG_TASKS_RCU */
> #define TASKS_RCU(x) do { } while (0)
> #define rcu_note_voluntary_context_switch_lite(t) do { } while (0)
> #define rcu_note_voluntary_context_switch(t) rcu_all_qs()
> +#define call_rcu_tasks call_rcu_sched
> +#define synchronize_rcu_tasks synchronize_sched
> #endif /* #else #ifdef CONFIG_TASKS_RCU */
>
> /**
> diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
> index be90c945063f..9210379c0353 100644
> --- a/kernel/rcu/Kconfig
> +++ b/kernel/rcu/Kconfig
> @@ -69,8 +69,7 @@ config TREE_SRCU
> This option selects the full-fledged version of SRCU.
>
> config TASKS_RCU
> - bool
> - default n
> + def_bool PREEMPT
> select SRCU
> help
> This option enables a task-based RCU implementation that uses
> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> index aedc8f2ad955..273032dc8f2d 100644
> --- a/kernel/rcu/rcutorture.c
> +++ b/kernel/rcu/rcutorture.c
> @@ -659,8 +659,6 @@ static struct rcu_torture_ops sched_ops = {
> .name = "sched"
> };
>
> -#ifdef CONFIG_TASKS_RCU
> -
> /*
> * Definitions for RCU-tasks torture testing.
> */
> @@ -698,24 +696,11 @@ static struct rcu_torture_ops tasks_ops = {
> .name = "tasks"
> };
>
> -#define RCUTORTURE_TASKS_OPS &tasks_ops,
> -
> static bool __maybe_unused torturing_tasks(void)
> {
> return cur_ops == &tasks_ops;
> }
>
> -#else /* #ifdef CONFIG_TASKS_RCU */
> -
> -#define RCUTORTURE_TASKS_OPS
> -
> -static bool __maybe_unused torturing_tasks(void)
> -{
> - return false;
> -}
> -
> -#endif /* #else #ifdef CONFIG_TASKS_RCU */
> -
> /*
> * RCU torture priority-boost testing. Runs one real-time thread per
> * CPU for moderate bursts, repeatedly registering RCU callbacks and
> @@ -1712,7 +1697,7 @@ rcu_torture_init(void)
> int firsterr = 0;
> static struct rcu_torture_ops *torture_ops[] = {
> &rcu_ops, &rcu_bh_ops, &rcu_busted_ops, &srcu_ops, &srcud_ops,
> - &sched_ops, RCUTORTURE_TASKS_OPS
> + &sched_ops, &tasks_ops,
> };
>
> if (!torture_init_begin(torture_type, verbose, &torture_runnable))
>
--
Masami Hiramatsu <mhiramat@...nel.org>
Powered by blists - more mailing lists