2024-01-24 17:36:26

by Frederic Weisbecker

[permalink] [raw]
Subject: [PATCH 00/15] timers/nohz cleanups and hotplug reorganization

Hi,

Here are some cleanups here and there and also some more rational tick
related CPU hotplug code reorganization.

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
timers/hotplug

HEAD: 378e195ce2fd96d90ed7c1cde9033cb7079a7274

Thanks,
Frederic
---

Frederic Weisbecker (13):
tick: Remove useless oneshot ifdeffery
tick: Use IS_ENABLED() whenever possible
tick: s/tick_nohz_stop_sched_tick/tick_nohz_full_stop_tick
tick: No need to clear ts->next_tick again
tick: Start centralizing tick related CPU hotplug operations
tick: Move tick cancellation up to CPUHP_AP_TICK_DYING
tick: Move broadcast cancellation up to CPUHP_AP_TICK_DYING
tick: Assume the tick can't be stopped in NOHZ_MODE_INACTIVE mode
tick: Move got_idle_tick away from common flags
tick: Move individual bit features to debuggable mask accesses
tick: Split nohz and highres features from nohz_mode
tick: Shut down low-res tick from dying CPU
tick: Assume timekeeping is correctly handed over upon last offline idle call

Peng Liu (2):
tick/nohz: Remove duplicate between tick_nohz_switch_to_nohz() and tick_setup_sched_timer()
tick/nohz: Remove duplicate between lowres and highres handlers


include/linux/cpuhotplug.h | 1 +
include/linux/tick.h | 16 +--
kernel/cpu.c | 11 +-
kernel/sched/idle.c | 1 -
kernel/time/hrtimer.c | 4 +-
kernel/time/tick-common.c | 31 +++--
kernel/time/tick-internal.h | 2 +
kernel/time/tick-sched.c | 296 ++++++++++++++++++++++----------------------
kernel/time/tick-sched.h | 40 +++---
kernel/time/timer_list.c | 10 +-
10 files changed, 213 insertions(+), 199 deletions(-)


2024-01-24 17:36:30

by Frederic Weisbecker

[permalink] [raw]
Subject: [PATCH 02/15] tick/nohz: Remove duplicate between lowres and highres handlers

From: Peng Liu <[email protected]>

tick_nohz_lowres_handler() does the same work as
tick_nohz_highres_handler() plus the clockevent device reprogramming, so
make the former reuse the latter.

Signed-off-by: Peng Liu <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
---
kernel/time/tick-sched.c | 96 ++++++++++++++++------------------------
1 file changed, 38 insertions(+), 58 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index e90dbb7ae70a..6f69ae88e47e 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -255,6 +255,41 @@ static void tick_sched_handle(struct tick_sched *ts, struct pt_regs *regs)
update_process_times(user_mode(regs));
profile_tick(CPU_PROFILING);
}
+
+/*
+ * We rearm the timer until we get disabled by the idle code.
+ * Called with interrupts disabled.
+ */
+static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer)
+{
+ struct tick_sched *ts =
+ container_of(timer, struct tick_sched, sched_timer);
+ struct pt_regs *regs = get_irq_regs();
+ ktime_t now = ktime_get();
+
+ tick_sched_do_timer(ts, now);
+
+ /*
+ * Do not call when we are not in IRQ context and have
+ * no valid 'regs' pointer
+ */
+ if (regs)
+ tick_sched_handle(ts, regs);
+ else
+ ts->next_tick = 0;
+
+ /*
+ * In dynticks mode, tick reprogram is deferred:
+ * - to the idle task if in dynticks-idle
+ * - to IRQ exit if in full-dynticks.
+ */
+ if (unlikely(ts->tick_stopped))
+ return HRTIMER_NORESTART;
+
+ hrtimer_forward(timer, now, TICK_NSEC);
+
+ return HRTIMER_RESTART;
+}
#endif

