From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757100Ab0CaPhE (ORCPT ); Wed, 31 Mar 2010 11:37:04 -0400 Received: from e4.ny.us.ibm.com ([32.97.182.144]:33105 "EHLO e4.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752199Ab0CaPg7 (ORCPT ); Wed, 31 Mar 2010 11:36:59 -0400 Date: Wed, 31 Mar 2010 08:36:56 -0700 From: "Paul E. McKenney" To: Lai Jiangshan Cc: Ingo Molnar , LKML Subject: Re: [PATCH] rcu: don't call rcu_preempt_note_context_switch() in rcu_check_callbacks() Message-ID: <20100331153656.GA4623@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <4BB014DF.9030905@cn.fujitsu.com> <20100329044236.GB2343@linux.vnet.ibm.com> <4BB1C7C5.5070700@cn.fujitsu.com> <20100330160339.GB2513@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100330160339.GB2513@linux.vnet.ibm.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 30, 2010 at 09:03:39AM -0700, Paul E. McKenney wrote: > On Tue, Mar 30, 2010 at 05:43:33PM +0800, Lai Jiangshan wrote: > > Paul E. McKenney wrote: > > > On Mon, Mar 29, 2010 at 10:47:59AM +0800, Lai Jiangshan wrote: > > >> Content-Type: text/plain; charset=UTF-8 > > >> Content-Transfer-Encoding: 7bit > > >> > > >> > > >> Even though in user mode or idle mode, rcu_check_callbacks() is not > > >> context switch, so we don't call rcu_preempt_note_context_switch() > > >> in rcu_check_callbacks(). > > >> > > >> Though there is no harm that calls rcu_preempt_note_context_switch() > > >> in rcu_check_callbacks(), but it is waste. > > >> > > >> rcu_check_callbacks() > > >> rcu_sched_qs() > > >> rcu_preempt_note_context_switch() > > >> Now, ->rcu_read_lock_nesting == 0, so we just calls > > >> rcu_preempt_qs(), but, rcu_preempt_check_callbacks() > > >> will call it again and set the ->rcu_read_unlock_special > > >> correct again. > > >> > > >> So let rcu_preempt_check_callbacks() handle things for us. > > > > > > Nice!!! > > > > > > But how about naming the new function that invokes > > > rcu_preempt_note_context_switch() something like > > > rcu_sched_note_context_switch(), and then leaving the > > > name of rcu_sched_qs() the same (rather than changing > > > it to __rcu_sched_qs(), as below)? > > > > > > This way, the names clearly call out what the function > > > is doing. > > > > > > > If I understand right, it will become this: > > > > schedule() / run_ksoftirqd() / rcu_needs_cpu() > > rcu_sched_note_context_switch() > > rcu_sched_qs() > > rcu_preempt_note_context_switch() > > Wow!!! That was a scare!!! I misread "run_ksoftirqd()" as > "do_softirq(). ;-) > > And I am not seeing a call to rcu_sched_qs() in rcu_needs_cpu()... > > Here is how I believe it needs to go: > > schedule(): > rcu_sched_note_context_switch() > rcu_sched_qs() > rcu_preempt_note_context_switch() > > run_ksoftirqd(): > rcu_sched_qs() > > rcu_check_callbacks(): > rcu_sched_qs() [if idle etc.] > rcu_bh_qs() [if not in softirq] > > The reason we don't need rcu_bh_qs() from run_ksoftirqd() is that > __do_softirq() already calls rcu_bh_qs(). > > Make sense, or am I missing something? And I was in fact missing something. The rcu_preempt_note_context_switch() function currently combines some work that needs to happen only at context-switch time with work that needs to happen all the time. At first glance, it appears that the big "if" statement in rcu_preempt_note_context_switch() need only happen for context switches. The remaining lines must happen unconditionally for context switches, and should be executed from rcu_check_callbacks() only if the current CPU is not in an RCU read-side critical section. Thanx, Paul