From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S942068AbdEYJEj (ORCPT ); Thu, 25 May 2017 05:04:39 -0400 Received: from mail.kernel.org ([198.145.29.99]:57292 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933767AbdEYJEf (ORCPT ); Thu, 25 May 2017 05:04:35 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0668923900 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=mhiramat@kernel.org Date: Thu, 25 May 2017 18:04:30 +0900 From: Masami Hiramatsu To: Ingo Molnar Cc: "Paul E . McKenney" , 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 Message-Id: <20170525180430.ab38cbb7d53cf7bc9b4de065@kernel.org> In-Reply-To: <20170525061555.m2ihe2dvjyedyzwn@gmail.com> References: <149562719270.15375.4565081030740506940.stgit@devbox> <20170525061555.m2ihe2dvjyedyzwn@gmail.com> X-Mailer: Sylpheed 3.5.0 (GTK+ 2.24.30; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 25 May 2017 08:15:55 +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? Hmm, if there are only 3 users, kprobes/ftrace/kpatch, and those use it same purpose (wait for tasks which preempted or interrupted), maybe we can switch the implementation of synchronize_rcu_tasks() in RCU level. > > 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. TASKS_RCU kconfig is already a hidden setting. It is selected if CONFIG_PREEMPT=y && CONFIG_KPROBES=y && HAVE_OPTPROBES=y for kprobes. Thank you, > > - Both ftrace andkprobes would use a simple synchronize_rcu_tasks() call. -- Masami Hiramatsu