From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S938702AbdEYPOI (ORCPT ); Thu, 25 May 2017 11:14:08 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:42536 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S934176AbdEYPOG (ORCPT ); Thu, 25 May 2017 11:14:06 -0400 Date: Thu, 25 May 2017 08:14:01 -0700 From: "Paul E. McKenney" To: Ingo Molnar Cc: Masami Hiramatsu , Steven Rostedt , linux-kernel@vger.kernel.org, Peter Zijlstra , Ananth N Mavinakayanahalli , Thomas Gleixner , "H . Peter Anvin" Subject: Re: [RFC PATCH tip/master] kprobes: Use synchronize_rcu_tasks() for optprobe wit CONFIG_PREEMPT Reply-To: paulmck@linux.vnet.ibm.com References: <149562719270.15375.4565081030740506940.stgit@devbox> <20170525061555.m2ihe2dvjyedyzwn@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170525061555.m2ihe2dvjyedyzwn@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 x-cbid: 17052515-0008-0000-0000-00000229964E X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007116; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000212; SDB=6.00865485; UDB=6.00429753; IPR=6.00645261; BA=6.00005374; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00015581; XFM=3.00000015; UTC=2017-05-25 15:14:03 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17052515-0009-0000-0000-000035686F49 Message-Id: <20170525151401.GP3956@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-05-25_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1705250288 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 25, 2017 at 08:15:55AM +0200, Ingo Molnar wrote: > > * Masami Hiramatsu 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 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 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 Signed-off-by: Paul E. McKenney --- 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))