linux-kernel - Re: How to turn scheduler tick on for current nohz

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190730173637.GG14271@linux.ibm.com>
Date:   Tue, 30 Jul 2019 10:36:37 -0700
From:   "Paul E. McKenney" <paulmck@...ux.ibm.com>
To:     Frederic Weisbecker <frederic@...nel.org>
Cc:     fweisbec@...il.com, tglx@...utronix.de, mingo@...nel.org,
        linux-kernel@...r.kernel.org, rcu@...r.kernel.org,
        peterz@...radead.org
Subject: Re: How to turn scheduler tick on for current nohz_full CPU?

On Tue, Jul 30, 2019 at 06:43:10PM +0200, Frederic Weisbecker wrote:
> 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.

You lost me on this one.  I would be turning off RCU's request to
maintain the tick on transition to nohz_full userspace or to idle.
Why would the tick get turned on by a later user->kernel transition?

> > 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.

Ah, good, that would remove my need to clear things on the scheduler
fastpaths.  My guess is that I use both the per-CPU and the per-task
variant in different places, but testing will tell!  ;-)

Thank you!

							Thanx, Paul

> > 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.
>