From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751716AbdBIG6B (ORCPT ); Thu, 9 Feb 2017 01:58:01 -0500 Received: from mail-wr0-f196.google.com ([209.85.128.196]:36839 "EHLO mail-wr0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751543AbdBIG54 (ORCPT ); Thu, 9 Feb 2017 01:57:56 -0500 Date: Thu, 9 Feb 2017 07:57:27 +0100 From: Ingo Molnar To: Peter Zijlstra Cc: Thomas Gleixner , Mike Galbraith , Ingo Molnar , Sebastian Andrzej Siewior , LKML Subject: Re: tip: demise of tsk_cpus_allowed() and tsk_nr_cpus_allowed() Message-ID: <20170209065727.GA6902@gmail.com> References: <1486355037.10462.17.camel@gmx.de> <20170206103156.GA18908@gmail.com> <1486383511.10462.43.camel@gmx.de> <20170206122928.GB9404@gmail.com> <20170206133242.GK6515@twins.programming.kicks-ass.net> <20170206222313.GA6061@gmail.com> <20170208114016.GX6500@twins.programming.kicks-ass.net> <20170209064501.GA27072@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170209064501.GA27072@gmail.com> 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 * Ingo Molnar wrote: > > * Peter Zijlstra wrote: > > > On Wed, Feb 08, 2017 at 11:20:19AM +0100, Thomas Gleixner wrote: > > > On Mon, 6 Feb 2017, Ingo Molnar wrote: > > > > * Peter Zijlstra wrote: > > > > > > > > > > cpumasks are a pain, the above avoids allocating more of them. > > > > > > Indeed. > > > > > > > Yeah, so this could then be done by pointerifying ->cpus_allowed - more robust > > > > than the wrappery, > > > > > > You mean: > > > > > > struct task_struct { > > > cpumask_t cpus_allowed; > > > cpumask_t *effective_cpus_allowed; > > > }; > > Yeah. I'd name it a bit differently and constify the pointer to get type > safety and to make sure the mask is never modified through the pointer: > > struct task_struct { > const cpumask_t *cpus_ptr; > cpumask_t cpus_mask; > }; > > ( I'd drop the 'allowed' part, it's obvious enough what task->cpus_mask does, > right? ) > > and upstream would essentially just do: > > t->cpus_allowed_ptr = &t->cpus_allowed; > > And -rt, when it wants to pin a task, would do: > > t->cpus_allowed_ptr = &cpumask_of(task_cpu(p)); > > The rules are: > > - Code that 'uses' ->cpus_allowed would use the pointer. > > - Code that 'modifies' ->cpus_allowed would use the direct mask. > > The upstream advantages are: > > - The type separation of modifications from usage. > > - Removal of wrappery. > > - Maybe sometime in the future upstream would want to disable migration too ... > > In fact -rt gains something too: > > - With this scheme we would AFAICS get slightly more optimal code on -rt. > (Because there's no __migration_disabled() branching anymore.) > > - Plus all new code is automatically -rt ready - no need to maintain the wrappery > space. Much less code path forking. > > So as I see it it's win-win for both upstream and for -rt! > > > > and make the scheduler use effective_cpus_allowed instead of cpus_allowed? Or > > > what do you have in mind? > > > > That scheme is weird for nr_cpus_allowed. Not to mention that the > > pointer to the integer is larger than the integer itself. > > So in the new scheme I don't think nr_cpus_allowed would have to be wrapped > at all: whenever the pointer (or mask) is changed in set_cpus_allowed_common() > nr_cpus_allowed is recalculated as well - like today. > > It should be self-maintaining. Am I missing something? And -rt would do something like this in migration_disable()/enable(): t->cpus_ptr = &cpumask_of(task_cpu(p)); t->nr_cpus = 1; ... t->cpus_ptr = &t->cpus_mask; t->nr_cpus = cpumask_weight(t->cpus_mask); In addition to that we could cache the weight of the cpumask as an additional optimization: t->cpus_ptr = &t->cpus_mask; t->nr_cpus = t->cpus_mask_weight; It all looks like a pretty natural construct to me. The migration_disabled() flag spreads almost a hundred branches all across the scheduler. Thanks, Ingo