Re: [PATCH RESEND] nohz: Affining unpinned timers

Re: [PATCH RESEND] nohz: Affining unpinned timers

[Preeti U Murthy]

On 06/23/2015 02:01 PM, Vatika Harlalka wrote:
> The problem addressed in this patch is about affining unpinned timers.
> Adaptive or Full Dynticks CPUs should not be disturbed by unnecessary
> jitter due to firing of such timers on them.
> This patch will affine timers to online CPUs which are not Full Dynticks
> in FULL_NOHZ configured systems. It will not bring about functional
> changes if NOHZ_FULL is not configured, because is_housekeeping_cpu()
> always returns true in CONFIG_NO_HZ_FULL=n.
> 
> Signed-off by: Vatika Harlalka <vatikaharlalka@gmail.com>
> ---

Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>

>  Changes: Patch sent to the right recipients.
> 
>  include/linux/tick.h | 9 ++++++++-
>  kernel/sched/core.c  | 9 +++++++--
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/tick.h b/include/linux/tick.h
> index f8492da5..145bcba 100644
> --- a/include/linux/tick.h
> +++ b/include/linux/tick.h
> @@ -133,13 +133,20 @@ static inline bool tick_nohz_full_cpu(int cpu)
> 
>  	return cpumask_test_cpu(cpu, tick_nohz_full_mask);
>  }
> -
> +static inline int housekeeping_any_cpu(void)
> +{
> +	return cpumask_any_and(housekeeping_mask, cpu_online_mask);
> +}
>  extern void __tick_nohz_full_check(void);
>  extern void tick_nohz_full_kick(void);
>  extern void tick_nohz_full_kick_cpu(int cpu);
>  extern void tick_nohz_full_kick_all(void);
>  extern void __tick_nohz_task_switch(struct task_struct *tsk);
>  #else
> +static inline int housekeeping_any_cpu(void)
> +{
> +	return smp_processor_id();
> +}
>  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_check(void) { }
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 57bd333..acee856 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -599,18 +599,23 @@ int get_nohz_timer_target(int pinned)
>  	int i;
>  	struct sched_domain *sd;
> 
> -	if (pinned || !get_sysctl_timer_migration() || !idle_cpu(cpu))
> +	if (pinned || !get_sysctl_timer_migration() ||
> +		(!idle_cpu(cpu) && is_housekeeping_cpu(cpu)))
>  		return cpu;
> 
>  	rcu_read_lock();
>  	for_each_domain(cpu, sd) {
>  		for_each_cpu(i, sched_domain_span(sd)) {
> -			if (!idle_cpu(i)) {
> +			if (!idle_cpu(i) && is_housekeeping_cpu(cpu)) {
>  				cpu = i;
>  				goto unlock;
>  			}
>  		}
>  	}
> +
> +	if (!is_housekeeping_cpu(cpu))
> +		cpu = housekeeping_any_cpu();
> +
>  unlock:
>  	rcu_read_unlock();
>  	return cpu;
> 

Re: [PATCH RESEND] nohz: Affining unpinned timers

[Frederic Weisbecker]

I hope everybody received the patch because there is a ";" after each address :-)

On Tue, Jun 23, 2015 at 02:01:33PM +0530, Vatika Harlalka wrote:
> The problem addressed in this patch is about affining unpinned timers.
> Adaptive or Full Dynticks CPUs should not be disturbed by unnecessary
> jitter due to firing of such timers on them.
> This patch will affine timers to online CPUs which are not Full Dynticks
> in FULL_NOHZ configured systems. It will not bring about functional
> changes if NOHZ_FULL is not configured, because is_housekeeping_cpu()
> always returns true in CONFIG_NO_HZ_FULL=n.
> 
> Signed-off by: Vatika Harlalka <vatikaharlalka@gmail.com>

The patch looks good to me. Peter, Thomas, are you ok with it too?

Thanks.

> ---
>  Changes: Patch sent to the right recipients.
> 
>  include/linux/tick.h | 9 ++++++++-
>  kernel/sched/core.c  | 9 +++++++--
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/tick.h b/include/linux/tick.h
> index f8492da5..145bcba 100644
> --- a/include/linux/tick.h
> +++ b/include/linux/tick.h
> @@ -133,13 +133,20 @@ static inline bool tick_nohz_full_cpu(int cpu)
>  
>  	return cpumask_test_cpu(cpu, tick_nohz_full_mask);
>  }
> -
> +static inline int housekeeping_any_cpu(void)
> +{
> +	return cpumask_any_and(housekeeping_mask, cpu_online_mask);
> +}
>  extern void __tick_nohz_full_check(void);
>  extern void tick_nohz_full_kick(void);
>  extern void tick_nohz_full_kick_cpu(int cpu);
>  extern void tick_nohz_full_kick_all(void);
>  extern void __tick_nohz_task_switch(struct task_struct *tsk);
>  #else
> +static inline int housekeeping_any_cpu(void)
> +{
> +	return smp_processor_id();
> +}
>  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_check(void) { }
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 57bd333..acee856 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -599,18 +599,23 @@ int get_nohz_timer_target(int pinned)
>  	int i;
>  	struct sched_domain *sd;
>  
> -	if (pinned || !get_sysctl_timer_migration() || !idle_cpu(cpu))
> +	if (pinned || !get_sysctl_timer_migration() ||
> +		(!idle_cpu(cpu) && is_housekeeping_cpu(cpu)))
>  		return cpu;
>  
>  	rcu_read_lock();
>  	for_each_domain(cpu, sd) {
>  		for_each_cpu(i, sched_domain_span(sd)) {
> -			if (!idle_cpu(i)) {
> +			if (!idle_cpu(i) && is_housekeeping_cpu(cpu)) {
>  				cpu = i;
>  				goto unlock;
>  			}
>  		}
>  	}
> +
> +	if (!is_housekeeping_cpu(cpu))
> +		cpu = housekeeping_any_cpu();
> +
>  unlock:
>  	rcu_read_unlock();
>  	return cpu;
> -- 
> 2.4.2
> 

Re: [PATCH RESEND] nohz: Affining unpinned timers

[Thomas Gleixner]

On Mon, 6 Jul 2015, Frederic Weisbecker wrote:
> I hope everybody received the patch because there is a ";" after each address :-)
> 
> On Tue, Jun 23, 2015 at 02:01:33PM +0530, Vatika Harlalka wrote:
> > The problem addressed in this patch is about affining unpinned timers.
> > Adaptive or Full Dynticks CPUs should not be disturbed by unnecessary
> > jitter due to firing of such timers on them.
> > This patch will affine timers to online CPUs which are not Full Dynticks
> > in FULL_NOHZ configured systems. It will not bring about functional
> > changes if NOHZ_FULL is not configured, because is_housekeeping_cpu()
> > always returns true in CONFIG_NO_HZ_FULL=n.
> > 
> > Signed-off by: Vatika Harlalka <vatikaharlalka@gmail.com>
> 
> The patch looks good to me. Peter, Thomas, are you ok with it too?

