From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751956AbbJLRh3 (ORCPT ); Mon, 12 Oct 2015 13:37:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60425 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751828AbbJLRhZ (ORCPT ); Mon, 12 Oct 2015 13:37:25 -0400 Date: Mon, 12 Oct 2015 19:34:02 +0200 From: Oleg Nesterov To: Peter Zijlstra Cc: heiko.carstens@de.ibm.com, Tejun Heo , Ingo Molnar , Rik van Riel , Thomas Gleixner , Vitaly Kuznetsov , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] sched: select_task_rq() should check cpu_active() like select_fallback_rq() Message-ID: <20151012173402.GA29647@redhat.com> References: <20151010185255.GA24075@redhat.com> <20151010185309.GA24089@redhat.com> <20151012121657.GP3816@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151012121657.GP3816@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/12, Peter Zijlstra wrote: > > On Sat, Oct 10, 2015 at 08:53:09PM +0200, Oleg Nesterov wrote: > > I do not understand the cpu_active() check in select_fallback_rq(). > > x86 doesn't need it, and the recent commit dd9d3843755d "sched: Fix > > cpu_active_mask/cpu_online_mask race" documents the fact that on any > > architecture we can ignore !active starting from CPU_ONLINE stage. > > > > But any possible reason why do we need this check in "fallback" must > > equally apply to select_task_rq(). > > So the reason, from vague memory, is that we want to allow per-cpu > threads to start/stop before/after active. I simply can't understand... To me it looks as if we can simply remove the cpu_active() check in select_fallback_rq(). If we race with cpu_down(), cpu_active() is cleared by sched_cpu_inactive() which is CPU_PRI_SCHED_INACTIVE = INT_MIN + 1 priority, so it seems that only cpuset_cpu_inactive() can be called after that and this check is obviously racy anyway. As for cpu_up(), I do not see any arch which does set_cpu_active(true), they all do set_cpu_online(true) which also marks it active. So why we can't simply remove select_fallback_rq()->cpu_active() ? > active 'should' really only govern load-balancer bits or so. OK, I don't understand the usage of cpu_active_mask in kernel/sched/, and of course I could easily miss something else. But I doubt very much this check can save us from something bad simply because it is racy. Yes, we also have synchronize_sched() after CPU_DOWN_PREPARE, but the only thing we do before stop_machine() is smpboot_park_threads(). So this can help (say) set_cpus_allowed_ptr() which uses active_mask, but I don't see how this can connect to ttwu paths. And again. If there is some reason we can not do this, say, because ipi to non-active CPU can trigger a warning, or something else, then we can hit the same problem because select_task_rq() does not check cpu_active(). The kernel threads like stoppers/workers are probably fine in any case, but userspace can trigger the race with cpu_up/down. In short, I am all confused ;) Oleg.