[patch 00/11] hrtimers: Cleanup hrtimer_forward() [ab]use

[patch 00/11] hrtimers: Cleanup hrtimer_forward() [ab]use

[Thomas Gleixner]

A recent syzbot report unearthed abuse of hrtimer_forward() which can cause
runaway timers hogging the CPU in timer expiry context by rearming the
timer in the past over and over.

This happens when the caller uses timer->expiry for the 'now' argument of
hrtimer_forward(). That works as long as the timer expiry is on time, but
can cause a long period of rearm/fire loops which hog the CPU. Expiring
late can have various causes, but obviously virtualization is prone to that
due to VCPU scheduling.

The correct usage of hrtimer_forward() is to hand the current time to the
'now' argument which ensures that the next event on the periodic time line
is past now. This is what hrtimer_forward_now() provides.

The following series addresses this:

    1) Add a debug mechanism to the hrtimer expiry loop

    2) Convert all hrtimer_forward() usage outside of kernel/time/ to
       use hrtimer_forward_now().

    3) Confine hrtimer_forward() to kernel/time/ core code.

The mac80211_hwsim patch has already been picked up by the wireless
maintainer and all other patches which affect usage outside the core code
can be picked up by the relevant subsystems. If a maintainer wants me to
pick a particular patch up, please let me know.

The last patch which confines hrtimer_forward() will be postponed until all
other patches have been merged into Linus tree.

The series is also available from git:

    git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git hrtimer

Thanks,

	tglx
---
 drivers/gpu/drm/i915/i915_pmu.c        |    2 -
 drivers/net/wireless/mac80211_hwsim.c  |    4 +-
 drivers/net/wwan/iosm/iosm_ipc_imem.c  |    4 +-
 drivers/power/reset/ltc2952-poweroff.c |    4 --
 include/linux/hrtimer.h                |   26 -----------------
 include/linux/posix-timers.h           |    3 ++
 kernel/signal.c                        |   14 +--------
 kernel/time/hrtimer.c                  |   48 ++++++++++++++++++++++++++++++++-
 kernel/time/itimer.c                   |   13 ++++++++
 kernel/time/posix-timers.c             |   42 +++++++++++-----------------
 kernel/time/tick-internal.h            |    1 
 net/can/bcm.c                          |    2 -
 sound/drivers/pcsp/pcsp_lib.c          |    2 -
 13 files changed, 92 insertions(+), 73 deletions(-)

[patch 01/11] hrtimer: Add a mechanism to catch runaway timers

[Thomas Gleixner]

A recent report from syzbot unearthed a problem with self rearming timers
which fire late and try to catch up to now with a short period. That causes
the timer to be rearmed in the past until it eventually catches up with
now. If that rearming happens from the timer callback the hard or soft
interrupt expiry loop can run for a long time with either interrupts or
bottom halves disabled which causes RCU stalls and other lockups.

There is no safety net to stop or at least identify such runaway timers.

Detection is trivial. Cache the pointer to the last expired timer. The next
invocation from the same loop compares the pointer with the next expiring
hrtimer pointer and if they match 10 times in a row (in the same hard or
soft interrupt expiry instance) then it's reasonable to declare it as a
runaway.

In that case emit a warning and skip the callback invocation which stops
the misbehaving timer right there.

It's obviously incomplete, but it's definitely better than nothing and would
have caught the reported issue in mac80211_hwsim.

Suggested-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/hrtimer.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1714,6 +1714,13 @@ static void __run_hrtimer(struct hrtimer
 	base->running = NULL;
 }
 
