Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

[Frederic Weisbecker]

On Fri, Aug 11, 2017 at 03:09:57PM -0400, Luiz Capitulino wrote:
> On Fri, 21 Jul 2017 15:21:28 +0200
> Frederic Weisbecker <fweisbec@xxxxxxxxx> wrote:
> > -void __init housekeeping_init(void)
> > +/* Parse the boot-time housekeeping CPU list from the kernel parameters. */
> > +static int __init housekeeping_setup(char *str)
> >  {
> > -	if (!tick_nohz_full_enabled())
> > -		return;
> > -
> > -	if (!alloc_cpumask_var(&housekeeping_mask, GFP_KERNEL)) {
> > -		WARN(1, "NO_HZ: Can't allocate not-full dynticks cpumask\n");
> > -		cpumask_clear(tick_nohz_full_mask);
> > -		tick_nohz_full_running = false;
> > -		return;
> > +	alloc_bootmem_cpumask_var(&housekeeping_mask);
> > +	if (cpulist_parse(str, housekeeping_mask) < 0) {
> > +		pr_warn("Housekeeping: Incorrect cpumask\n");
> > +		free_bootmem_cpumask_var(housekeeping_mask);
> > +		return 1;
> >  	}
> >  
> > -	cpumask_andnot(housekeeping_mask,
> > -		       cpu_possible_mask, tick_nohz_full_mask);
> > -
> >  	static_branch_enable(&housekeeping_overriden);
> >  
> >  	/* We need at least one CPU to handle housekeeping work */
> >  	WARN_ON_ONCE(cpumask_empty(housekeeping_mask));
> > +
> > +	return 1;
> >  }
> > +__setup("housekeeping=", housekeeping_setup);
> 
> Am I right that from now on nohz_full= users will also have
> to specify housekeeping= in order to get nohz_full working?
> If that's correct, then won't this patch break nohz_full for
> existing setups?

nohz_full= will still work but will only imply tick stop. A few isolation
details that were enabled by nohz_full= won't be handled anymore such as:
unbound timers affinity, watchdog disablement, rcu threads affinity, sched idle
load balancing... Those are now handled by housekeeping=

So yes in a sense, this can break some setup that assume nohz_full= does more
than stopping the tick.

Perhaps I should remove the nohz_full= parameter altogether and let nohz_full controlled
by housekeeping= only. How much can kernel parameters be considered as kernel ABIs?

Also I'm wondering if "housekeeping=" is a clear name for users. "isolation=" or
"cpu_isolation=" would be better and more obvious. Housekeeping based naming would only be
internal implementation detail. And deactivating the tick through "cpu_isolation=" would
be clearer than if we did through "housekeeping=".

Of course the problem is that we already have "isolcpus=". But re-implementing isolcpus
on top of housekeeping might be a good idea. I believe that the current implementation on
top of NULL domains isn't much beloved. A less controversial implementation might even
allow us to control it though cpusets.
 
> 
> Also, I just give this series a try and got this:
> 
> [    0.000000] Kernel command line: BOOT_IMAGE=/vmlinuz-4.13.0-rc4+ root=/dev/mapper/rhel_virtlab508-root ro crashkernel=auto rd.lvm.lv=rhel_virtlab508/root rd.lvm.lv=rhel_virtlab508/swap console=ttyS1,115200 LANG=en_US.UTF-8 housekeeping=0,2,4,6,8,10,12,14,1 isolcpus=15 nohz_full=15 intel_pstate=disable
> [    0.000000] static_key_slow_inc used before call to jump_label_init
> [    0.000000] ------------[ cut here ]------------
> [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:108 static_key_slow_inc+0x86/0xa0

Oops ^_^

Thanks.