#ifdef CONFIG_NO_HZ_FULL
@@ -1392,30 +1427,17 @@ void tick_nohz_idle_exit(void)
* infrastructure actually relies on the tick itself as a backend in
* low-resolution mode (see hrtimer_run_queues()).
*
- * This low-resolution handler still makes use of some hrtimer APIs meanwhile
- * for convenience with expiration calculation and forwarding.
+ * This low-resolution handler still reuse tick_nohz_highres_handler() since
+ * most of the work is independent from the clockevent level.
*/
static void tick_nohz_lowres_handler(struct clock_event_device *dev)
{
struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched);
- struct pt_regs *regs = get_irq_regs();
- ktime_t now = ktime_get();

dev->next_event = KTIME_MAX;

- tick_sched_do_timer(ts, now);
- tick_sched_handle(ts, regs);
-
- /*
- * In dynticks mode, tick reprogram is deferred:
- * - to the idle task if in dynticks-idle
- * - to IRQ exit if in full-dynticks.
- */
- if (likely(!ts->tick_stopped)) {
- hrtimer_forward(&ts->sched_timer, now, TICK_NSEC);
+ if (likely(tick_nohz_highres_handler(&ts->sched_timer) == HRTIMER_RESTART))
tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
- }
-
}

static inline void tick_nohz_activate(struct tick_sched *ts, int mode)
@@ -1484,46 +1506,6 @@ void tick_irq_enter(void)
tick_nohz_irq_enter();
}

-/*
- * High resolution timer specific code
- */
-#ifdef CONFIG_HIGH_RES_TIMERS
-/*
- * We rearm the timer until we get disabled by the idle code.
- * Called with interrupts disabled.
- */
-static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer)
-{
- struct tick_sched *ts =
- container_of(timer, struct tick_sched, sched_timer);
- struct pt_regs *regs = get_irq_regs();
- ktime_t now = ktime_get();
-
- tick_sched_do_timer(ts, now);
-
- /*
- * Do not call when we are not in IRQ context and have
- * no valid 'regs' pointer
- */
- if (regs)
- tick_sched_handle(ts, regs);
- else
- ts->next_tick = 0;
-
- /*
- * In dynticks mode, tick reprogram is deferred:
- * - to the idle task if in dynticks-idle
- * - to IRQ exit if in full-dynticks.
- */
- if (unlikely(ts->tick_stopped))
- return HRTIMER_NORESTART;
-
- hrtimer_forward(timer, now, TICK_NSEC);
-
- return HRTIMER_RESTART;
-}
-#endif /* HIGH_RES_TIMERS */
-
#if defined CONFIG_NO_HZ_COMMON || defined CONFIG_HIGH_RES_TIMERS
static int sched_skew_tick;

@@ -1545,10 +1527,8 @@ void tick_setup_sched_timer(int mode)

/* Emulate tick processing via per-CPU hrtimers: */
hrtimer_init(&ts->sched_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_HARD);
-#ifdef CONFIG_HIGH_RES_TIMERS
if (mode == NOHZ_MODE_HIGHRES)
ts->sched_timer.function = tick_nohz_highres_handler;
-#endif

/* Get the next period (per-CPU) */
hrtimer_set_expires(&ts->sched_timer, tick_init_jiffy_update());
--
2.43.0


2024-01-24 17:37:13

by Frederic Weisbecker

[permalink] [raw]
Subject: [PATCH 08/15] tick: Move tick cancellation up to CPUHP_AP_TICK_DYING

The tick hrtimer is cancelled right before hrtimers are migrated. This
is done from the hrtimer subsystem even though it shouldn't know about
its actual users.

Move instead the tick hrtimer cancellation to the relevant CPU hotplug
state that aims at centralizing high level tick shutdown operations so
that the related flow is easy to follow.

Signed-off-by: Frederic Weisbecker <[email protected]>
---
kernel/time/hrtimer.c | 2 --
kernel/time/tick-common.c | 2 ++
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 355b5a957f7f..3b456ec7d4fb 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -2224,8 +2224,6 @@ int hrtimers_cpu_dying(unsigned int dying_cpu)
struct hrtimer_cpu_base *old_base, *new_base;
int i, ncpu = cpumask_first(cpu_active_mask);

