Re: [GIT PULL] nohz: Tick dependency mask v2

[Ingo Molnar <mingo@kernel.org>]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Tue, Feb 16, 2016 at 09:03:45AM +0100, Ingo Molnar wrote:
> > 
> > 
> > So I think it would be useful to name this in a way the expresses that this is a 
> > mask.
> > 
> > 'tick_dep_mask' or so?
> 
> [...]
> 
> > >  
> > > +enum tick_dependency_bit {
> > 
> > s/tick_dep_bits
> > 
> > > +	TICK_POSIX_TIMER_BIT	= 0,
> > > +	TICK_PERF_EVENTS_BIT	= 1,
> > > +	TICK_SCHED_BIT		= 2,
> > > +	TICK_CLOCK_UNSTABLE_BIT	= 3
> > 
> > s/TICK_DEP_BIT_...
> > 
> > > +};
> > > +
> > > +#define TICK_POSIX_TIMER_MASK		(1 << TICK_POSIX_TIMER_BIT)
> > > +#define TICK_PERF_EVENTS_MASK		(1 << TICK_PERF_EVENTS_BIT)
> > > +#define TICK_SCHED_MASK			(1 << TICK_SCHED_BIT)
> > > +#define TICK_CLOCK_UNSTABLE_MASK	(1 << TICK_CLOCK_UNSTABLE_BIT)
> > 
> > So I'd rename this to:
> > 
> > #define TICK_DEP_MASK_POSIX_TIMER	(1 << TICK_POSIX_TIMER_BIT)
> > #define TICK_DEP_MASK_PERF_EVENTS	(1 << TICK_PERF_EVENTS_BIT)
> > #define TICK_DEP_MASK_SCHED		(1 << TICK_SCHED_BIT)
> > #define TICK_DEP_MASK_CLOCK_UNSTABLE	(1 << TICK_CLOCK_UNSTABLE_BIT)
> > 
> > i.e. the 'tick_dep' and 'TICK_DEP' nomenclature would be used throughout the code 
> > and the pattern would be easy to grep for.
> > 
> > > +extern void tick_nohz_set_dep(enum tick_dependency_bit bit);
> > > +extern void tick_nohz_clear_dep(enum tick_dependency_bit bit);
> > > +extern void tick_nohz_set_dep_cpu(int cpu, enum tick_dependency_bit bit);
> > > +extern void tick_nohz_clear_dep_cpu(int cpu, enum tick_dependency_bit bit);
> > > +extern void tick_nohz_set_dep_task(struct task_struct *tsk,
> > > +				   enum tick_dependency_bit bit);
> > > +extern void tick_nohz_clear_dep_task(struct task_struct *tsk,
> > > +				     enum tick_dependency_bit bit);
> > > +extern void tick_nohz_set_dep_signal(struct signal_struct *signal,
> > > +				     enum tick_dependency_bit bit);
> > > +extern void tick_nohz_clear_dep_signal(struct signal_struct *signal,
> > > +				       enum tick_dependency_bit bit);
> > 
> > Ditto, please rename it all to:
> > 
> > 	tick_dep_set()
> > 	tick_dep_clear()
> > 	tick_dep_set_cpu()
> > 	tick_dep_clear_cpu()
> > 	tick_dep_set_task()
> > 	...
> 
> Ok, I fixed all the above.
> 
> > 
> > also, please don't line-break function prototypes, it only makes the result harder 
> > to read.
> 
> I couldn't fix that though, I'm limited by the 80 columns.
> 
> 
> If you're ok with it, please pull the branch:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> 	timers/core-v9
> 
> HEAD: 4f49b90abb4aca6fe677c95fc352fd0674d489bd
> 
> --- Summary ---
> 
> Currently in nohz full configs, the tick dependency is checked
> asynchronously by nohz code from interrupt and context switch for each
> concerned subsystem with a set of function provided by these. Such
> functions are made of many conditions and details that can be heavyweight
> as they are called on fastpath: sched_can_stop_tick(),
> posix_cpu_timer_can_stop_tick(), perf_event_can_stop_tick()...
> 
> Thomas suggested a few month ago to make that tick dependency check
> synchronous. Instead of checking subsystems details from each interrupt
> to guess if the tick can be stopped, every subsystem that may have a tick
> dependency should set itself a flag specifying the state of that
> dependency. This way we can verify if we can stop the tick with a single
> lightweight mask check on fast path.
> 
> This conversion from a pull to a push model to implement tick dependency
> is the core feature of this patchset that is split into:
> 
> * Nohz wide kick simplification
> * Improve nohz tracing
> * Introduce tick dependency mask
> * Migrate scheduler, posix timers, perf events and sched clock tick
>   dependencies to the tick dependency mask.

Pulled into tip:timers/nohz, thanks a lot Frederic!

	Ingo