Re: How to turn scheduler tick on for current nohz_full CPU?

[Frederic Weisbecker <frederic@kernel.org>]

On Mon, Jul 29, 2019 at 03:32:38PM -0700, Paul E. McKenney wrote:
> On Wed, Jul 24, 2019 at 06:12:43PM -0700, Paul E. McKenney wrote:
> 
> The patch below (which includes your patch) does help considerably.
> However, it does have some shortcomings:
> 
> 1.	Adds an atomic operation (albeit a cache-local one) to
> 	the scheduler fastpath.  One approach would be to have
> 	a way of testing this bit and clearing it only if set.
> 
> 	Another approach would be to instead clear it on the
> 	transition to nohz_full userspace or to idle.

Well, the latter would be costly as it is going to restart the tick on every
user -> kernel transitions.

> 
> 2.	There are a lot of other places in the kernel that are in
> 	need of this bit being set.  I am therefore considering making
> 	multi_cpu_stop() or its callers set this bit on all CPUs upon
> 	entry and clear it upon exit.  While in this state, it is
> 	likely necessary to disable clearing this bit.  Or it would
> 	be necessary to make multi_cpu_stop() repeat clearing the bit
> 	every so often.
> 
> 	As it stands, I have CPU hotplug removal operations taking
> 	more than 400 seconds.
> 
> 3.	It was tempting to ask for this bit to be tracked on a per-task
> 	basis, but from what I can see that adds at least as much
> 	complexity as it removes.

Yeah I forgot to answer, you can use tick_dep_set_task() for that.

> 
> Thoughts?
> 
> 							Thanx, Paul
> 
> PS.  Outage on @linux.ibm.com, hence the CC of my gmail address.
> 
> ------------------------------------------------------------------------
> 
> diff --git a/include/linux/tick.h b/include/linux/tick.h
> index 196a0a7bfc4f..0dea6fb33a11 100644
> --- a/include/linux/tick.h
> +++ b/include/linux/tick.h
> @@ -108,7 +108,8 @@ enum tick_dep_bits {
>  	TICK_DEP_BIT_POSIX_TIMER	= 0,
>  	TICK_DEP_BIT_PERF_EVENTS	= 1,
>  	TICK_DEP_BIT_SCHED		= 2,
> -	TICK_DEP_BIT_CLOCK_UNSTABLE	= 3
> +	TICK_DEP_BIT_CLOCK_UNSTABLE	= 3,
> +	TICK_DEP_BIT_RCU		= 4
>  };
>  
>  #define TICK_DEP_MASK_NONE		0
> @@ -116,6 +117,7 @@ enum tick_dep_bits {
>  #define TICK_DEP_MASK_PERF_EVENTS	(1 << TICK_DEP_BIT_PERF_EVENTS)
>  #define TICK_DEP_MASK_SCHED		(1 << TICK_DEP_BIT_SCHED)
>  #define TICK_DEP_MASK_CLOCK_UNSTABLE	(1 << TICK_DEP_BIT_CLOCK_UNSTABLE)
> +#define TICK_DEP_MASK_RCU		(1 << TICK_DEP_BIT_RCU)
>  
>  #ifdef CONFIG_NO_HZ_COMMON
>  extern bool tick_nohz_enabled;
> @@ -258,6 +260,9 @@ static inline bool tick_nohz_full_enabled(void) { return false; }
>  static inline bool tick_nohz_full_cpu(int cpu) { return false; }
>  static inline void tick_nohz_full_add_cpus_to(struct cpumask *mask) { }
>  
> +static inline void tick_nohz_dep_set_cpu(int cpu, enum tick_dep_bits bit) { }
> +static inline void tick_nohz_dep_clear_cpu(int cpu, enum tick_dep_bits bit) { }

And I gave you the wrong APIs. Please consider_using tick_dep_set_cpu()
and tick_dep_clear_cpu() that first check if the CPU uses nohz_full.

Those should have the !CONFIG_NO_HZ_FULL stub implemented as well.

Thanks.