From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756900AbdDFLC2 (ORCPT ); Thu, 6 Apr 2017 07:02:28 -0400 Received: from mail-wr0-f194.google.com ([209.85.128.194]:33579 "EHLO mail-wr0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752603AbdDFLCT (ORCPT ); Thu, 6 Apr 2017 07:02:19 -0400 Date: Thu, 6 Apr 2017 13:02:15 +0200 From: Ingo Molnar To: Thomas Gleixner Cc: Sebastian Andrzej Siewior , linux-kernel@vger.kernel.org, Peter Zijlstra , Mike Galbraith , Ingo Molnar , "Rafael J . Wysocki" Subject: Re: [RFC PATCH] kernel: sched: Provide a pointer to the valid CPU mask Message-ID: <20170406110215.GA1367@gmail.com> References: <20170404184202.20376-1-bigeasy@linutronix.de> <20170405073943.GA17266@gmail.com> <20170405083753.7eszej2njds4ovdb@linutronix.de> <20170406061622.GA19979@gmail.com> <20170406073832.e7bu4ldpfuq44ui6@linutronix.de> <20170406080139.GA22069@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Thomas Gleixner wrote: > On Thu, 6 Apr 2017, Ingo Molnar wrote: > > > Sorry if this is a back and forth - I was somehow convinced that we do need to > > frob the cpus_allowed mask to get this functionality - but in hindsight I > > think the counter should be enough. > > > > I.e. just have a counter and these two APIs: > > > > static inline void migrate_disable(void) > > { > > current->migration_disabled++; > > } > > > > ... > > > > static inline void migrate_enable(void) > > { > > current->migration_disabled--; > > } > > > > ... and make sure the scheduler migration code plus the CPU hotplug code considers > > the counter. > > We tried that some time ago, but there are a lot of places in the scheduler > which just rely on the cpus_allowed_mask, so we have to chase all of them and > make sure that new users do not ignore that counter. That's why we chose the > cpus_allowed_mask approach. And I still think that's the proper thing to do. But but ... The number of places in the scheduler where we actually end up migrating a task is pretty limited: try_to_wake_up(): - main wakeup code migrate_swap(): - active NUMA-balancing feature move_queued_task(): - hotplug CPU-down migration - changing the affinity mask The wakeup and NUMA balancing case is trivial to solve: it's an optimization and we can skip the migration if migration is disabled. CPU hotplug and changing the affinity mask are the more complex cases, because there migrating or not migrating is a correctness issue: - CPU hotplug has to be aware of this anyway, regardless of whether it's solved via a counter of the affinity mask. - Changing the affinity mask (set_cpus_allowed()) has two main cases: the synchronous and asynchronous case: - synchronous is when the current task changes its own affinity mask, this should work fine mostly out of box, as we don't call set_cpus_allowed() from inside migration disabled regions. (We can enforce this via a debugging check.) - The asynchronous case is when the affinity task of some other task is changed - this would not have an immediate effect with migration-disabled logic, the migration would be delayed to when migration is re-enabled again. As for general fragility, is there any reason why a simple debugging check in set_task_cpu() would not catch most mishaps: WARN_ON_ONCE(p->state != TASK_RUNNING && p->migration_disabled); ... or something like that? I.e. my point is that I think using a counter would be much simpler, yet still as robust and maintainable. We could in fact move migrate_disable()/enable() upstream straight away and eliminate this small fork of functionality between mainline and -rt. Thanks, Ingo