+static void hrtimer_del_runaway(struct hrtimer_clock_base *base,
+				struct hrtimer *timer)
+{
+	__remove_hrtimer(timer, base, HRTIMER_STATE_INACTIVE, 0);
+	pr_warn("Runaway hrtimer %p %ps stopped\n", timer, timer->function);
+}
+
 static void __hrtimer_run_queues(struct hrtimer_cpu_base *cpu_base, ktime_t now,
 				 unsigned long flags, unsigned int active_mask)
 {
@@ -1722,6 +1729,8 @@ static void __hrtimer_run_queues(struct
 
 	for_each_active_base(base, cpu_base, active) {
 		struct timerqueue_node *node;
+		struct hrtimer *last = NULL;
+		unsigned int cnt = 0;
 		ktime_t basenow;
 
 		basenow = ktime_add(now, base->offset);
@@ -1732,6 +1741,22 @@ static void __hrtimer_run_queues(struct
 			timer = container_of(node, struct hrtimer, node);
 
 			/*
+			 * Catch timers which rearm themself with a expiry
+			 * time in the past over and over which makes this
+			 * loop run forever.
+			 */
+			if (IS_ENABLED(CONFIG_DEBUG_OBJECTS_TIMERS)) {
+				if (unlikely(last == timer)) {
+					if (++cnt == 10) {
+						hrtimer_del_runaway(base, timer);
+						continue;
+					}
+				}
+				last = timer;
+				cnt = 0;
+			}
+
+			/*
 			 * The immediate goal for using the softexpires is
 			 * minimizing wakeups, not running timers at the
 			 * earliest interrupt after their soft expiration.

[patch 02/11] mac80211-hwsim: Fix late beacon hrtimer handling

[Thomas Gleixner]

From: Johannes Berg <johannes.berg@intel.com>

Thomas explained in https://lore.kernel.org/r/87mtoeb4hb.ffs@tglx
that our handling of the hrtimer here is wrong: If the timer fires
late (e.g. due to vCPU scheduling, as reported by Dmitry/syzbot)
then it tries to actually rearm the timer at the next deadline,
which might be in the past already:

 1          2          3          N          N+1
 |          |          |   ...    |          |

 ^ intended to fire here (1)
            ^ next deadline here (2)
                                      ^ actually fired here

The next time it fires, it's later, but will still try to schedule
for the next deadline (now 3), etc. until it catches up with N,
but that might take a long time, causing stalls etc.

Now, all of this is simulation, so we just have to fix it, but
note that the behaviour is wrong even per spec, since there's no
value then in sending all those beacons unaligned - they should be
aligned to the TBTT (1, 2, 3, ... in the picture), and if we're a
bit (or a lot) late, then just resume at that point.

Therefore, change the code to use hrtimer_forward_now() which will
ensure that the next firing of the timer would be at N+1 (in the
picture), i.e. the next interval point after the current time.

Fixes: 01e59e467ecf ("mac80211_hwsim: hrtimer beacon")
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Reported-by: syzbot+0e964fad69a9c462bc1e@syzkaller.appspotmail.com
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

---

Note: This patch has already been applied in the wireless tree. It's just
carried here for completeness sake so that the confinement of
hrtimer_forward() in the last patch works.

v2: add fixes tag - it's kind of old and the patch won't apply,
    but even the original hrtimer code here had this problem
---
 drivers/net/wireless/mac80211_hwsim.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index ffa894f7312a..0adae76eb8df 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -1867,8 +1867,8 @@ mac80211_hwsim_beacon(struct hrtimer *timer)
 		bcn_int -= data->bcn_delta;
 		data->bcn_delta = 0;
 	}
-	hrtimer_forward(&data->beacon_timer, hrtimer_get_expires(timer),
-			ns_to_ktime(bcn_int * NSEC_PER_USEC));
+	hrtimer_forward_now(&data->beacon_timer,
+			    ns_to_ktime(bcn_int * NSEC_PER_USEC));
 	return HRTIMER_RESTART;
 }
 
-- 
2.31.1

[patch 03/11] net: iosm: Use hrtimer_forward_now()

[Thomas Gleixner]

hrtimer_forward_now() is providing the same functionality. Preparation for
making hrtimer_forward() timer core code only.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Loic Poulain <loic.poulain@linaro.org>
Cc: netdev@vger.kernel.org
Cc: Sergey Ryazanov <ryazanov.s.a@gmail.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: M Chetan Kumar <m.chetan.kumar@intel.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Intel Corporation <linuxwwan@intel.com>
---
 drivers/net/wwan/iosm/iosm_ipc_imem.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/drivers/net/wwan/iosm/iosm_ipc_imem.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem.c
@@ -482,8 +482,8 @@ static enum hrtimer_restart ipc_imem_sta
 		container_of(hr_timer, struct iosm_imem, startup_timer);
 
 	if (ktime_to_ns(ipc_imem->hrtimer_period)) {
-		hrtimer_forward(&ipc_imem->startup_timer, ktime_get(),
-				ipc_imem->hrtimer_period);
+		hrtimer_forward_now(&ipc_imem->startup_timer,
+				    ipc_imem->hrtimer_period);
 		result = HRTIMER_RESTART;
 	}
 

[patch 04/11] ALSA: pcsp: Make hrtimer forwarding more robust

[Thomas Gleixner]

The hrtimer callback pcsp_do_timer() prepares rearming of the timer with
hrtimer_forward(). hrtimer_forward() is intended to provide a mechanism to
forward the expiry time of the hrtimer by a multiple of the period argument
so that the expiry time greater than the time provided in the 'now'
argument.

pcsp_do_timer() invokes hrtimer_forward() with the current timer expiry
time as 'now' argument. That's providing a periodic timer expiry, but is
not really robust when the timer callback is delayed so that the resulting
new expiry time is already in the past which causes the callback to be
invoked immediately again. If the timer is delayed then the back to back
invocation is not really making it better than skipping the missed
periods. Sound is distorted in any case.

Use hrtimer_forward_now() which ensures that the next expiry is in the
future. This prevents hogging the CPU in the timer expiry code and allows
later on to remove hrtimer_forward() from the public interfaces.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: alsa-devel@alsa-project.org
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Jaroslav Kysela <perex@perex.cz>
---
 sound/drivers/pcsp/pcsp_lib.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/sound/drivers/pcsp/pcsp_lib.c
+++ b/sound/drivers/pcsp/pcsp_lib.c
@@ -143,7 +143,7 @@ enum hrtimer_restart pcsp_do_timer(struc
 	if (pointer_update)
 		pcsp_pointer_update(chip);
 
-	hrtimer_forward(handle, hrtimer_get_expires(handle), ns_to_ktime(ns));
+	hrtimer_forward_now(handle, ns_to_ktime(ns));
 
 	return HRTIMER_RESTART;
 }

[patch 05/11] can: bcm: Use hrtimer_forward_now()

[Thomas Gleixner]

hrtimer_forward_now() provides the same functionality as the open coded
hrimer_forward() invocation. Prepares for removal of hrtimer_forward() from
the public interfaces.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: linux-can@vger.kernel.org
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: netdev@vger.kernel.org
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
---
 net/can/bcm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -625,7 +625,7 @@ static enum hrtimer_restart bcm_rx_thr_h
 	struct bcm_op *op = container_of(hrtimer, struct bcm_op, thrtimer);
 
 	if (bcm_rx_thr_flush(op)) {
-		hrtimer_forward(hrtimer, ktime_get(), op->kt_ival2);
+		hrtimer_forward_now(hrtimer, op->kt_ival2);
 		return HRTIMER_RESTART;
 	} else {
 		/* rearm throttle handling */

[patch 06/11] power: reset: ltc2952: Use hrtimer_forward_now()

[Thomas Gleixner]

hrtimer_forward_now() provides the same functionality as the open coded
hrtimer_forward() invocation. Prepares for removal of hrtimer_forward()
from the public interfaces.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: linux-pm@vger.kernel.org
---
 drivers/power/reset/ltc2952-poweroff.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -94,7 +94,6 @@ static struct ltc2952_poweroff *ltc2952_
  */
 static enum hrtimer_restart ltc2952_poweroff_timer_wde(struct hrtimer *timer)
 {
-	ktime_t now;
 	int state;
 	struct ltc2952_poweroff *data = to_ltc2952(timer, timer_wde);
 
@@ -104,8 +103,7 @@ static enum hrtimer_restart ltc2952_powe
 	state = gpiod_get_value(data->gpio_watchdog);
 	gpiod_set_value(data->gpio_watchdog, !state);
 
-	now = hrtimer_cb_get_time(timer);
-	hrtimer_forward(timer, now, data->wde_interval);
+	hrtimer_forward_now(timer, data->wde_interval);
 
 	return HRTIMER_RESTART;
 }

[patch 07/11] drm/i915/pmu: Use hrtimer_forward_now()

[Thomas Gleixner]

hrtimer_forward() is about to be removed from the public
interfaces. Replace it with hrtimer_forward_now() which provides the same
functionality.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: David Airlie <airlied@linux.ie>
Cc: intel-gfx@lists.freedesktop.org
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -439,7 +439,7 @@ static enum hrtimer_restart i915_sample(
 	engines_sample(gt, period_ns);
 	frequency_sample(gt, period_ns);
 
-	hrtimer_forward(hrtimer, now, ns_to_ktime(PERIOD));
+	hrtimer_forward_now(hrtimer, ns_to_ktime(PERIOD));
 
 	return HRTIMER_RESTART;
 }

[patch 08/11] signal: Move itimer rearming into itimer code

[Thomas Gleixner]

Move the itimer rearming functionality into itimer.c which cleans up the
ifdeffery in the signal code.

While at it replace the open coded hrtimer_forward() invocation with
hrtimer_forward_now() which is equivalent.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
---
 include/linux/posix-timers.h |    3 +++
 kernel/signal.c              |   14 ++------------
 kernel/time/itimer.c         |   13 +++++++++++++
 3 files changed, 18 insertions(+), 12 deletions(-)

--- a/include/linux/posix-timers.h
+++ b/include/linux/posix-timers.h
@@ -174,6 +174,8 @@ static inline void posix_cputimers_rt_wa
 	.posix_cputimers = {						\
 		.bases = INIT_CPU_TIMERBASES(s.posix_cputimers.bases),	\
 	},
+
+void itimer_restart(void);
 #else
 struct posix_cputimers { };
 struct cpu_timer { };
@@ -181,6 +183,7 @@ struct cpu_timer { };
 static inline void posix_cputimers_init(struct posix_cputimers *pct) { }
 static inline void posix_cputimers_group_init(struct posix_cputimers *pct,
 					      u64 cpu_limit) { }
+static inline void itimer_restart(void) { }
 #endif
 
 #ifdef CONFIG_POSIX_CPU_TIMERS_TASK_WORK
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -651,7 +651,6 @@ int dequeue_signal(struct task_struct *t
 	if (!signr) {
 		signr = __dequeue_signal(&tsk->signal->shared_pending,
 					 mask, info, &resched_timer);
-#ifdef CONFIG_POSIX_TIMERS
 		/*
 		 * itimer signal ?
 		 *
@@ -665,17 +664,8 @@ int dequeue_signal(struct task_struct *t
 		 * reducing the timer noise on heavy loaded !highres
 		 * systems too.
 		 */
-		if (unlikely(signr == SIGALRM)) {
-			struct hrtimer *tmr = &tsk->signal->real_timer;
-
-			if (!hrtimer_is_queued(tmr) &&
-			    tsk->signal->it_real_incr != 0) {
-				hrtimer_forward(tmr, tmr->base->get_time(),
-						tsk->signal->it_real_incr);
-				hrtimer_restart(tmr);
-			}
-		}
-#endif
+		if (unlikely(signr == SIGALRM))
+			itimer_restart();
 	}
 
 	recalc_sigpending();
--- a/kernel/time/itimer.c
+++ b/kernel/time/itimer.c
@@ -150,6 +150,19 @@ COMPAT_SYSCALL_DEFINE2(getitimer, int, w
 }
 #endif
 
+void itimer_restart(void)
+{
+	struct task_struct *tsk = current;
+	struct hrtimer *tmr = &tsk->signal->real_timer;
+
+	lockdep_assert_task_sighand_held(current);
+
+	if (!hrtimer_is_queued(tmr) && tsk->signal->it_real_incr != 0) {
+		hrtimer_forward_now(tmr, tsk->signal->it_real_incr);
+		hrtimer_restart(tmr);
+	}
+}
+
 /*
  * The timer is automagically restarted, when interval != 0
  */

[patch 09/11] posix-timers: Fixup stale commnt and reduce ifdeffery

[Thomas Gleixner]

When chasing all invocations of hrtimer_forward() the comment in
posix_timer_fn() turned out to be stale and misleading.

When it was written there was an idea floating around to actually let
periodic posix timers which fail signal delivery due to SIG_IGN not rearm
the underlying hrtimer and restart the timer when SIG_IGN is lifted. In
principle a good idea, but all implementation attempts ended in creating
more problems than they solved. But the comment was left around and
suggests for 14 years now that this is going to be fixed soonish.

Rewrite it to reflect reality and replace the ifdeffery close to it with
IS_ENABLED().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/posix-timers.c |   38 +++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 23 deletions(-)

--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -387,35 +387,27 @@ static enum hrtimer_restart posix_timer_
 			ktime_t now = hrtimer_cb_get_time(timer);
 
 			/*
-			 * FIXME: What we really want, is to stop this
-			 * timer completely and restart it in case the
-			 * SIG_IGN is removed. This is a non trivial
-			 * change which involves sighand locking
-			 * (sigh !), which we don't want to do late in
-			 * the release cycle.
-			 *
-			 * For now we just let timers with an interval
-			 * less than a jiffie expire every jiffie to
-			 * avoid softirq starvation in case of SIG_IGN
-			 * and a very small interval, which would put
-			 * the timer right back on the softirq pending
-			 * list. By moving now ahead of time we trick
-			 * hrtimer_forward() to expire the timer
-			 * later, while we still maintain the overrun
-			 * accuracy, but have some inconsistency in
-			 * the timer_gettime() case. This is at least
-			 * better than a starved softirq. A more
-			 * complex fix which solves also another related
-			 * inconsistency is already in the pipeline.
+			 * What we really want, is to stop this timer
+			 * completely and restart it in case the SIG_IGN is
+			 * removed. That's not trivial. For now we just let
+			 * timers with an interval less than a jiffie
+			 * expire every jiffie to avoid softirq starvation
+			 * in case of SIG_IGN and a very small interval,
+			 * which would put the timer right back on the
+			 * softirq pending list. By moving now ahead of
+			 * time we trick hrtimer_forward() to expire the
+			 * timer later, while we still maintain the overrun
+			 * accuracy, but have some inconsistency in the
+			 * timer_gettime() case. This is at least better
+			 * than a starved softirq.
 			 */
-#ifdef CONFIG_HIGH_RES_TIMERS
-			{
+			if (IS_ENABLED(CONFIG_HIGH_RES_TIMERS)) {
 				ktime_t kj = NSEC_PER_SEC / HZ;
 
 				if (timr->it_interval < kj)
 					now = ktime_add(now, kj);
 			}
-#endif
+
 			timr->it_overrun += hrtimer_forward(timer, now,
 							    timr->it_interval);
 			ret = HRTIMER_RESTART;

[patch 10/11] posix-timers: Use hrtimer_forward_now()

[Thomas Gleixner]

Replace the open coded invocation of hrtimer_forward() with
hrtimer_forward_now() which does exactly the same.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/posix-timers.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -294,8 +294,7 @@ static void common_hrtimer_rearm(struct
 {
 	struct hrtimer *timer = &timr->it.real.timer;
 
-	timr->it_overrun += hrtimer_forward(timer, timer->base->get_time(),
-					    timr->it_interval);
+	timr->it_overrun += hrtimer_forward_now(timer, timr->it_interval);
 	hrtimer_restart(timer);
 }
 

[patch 11/11] hrtimer: Make hrtimer_forward() private to core timer code

[Thomas Gleixner]

hrtimer_forward() invites to be used in the wrong way and there is no real
requirement to have it outside of the timers core code.

All users outside of the timer core code have been converted to
hrtimer_forward_now() which provides all what the non-core code usage
needs.

Export hrtimer_forward_now() instead and move the prototype to a timer core
header.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/hrtimer.h     |   26 +-------------------------
 kernel/time/hrtimer.c       |   23 ++++++++++++++++++++++-
 kernel/time/posix-timers.c  |    1 +
 kernel/time/tick-internal.h |    1 +
 4 files changed, 25 insertions(+), 26 deletions(-)

--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -480,31 +480,7 @@ static inline int hrtimer_callback_runni
 	return timer->base->running == timer;
 }
 
-/* Forward a hrtimer so it expires after now: */
-extern u64
-hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval);
-
-/**
- * hrtimer_forward_now - forward the timer expiry so it expires after now
- * @timer:	hrtimer to forward
- * @interval:	the interval to forward
- *
- * Forward the timer expiry so it will expire after the current time
- * of the hrtimer clock base. Returns the number of overruns.
- *
- * Can be safely called from the callback function of @timer. If
- * called from other contexts @timer must neither be enqueued nor
- * running the callback and the caller needs to take care of
- * serialization.
- *
- * Note: This only updates the timer expiry value and does not requeue
- * the timer.
- */
-static inline u64 hrtimer_forward_now(struct hrtimer *timer,
-				      ktime_t interval)
-{
-	return hrtimer_forward(timer, timer->base->get_time(), interval);
-}
+extern u64 hrtimer_forward_now(struct hrtimer *timer, ktime_t interval);
 
 /* Precise sleep: */
 
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1067,7 +1067,28 @@ u64 hrtimer_forward(struct hrtimer *time
 
 	return orun;
 }
-EXPORT_SYMBOL_GPL(hrtimer_forward);
+
+/**
+ * hrtimer_forward_now - forward the timer expiry so it expires after now
+ * @timer:	hrtimer to forward
+ * @interval:	the interval to forward
+ *
+ * Forward the timer expiry so it will expire after the current time
+ * of the hrtimer clock base. Returns the number of overruns.
+ *
+ * Can be safely called from the callback function of @timer. If
+ * called from other contexts @timer must neither be enqueued nor
+ * running the callback and the caller needs to take care of
+ * serialization.
+ *
+ * Note: This only updates the timer expiry value and does not requeue
+ * the timer.
+ */
+extern u64 hrtimer_forward_now(struct hrtimer *timer, ktime_t interval)
+{
+	return hrtimer_forward(timer, timer->base->get_time(), interval);
+}
+EXPORT_SYMBOL_GPL(hrtimer_forward_now);
 
 /*
  * enqueue_hrtimer - internal function to (re)start a timer
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -33,6 +33,7 @@
 #include <linux/time_namespace.h>
 
 #include "timekeeping.h"
+#include "tick-internal.h"
 #include "posix-timers.h"
 
 /*
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -177,6 +177,7 @@ void clock_was_set(unsigned int bases);
 void clock_was_set_delayed(void);
 
 void hrtimers_resume_local(void);
+u64 hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval);
 
 /* Since jiffies uses a simple TICK_NSEC multiplier
  * conversion, the .shift value could be zero. However

Re: [patch 01/11] hrtimer: Add a mechanism to catch runaway timers

[Dmitry Vyukov]

On Thu, 23 Sept 2021 at 18:04, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> A recent report from syzbot unearthed a problem with self rearming timers
> which fire late and try to catch up to now with a short period. That causes
> the timer to be rearmed in the past until it eventually catches up with
> now. If that rearming happens from the timer callback the hard or soft
> interrupt expiry loop can run for a long time with either interrupts or
> bottom halves disabled which causes RCU stalls and other lockups.
>
> There is no safety net to stop or at least identify such runaway timers.
>
> Detection is trivial. Cache the pointer to the last expired timer. The next
> invocation from the same loop compares the pointer with the next expiring
> hrtimer pointer and if they match 10 times in a row (in the same hard or
> soft interrupt expiry instance) then it's reasonable to declare it as a
> runaway.
>
> In that case emit a warning and skip the callback invocation which stops
> the misbehaving timer right there.
>
> It's obviously incomplete, but it's definitely better than nothing and would
> have caught the reported issue in mac80211_hwsim.
>
> Suggested-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  kernel/time/hrtimer.c |   25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> --- a/kernel/time/hrtimer.c
> +++ b/kernel/time/hrtimer.c
> @@ -1714,6 +1714,13 @@ static void __run_hrtimer(struct hrtimer
>         base->running = NULL;
>  }
>
> +static void hrtimer_del_runaway(struct hrtimer_clock_base *base,
> +                               struct hrtimer *timer)
> +{
> +       __remove_hrtimer(timer, base, HRTIMER_STATE_INACTIVE, 0);
> +       pr_warn("Runaway hrtimer %p %ps stopped\n", timer, timer->function);

Thanks for implementing this, Thomas.
Please use some standard kernel bug reporting facility here, e.g. WARN
I think will be appropriate. The ad-hoc format won't be recognized by
any testing system.
Otherwise looks good to me.

> +}
> +
>  static void __hrtimer_run_queues(struct hrtimer_cpu_base *cpu_base, ktime_t now,
>                                  unsigned long flags, unsigned int active_mask)
>  {
> @@ -1722,6 +1729,8 @@ static void __hrtimer_run_queues(struct
>
>         for_each_active_base(base, cpu_base, active) {
>                 struct timerqueue_node *node;
> +               struct hrtimer *last = NULL;
> +               unsigned int cnt = 0;
>                 ktime_t basenow;
>
>                 basenow = ktime_add(now, base->offset);
> @@ -1732,6 +1741,22 @@ static void __hrtimer_run_queues(struct
>                         timer = container_of(node, struct hrtimer, node);
>
>                         /*
> +                        * Catch timers which rearm themself with a expiry
> +                        * time in the past over and over which makes this
> +                        * loop run forever.
> +                        */
> +                       if (IS_ENABLED(CONFIG_DEBUG_OBJECTS_TIMERS)) {
> +                               if (unlikely(last == timer)) {
> +                                       if (++cnt == 10) {
> +                                               hrtimer_del_runaway(base, timer);
> +                                               continue;
> +                                       }
> +                               }
> +                               last = timer;
> +                               cnt = 0;
> +                       }
> +
> +                       /*
>                          * The immediate goal for using the softexpires is
>                          * minimizing wakeups, not running timers at the
>                          * earliest interrupt after their soft expiration.
>

Re: [Intel-gfx] [patch 07/11] drm/i915/pmu: Use hrtimer_forward_now()

[Tvrtko Ursulin]

On 23/09/2021 17:04, Thomas Gleixner wrote:
> hrtimer_forward() is about to be removed from the public
> interfaces. Replace it with hrtimer_forward_now() which provides the same
> functionality.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_pmu.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -439,7 +439,7 @@ static enum hrtimer_restart i915_sample(
>   	engines_sample(gt, period_ns);
>   	frequency_sample(gt, period_ns);
>   
> -	hrtimer_forward(hrtimer, now, ns_to_ktime(PERIOD));
> +	hrtimer_forward_now(hrtimer, ns_to_ktime(PERIOD));
>   
>   	return HRTIMER_RESTART;
>   }
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

Re: [patch 06/11] power: reset: ltc2952: Use hrtimer_forward_now()

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1296 bytes --]

Hi,

On Thu, Sep 23, 2021 at 06:04:28PM +0200, Thomas Gleixner wrote:
> hrtimer_forward_now() provides the same functionality as the open coded
> hrtimer_forward() invocation. Prepares for removal of hrtimer_forward()
> from the public interfaces.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: linux-pm@vger.kernel.org
> ---

Thanks, queued to power-supply's for-next branch.

-- Sebastian

>  drivers/power/reset/ltc2952-poweroff.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> --- a/drivers/power/reset/ltc2952-poweroff.c
> +++ b/drivers/power/reset/ltc2952-poweroff.c
> @@ -94,7 +94,6 @@ static struct ltc2952_poweroff *ltc2952_
>   */
>  static enum hrtimer_restart ltc2952_poweroff_timer_wde(struct hrtimer *timer)
>  {
> -	ktime_t now;
>  	int state;
>  	struct ltc2952_poweroff *data = to_ltc2952(timer, timer_wde);
>  
> @@ -104,8 +103,7 @@ static enum hrtimer_restart ltc2952_powe
>  	state = gpiod_get_value(data->gpio_watchdog);
>  	gpiod_set_value(data->gpio_watchdog, !state);
>  
> -	now = hrtimer_cb_get_time(timer);
> -	hrtimer_forward(timer, now, data->wde_interval);
> +	hrtimer_forward_now(timer, data->wde_interval);
>  
>  	return HRTIMER_RESTART;
>  }
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [patch 04/11] ALSA: pcsp: Make hrtimer forwarding more robust

[Takashi Iwai]

On Thu, 23 Sep 2021 18:04:25 +0200,
Thomas Gleixner wrote:
> 
> The hrtimer callback pcsp_do_timer() prepares rearming of the timer with
> hrtimer_forward(). hrtimer_forward() is intended to provide a mechanism to
> forward the expiry time of the hrtimer by a multiple of the period argument
> so that the expiry time greater than the time provided in the 'now'
> argument.
> 
> pcsp_do_timer() invokes hrtimer_forward() with the current timer expiry
> time as 'now' argument. That's providing a periodic timer expiry, but is
> not really robust when the timer callback is delayed so that the resulting
> new expiry time is already in the past which causes the callback to be
> invoked immediately again. If the timer is delayed then the back to back
> invocation is not really making it better than skipping the missed
> periods. Sound is distorted in any case.
> 
> Use hrtimer_forward_now() which ensures that the next expiry is in the
> future. This prevents hogging the CPU in the timer expiry code and allows
> later on to remove hrtimer_forward() from the public interfaces.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: alsa-devel@alsa-project.org
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: Jaroslav Kysela <perex@perex.cz>

Thanks, applied now to sound git tree.


Takashi

Re: [patch 05/11] can: bcm: Use hrtimer_forward_now()

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

On 23.09.2021 18:04:27, Thomas Gleixner wrote:
> hrtimer_forward_now() provides the same functionality as the open coded
> hrimer_forward() invocation. Prepares for removal of hrtimer_forward() from
> the public interfaces.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Oliver Hartkopp <socketcan@hartkopp.net>
> Cc: linux-can@vger.kernel.org
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: netdev@vger.kernel.org
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>

Tnx, applied to linux-can-next/testing.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]