Re: [PATCH v2 2/2] nohz: make nohz_full imply isolcpus

[Frederic Weisbecker]

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().
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/