From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755239AbbDIMpa (ORCPT ); Thu, 9 Apr 2015 08:45:30 -0400 Received: from mail-wi0-f173.google.com ([209.85.212.173]:36434 "EHLO mail-wi0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751372AbbDIMp2 (ORCPT ); Thu, 9 Apr 2015 08:45:28 -0400 Date: Thu, 9 Apr 2015 14:45:25 +0200 From: Frederic Weisbecker To: Chris Metcalf Cc: Peter Zijlstra , "Paul E. McKenney" , "Rafael J. Wysocki" , Martin Schwidefsky , Ingo Molnar , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/2] nohz: make nohz_full imply isolcpus Message-ID: <20150409124524.GA17709@lerouge> References: <1428344205-27678-1-git-send-email-cmetcalf@ezchip.com> <1428344205-27678-2-git-send-email-cmetcalf@ezchip.com> <20150408094114.GX23123@twins.programming.kicks-ass.net> <5525357B.2080205@ezchip.com> <20150408142641.GG5029@twins.programming.kicks-ass.net> <55254794.4070503@ezchip.com> <20150408172746.GK5029@twins.programming.kicks-ass.net> <55256F92.1010606@ezchip.com> <20150409082921.GP27490@worktop.programming.kicks-ass.net> <55266A54.30304@ezchip.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55266A54.30304@ezchip.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Apr 09, 2015 at 08:02:28AM -0400, Chris Metcalf wrote: > On 4/9/2015 4:29 AM, Peter Zijlstra wrote: > >On Wed, Apr 08, 2015 at 02:12:34PM -0400, Chris Metcalf wrote: > > > >>>But you're doing the reverse! You're setting nohz_full for isolcpus, not > >>>limiting the nohz_full mask to isolcpus. > >>Ah, I see. Yes, that's right. > >No its not, you should correct me when I'm wrong ;-) > > Oh, I have no problem with that :-) But, now that I know what was confusing > you about the patch I see what it was you were saying with your English > above too. I thought you were saying something like "making nohz_full > imply isolcpus" again, but you weren't. Phew, OK, I think we're done > talking at cross-purposes. > > >So the problem is that: > > > >+ tick_nohz_full_set_cpus(cpu_isolated_map); > > > >reads like you're doing: > > > > nohz_full_map |= isolcpus_map > > > >But in actual fact you're doing: > > > > isolcpus_map |= nohz_full_map > > > >So that function is retarded, but the logic is fine. > > > >So NAK on both patches for the reason that they're utterly confusing as > >to wtf they actually do. > > What about tick_nohz_full_cpumask_or(cpu_isolated_map) ? > At that point maybe the similarity to the existing cpumask API will make > it more clear that we are modifying the argument? > > If not, do you have any suggestions what might do better? Obviously > the goal is to make it something that macroizes away, otherwise I'd > suggest just explicitly using an #ifdef and cpumask_or(). Possible alternative: do it the other way around. cpu_isolated_map is allocated and filled early (__setup or sched_init()) before tick_init() and tick_init() is before sched_init_smp() which first uses cpu_isolated_map(). So we can call some sched_isolated_map_add(struct cpumask *cpumask) from tick_nohz_init().