By some definition of OK. The overhead of this is growing and growing.

We really need to make this a pull not a push model.

Thanks,

	tglx

Re: [PATCH RESEND] nohz: Affining unpinned timers

[Frederic Weisbecker]

On Mon, Jul 06, 2015 at 07:18:22PM +0200, Thomas Gleixner wrote:
> On Mon, 6 Jul 2015, Frederic Weisbecker wrote:
> > I hope everybody received the patch because there is a ";" after each address :-)
> > 
> > On Tue, Jun 23, 2015 at 02:01:33PM +0530, Vatika Harlalka wrote:
> > > The problem addressed in this patch is about affining unpinned timers.
> > > Adaptive or Full Dynticks CPUs should not be disturbed by unnecessary
> > > jitter due to firing of such timers on them.
> > > This patch will affine timers to online CPUs which are not Full Dynticks
> > > in FULL_NOHZ configured systems. It will not bring about functional
> > > changes if NOHZ_FULL is not configured, because is_housekeeping_cpu()
> > > always returns true in CONFIG_NO_HZ_FULL=n.
> > > 
> > > Signed-off by: Vatika Harlalka <vatikaharlalka@gmail.com>
> > 
> > The patch looks good to me. Peter, Thomas, are you ok with it too?
> 
> By some definition of OK. The overhead of this is growing and growing.
> 
> We really need to make this a pull not a push model.

I'm currently working toward that.
See "[PATCH 0/8] tick/nohz: Tick dependency quick check + cleanups" as a
first step.

Now for this very patch, I don't know how we could make it better. Any
suggestion?

> Thanks,
> 
> 	tglx

Re: [PATCH RESEND] nohz: Affining unpinned timers

[Thomas Gleixner]

On Tue, 7 Jul 2015, Frederic Weisbecker wrote:
> On Mon, Jul 06, 2015 at 07:18:22PM +0200, Thomas Gleixner wrote:
> > On Mon, 6 Jul 2015, Frederic Weisbecker wrote:
> > > I hope everybody received the patch because there is a ";" after each address :-)
> > > 
> > > On Tue, Jun 23, 2015 at 02:01:33PM +0530, Vatika Harlalka wrote:
> > > > The problem addressed in this patch is about affining unpinned timers.
> > > > Adaptive or Full Dynticks CPUs should not be disturbed by unnecessary
> > > > jitter due to firing of such timers on them.
> > > > This patch will affine timers to online CPUs which are not Full Dynticks
> > > > in FULL_NOHZ configured systems. It will not bring about functional
> > > > changes if NOHZ_FULL is not configured, because is_housekeeping_cpu()
> > > > always returns true in CONFIG_NO_HZ_FULL=n.
> > > > 
> > > > Signed-off by: Vatika Harlalka <vatikaharlalka@gmail.com>
> > > 
> > > The patch looks good to me. Peter, Thomas, are you ok with it too?
> > 
> > By some definition of OK. The overhead of this is growing and growing.
> > 
> > We really need to make this a pull not a push model.
> 
> I'm currently working toward that.
> See "[PATCH 0/8] tick/nohz: Tick dependency quick check + cleanups" as a
> first step.
> 
> Now for this very patch, I don't know how we could make it better. Any
> suggestion?

Not for the time being. When I'm done with the timer wheel overhaul we
should look into queueing stuff in different wheels and let the non
idle cpus pull from there instead of pushing stuff at enqueue
time. The latter is really silly because most of these timers are
removed before they expire. Needs some thoughts, but anything which
avoids the whole target cpu crap at enqueue time is better than what
we have now.

Thanks,

	tglx

Re: [PATCH RESEND] nohz: Affining unpinned timers

[Vatika Harlalka]

If everyone thinks its alright, could this patch be applied please? :)

Thanks
Vatika

Re: [PATCH RESEND] nohz: Affining unpinned timers

[Frederic Weisbecker]

I'm going to resend the patch :)


On Sun, Aug 02, 2015 at 03:05:53PM +0530, Vatika Harlalka wrote:
> If everyone thinks its alright, could this patch be applied please? :)
> 
> Thanks
> Vatika