* Frederic Weisbecker <fweis...@gmail.com> wrote:
> The tick dependency is evaluated on every IRQ and context switch. This
> consists is a batch of checks which determine whether it is safe to
> stop the tick or not. These checks are often split in many details:
> posix cpu timers, scheduler, sched clock, perf events.... each of which
> are made of smaller details: posix cpu timer involves checking process
> wide timers then thread wide timers. Perf involves checking freq events
> then more per cpu details.
>
> Checking these informations asynchronously every time we update the full
> dynticks state bring avoidable overhead and a messy layout.
>
> Let's introduce instead tick dependency masks: one for system wide
> dependency (unstable sched clock, freq based perf events), one for CPU
> wide dependency (sched, throttling perf events), and task/signal level
> dependencies (posix cpu timers). The subsystems are responsible
> for setting and clearing their dependency through a set of APIs that will
> take care of concurrent dependency mask modifications and kick targets
> to restart the relevant CPU tick whenever needed.
>
> This new dependency engine stays beside the old one until all subsystems
> having a tick dependency are converted to it.
>
> Suggested-by: Thomas Gleixner <t...@linutronix.de>
> Suggested-by: Peter Zijlstra <pet...@infradead.org>
> Reviewed-by: Chris Metcalf <cmetc...@ezchip.com>
> Cc: Christoph Lameter <c...@linux.com>
> Cc: Chris Metcalf <cmetc...@ezchip.com>
> Cc: Ingo Molnar <mi...@kernel.org>
> Cc: Luiz Capitulino <lcapitul...@redhat.com>
> Cc: Peter Zijlstra <pet...@infradead.org>
> Cc: Rik van Riel <r...@redhat.com>
> Cc: Thomas Gleixner <t...@linutronix.de>
> Cc: Viresh Kumar <viresh.ku...@linaro.org>
> Signed-off-by: Frederic Weisbecker <fweis...@gmail.com>
> ---
> include/linux/sched.h | 8 +++
> include/linux/tick.h | 92 +++++++++++++++++++++++++++++
> kernel/time/tick-sched.c | 150
> ++++++++++++++++++++++++++++++++++++++++++++---
> kernel/time/tick-sched.h | 1 +
> 4 files changed, 244 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index a10494a..d482cc8 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -719,6 +719,10 @@ struct signal_struct {
> /* Earliest-expiration cache. */
> struct task_cputime cputime_expires;
>
> +#ifdef CONFIG_NO_HZ_FULL
> + unsigned long tick_dependency;
> +#endif
> +
> struct list_head cpu_timers[3];
>
> struct pid *tty_old_pgrp;
> @@ -1542,6 +1546,10 @@ struct task_struct {
> VTIME_SYS,
> } vtime_snap_whence;
> #endif
> +
> +#ifdef CONFIG_NO_HZ_FULL
> + unsigned long tick_dependency;
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?
> +#endif
> unsigned long nvcsw, nivcsw; /* context switch counts */
> u64 start_time; /* monotonic time in nsec */
> u64 real_start_time; /* boot based time in nsec */
> diff --git a/include/linux/tick.h b/include/linux/tick.h
> index 97fd4e5..a33adab 100644
> --- a/include/linux/tick.h
> +++ b/include/linux/tick.h
> @@ -97,6 +97,18 @@ static inline void tick_broadcast_exit(void)
> tick_broadcast_oneshot_control(TICK_BROADCAST_EXIT);
> }
>
> +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()
...
also, please don't line-break function prototypes, it only makes the result
harder
to read.
Thanks,
Ingo
