From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from e23smtp08.au.ibm.com (e23smtp08.au.ibm.com [202.81.31.141]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "e23smtp08.au.ibm.com", Issuer "GeoTrust SSL CA" (not verified)) by ozlabs.org (Postfix) with ESMTPS id F2B092C00A1 for ; Tue, 2 Jul 2013 18:28:43 +1000 (EST) Received: from /spool/local by e23smtp08.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 2 Jul 2013 18:25:51 +1000 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [9.190.235.152]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id A15423578051 for ; Tue, 2 Jul 2013 18:28:37 +1000 (EST) Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r628DUPt4587932 for ; Tue, 2 Jul 2013 18:13:31 +1000 Received: from d23av01.au.ibm.com (loopback [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r628SZ4r031404 for ; Tue, 2 Jul 2013 18:28:37 +1000 Message-ID: <51D28E69.9060205@linux.vnet.ibm.com> Date: Tue, 02 Jul 2013 13:55:13 +0530 From: "Srivatsa S. Bhat" MIME-Version: 1.0 To: Michael Wang Subject: Re: [PATCH v3 10/45] smp: Use get/put_online_cpus_atomic() to prevent CPU offline References: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com> <20130627195418.29830.34958.stgit@srivatsabhat.in.ibm.com> <51D2660A.8000401@linux.vnet.ibm.com> In-Reply-To: <51D2660A.8000401@linux.vnet.ibm.com> Content-Type: text/plain; charset=UTF-8 Cc: peterz@infradead.org, fweisbec@gmail.com, Shaohua Li , linux-kernel@vger.kernel.org, Jan Beulich , walken@google.com, mingo@kernel.org, linux-arch@vger.kernel.org, vincent.guittot@linaro.org, xiaoguangrong@linux.vnet.ibm.com, paulmck@linux.vnet.ibm.com, nikunj@linux.vnet.ibm.com, linux-pm@vger.kernel.org, rusty@rustcorp.com.au, rostedt@goodmis.org, namhyung@kernel.org, tglx@linutronix.de, Wang YanQing , laijs@cn.fujitsu.com, zhong@linux.vnet.ibm.com, netdev@vger.kernel.org, oleg@redhat.com, sbw@mit.edu, David.Laight@aculab.com, tj@kernel.org, akpm@linux-foundation.org, linuxppc-dev@lists.ozlabs.org, liguang List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Hi Michael, On 07/02/2013 11:02 AM, Michael Wang wrote: > Hi, Srivatsa > > On 06/28/2013 03:54 AM, Srivatsa S. Bhat wrote: > [snip] >> @@ -625,8 +632,9 @@ EXPORT_SYMBOL(on_each_cpu_mask); >> * The function might sleep if the GFP flags indicates a non >> * atomic allocation is allowed. >> * >> - * Preemption is disabled to protect against CPUs going offline but not online. >> - * CPUs going online during the call will not be seen or sent an IPI. >> + * We use get/put_online_cpus_atomic() to protect against CPUs going >> + * offline but not online. CPUs going online during the call will >> + * not be seen or sent an IPI. > > I was a little confused about this comment, if the offline-cpu still > have chances to become online, then there is chances that we will pick > it from for_each_online_cpu(), isn't it? Did I miss some point? > Whether or not the newly onlined CPU is observed in our for_each_online_cpu() loop, is dependent on timing. On top of that, there are 2 paths in the code: one which uses a temporary cpumask and the other which doesn't. In the former case, if a CPU comes online _after_ we populate the temporary cpumask, then we won't send an IPI to that cpu, since the temporary cpumask doesn't contain that CPU. Whereas, if we observe the newly onlined CPU in the for_each_online_cpu() loop itself (either in the former or the latter case), then yes, we will send the IPI to that CPU. Regards, Srivatsa S. Bhat > >> * >> * You must not call this function with disabled interrupts or >> * from a hardware interrupt handler or from a bottom half handler. >> @@ -641,26 +649,26 @@ void on_each_cpu_cond(bool (*cond_func)(int cpu, void *info), >> might_sleep_if(gfp_flags & __GFP_WAIT); >> >> if (likely(zalloc_cpumask_var(&cpus, (gfp_flags|__GFP_NOWARN)))) { >> - preempt_disable(); >> + get_online_cpus_atomic(); >> for_each_online_cpu(cpu) >> if (cond_func(cpu, info)) >> cpumask_set_cpu(cpu, cpus); >> on_each_cpu_mask(cpus, func, info, wait); >> - preempt_enable(); >> + put_online_cpus_atomic(); >> free_cpumask_var(cpus); >> } else { >> /* >> * No free cpumask, bother. No matter, we'll >> * just have to IPI them one by one. >> */ >> - preempt_disable(); >> + get_online_cpus_atomic(); >> for_each_online_cpu(cpu) >> if (cond_func(cpu, info)) { >> ret = smp_call_function_single(cpu, func, >> info, wait); >> WARN_ON_ONCE(!ret); >> } >> - preempt_enable(); >> + put_online_cpus_atomic(); >> } >> } >> EXPORT_SYMBOL(on_each_cpu_cond); >> >