From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757242AbYGYWcg (ORCPT ); Fri, 25 Jul 2008 18:32:36 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752162AbYGYWc2 (ORCPT ); Fri, 25 Jul 2008 18:32:28 -0400 Received: from E23SMTP02.au.ibm.com ([202.81.18.163]:59972 "EHLO e23smtp02.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752129AbYGYWc1 (ORCPT ); Fri, 25 Jul 2008 18:32:27 -0400 Date: Fri, 25 Jul 2008 18:31:59 -0400 From: Gautham R Shenoy To: Dmitry Adamushko Cc: Peter Zijlstra , Ingo Molnar , LKML , Steven Rostedt , Thomas Gleixner Subject: Re: [patch, rfc: 1/2] sched, hotplug: safe use of rq->migration_thread and find_busiest_queue() Message-ID: <20080725223159.GA16433@in.ibm.com> Reply-To: ego@in.ibm.com References: <1216937517.5368.11.camel@earth> <1216985986.7257.375.camel@twins> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17 (2007-11-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jul 25, 2008 at 01:52:17PM +0200, Dmitry Adamushko wrote: > 2008/7/25 Peter Zijlstra : > > On Fri, 2008-07-25 at 00:11 +0200, Dmitry Adamushko wrote: > >> From: Dmitry Adamushko > >> Subject: sched, hotplug: safe use of rq->migration_thread > >> and find_busiest_queue() > >> > >> --- > >> > >> sched, hotplug: safe use of rq->migration_thread and find_busiest_queue() > >> > >> (1) make usre rq->migration_thread is valid when we access it in set_cpus_allowed_ptr() > >> after releasing the rq-lock; > >> > >> (2) in load_balance() and load_balance_idle() > >> > >> ensure that we don't get 'busiest' which can disappear as a result of cpu_down() > >> while we are manipulating it. For this goal, we choose 'busiest' only amongst > >> 'cpu_active_map' cpus. > >> > >> load_balance() and load_balance_idle() get called with preemption being disabled > >> so synchronize_sched() in cpu_down() should get us synced. > >> > >> IOW, as soon as synchronize_sched() has been done in cpu_down(cpu), the run-queue for > >> can't be manipulated/accessed by the load-balancer. > >> > >> Signed-off-by: Dmitry Adamushko > > > > Acked-by: Peter Zijlstra > > Thanks. > > > > >> diff --git a/kernel/sched.c b/kernel/sched.c > >> index 6acf749..b4ccc8b 100644 > >> --- a/kernel/sched.c > >> +++ b/kernel/sched.c > >> @@ -3409,7 +3409,14 @@ static int load_balance(int this_cpu, struct rq *this_rq, > >> struct rq *busiest; > >> unsigned long flags; > >> > >> - cpus_setall(*cpus); > >> + /* > >> + * Ensure that we don't get 'busiest' which can disappear > >> + * as a result of cpu_down() while we are manipulating it. > >> + * > >> + * load_balance() gets called with preemption being disabled > >> + * so synchronize_sched() in cpu_down() should get us synced. > >> + */ > >> + *cpus = cpu_active_map; > > > > This is going to be painful on -rt... there it can be preempted. I guess > > we can put get_online_cpus() around it or something.. > > I've considered using get_online_cpus() for a moment but dropped this > idea exactly because I thought it would harm us latency-wise. get_online_cpus() can be made to be extremely lightweight (as simple as updating a per_cpu variable). But yes, if a cpu-hotplug operation is in progress one might block there.. So probably we need the try_ variant here.. > cpu_down() and cpu_up() may take quite a long time to complete and > load_balance() && load_balance_idle() would need to wait all this > time. And they both are kind of generic (primary) scheduler > operations. > > but yea, my scheme relies on the fact that load_balance() && > load_balance_idle() are atomic one way or another wrt. cpu_clear() + > synchronize_sched() in cpu_down(). > > [ speculating here ] I'd rather add an additional mechanism which > would be light-weight for load_balance() and add > synch_this_mechanism() (alike to synchonise_sched()) in cpu_down() as > perhaps we don't care that much on how fast the later one is. > > > -- > Best regards, > Dmitry Adamushko > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- Thanks and Regards gautham