| 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
| ||
|
Message-ID: <a269c869-b966-75d5-5fe1-6ed6921c1b83@nextfour.com>
Date: Fri, 21 May 2021 05:25:41 +0300
From: Mika Penttilä <mika.penttila@...tfour.com>
To: Will Deacon <will@...nel.org>, linux-arm-kernel@...ts.infradead.org
Cc: linux-kernel@...r.kernel.org,
Frederic Weisbecker <fweisbec@...il.com>,
Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <maz@...nel.org>,
Lorenzo Colitti <lorenzo@...gle.com>,
John Stultz <john.stultz@...aro.org>,
Stephen Boyd <sboyd@...nel.org>, kernel-team@...roid.com
Subject: Re: [PATCH 3/5] tick/broadcast: Prefer per-cpu oneshot wakeup timers
to broadcast
Hi!
On 20.5.2021 21.47, Will Deacon wrote:
> Some SoCs have two per-cpu timer implementations where the timer with
> the higher rating stops in deep idle (i.e. suffers from
> CLOCK_EVT_FEAT_C3STOP) but is otherwise preferable to the timer with the
> lower rating. In such a design, we rely on a global broadcast timer and
> IPIs to wake up from deep idle states.
>
> To avoid the reliance on a global broadcast timer and also to reduce the
> overhead associated with the IPI wakeups, extend
> tick_install_broadcast_device() to manage per-cpu wakeup timers
> separately from the broadcast device.
>
> For now, these timers remain unused.
>
> Cc: Frederic Weisbecker <fweisbec@...il.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Marc Zyngier <maz@...nel.org>
> Cc: Lorenzo Colitti <lorenzo@...gle.com>
> Cc: John Stultz <john.stultz@...aro.org>
> Cc: Stephen Boyd <sboyd@...nel.org>
> Signed-off-by: Will Deacon <will@...nel.org>
> ---
> kernel/time/tick-broadcast.c | 57 +++++++++++++++++++++++++++++++++++-
> kernel/time/tick-common.c | 2 +-
> kernel/time/tick-internal.h | 4 +--
> 3 files changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
> index f3f2f4ba4321..8bd8cd69c8c9 100644
> --- a/kernel/time/tick-broadcast.c
> +++ b/kernel/time/tick-broadcast.c
> @@ -33,6 +33,8 @@ static int tick_broadcast_forced;
> static __cacheline_aligned_in_smp DEFINE_RAW_SPINLOCK(tick_broadcast_lock);
>
> #ifdef CONFIG_TICK_ONESHOT
> +static DEFINE_PER_CPU(struct clock_event_device *, tick_oneshot_wakeup_device);
> +
> static void tick_broadcast_setup_oneshot(struct clock_event_device *bc);
> static void tick_broadcast_clear_oneshot(int cpu);
> static void tick_resume_broadcast_oneshot(struct clock_event_device *bc);
> @@ -88,13 +90,59 @@ static bool tick_check_broadcast_device(struct clock_event_device *curdev,
> return !curdev || newdev->rating > curdev->rating;
> }
>
> +#ifdef CONFIG_TICK_ONESHOT
> +static struct clock_event_device *tick_get_oneshot_wakeup_device(int cpu)
> +{
> + return per_cpu(tick_oneshot_wakeup_device, cpu);
> +}
> +
> +static bool tick_set_oneshot_wakeup_device(struct clock_event_device *newdev,
> + int cpu)
> +{
> + struct clock_event_device *curdev;
> +
> + if (!newdev)
> + goto set_device;
> +
> + if ((newdev->features & CLOCK_EVT_FEAT_DUMMY) ||
> + (newdev->features & CLOCK_EVT_FEAT_C3STOP) ||
> + !(newdev->features & CLOCK_EVT_FEAT_ONESHOT))
> + return false;
> +
> + if (!cpumask_equal(newdev->cpumask, cpumask_of(cpu)))
> + return false;
> +
> + curdev = tick_get_oneshot_wakeup_device(cpu);
> + if (curdev && newdev->rating <= curdev->rating)
> + return false;
> +
> +set_device:
> + per_cpu(tick_oneshot_wakeup_device, cpu) = newdev;
> + return true;
> +}
> +#else
> +static struct clock_event_device *tick_get_oneshot_wakeup_device(int cpu)
> +{
> + return NULL;
> +}
> +
> +static bool tick_set_oneshot_wakeup_device(struct clock_event_device *newdev,
> + int cpu)
> +{
> + return false;
> +}
> +#endif
> +
> /*
> * Conditionally install/replace broadcast device
> */
> -void tick_install_broadcast_device(struct clock_event_device *dev)
> +void tick_install_broadcast_device(struct clock_event_device *dev, int cpu)
> {
> struct clock_event_device *cur = tick_broadcast_device.evtdev;
>
> + if (tick_set_oneshot_wakeup_device(dev, cpu))
> + return;
> +
> if (!tick_check_broadcast_device(cur, dev))
> return;
>
Does this disable hpet registering as a global broadcast device on x86 ?
I think it starts with cpumask = cpu0 so it qualifies for a percpu
wakeup timer.
> @@ -996,6 +1044,13 @@ void hotplug_cpu__broadcast_tick_pull(int deadcpu)
> */
> static void tick_broadcast_oneshot_offline(unsigned int cpu)
> {
> + struct clock_event_device *dev = tick_get_oneshot_wakeup_device(cpu);
> +
> + if (dev) {
> + clockevents_switch_state(dev, CLOCK_EVT_STATE_DETACHED);
> + tick_set_oneshot_wakeup_device(NULL, cpu);
> + }
> +
> /*
> * Clear the broadcast masks for the dead cpu, but do not stop
> * the broadcast device!
> diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
> index e15bc0ef1912..d663249652ef 100644
> --- a/kernel/time/tick-common.c
> +++ b/kernel/time/tick-common.c
> @@ -373,7 +373,7 @@ void tick_check_new_device(struct clock_event_device *newdev)
> /*
> * Can the new device be used as a broadcast device ?
> */
> - tick_install_broadcast_device(newdev);
> + tick_install_broadcast_device(newdev, cpu);
> }
>
> /**
> diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
> index 7a981c9e87a4..30c89639e305 100644
> --- a/kernel/time/tick-internal.h
> +++ b/kernel/time/tick-internal.h
> @@ -61,7 +61,7 @@ extern ssize_t sysfs_get_uname(const char *buf, char *dst, size_t cnt);
> /* Broadcasting support */
> # ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
> extern int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu);
> -extern void tick_install_broadcast_device(struct clock_event_device *dev);
> +extern void tick_install_broadcast_device(struct clock_event_device *dev, int cpu);
> extern int tick_is_broadcast_device(struct clock_event_device *dev);
> extern void tick_suspend_broadcast(void);
> extern void tick_resume_broadcast(void);
> @@ -72,7 +72,7 @@ extern int tick_broadcast_update_freq(struct clock_event_device *dev, u32 freq);
> extern struct tick_device *tick_get_broadcast_device(void);
> extern struct cpumask *tick_get_broadcast_mask(void);
> # else /* !CONFIG_GENERIC_CLOCKEVENTS_BROADCAST: */
> -static inline void tick_install_broadcast_device(struct clock_event_device *dev) { }
> +static inline void tick_install_broadcast_device(struct clock_event_device *dev, int cpu) { }
> static inline int tick_is_broadcast_device(struct clock_event_device *dev) { return 0; }
> static inline int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu) { return 0; }
> static inline void tick_do_periodic_broadcast(struct clock_event_device *d) { }
Powered by blists - more mailing lists