- tick_cancel_sched_timer(dying_cpu);
-
old_base = this_cpu_ptr(&hrtimer_bases);
new_base = &per_cpu(hrtimer_bases, ncpu);

diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index a89ef450fda7..b4af8c743b73 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -410,6 +410,8 @@ int tick_cpu_dying(unsigned int dying_cpu)
if (tick_do_timer_cpu == dying_cpu)
tick_do_timer_cpu = cpumask_first(cpu_online_mask);

+ tick_cancel_sched_timer(dying_cpu);
+
return 0;
}

--
2.43.0


2024-01-25 09:38:34

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 08/15] tick: Move tick cancellation up to CPUHP_AP_TICK_DYING

On Wed, Jan 24 2024 at 18:04, Frederic Weisbecker wrote:

> The tick hrtimer is cancelled right before hrtimers are migrated. This
> is done from the hrtimer subsystem even though it shouldn't know about
> its actual users.
>
> Move instead the tick hrtimer cancellation to the relevant CPU hotplug
> state that aims at centralizing high level tick shutdown operations so
> that the related flow is easy to follow.
>
> Signed-off-by: Frederic Weisbecker <[email protected]>

Reviewed-by: Thomas Gleixner <[email protected]>

2024-01-25 10:11:57

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 02/15] tick/nohz: Remove duplicate between lowres and highres handlers

On Wed, Jan 24 2024 at 18:04, Frederic Weisbecker wrote:
> +/*
> + * We rearm the timer until we get disabled by the idle code.
> + * Called with interrupts disabled.
> + */
> +static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer)

Isn't that a misnomer now?

> +{
> + struct tick_sched *ts =
> + container_of(timer, struct tick_sched, sched_timer);

Let it stick out please.

> + struct pt_regs *regs = get_irq_regs();
> + ktime_t now = ktime_get();
> + if (likely(tick_nohz_highres_handler(&ts->sched_timer) == HRTIMER_RESTART))
> tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);

Thanks,

tglx

2024-01-25 11:59:40

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH 02/15] tick/nohz: Remove duplicate between lowres and highres handlers

On Thu, Jan 25, 2024 at 10:32:09AM +0100, Thomas Gleixner wrote:
> On Wed, Jan 24 2024 at 18:04, Frederic Weisbecker wrote:
> > +/*
> > + * We rearm the timer until we get disabled by the idle code.
> > + * Called with interrupts disabled.
> > + */
> > +static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer)
>
> Isn't that a misnomer now?

Would tick_nohz_hrtimer_handler() make more sense then? Because it's clearly
an hrtimer handler, just emulated in low-res mode.

>
> > +{
> > + struct tick_sched *ts =
> > + container_of(timer, struct tick_sched, sched_timer);
>
> Let it stick out please.

Ok.

Thanks.
>
> > + struct pt_regs *regs = get_irq_regs();
> > + ktime_t now = ktime_get();
> > + if (likely(tick_nohz_highres_handler(&ts->sched_timer) == HRTIMER_RESTART))
> > tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
>
> Thanks,
>
> tglx

2024-01-25 13:30:50

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 02/15] tick/nohz: Remove duplicate between lowres and highres handlers

On Thu, Jan 25 2024 at 12:58, Frederic Weisbecker wrote:

> On Thu, Jan 25, 2024 at 10:32:09AM +0100, Thomas Gleixner wrote:
>> On Wed, Jan 24 2024 at 18:04, Frederic Weisbecker wrote:
>> > +/*
>> > + * We rearm the timer until we get disabled by the idle code.
>> > + * Called with interrupts disabled.
>> > + */
>> > +static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer)
>>
>> Isn't that a misnomer now?
>
> Would tick_nohz_hrtimer_handler() make more sense then? Because it's clearly
> an hrtimer handler, just emulated in low-res mode.

Kinda. tick_nohz_handler() would be sufficient too.