From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932576AbdKFSxX (ORCPT ); Mon, 6 Nov 2017 13:53:23 -0500 Received: from foss.arm.com ([217.140.101.70]:39470 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753523AbdKFSxW (ORCPT ); Mon, 6 Nov 2017 13:53:22 -0500 Message-ID: <5A00AF37.7030606@arm.com> Date: Mon, 06 Nov 2017 18:51:35 +0000 From: James Morse User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.6.0 MIME-Version: 1.0 To: Peter Zijlstra CC: Thomas Gleixner , "linux-kernel@vger.kernel.org" Subject: Re: get_online_cpus() from a preemptible() context (bug?) References: <59FC8119.8030608@arm.com> <20171106103212.GG3165@worktop.lehotels.local> In-Reply-To: <20171106103212.GG3165@worktop.lehotels.local> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Peter, (combining your replies) On 06/11/17 10:32, Peter Zijlstra wrote: > On Fri, Nov 03, 2017 at 02:45:45PM +0000, James Morse wrote: >> I'm trying to work out what stops a thread being pre-empted and migrated between >> calling get_online_cpus() and put_online_cpus(). > Nothing; why would you think it would? To stop the this_cpu_*() operations in down/up being applied on different CPUs, affecting a different percpu:read_count. > All those functions guarantee is > that any CPU observed as being online says online (and its converse, > that a CPU observed as being offline, says offline, although less people > care about that one). >> According to __percpu_down_read(), its the pre-empt count: >>> * Due to having preemption disabled the decrement happens on >>> * the same CPU as the increment, avoiding the >>> * increment-on-one-CPU-and-decrement-on-another problem. >> >> >> So this: >>> void cpus_read_lock(void) >>> { >>> percpu_down_read(&cpu_hotplug_lock); >>> + >>> + /* Can we migrated before we release this per-cpu lock? */ >>> + WARN_ON(preemptible()); >>> } >> >> should never fire? > It should.. You're reading a comment on __percpu_down_read() and using > percpu_down_read(), _not_ the same function ;-) Yes, sorry, I thought you did a better job of describing the case I'm trying to work-out. > If you look at percpu_down_read(), you'll note it'll disable preemption > before calling __percpu_down_read(). Yes, this is how __percpu_down_read() protects the combination of it's fast/slow paths. But next percpu_down_read() calls preempt_enable(), I can't see what stops us migrating before percpu_up_read() preempt_disable()s to call __this_cpu_dec(), which now affects a different variable. > And yes, that whole percpu-rwsem code is fairly magical :-) I think I'll file this under magical. That rcu_sync_is_idle() must know something I don't! Thanks! James