Skip to contentSkip to conversation

hrtimer: Provide softirq context hrtimers

PATCH In review Anna-Maria Gleixner 7 participants 47 messages Oct 22, 2017 – Nov 13, 2017

Subsystems (11)
Networking Network drivers USB Sound +7 more subsystems
lore
Share & tools
lore

Anna-Maria Gleixner

author maintainer
#0 lore raw

v2 cover hrtimer: Provide softirq context hrtimers

There are quite some places in the kernel which use a combination of hrtimers and tasklets to make use of the precise expiry of hrtimers, which schedule a tasklet to bring the actual function into softirq context.

This was introduced when the previous hrtimer softirq code was removed. That code was implemented by expiring the timer in hard irq context and then deferring the execution of the callback into softirq context. That caused a lot of pointless shuffling between the rbtree and a linked list.

In recent discussions it turned out that more potential users of hrtimers in softirq context might come up. Aside of that the RT patches need this functionality as well to defer hrtimers into softirq context if their callbacks are not interrupt safe on RT.

This series implements a new approach by adding SOFT hrtimer mode and instead of doing the list shuffle, timers started with this mode are put into separate soft expiry hrtimer queues. These queues are evaluated only when the hardirq context detects that the first expiring timer in the softirq queues has expired. That makes the overhead in the hardirq context minimal.

The series reworks the code to reuse as much as possible from the existing facilities for the new softirq hrtimers and integrates them with all flavours of hrtimers (HIGH_RES=y/n - NOHZ=y/n).

To achieve this quite some of the conditionals in the existing code are removed for the price of adding some pointless data and state tracking to the HIGH_RES=n case. That's minimal, but well worth it as it increases the readability and maintainability of the code.

The first part of the series implements the new functionality and the second part converts the hrtimer/tasklet users to make use of it and removes struct hrtimer_tasklet and the surrounding helper functions.

This series is available from git as well:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.timers

Thanks,

Anna-Maria

---
v1..v2:

- integration of Peter Zijlstras patch:

    https://lkml.kernel.org/r/[email protected]
  - using hrtimer_mode instead of additional hrtimer clock bases
  - folding the fix for updating the base offsets:
    https://lkml.kernel.org/r/[email protected]

- rework of 08/25 - 10/25 (all of those patches are facing reduction of conditional code) and make hrtimer_force_reprogram() unconditional as well
- integration of new versions of "ALSA/dummy: Replace tasklet with softirq hrtimer" and "net/cdc_ncm: Replace tasklet with softirq hrtimer"
- additional hrtimer/tasklet user conversion: "net/mvpp2: Replace tasklet with softirq hrtimer"
- additional fixes of several wrong comments
- update hrtimer tracing (mode and clock bases)

 drivers/net/ethernet/marvell/mvpp2.c  |   62 +--
 drivers/net/usb/cdc_ncm.c             |   38 --
 drivers/net/wireless/mac80211_hwsim.c |   44 +-
 drivers/usb/gadget/function/f_ncm.c   |   30 -
 include/linux/hrtimer.h               |  110 +++---
 include/linux/interrupt.h             |   25 -
 include/linux/usb/cdc_ncm.h           |    1 
 include/net/xfrm.h                    |    2 
 include/trace/events/timer.h          |   37 +-
 kernel/softirq.c                      |   51 --
 kernel/time/hrtimer.c                 |  623 +++++++++++++++++++++-------------
 kernel/time/timer.c                   |    6 
 net/can/bcm.c                         |  156 ++------
 net/xfrm/xfrm_state.c                 |   30 -
 sound/drivers/dummy.c                 |   27 -
 15 files changed, 652 insertions(+), 590 deletions(-)
author maintainer
#1 lore raw

PATCH v2 1/37 hrtimer: Correct blantanly wrong comment

From: Thomas Gleixner <[email protected]>

The protection of a hrtimer which runs its callback against migration to a different CPU has nothing to do with hard interrupt context.

The protection against migration of a hrtimer running the expiry callback is the pointer in the cpu_base which holds a pointer to the currently running timer. This pointer is evaluated in the code which potentially switches the timer base and makes sure it's kept on the CPU on which the callback is running.

---
 kernel/time/hrtimer.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
kernel/time/hrtimer.c +3 -3
@@ -1204,9 +1204,9 @@ static void __run_hrtimer(struct hrtimer
 		timer->is_rel = false;
 
 	/*
-	 * Because we run timers from hardirq context, there is no chance
-	 * they get migrated to another cpu, therefore its safe to unlock
-	 * the timer base.
+	 * The timer is marked as running in the cpu base, so it is
+	 * protected against migration to a different CPU even if the lock
+	 * is dropped.
 	 */
 	raw_spin_unlock(&cpu_base->lock);
 	trace_hrtimer_expire_entry(timer, now);
author maintainer
#2 lore raw

PATCH v2 2/37 hrtimer: Fix kerneldoc for struct hrtimer_cpu_base

In reply to Anna-Maria Gleixner

From: Anna-Maria Gleixner <[email protected]>

The sequence '/**' marks the start of a struct description. Add the missing second asterisk. While at it adapt the ordering of the struct members to the struct definition and document the purpose of expires_next more precisely.

---
 include/linux/hrtimer.h |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
include/linux/hrtimer.h +4 -4
@@ -144,7 +144,7 @@ enum  hrtimer_base_type {
 	HRTIMER_MAX_CLOCK_BASES,
 };
 
-/*
+/**
  * struct hrtimer_cpu_base - the per cpu clock bases
  * @lock:		lock protecting the base and associated clock bases
  *			and timers
@@ -155,12 +155,12 @@ enum  hrtimer_base_type {
  * @clock_was_set_seq:	Sequence counter of clock was set events
  * @migration_enabled:	The migration of hrtimers to other cpus is enabled
  * @nohz_active:	The nohz functionality is enabled
- * @expires_next:	absolute time of the next event which was scheduled
- *			via clock_set_next_event()
- * @next_timer:		Pointer to the first expiring timer
  * @in_hrtirq:		hrtimer_interrupt() is currently executing
  * @hres_active:	State of high resolution mode
  * @hang_detected:	The last hrtimer interrupt detected a hang
+ * @expires_next:	absolute time of the next event, is required for remote
+ *			hrtimer enqueue
+ * @next_timer:		Pointer to the first expiring timer
  * @nr_events:		Total number of hrtimer interrupt events
  * @nr_retries:		Total number of hrtimer interrupt retries
  * @nr_hangs:		Total number of hrtimer interrupt hangs
author maintainer
#3 lore raw

PATCH v2 3/37 hrtimer: Cleanup clock argument in schedule_hrtimeout_range_clock()

In reply to Anna-Maria Gleixner

From: Anna-Maria Gleixner <[email protected]>

schedule_hrtimeout_range_clock() uses an integer for the clock id instead of the predefined type "clockid_t". The ID of the clock is indicated in hrtimer code as clock_id. Therefore change the name of the variable as well to make it consistent.

While at it, clean up the description for the function parameters clock_id and mode. The clock modes and the clock ids are not restricted as the comment suggests. Fix the mode description as well for the callers of schedule_hrtimeout_range_clock().

No functional change.

---
 include/linux/hrtimer.h |    2 +-
 kernel/time/hrtimer.c   |   12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)
include/linux/hrtimer.h +1 -1
@@ -466,7 +466,7 @@ extern int schedule_hrtimeout_range(ktim
 extern int schedule_hrtimeout_range_clock(ktime_t *expires,
 					  u64 delta,
 					  const enum hrtimer_mode mode,
-					  int clock);
+					  clockid_t clock_id);
 extern int schedule_hrtimeout(ktime_t *expires, const enum hrtimer_mode mode);
 
 /* Soft interrupt function to run the hrtimer queues: */
kernel/time/hrtimer.c +6 -6
@@ -1671,12 +1671,12 @@ void __init hrtimers_init(void)
  * schedule_hrtimeout_range_clock - sleep until timeout
  * @expires:	timeout value (ktime_t)
  * @delta:	slack in expires timeout (ktime_t)
- * @mode:	timer mode, HRTIMER_MODE_ABS or HRTIMER_MODE_REL
- * @clock:	timer clock, CLOCK_MONOTONIC or CLOCK_REALTIME
+ * @mode:	timer mode
+ * @clock_id:	timer clock to be used
  */
 int __sched
 schedule_hrtimeout_range_clock(ktime_t *expires, u64 delta,
-			       const enum hrtimer_mode mode, int clock)
+			       const enum hrtimer_mode mode, clockid_t clock_id)
 {
 	struct hrtimer_sleeper t;
 
@@ -1697,7 +1697,7 @@ schedule_hrtimeout_range_clock(ktime_t *
 		return -EINTR;
 	}
 
-	hrtimer_init_on_stack(&t.timer, clock, mode);
+	hrtimer_init_on_stack(&t.timer, clock_id, mode);
 	hrtimer_set_expires_range_ns(&t.timer, *expires, delta);
 
 	hrtimer_init_sleeper(&t, current);
@@ -1719,7 +1719,7 @@ schedule_hrtimeout_range_clock(ktime_t *
  * schedule_hrtimeout_range - sleep until timeout
  * @expires:	timeout value (ktime_t)
  * @delta:	slack in expires timeout (ktime_t)
- * @mode:	timer mode, HRTIMER_MODE_ABS or HRTIMER_MODE_REL
+ * @mode:	timer mode
  *
  * Make the current task sleep until the given expiry time has
  * elapsed. The routine will return immediately unless
@@ -1758,7 +1758,7 @@ EXPORT_SYMBOL_GPL(schedule_hrtimeout_ran
 /**
  * schedule_hrtimeout - sleep until timeout
  * @expires:	timeout value (ktime_t)
- * @mode:	timer mode, HRTIMER_MODE_ABS or HRTIMER_MODE_REL
+ * @mode:	timer mode
  *
  * Make the current task sleep until the given expiry time has
  * elapsed. The routine will return immediately unless
author maintainer
#4 lore raw

PATCH v2 4/37 hrtimer: Fix hrtimer function description

In reply to Anna-Maria Gleixner

The hrtimer_start[_range_ns]() starts a timer reliable on this CPU only when HRTIMER_MODE_PINNED is set. Furthermore the HRTIMER_MODE_PINNED mode is not considered, when a hrtimer is initialized.

---
 include/linux/hrtimer.h |    6 +++---
 kernel/time/hrtimer.c   |    9 +++++----
 2 files changed, 8 insertions(+), 7 deletions(-)
include/linux/hrtimer.h +3 -3
@@ -365,11 +365,11 @@ extern void hrtimer_start_range_ns(struc
 				   u64 range_ns, const enum hrtimer_mode mode);
 
 /**
- * hrtimer_start - (re)start an hrtimer on the current CPU
+ * hrtimer_start - (re)start an hrtimer
  * @timer:	the timer to be added
  * @tim:	expiry time
- * @mode:	expiry mode: absolute (HRTIMER_MODE_ABS) or
- *		relative (HRTIMER_MODE_REL)
+ * @mode:	timer mode: absolute (HRTIMER_MODE_ABS) or
+ *		relative (HRTIMER_MODE_REL), and pinned (HRTIMER_MODE_PINNED)
  */
 static inline void hrtimer_start(struct hrtimer *timer, ktime_t tim,
 				 const enum hrtimer_mode mode)
kernel/time/hrtimer.c +5 -4
@@ -933,12 +933,12 @@ static inline ktime_t hrtimer_update_low
 }
 
 /**
- * hrtimer_start_range_ns - (re)start an hrtimer on the current CPU
+ * hrtimer_start_range_ns - (re)start an hrtimer
  * @timer:	the timer to be added
  * @tim:	expiry time
  * @delta_ns:	"slack" range for the timer
- * @mode:	expiry mode: absolute (HRTIMER_MODE_ABS) or
- *		relative (HRTIMER_MODE_REL)
+ * @mode:	timer mode: absolute (HRTIMER_MODE_ABS) or
+ *		relative (HRTIMER_MODE_REL), and pinned (HRTIMER_MODE_PINNED)
  */
 void hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 			    u64 delta_ns, const enum hrtimer_mode mode)
@@ -1116,7 +1116,8 @@ static void __hrtimer_init(struct hrtime
  * hrtimer_init - initialize a timer to the given clock
  * @timer:	the timer to be initialized
  * @clock_id:	the clock to be used
- * @mode:	timer mode abs/rel
+ * @mode:	timer mode: absolute (HRTIMER_MODE_ABS) or
+ *		relative (HRTIMER_MODE_REL); pinned is not considered here!
  */
 void hrtimer_init(struct hrtimer *timer, clockid_t clock_id,
 		  enum hrtimer_mode mode)
author maintainer
#5 lore raw

PATCH v2 5/37 hrtimer: Ensure POSIX compliance (relative CLOCK_REALTIME hrtimers)

In reply to Anna-Maria Gleixner

POSIX specification defines, that relative CLOCK_REALTIME timers are not affected by clock modifications. Those timers have to use CLOCK_MONOTONIC to ensure POSIX compliance.

The introduction of the additional mode HRTIMER_MODE_PINNED broke this requirement for pinned timers. There is no user space visible impact because user space timers are not using the pinned mode, but for consistency reasons this needs to be fixed.

Check whether the mode has the HRTIMER_MODE_REL bit set instead of comparing with HRTIMER_MODE_ABS.

Fixes 597d0275736d ("timers: Framework for identifying pinned timers")
---
 kernel/time/hrtimer.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
kernel/time/hrtimer.c +6 -1
@@ -1104,7 +1104,12 @@ static void __hrtimer_init(struct hrtime
 
 	cpu_base = raw_cpu_ptr(&hrtimer_bases);
 
-	if (clock_id == CLOCK_REALTIME && mode != HRTIMER_MODE_ABS)
+	/*
+	 * Posix magic: Relative CLOCK_REALTIME timers are not affected by
+	 * clock modifications, so they needs to become CLOCK_MONOTONIC to
+	 * ensure Posix compliance.
+	 */
+	if (clock_id == CLOCK_REALTIME && mode & HRTIMER_MODE_REL)
 		clock_id = CLOCK_MONOTONIC;
 
 	base = hrtimer_clockid_to_base(clock_id);
author maintainer
#6 lore raw

PATCH v2 6/37 hrtimer: Cleanup hrtimer_mode enum

In reply to Anna-Maria Gleixner

It's not obvious that the HRTIMER_MODE variants are bit combinations because all modes are hard coded constants.

Change it so the bit meanings are clear and use the symbols for creating modes which combine bits.

While at it get rid of the ugly tail comments.

---
 include/linux/hrtimer.h |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
include/linux/hrtimer.h +11 -5
@@ -28,13 +28,19 @@ struct hrtimer_cpu_base;
 
 /*
  * Mode arguments of xxx_hrtimer functions:
+ *
+ * HRTIMER_MODE_ABS		- Time value is absolute
+ * HRTIMER_MODE_REL		- Time value is relative to now
+ * HRTIMER_MODE_PINNED		- Timer is bound to CPU (is only considered
+ *				  when starting the timer)
  */
 enum hrtimer_mode {
-	HRTIMER_MODE_ABS = 0x0,		/* Time value is absolute */
-	HRTIMER_MODE_REL = 0x1,		/* Time value is relative to now */
-	HRTIMER_MODE_PINNED = 0x02,	/* Timer is bound to CPU */
-	HRTIMER_MODE_ABS_PINNED = 0x02,
-	HRTIMER_MODE_REL_PINNED = 0x03,
+	HRTIMER_MODE_ABS	= 0x00,
+	HRTIMER_MODE_REL	= 0x01,
+	HRTIMER_MODE_PINNED	= 0x02,
+
+	HRTIMER_MODE_ABS_PINNED = HRTIMER_MODE_ABS | HRTIMER_MODE_PINNED,
+	HRTIMER_MODE_REL_PINNED = HRTIMER_MODE_REL | HRTIMER_MODE_PINNED,
 };
 
 /*
author maintainer
#7 lore raw

PATCH v2 7/37 tracing: hrtimer: Take all clock bases and modes into account

In reply to Anna-Maria Gleixner

So far only CLOCK_MONOTONIC and CLOCK_REALTIME were taken into account as well as HRTIMER_MODE_ABS/REL in hrtimer_init tracepoint. The query for detecting timer mode ABS or REL is not valid, since the introduction of HRTIMER_MODE_PINNED.

HRTIMER_MODE_PINNED is not evaluated in hrtimer_init() call. But for the sake of completeness print all given modes.

---
 include/trace/events/timer.h |   20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
include/trace/events/timer.h +16 -4
@@ -135,6 +135,20 @@ DEFINE_EVENT(timer_class, timer_cancel,
 	TP_ARGS(timer)
 );
 
+#define decode_clockid(type)						\
+	__print_symbolic(type,						\
+		{ CLOCK_REALTIME,	"CLOCK_REALTIME"	},	\
+		{ CLOCK_MONOTONIC,	"CLOCK_MONOTONIC"	},	\
+		{ CLOCK_BOOTTIME,	"CLOCK_BOOTTIME"	},	\
+		{ CLOCK_TAI,		"CLOCK_TAI"		})
+
+#define decode_hrtimer_mode(mode)					\
+	__print_symbolic(mode,						\
+		{ HRTIMER_MODE_ABS,		"ABS"		},	\
+		{ HRTIMER_MODE_REL,		"REL"		},	\
+		{ HRTIMER_MODE_ABS_PINNED,	"ABS|PINNED"	},	\
+		{ HRTIMER_MODE_REL_PINNED,	"REL|PINNED"	})
+
 /**
  * hrtimer_init - called when the hrtimer is initialized
  * @hrtimer:	pointer to struct hrtimer
@@ -161,10 +175,8 @@ TRACE_EVENT(hrtimer_init,
 	),
 
 	TP_printk("hrtimer=%p clockid=%s mode=%s", __entry->hrtimer,
-		  __entry->clockid == CLOCK_REALTIME ?
-			"CLOCK_REALTIME" : "CLOCK_MONOTONIC",
-		  __entry->mode == HRTIMER_MODE_ABS ?
-			"HRTIMER_MODE_ABS" : "HRTIMER_MODE_REL")
+		  decode_clockid(__entry->clockid),
+		  decode_hrtimer_mode(__entry->mode))
 );
 
 /**
author maintainer
#8 lore raw

PATCH v2 8/37 tracing: hrtimer: Print hrtimer mode in hrtimer_start tracepoint

In reply to Anna-Maria Gleixner

The hrtimer_start tracepoint lacks the mode information. The mode is important because consecutive starts can switch from ABS to REL or from PINNED to non PINNED.

Add the mode information.

---
 include/trace/events/timer.h |   13 ++++++++-----
 kernel/time/hrtimer.c        |   16 +++++++++-------
 2 files changed, 17 insertions(+), 12 deletions(-)
include/trace/events/timer.h +8 -5
@@ -185,15 +185,16 @@ TRACE_EVENT(hrtimer_init,
  */
 TRACE_EVENT(hrtimer_start,
 
-	TP_PROTO(struct hrtimer *hrtimer),
+	TP_PROTO(struct hrtimer *hrtimer, enum hrtimer_mode mode),
 
-	TP_ARGS(hrtimer),
+	TP_ARGS(hrtimer, mode),
 
 	TP_STRUCT__entry(
 		__field( void *,	hrtimer		)
 		__field( void *,	function	)
 		__field( s64,		expires		)
 		__field( s64,		softexpires	)
+		__field( enum hrtimer_mode,	mode	)
 	),
 
 	TP_fast_assign(
@@ -201,12 +202,14 @@ TRACE_EVENT(hrtimer_start,
 		__entry->function	= hrtimer->function;
 		__entry->expires	= hrtimer_get_expires(hrtimer);
 		__entry->softexpires	= hrtimer_get_softexpires(hrtimer);
+		__entry->mode		= mode;
 	),
 
-	TP_printk("hrtimer=%p function=%pf expires=%llu softexpires=%llu",
-		  __entry->hrtimer, __entry->function,
+	TP_printk("hrtimer=%p function=%pf expires=%llu softexpires=%llu "
+		  "mode=%s", __entry->hrtimer, __entry->function,
 		  (unsigned long long) __entry->expires,
-		  (unsigned long long) __entry->softexpires)
+		  (unsigned long long) __entry->softexpires,
+		  decode_hrtimer_mode(__entry->mode))
 );
 
 /**
kernel/time/hrtimer.c +9 -7
@@ -442,10 +442,11 @@ debug_init(struct hrtimer *timer, clocki
 	trace_hrtimer_init(timer, clockid, mode);
 }
 
-static inline void debug_activate(struct hrtimer *timer)
+static inline void debug_activate(struct hrtimer *timer,
+				  enum hrtimer_mode mode)
 {
 	debug_hrtimer_activate(timer);
-	trace_hrtimer_start(timer);
+	trace_hrtimer_start(timer, mode);
 }
 
 static inline void debug_deactivate(struct hrtimer *timer)
@@ -837,9 +838,10 @@ EXPORT_SYMBOL_GPL(hrtimer_forward);
  * Returns 1 when the new timer is the leftmost timer in the tree.
  */
 static int enqueue_hrtimer(struct hrtimer *timer,
-			   struct hrtimer_clock_base *base)
+			   struct hrtimer_clock_base *base,
+			   enum hrtimer_mode mode)
 {
-	debug_activate(timer);
+	debug_activate(timer, mode);
 
 	base->cpu_base->active_bases |= 1 << base->index;
 
@@ -962,7 +964,7 @@ void hrtimer_start_range_ns(struct hrtim
 	/* Switch the timer base, if necessary: */
 	new_base = switch_hrtimer_base(timer, base, mode & HRTIMER_MODE_PINNED);
 
-	leftmost = enqueue_hrtimer(timer, new_base);
+	leftmost = enqueue_hrtimer(timer, new_base, mode);
 	if (!leftmost)
 		goto unlock;
 
@@ -1231,7 +1233,7 @@ static void __run_hrtimer(struct hrtimer
 	 */
 	if (restart != HRTIMER_NORESTART &&
 	    !(timer->state & HRTIMER_STATE_ENQUEUED))
-		enqueue_hrtimer(timer, base);
+		enqueue_hrtimer(timer, base, HRTIMER_MODE_ABS);
 
 	/*
 	 * Separate the ->running assignment from the ->state assignment.
@@ -1630,7 +1632,7 @@ static void migrate_hrtimer_list(struct
 		 * sort out already expired timers and reprogram the
 		 * event device.
 		 */
-		enqueue_hrtimer(timer, new_base);
+		enqueue_hrtimer(timer, new_base, HRTIMER_MODE_ABS);
 	}
 }
 
author maintainer
#9 lore raw

PATCH v2 9/37 hrtimer: Switch for loop to _ffs() evaluation

In reply to Anna-Maria Gleixner

From: Anna-Maria Gleixner <[email protected]>

Looping over all clock bases to find active bits is suboptimal if not all bases are active.

Avoid this by converting it to a __ffs() evaluation. The functionallity is outsourced into an own function and is called via a macro as suggested by Peter Zijlstra.

---
 kernel/time/hrtimer.c |   31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)
kernel/time/hrtimer.c +21 -10
@@ -464,20 +464,34 @@ static inline void hrtimer_update_next_t
 #endif
 }
 
+static struct hrtimer_clock_base *
+__next_base(struct hrtimer_cpu_base *cpu_base, unsigned int *active)
+{
+	struct hrtimer_clock_base *base = NULL;
+
+	if (*active) {
+		unsigned int idx = __ffs(*active);
+		*active &= ~(1U << idx);
+		base = &cpu_base->clock_base[idx];
+	}
+
+	return base;
+}
+
+#define for_each_active_base(base, cpu_base, active)	\
+	while ((base = __next_base((cpu_base), &(active))))
+
 static ktime_t __hrtimer_get_next_event(struct hrtimer_cpu_base *cpu_base)
 {
-	struct hrtimer_clock_base *base = cpu_base->clock_base;
+	struct hrtimer_clock_base *base;
 	unsigned int active = cpu_base->active_bases;
 	ktime_t expires, expires_next = KTIME_MAX;
 
 	hrtimer_update_next_timer(cpu_base, NULL);
-	for (; active; base++, active >>= 1) {
+	for_each_active_base(base, cpu_base, active) {
 		struct timerqueue_node *next;
 		struct hrtimer *timer;
 
-		if (!(active & 0x01))
-			continue;
-
 		next = timerqueue_getnext(&base->active);
 		timer = container_of(next, struct hrtimer, node);
 		expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
@@ -1250,16 +1264,13 @@ static void __run_hrtimer(struct hrtimer
 
 static void __hrtimer_run_queues(struct hrtimer_cpu_base *cpu_base, ktime_t now)
 {
-	struct hrtimer_clock_base *base = cpu_base->clock_base;
+	struct hrtimer_clock_base *base;
 	unsigned int active = cpu_base->active_bases;
 
-	for (; active; base++, active >>= 1) {
+	for_each_active_base(base, cpu_base, active) {
 		struct timerqueue_node *node;
 		ktime_t basenow;
 
-		if (!(active & 0x01))
-			continue;
-
 		basenow = ktime_add(now, base->offset);
 
 		while ((node = timerqueue_getnext(&base->active))) {
author maintainer
#10 lore raw

PATCH v2 10/37 hrtimer: Store running timer in hrtimer_clock_base

In reply to Anna-Maria Gleixner

From: Anna-Maria Gleixner <[email protected]>

The pointer to the currently running timer is stored in hrtimer_cpu_base before the base lock is dropped and the callback is invoked.

This results in two levels of indirections and the upcoming support for softirq based hrtimer requires splitting the "running" storage into soft and hard irq context expiry.

Storing both in the cpu base would require conditionals in all code paths accessing that information.

It's possible to have a per clock base sequence count and running pointer without changing the semantics of the related mechanisms because the timer base pointer cannot be changed while a timer is running the callback.

Unfortunately this makes cpu_clock base larger than 32 bytes on 32bit kernels. Instead of having huge gaps due to alignment, remove the alignment and let the compiler pack cpu base for 32bit. The resulting cache access patterns are fortunately not really different from the current behaviour. On 64bit kernels the 64byte alignment stays and the behaviour is unchanged. This was determined by analyzing the resulting layout and looking at the number of cache lines involved for the frequently used clocks.

---
 include/linux/hrtimer.h |   20 +++++++++-----------
 kernel/time/hrtimer.c   |   28 +++++++++++++---------------
 2 files changed, 22 insertions(+), 26 deletions(-)
include/linux/hrtimer.h +9 -11
@@ -118,9 +118,9 @@ struct hrtimer_sleeper {
 };
 
 #ifdef CONFIG_64BIT
-# define HRTIMER_CLOCK_BASE_ALIGN	64
+# define __hrtimer_clock_base_align	____cacheline_aligned
 #else
-# define HRTIMER_CLOCK_BASE_ALIGN	32
+# define __hrtimer_clock_base_align
 #endif
 
 /**
@@ -129,18 +129,22 @@ struct hrtimer_sleeper {
  * @index:		clock type index for per_cpu support when moving a
  *			timer to a base on another cpu.
  * @clockid:		clock id for per_cpu support
+ * @seq:		seqcount around __run_hrtimer
+ * @running:		pointer to the currently running hrtimer
  * @active:		red black tree root node for the active timers
  * @get_time:		function to retrieve the current time of the clock
  * @offset:		offset of this clock to the monotonic base
  */
 struct hrtimer_clock_base {
 	struct hrtimer_cpu_base	*cpu_base;
-	int			index;
+	unsigned int		index;
 	clockid_t		clockid;
+	seqcount_t		seq;
+	struct hrtimer		*running;
 	struct timerqueue_head	active;
 	ktime_t			(*get_time)(void);
 	ktime_t			offset;
-} __attribute__((__aligned__(HRTIMER_CLOCK_BASE_ALIGN)));
+} __hrtimer_clock_base_align;
 
 enum  hrtimer_base_type {
 	HRTIMER_BASE_MONOTONIC,
@@ -154,8 +158,6 @@ enum  hrtimer_base_type {
  * struct hrtimer_cpu_base - the per cpu clock bases
  * @lock:		lock protecting the base and associated clock bases
  *			and timers
- * @seq:		seqcount around __run_hrtimer
- * @running:		pointer to the currently running hrtimer
  * @cpu:		cpu number
  * @active_bases:	Bitfield to mark bases with active timers
  * @clock_was_set_seq:	Sequence counter of clock was set events
@@ -179,8 +181,6 @@ enum  hrtimer_base_type {
  */
 struct hrtimer_cpu_base {
 	raw_spinlock_t			lock;
-	seqcount_t			seq;
-	struct hrtimer			*running;
 	unsigned int			cpu;
 	unsigned int			active_bases;
 	unsigned int			clock_was_set_seq;
@@ -202,8 +202,6 @@ struct hrtimer_cpu_base {
 
 static inline void hrtimer_set_expires(struct hrtimer *timer, ktime_t time)
 {
-	BUILD_BUG_ON(sizeof(struct hrtimer_clock_base) > HRTIMER_CLOCK_BASE_ALIGN);
-
 	timer->node.expires = time;
 	timer->_softexpires = time;
 }
@@ -428,7 +426,7 @@ static inline int hrtimer_is_queued(stru
  */
 static inline int hrtimer_callback_running(struct hrtimer *timer)
 {
-	return timer->base->cpu_base->running == timer;
+	return timer->base->running == timer;
 }
 
 /* Forward a hrtimer so it expires after now: */
kernel/time/hrtimer.c +13 -15
@@ -70,7 +70,6 @@
 DEFINE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases) =
 {
 	.lock = __RAW_SPIN_LOCK_UNLOCKED(hrtimer_bases.lock),
-	.seq = SEQCNT_ZERO(hrtimer_bases.seq),
 	.clock_base =
 	{
 		{
@@ -118,7 +117,6 @@ static const int hrtimer_clock_to_base_t
  * timer->base->cpu_base
  */
 static struct hrtimer_cpu_base migration_cpu_base = {
-	.seq = SEQCNT_ZERO(migration_cpu_base),
 	.clock_base = { { .cpu_base = &migration_cpu_base, }, },
 };
 
@@ -1157,19 +1155,19 @@ EXPORT_SYMBOL_GPL(hrtimer_init);
  */
 bool hrtimer_active(const struct hrtimer *timer)
 {
-	struct hrtimer_cpu_base *cpu_base;
+	struct hrtimer_clock_base *base;
 	unsigned int seq;
 
 	do {
-		cpu_base = READ_ONCE(timer->base->cpu_base);
-		seq = raw_read_seqcount_begin(&cpu_base->seq);
+		base = READ_ONCE(timer->base);
+		seq = raw_read_seqcount_begin(&base->seq);
 
 		if (timer->state != HRTIMER_STATE_INACTIVE ||
-		    cpu_base->running == timer)
+		    base->running == timer)
 			return true;
 
-	} while (read_seqcount_retry(&cpu_base->seq, seq) ||
-		 cpu_base != READ_ONCE(timer->base->cpu_base));
+	} while (read_seqcount_retry(&base->seq, seq) ||
+		 base != READ_ONCE(timer->base));
 
 	return false;
 }
@@ -1203,16 +1201,16 @@ static void __run_hrtimer(struct hrtimer
 	lockdep_assert_held(&cpu_base->lock);
 
 	debug_deactivate(timer);
-	cpu_base->running = timer;
+	base->running = timer;
 
 	/*
 	 * Separate the ->running assignment from the ->state assignment.
 	 *
 	 * As with a regular write barrier, this ensures the read side in
-	 * hrtimer_active() cannot observe cpu_base->running == NULL &&
+	 * hrtimer_active() cannot observe base->running == NULL &&
 	 * timer->state == INACTIVE.
 	 */
-	raw_write_seqcount_barrier(&cpu_base->seq);
+	raw_write_seqcount_barrier(&base->seq);
 
 	__remove_hrtimer(timer, base, HRTIMER_STATE_INACTIVE, 0);
 	fn = timer->function;
@@ -1253,13 +1251,13 @@ static void __run_hrtimer(struct hrtimer
 	 * Separate the ->running assignment from the ->state assignment.
 	 *
 	 * As with a regular write barrier, this ensures the read side in
-	 * hrtimer_active() cannot observe cpu_base->running == NULL &&
+	 * hrtimer_active() cannot observe base->running.timer == NULL &&
 	 * timer->state == INACTIVE.
 	 */
-	raw_write_seqcount_barrier(&cpu_base->seq);
+	raw_write_seqcount_barrier(&base->seq);
 
-	WARN_ON_ONCE(cpu_base->running != timer);
-	cpu_base->running = NULL;
+	WARN_ON_ONCE(base->running != timer);
+	base->running = NULL;
 }
 
 static void __hrtimer_run_queues(struct hrtimer_cpu_base *cpu_base, ktime_t now)
author maintainer
#11 lore raw

PATCH v2 11/37 hrtimer: Change boolean struct members into bitfield

In reply to Anna-Maria Gleixner

The struct member migration_enabled and nohz_active of struct hrtimer_cpu_base are boolean. Thus, they cannot easily be packed with the subsequent bitfield when CONFIG_HIGH_RES_TIMERS is enabled.

Change the type of boolean struct members into bitfield. Adapt the function timers_update_migration() which updates those members.

No functional change.

---
 include/linux/hrtimer.h |    4 ++--
 kernel/time/timer.c     |    6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)
include/linux/hrtimer.h +2 -2
@@ -184,8 +184,8 @@ struct hrtimer_cpu_base {
 	unsigned int			cpu;
 	unsigned int			active_bases;
 	unsigned int			clock_was_set_seq;
-	bool				migration_enabled;
-	bool				nohz_active;
+	unsigned int			migration_enabled	: 1,
+					nohz_active		: 1;
 #ifdef CONFIG_HIGH_RES_TIMERS
 	unsigned int			in_hrtirq	: 1,
 					hres_active	: 1,
kernel/time/timer.c +4 -2
@@ -216,6 +216,7 @@ unsigned int sysctl_timer_migration = 1;
 void timers_update_migration(bool update_nohz)
 {
 	bool on = sysctl_timer_migration && tick_nohz_active;
+	struct hrtimer_cpu_base *base;
 	unsigned int cpu;
 
 	/* Avoid the loop, if nothing to update */
@@ -225,12 +226,13 @@ void timers_update_migration(bool update
 	for_each_possible_cpu(cpu) {
 		per_cpu(timer_bases[BASE_STD].migration_enabled, cpu) = on;
 		per_cpu(timer_bases[BASE_DEF].migration_enabled, cpu) = on;
-		per_cpu(hrtimer_bases.migration_enabled, cpu) = on;
+		base = per_cpu_ptr(&hrtimer_bases, cpu);
+		base->migration_enabled = on;
 		if (!update_nohz)
 			continue;
 		per_cpu(timer_bases[BASE_STD].nohz_active, cpu) = true;
 		per_cpu(timer_bases[BASE_DEF].nohz_active, cpu) = true;
-		per_cpu(hrtimer_bases.nohz_active, cpu) = true;
+		base->nohz_active = true;
 	}
 }
 
author maintainer
#12 lore raw

PATCH v2 12/37 hrtimer: Make room in struct hrtimer_cpu_base

In reply to Anna-Maria Gleixner

The upcoming softirq based hrtimers support requires an additional field in the hrtimer_cpu_base struct, which would grow the struct size beyond a cache line.

The struct members nr_retries and nr_hangs of hrtimer_cpu_base are solely used for diagnostic output and have no requirement to be unsigned int.

Make them unsigned short to create room for the new struct member. No functional change.

---
 include/linux/hrtimer.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/hrtimer.h +2 -2
@@ -193,8 +193,8 @@ struct hrtimer_cpu_base {
 	ktime_t				expires_next;
 	struct hrtimer			*next_timer;
 	unsigned int			nr_events;
-	unsigned int			nr_retries;
-	unsigned int			nr_hangs;
+	unsigned short			nr_retries;
+	unsigned short			nr_hangs;
 	unsigned int			max_hang_time;
 #endif
 	struct hrtimer_clock_base	clock_base[HRTIMER_MAX_CLOCK_BASES];
author maintainer
#13 lore raw

PATCH v2 13/37 hrtimer: Reduce conditional code (hres_active)

In reply to Anna-Maria Gleixner

The hrtimer_cpu_base struct has the CONFIG_HIGH_RES_TIMERS conditional struct member hres_active. All related functions to this member are conditional as well.

There is no functional change, when the hres_active member is unconditional with all related functions and is set to zero during initialization.

The conditional code sections can be avoided by adding IS_ENABLED(HIGHRES) conditionals into common functions, which ensures dead code elimination.

---
 include/linux/hrtimer.h |   20 ++++++++------------
 kernel/time/hrtimer.c   |   31 +++++++++++++++----------------
 2 files changed, 23 insertions(+), 28 deletions(-)
include/linux/hrtimer.h +8 -12
@@ -185,10 +185,10 @@ struct hrtimer_cpu_base {
 	unsigned int			active_bases;
 	unsigned int			clock_was_set_seq;
 	unsigned int			migration_enabled	: 1,
-					nohz_active		: 1;
+					nohz_active		: 1,
+					hres_active		: 1;
 #ifdef CONFIG_HIGH_RES_TIMERS
 	unsigned int			in_hrtirq	: 1,
-					hres_active	: 1,
 					hang_detected	: 1;
 	ktime_t				expires_next;
 	struct hrtimer			*next_timer;
@@ -270,16 +270,17 @@ static inline ktime_t hrtimer_cb_get_tim
 	return timer->base->get_time();
 }
 
+static inline int hrtimer_is_hres_active(struct hrtimer *timer)
+{
+	return IS_ENABLED(CONFIG_HIGH_RES_TIMERS) ?
+		timer->base->cpu_base->hres_active : 0;
+}
+
 #ifdef CONFIG_HIGH_RES_TIMERS
 struct clock_event_device;
 
 extern void hrtimer_interrupt(struct clock_event_device *dev);
 
-static inline int hrtimer_is_hres_active(struct hrtimer *timer)
-{
-	return timer->base->cpu_base->hres_active;
-}
-
 /*
  * The resolution of the clocks. The resolution value is returned in
  * the clock_getres() system call to give application programmers an
@@ -302,11 +303,6 @@ extern unsigned int hrtimer_resolution;
 
 #define hrtimer_resolution	(unsigned int)LOW_RES_NSEC
 
-static inline int hrtimer_is_hres_active(struct hrtimer *timer)
-{
-	return 0;
-}
-
 static inline void clock_was_set_delayed(void) { }
 
 #endif
kernel/time/hrtimer.c +15 -16
@@ -519,6 +519,20 @@ static inline ktime_t hrtimer_update_bas
 					    offs_real, offs_boot, offs_tai);
 }
 
+/*
+ * Is the high resolution mode active ?
+ */
+static inline int __hrtimer_hres_active(struct hrtimer_cpu_base *cpu_base)
+{
+	return IS_ENABLED(CONFIG_HIGH_RES_TIMERS) ?
+		cpu_base->hres_active : 0;
+}
+
+static inline int hrtimer_hres_active(void)
+{
+	return __hrtimer_hres_active(this_cpu_ptr(&hrtimer_bases));
+}
+
 /* High resolution timer related functions */
 #ifdef CONFIG_HIGH_RES_TIMERS
 
@@ -548,19 +562,6 @@ static inline int hrtimer_is_hres_enable
 }
 
 /*
- * Is the high resolution mode active ?
- */
-static inline int __hrtimer_hres_active(struct hrtimer_cpu_base *cpu_base)
-{
-	return cpu_base->hres_active;
-}
-
-static inline int hrtimer_hres_active(void)
-{
-	return __hrtimer_hres_active(this_cpu_ptr(&hrtimer_bases));
-}
-
-/*
  * Reprogram the event source with checking both queues for the
  * next event
  * Called with interrupts disabled and base->lock held
@@ -668,7 +669,6 @@ static void hrtimer_reprogram(struct hrt
 static inline void hrtimer_init_hres(struct hrtimer_cpu_base *base)
 {
 	base->expires_next = KTIME_MAX;
-	base->hres_active = 0;
 }
 
 /*
@@ -727,8 +727,6 @@ void clock_was_set_delayed(void)
 
 #else
 
-static inline int __hrtimer_hres_active(struct hrtimer_cpu_base *b) { return 0; }
-static inline int hrtimer_hres_active(void) { return 0; }
 static inline int hrtimer_is_hres_enabled(void) { return 0; }
 static inline void hrtimer_switch_to_hres(void) { }
 static inline void
@@ -1609,6 +1607,7 @@ int hrtimers_prepare_cpu(unsigned int cp
 	}
 
 	cpu_base->cpu = cpu;
+	cpu_base->hres_active = 0;
 	hrtimer_init_hres(cpu_base);
 	return 0;
 }
author maintainer
#14 lore raw

PATCH v2 14/37 hrtimer: Use accesor functions instead of direct access

In reply to Anna-Maria Gleixner

__hrtimer_hres_active() is now available unconditionally. Replace the direct access to hrtimer_cpu_base.hres_active.

No functional change.

---
 kernel/time/hrtimer.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/time/hrtimer.c +2 -2
@@ -571,7 +571,7 @@ hrtimer_force_reprogram(struct hrtimer_c
 {
 	ktime_t expires_next;
 
-	if (!cpu_base->hres_active)
+	if (!__hrtimer_hres_active(cpu_base))
 		return;
 
 	expires_next = __hrtimer_get_next_event(cpu_base);
@@ -680,7 +680,7 @@ static void retrigger_next_event(void *a
 {
 	struct hrtimer_cpu_base *base = this_cpu_ptr(&hrtimer_bases);
 
-	if (!base->hres_active)
+	if (!__hrtimer_hres_active(base))
 		return;
 
 	raw_spin_lock(&base->lock);
author maintainer
#15 lore raw

PATCH v2 15/37 hrtimer: Make the remote enqueue check unconditional

In reply to Anna-Maria Gleixner

hrtimer_cpu_base.expires_next is used to cache the next event armed in the timer hardware. The value is used to check whether an hrtimer can be enqueued remotely. If the new hrtimer is expiring before expires_next, then remote enqueue is not possible as the remote hrtimer hardware cannot be accessed for reprogramming to an earlier expiry time.

The remote enqueue check is currently conditional on CONFIG_HIGH_RES_TIMERS=y and hrtimer_cpu_base.hres_active. There is no compelling reason to make this conditional.

Move hrtimer_cpu_base.expires_next out of the CONFIG_HIGH_RES_TIMERS=y guarded area and remove the conditionals in hrtimer_check_target().

The check is currently a NOOP for the CONFIG_HIGH_RES_TIMERS=n and the !hrtimer_cpu_base.hres_active case because in these cases nothing updates hrtimer_cpu_base.expires_next yet. This will be changed with later patches which further reduce the #ifdef zoo in this code.

---
 include/linux/hrtimer.h |    6 +++---
 kernel/time/hrtimer.c   |   26 ++++++--------------------
 2 files changed, 9 insertions(+), 23 deletions(-)
include/linux/hrtimer.h +3 -3
@@ -166,13 +166,13 @@ enum  hrtimer_base_type {
  * @in_hrtirq:		hrtimer_interrupt() is currently executing
  * @hres_active:	State of high resolution mode
  * @hang_detected:	The last hrtimer interrupt detected a hang
- * @expires_next:	absolute time of the next event, is required for remote
- *			hrtimer enqueue
  * @next_timer:		Pointer to the first expiring timer
  * @nr_events:		Total number of hrtimer interrupt events
  * @nr_retries:		Total number of hrtimer interrupt retries
  * @nr_hangs:		Total number of hrtimer interrupt hangs
  * @max_hang_time:	Maximum time spent in hrtimer_interrupt
+ * @expires_next:	absolute time of the next event, is required for remote
+ *			hrtimer enqueue
  * @clock_base:		array of clock bases for this cpu
  *
  * Note: next_timer is just an optimization for __remove_hrtimer().
@@ -190,13 +190,13 @@ struct hrtimer_cpu_base {
 #ifdef CONFIG_HIGH_RES_TIMERS
 	unsigned int			in_hrtirq	: 1,
 					hang_detected	: 1;
-	ktime_t				expires_next;
 	struct hrtimer			*next_timer;
 	unsigned int			nr_events;
 	unsigned short			nr_retries;
 	unsigned short			nr_hangs;
 	unsigned int			max_hang_time;
 #endif
+	ktime_t				expires_next;
 	struct hrtimer_clock_base	clock_base[HRTIMER_MAX_CLOCK_BASES];
 } ____cacheline_aligned;
 
kernel/time/hrtimer.c +6 -20
@@ -154,26 +154,21 @@ struct hrtimer_clock_base *lock_hrtimer_
 }
 
 /*
- * With HIGHRES=y we do not migrate the timer when it is expiring
- * before the next event on the target cpu because we cannot reprogram
- * the target cpu hardware and we would cause it to fire late.
+ * We do not migrate the timer when it is expiring before the next
+ * event on the target cpu. When high resolution is enabled, we cannot
+ * reprogram the target cpu hardware and we would cause it to fire
+ * late. To keep it simple, we handle the high resolution enabled and
+ * disabled case similar.
  *
  * Called with cpu_base->lock of target cpu held.
  */
 static int
 hrtimer_check_target(struct hrtimer *timer, struct hrtimer_clock_base *new_base)
 {
-#ifdef CONFIG_HIGH_RES_TIMERS
 	ktime_t expires;
 
-	if (!new_base->cpu_base->hres_active)
-		return 0;
-
 	expires = ktime_sub(hrtimer_get_expires(timer), new_base->offset);
 	return expires <= new_base->cpu_base->expires_next;
-#else
-	return 0;
-#endif
 }
 
 #ifdef CONFIG_NO_HZ_COMMON
@@ -664,14 +659,6 @@ static void hrtimer_reprogram(struct hrt
 }
 
 /*
- * Initialize the high resolution related parts of cpu_base
- */
-static inline void hrtimer_init_hres(struct hrtimer_cpu_base *base)
-{
-	base->expires_next = KTIME_MAX;
-}
-
-/*
  * Retrigger next event is called after clock was set
  *
  * Called with interrupts disabled via on_each_cpu()
@@ -736,7 +723,6 @@ static inline int hrtimer_reprogram(stru
 {
 	return 0;
 }
-static inline void hrtimer_init_hres(struct hrtimer_cpu_base *base) { }
 static inline void retrigger_next_event(void *arg) { }
 
 #endif /* CONFIG_HIGH_RES_TIMERS */
@@ -1608,7 +1594,7 @@ int hrtimers_prepare_cpu(unsigned int cp
 
 	cpu_base->cpu = cpu;
 	cpu_base->hres_active = 0;
-	hrtimer_init_hres(cpu_base);
+	cpu_base->expires_next = KTIME_MAX;
 	return 0;
 }
 
author maintainer
#16 lore raw

PATCH v2 16/37 hrtimer: Make hrtimer_cpu_base.next_timer handling unconditional

In reply to Anna-Maria Gleixner

hrtimer_cpu_base.next_timer stores the pointer to the next expiring timer in a cpu base.

This pointer cannot be dereferenced and is solely used to check whether a hrtimer which is removed is the hrtimer which is the first to expire in the CPU base. If this is the case, then the timer hardware needs to be reprogrammed to avoid an extra interrupt for nothing.

Again, this is conditional functionality, but there is no compelling reason to make this conditional. As a preparation, hrtimer_cpu_base.next_timer needs to be available unconditonal. Aside of that the upcoming support for softirq based hrtimers requires access to this pointer unconditionally.

Make the update of hrtimer_cpu_base.next_timer unconditional and remove the ifdef cruft. The impact on CONFIG_HIGH_RES_TIMERS=n && CONFIG_NOHZ=n is marginal as it's just a store on an already dirtied cacheline.

No functional change.

---
 include/linux/hrtimer.h |    4 ++--
 kernel/time/hrtimer.c   |   12 ++----------
 2 files changed, 4 insertions(+), 12 deletions(-)
include/linux/hrtimer.h +2 -2
@@ -166,13 +166,13 @@ enum  hrtimer_base_type {
  * @in_hrtirq:		hrtimer_interrupt() is currently executing
  * @hres_active:	State of high resolution mode
  * @hang_detected:	The last hrtimer interrupt detected a hang
- * @next_timer:		Pointer to the first expiring timer
  * @nr_events:		Total number of hrtimer interrupt events
  * @nr_retries:		Total number of hrtimer interrupt retries
  * @nr_hangs:		Total number of hrtimer interrupt hangs
  * @max_hang_time:	Maximum time spent in hrtimer_interrupt
  * @expires_next:	absolute time of the next event, is required for remote
  *			hrtimer enqueue
+ * @next_timer:		Pointer to the first expiring timer
  * @clock_base:		array of clock bases for this cpu
  *
  * Note: next_timer is just an optimization for __remove_hrtimer().
@@ -190,13 +190,13 @@ struct hrtimer_cpu_base {
 #ifdef CONFIG_HIGH_RES_TIMERS
 	unsigned int			in_hrtirq	: 1,
 					hang_detected	: 1;
-	struct hrtimer			*next_timer;
 	unsigned int			nr_events;
 	unsigned short			nr_retries;
 	unsigned short			nr_hangs;
 	unsigned int			max_hang_time;
 #endif
 	ktime_t				expires_next;
+	struct hrtimer			*next_timer;
 	struct hrtimer_clock_base	clock_base[HRTIMER_MAX_CLOCK_BASES];
 } ____cacheline_aligned;
 
kernel/time/hrtimer.c +2 -10
@@ -449,14 +449,6 @@ static inline void debug_deactivate(stru
 }
 
 #if defined(CONFIG_NO_HZ_COMMON) || defined(CONFIG_HIGH_RES_TIMERS)
-static inline void hrtimer_update_next_timer(struct hrtimer_cpu_base *cpu_base,
-					     struct hrtimer *timer)
-{
-#ifdef CONFIG_HIGH_RES_TIMERS
-	cpu_base->next_timer = timer;
-#endif
-}
-
 static struct hrtimer_clock_base *
 __next_base(struct hrtimer_cpu_base *cpu_base, unsigned int *active)
 {
@@ -480,7 +472,7 @@ static ktime_t __hrtimer_get_next_event(
 	unsigned int active = cpu_base->active_bases;
 	ktime_t expires, expires_next = KTIME_MAX;
 
-	hrtimer_update_next_timer(cpu_base, NULL);
+	cpu_base->next_timer = NULL;
 	for_each_active_base(base, cpu_base, active) {
 		struct timerqueue_node *next;
 		struct hrtimer *timer;
@@ -490,7 +482,7 @@ static ktime_t __hrtimer_get_next_event(
 		expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
 		if (expires < expires_next) {
 			expires_next = expires;
-			hrtimer_update_next_timer(cpu_base, timer);
+			cpu_base->next_timer = timer;
 		}
 	}
 	/*
author maintainer
#17 lore raw

PATCH v2 17/37 hrtimer: Make hrtimer_reprogramm() unconditional

In reply to Anna-Maria Gleixner

hrtimer_reprogram() needs to be available unconditionally for softirq based hrtimers. Move the function and all required struct members out of the CONFIG_HIGH_RES_TIMERS #ifdef.

There is no functional change because hrtimer_reprogram() is only invoked when hrtimer_cpu_base.hres_active is true. Making it unconditional increases the text size for the CONFIG_HIGH_RES_TIMERS=n case, but avoids replication of that code for the upcoming softirq based hrtimers support.

---
 include/linux/hrtimer.h |    6 +-
 kernel/time/hrtimer.c   |  129 +++++++++++++++++++++++-------------------------
 2 files changed, 65 insertions(+), 70 deletions(-)
include/linux/hrtimer.h +3 -3
@@ -186,10 +186,10 @@ struct hrtimer_cpu_base {
 	unsigned int			clock_was_set_seq;
 	unsigned int			migration_enabled	: 1,
 					nohz_active		: 1,
-					hres_active		: 1;
+					in_hrtirq		: 1,
+					hres_active		: 1,
+					hang_detected		: 1;
 #ifdef CONFIG_HIGH_RES_TIMERS
-	unsigned int			in_hrtirq	: 1,
-					hang_detected	: 1;
 	unsigned int			nr_events;
 	unsigned short			nr_retries;
 	unsigned short			nr_hangs;
kernel/time/hrtimer.c +62 -67

146 lines not on this page · raw message

author maintainer
#18 lore raw

PATCH v2 18/37 hrtimer: Reduce conditional code and make hrtimer_force_reprogramm() unconditional

In reply to Anna-Maria Gleixner

hrtimer_force_reprogram() needs to be available unconditionally for softirq based hrtimers. Move the function and all required struct members out of the CONFIG_HIGH_RES_TIMERS #ifdef.

There is no functional change because hrtimer_force_reprogram() is only invoked when hrtimer_cpu_base.hres_active is true and CONFIG_HIGH_RES_TIMERS=y. Making it unconditional increases the text size for the CONFIG_HIGH_RES_TIMERS=n case slightly, but avoids replication of that code for the upcoming softirq based hrtimers support. Most of the code gets eliminated in the CONFIG_HIGH_RES_TIMERS=n case by the compiler.

---
 kernel/time/hrtimer.c |   58 ++++++++++++++++++++++++--------------------------
 1 file changed, 28 insertions(+), 30 deletions(-)
kernel/time/hrtimer.c +28 -30
@@ -520,34 +520,6 @@ static inline int hrtimer_hres_active(vo
 	return __hrtimer_hres_active(this_cpu_ptr(&hrtimer_bases));
 }
 
-/* High resolution timer related functions */
-#ifdef CONFIG_HIGH_RES_TIMERS
-
-/*
- * High resolution timer enabled ?
- */
-static bool hrtimer_hres_enabled __read_mostly  = true;
-unsigned int hrtimer_resolution __read_mostly = LOW_RES_NSEC;
-EXPORT_SYMBOL_GPL(hrtimer_resolution);
-
-/*
- * Enable / Disable high resolution mode
- */
-static int __init setup_hrtimer_hres(char *str)
-{
-	return (kstrtobool(str, &hrtimer_hres_enabled) == 0);
-}
-
-__setup("highres=", setup_hrtimer_hres);
-
-/*
- * hrtimer_high_res_enabled - query, if the highres mode is enabled
- */
-static inline int hrtimer_is_hres_enabled(void)
-{
-	return hrtimer_hres_enabled;
-}
-
 /*
  * Reprogram the event source with checking both queues for the
  * next event
@@ -588,6 +560,34 @@ hrtimer_force_reprogram(struct hrtimer_c
 	tick_program_event(cpu_base->expires_next, 1);
 }
 
+/* High resolution timer related functions */
+#ifdef CONFIG_HIGH_RES_TIMERS
+
+/*
+ * High resolution timer enabled ?
+ */
+static bool hrtimer_hres_enabled __read_mostly  = true;
+unsigned int hrtimer_resolution __read_mostly = LOW_RES_NSEC;
+EXPORT_SYMBOL_GPL(hrtimer_resolution);
+
+/*
+ * Enable / Disable high resolution mode
+ */
+static int __init setup_hrtimer_hres(char *str)
+{
+	return (kstrtobool(str, &hrtimer_hres_enabled) == 0);
+}
+
+__setup("highres=", setup_hrtimer_hres);
+
+/*
+ * hrtimer_high_res_enabled - query, if the highres mode is enabled
+ */
+static inline int hrtimer_is_hres_enabled(void)
+{
+	return hrtimer_hres_enabled;
+}
+
 /*
  * Retrigger next event is called after clock was set
  *
@@ -646,8 +646,6 @@ void clock_was_set_delayed(void)
 
 static inline int hrtimer_is_hres_enabled(void) { return 0; }
 static inline void hrtimer_switch_to_hres(void) { }
-static inline void
-hrtimer_force_reprogram(struct hrtimer_cpu_base *base, int skip_equal) { }
 static inline void retrigger_next_event(void *arg) { }
 
 #endif /* CONFIG_HIGH_RES_TIMERS */
author maintainer
#19 lore raw

PATCH v2 19/37 hrtimer: Unify handling of hrtimer remove

In reply to Anna-Maria Gleixner

When the first hrtimer on the current CPU is removed, hrtimer_force_reprogram() is invoked but only when CONFIG_HIGH_RES_TIMERS=y and hrtimer_cpu_base.hres_active is set.

hrtimer_force_reprogram() updates hrtimer_cpu_base.expires_next and reprograms the clock event device. When CONFIG_HIGH_RES_TIMERS=y and hrtimer_cpu_base.hres_active is set, a pointless hrtimer interrupt can be prevented.

hrtimer_check_target() makes the 'can remote enqueue' decision. As soon as hrtimer_check_target() is unconditionally available and hrtimer_cpu_base.expires_next is updated by hrtimer_reprogram(), hrtimer_force_reprogram() needs to be available unconditionally as well to prevent the following scenario with CONFIG_HIGH_RES_TIMERS=n:

- the first hrtimer on this CPU is removed and hrtimer_force_reprogram() is not executed

- CPU goes idle (next timer is calculated and hrtimers are taken into account)

- a hrtimer is enqueued remote on the idle CPU: hrtimer_check_target() compares expiry value and hrtimer_cpu_base.expires_next. The expiry value is after expires_next, so the hrtimer is enqueued. This timer will fire late, if it expires before the effective first hrtimer on this CPU and the comparison was with an outdated expires_next value.

To prevent this scenario, make hrtimer_force_reprogram() unconditional except the effective reprogramming part, which gets eliminated by the compiler in the CONFIG_HIGH_RES_TIMERS=n case.

---
 kernel/time/hrtimer.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
kernel/time/hrtimer.c +4 -6
@@ -530,9 +530,6 @@ hrtimer_force_reprogram(struct hrtimer_c
 {
 	ktime_t expires_next;
 
-	if (!__hrtimer_hres_active(cpu_base))
-		return;
-
 	expires_next = __hrtimer_get_next_event(cpu_base);
 
 	if (skip_equal && expires_next == cpu_base->expires_next)
@@ -541,6 +538,9 @@ hrtimer_force_reprogram(struct hrtimer_c
 	cpu_base->expires_next = expires_next;
 
 	/*
+	 * If hres is not active, hardware does not have to be
+	 * reprogrammed yet.
+	 *
 	 * If a hang was detected in the last timer interrupt then we
 	 * leave the hang delay active in the hardware. We want the
 	 * system to make progress. That also prevents the following
@@ -554,7 +554,7 @@ hrtimer_force_reprogram(struct hrtimer_c
 	 * set. So we'd effectivly block all timers until the T2 event
 	 * fires.
 	 */
-	if (cpu_base->hang_detected)
+	if (!__hrtimer_hres_active(cpu_base) || cpu_base->hang_detected)
 		return;
 
 	tick_program_event(cpu_base->expires_next, 1);
@@ -855,7 +855,6 @@ static void __remove_hrtimer(struct hrti
 	if (!timerqueue_del(&base->active, &timer->node))
 		cpu_base->active_bases &= ~(1 << base->index);
 
-#ifdef CONFIG_HIGH_RES_TIMERS
 	/*
 	 * Note: If reprogram is false we do not update
 	 * cpu_base->next_timer. This happens when we remove the first
@@ -866,7 +865,6 @@ static void __remove_hrtimer(struct hrti
 	 */
 	if (reprogram && timer == cpu_base->next_timer)
 		hrtimer_force_reprogram(cpu_base, 1);
-#endif
 }
 
 /*
author maintainer
#20 lore raw

PATCH v2 20/37 hrtimer: Unify handling of remote enqueue

In reply to Anna-Maria Gleixner

hrtimer_reprogram() is conditionally invoked from hrtimer_start_range_ns() when hrtimer_cpu_base.hres_active is true.

In the !hres_active case there is a special condition for the nohz_active case:

If the newly enqueued timer expires before the first expiring timer on a remote CPU then the remote CPU needs to be notified and woken up from a NOHZ idle sleep to take the new first expiring timer into account.

Previous changes have already established the prerequisites to make the remote enqueue behaviour the same whether high resolution mode is active or not:

If the to be enqueued timer expires before the first expiring timer on a remote CPU, then it cannot be enqueued there.

This was done for the high resolution mode because there is no way to access the remote CPU timer hardware. The same is true for NOHZ, but was handled differently by unconditionally enqueuing the timer and waking up the remote CPU so it can reprogram its timer. Again there is no compelling reason for this difference.

hrtimer_check_target(), which makes the 'can remote enqueue' decision is already unconditional, but not yet functional because nothing updates hrtimer_cpu_base.expires_next in the !hres_active case.

To unify this the following changes are required:

1) Make the store of the new first expiry time unconditonal in hrtimer_reprogram() and check __hrtimer_hres_active() before proceeding to the actual hardware access. This check also lets the compiler eliminate the rest of the function in case of CONFIG_HIGH_RES_TIMERS=n.

2) Invoke hrtimer_reprogram() unconditionally from hrtimer_start_range_ns()

3) Remove the remote wakeup special case for the !high_res && nohz_active case.

---
 kernel/time/hrtimer.c |   19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)
kernel/time/hrtimer.c +7 -12
@@ -694,21 +694,24 @@ static void hrtimer_reprogram(struct hrt
 
 	/* Update the pointer to the next expiring timer */
 	cpu_base->next_timer = timer;
+	cpu_base->expires_next = expires;
 
 	/*
+	 * If hres is not active, hardware does not have to be
+	 * programmed yet.
+	 *
 	 * If a hang was detected in the last timer interrupt then we
 	 * do not schedule a timer which is earlier than the expiry
 	 * which we enforced in the hang detection. We want the system
 	 * to make progress.
 	 */
-	if (cpu_base->hang_detected)
+	if (!__hrtimer_hres_active(cpu_base) || cpu_base->hang_detected)
 		return;
 
 	/*
 	 * Program the timer hardware. We enforce the expiry for
 	 * events which are already in the past.
 	 */
-	cpu_base->expires_next = expires;
 	tick_program_event(expires, 1);
 }
 
@@ -947,16 +950,8 @@ void hrtimer_start_range_ns(struct hrtim
 	if (!leftmost)
 		goto unlock;
 
-	if (!hrtimer_is_hres_active(timer)) {
-		/*
-		 * Kick to reschedule the next tick to handle the new timer
-		 * on dynticks target.
-		 */
-		if (new_base->cpu_base->nohz_active)
-			wake_up_nohz_cpu(new_base->cpu_base->cpu);
-	} else {
-		hrtimer_reprogram(timer, new_base);
-	}
+	hrtimer_reprogram(timer, new_base);
+
 unlock:
 	unlock_hrtimer_base(timer, &flags);
 }
author maintainer
#21 lore raw

PATCH v2 21/37 hrtimer: Make remote enqueue decision less restrictive

In reply to Anna-Maria Gleixner

The current decision whether a timer can be queued on a remote CPU checks for timer->expiry <= remote_cpu_base.expires_next.

This is too restrictive because a timer with the same expiry time as an existing timer will be enqueued on right-hand size of the existing timer inside the rbtree, i.e. behind the first expiring timer.

So its safe to allow enqueuing timers with the same expiry time as the first expiring timer on a remote CPU base.

---
 kernel/time/hrtimer.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/time/hrtimer.c +1 -1
@@ -168,7 +168,7 @@ hrtimer_check_target(struct hrtimer *tim
 	ktime_t expires;
 
 	expires = ktime_sub(hrtimer_get_expires(timer), new_base->offset);
-	return expires <= new_base->cpu_base->expires_next;
+	return expires < new_base->cpu_base->expires_next;
 }
 
 #ifdef CONFIG_NO_HZ_COMMON
author maintainer
#22 lore raw

PATCH v2 22/37 hrtimer: Remove base argument from hrtimer_reprogram()

In reply to Anna-Maria Gleixner

hrtimer_reprogram() must have access to the hrtimer_clock_base of the new first expiring timer to access hrtimer_clock_base.offset for adjusting the expiry time to CLOCK_MONOTONIC. This is required to evaluate whether the new left most timer in the hrtimer_clock_base is the first expiring timer of all clock bases in a hrtimer_cpu_base.

The only user of hrtimer_reprogram() is hrtimer_start_range_ns(), which has a pointer to hrtimer_clock_base already and hands it in as an argument. But hrtimer_start_range_ns() will be split for the upcoming support for softirq based hrtimers to avoid code duplication and will lose the direct access to the clock base pointer.

Instead of handing in timer and timer->base as an argument remove the base argument from hrtimer_reprogram() and retrieve the clock base internally.

---
 kernel/time/hrtimer.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
kernel/time/hrtimer.c +3 -3
@@ -657,10 +657,10 @@ static inline void retrigger_next_event(
  *
  * Called with interrupts disabled and base->cpu_base.lock held
  */
-static void hrtimer_reprogram(struct hrtimer *timer,
-			      struct hrtimer_clock_base *base)
+static void hrtimer_reprogram(struct hrtimer *timer)
 {
 	struct hrtimer_cpu_base *cpu_base = this_cpu_ptr(&hrtimer_bases);
+	struct hrtimer_clock_base *base = timer->base;
 	ktime_t expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
 
 	WARN_ON_ONCE(hrtimer_get_expires_tv64(timer) < 0);
@@ -950,7 +950,7 @@ void hrtimer_start_range_ns(struct hrtim
 	if (!leftmost)
 		goto unlock;
 
-	hrtimer_reprogram(timer, new_base);
+	hrtimer_reprogram(timer);
 
 unlock:
 	unlock_hrtimer_base(timer, &flags);
author maintainer
#23 lore raw

PATCH v2 23/37 hrtimer: Split hrtimer_start_range_ns()

In reply to Anna-Maria Gleixner

From: Anna-Maria Gleixner <[email protected]>

Preparatory patch for softirq based hrtimers to avoid code duplication. No functional change.

---
 kernel/time/hrtimer.c |   44 ++++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 20 deletions(-)
kernel/time/hrtimer.c +24 -20
@@ -916,22 +916,11 @@ static inline ktime_t hrtimer_update_low
 	return tim;
 }
 
-/**
- * hrtimer_start_range_ns - (re)start an hrtimer
- * @timer:	the timer to be added
- * @tim:	expiry time
- * @delta_ns:	"slack" range for the timer
- * @mode:	timer mode: absolute (HRTIMER_MODE_ABS) or
- *		relative (HRTIMER_MODE_REL), and pinned (HRTIMER_MODE_PINNED)
- */
-void hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
-			    u64 delta_ns, const enum hrtimer_mode mode)
+static int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
+				    u64 delta_ns, const enum hrtimer_mode mode,
+				    struct hrtimer_clock_base *base)
 {
-	struct hrtimer_clock_base *base, *new_base;
-	unsigned long flags;
-	int leftmost;
-
-	base = lock_hrtimer_base(timer, &flags);
+	struct hrtimer_clock_base *new_base;
 
 	/* Remove an active timer from the queue: */
 	remove_hrtimer(timer, base, true);
@@ -946,13 +935,28 @@ void hrtimer_start_range_ns(struct hrtim
 	/* Switch the timer base, if necessary: */
 	new_base = switch_hrtimer_base(timer, base, mode & HRTIMER_MODE_PINNED);
 
-	leftmost = enqueue_hrtimer(timer, new_base, mode);
-	if (!leftmost)
-		goto unlock;
+	return enqueue_hrtimer(timer, new_base, mode);
+}
+
+/**
+ * hrtimer_start_range_ns - (re)start an hrtimer
+ * @timer:	the timer to be added
+ * @tim:	expiry time
+ * @delta_ns:	"slack" range for the timer
+ * @mode:	timer mode: absolute (HRTIMER_MODE_ABS) or
+ *		relative (HRTIMER_MODE_REL), and pinned (HRTIMER_MODE_PINNED)
+ */
+void hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
+			    u64 delta_ns, const enum hrtimer_mode mode)
+{
+	struct hrtimer_clock_base *base;
+	unsigned long flags;
+
+	base = lock_hrtimer_base(timer, &flags);
 
-	hrtimer_reprogram(timer);
+	if (__hrtimer_start_range_ns(timer, tim, delta_ns, mode, base))
+		hrtimer_reprogram(timer);
 
-unlock:
 	unlock_hrtimer_base(timer, &flags);
 }
 EXPORT_SYMBOL_GPL(hrtimer_start_range_ns);
author maintainer
#24 lore raw

PATCH v2 24/37 hrtimer: Split __hrtimer_get_next_event()

In reply to Anna-Maria Gleixner

From: Anna-Maria Gleixner <[email protected]>

Preparatory patch for softirq based hrtimers to avoid code duplication. No functional change.

---
 kernel/time/hrtimer.c |   20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
kernel/time/hrtimer.c +16 -4
@@ -466,13 +466,13 @@ static struct hrtimer_clock_base *
 #define for_each_active_base(base, cpu_base, active)	\
 	while ((base = __next_base((cpu_base), &(active))))
 
-static ktime_t __hrtimer_get_next_event(struct hrtimer_cpu_base *cpu_base)
+static ktime_t __hrtimer_next_event_base(struct hrtimer_cpu_base *cpu_base,
+					 unsigned int active,
+					 ktime_t expires_next)
 {
 	struct hrtimer_clock_base *base;
-	unsigned int active = cpu_base->active_bases;
-	ktime_t expires, expires_next = KTIME_MAX;
+	ktime_t expires;
 
-	cpu_base->next_timer = NULL;
 	for_each_active_base(base, cpu_base, active) {
 		struct timerqueue_node *next;
 		struct hrtimer *timer;
@@ -494,6 +494,18 @@ static ktime_t __hrtimer_get_next_event(
 		expires_next = 0;
 	return expires_next;
 }
+
+static ktime_t __hrtimer_get_next_event(struct hrtimer_cpu_base *cpu_base)
+{
+	unsigned int active = cpu_base->active_bases;
+	ktime_t expires_next = KTIME_MAX;
+
+	cpu_base->next_timer = NULL;
+
+	expires_next = __hrtimer_next_event_base(cpu_base, active, expires_next);
+
+	return expires_next;
+}
 #endif
 
 static inline ktime_t hrtimer_update_base(struct hrtimer_cpu_base *base)
author maintainer
#25 lore raw

PATCH v2 25/37 hrtimer: Use irqsave/irqrestore around __run_hrtimer()

In reply to Anna-Maria Gleixner

__run_hrtimer() is called with the hrtimer_cpu_base.lock held and interrupts disabled. Before invoking the timer callback the base lock is dropped, but interrupts stay disabled.

The upcoming support for softirq based hrtimers requires that interrupts are enabled before the timer callback is invoked.

To avoid code duplication, take hrtimer_cpu_base.lock with raw_spin_lock_irqsave(flags) at the call site and hand in the flags as argument. So raw_spin_unlock_irqrestore() before the callback invocation will either keep interrupts disabled in interrupt context or restore to interrupt enabled state when called from softirq context.

---
 kernel/time/hrtimer.c |   31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)
kernel/time/hrtimer.c +18 -13
@@ -1171,7 +1171,8 @@ EXPORT_SYMBOL_GPL(hrtimer_active);
 
 static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base,
 			  struct hrtimer_clock_base *base,
-			  struct hrtimer *timer, ktime_t *now)
+			  struct hrtimer *timer, ktime_t *now,
+			  unsigned long flags)
 {
 	enum hrtimer_restart (*fn)(struct hrtimer *);
 	int restart;
@@ -1206,11 +1207,11 @@ static void __run_hrtimer(struct hrtimer
 	 * protected against migration to a different CPU even if the lock
 	 * is dropped.
 	 */
-	raw_spin_unlock(&cpu_base->lock);
+	raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
 	trace_hrtimer_expire_entry(timer, now);
 	restart = fn(timer);
 	trace_hrtimer_expire_exit(timer);
-	raw_spin_lock(&cpu_base->lock);
+	raw_spin_lock_irq(&cpu_base->lock);
 
 	/*
 	 * Note: We clear the running state after enqueue_hrtimer and
@@ -1238,7 +1239,8 @@ static void __run_hrtimer(struct hrtimer
 	base->running = NULL;
 }
 
-static void __hrtimer_run_queues(struct hrtimer_cpu_base *cpu_base, ktime_t now)
+static void __hrtimer_run_queues(struct hrtimer_cpu_base *cpu_base, ktime_t now,
+				 unsigned long flags)
 {
 	struct hrtimer_clock_base *base;
 	unsigned int active = cpu_base->active_bases;
@@ -1269,7 +1271,7 @@ static void __hrtimer_run_queues(struct
 			if (basenow < hrtimer_get_softexpires_tv64(timer))
 				break;
 
-			__run_hrtimer(cpu_base, base, timer, &basenow);
+			__run_hrtimer(cpu_base, base, timer, &basenow, flags);
 		}
 	}
 }
@@ -1284,13 +1286,14 @@ void hrtimer_interrupt(struct clock_even
 {
 	struct hrtimer_cpu_base *cpu_base = this_cpu_ptr(&hrtimer_bases);
 	ktime_t expires_next, now, entry_time, delta;
+	unsigned long flags;
 	int retries = 0;
 
 	BUG_ON(!cpu_base->hres_active);
 	cpu_base->nr_events++;
 	dev->next_event = KTIME_MAX;
 
-	raw_spin_lock(&cpu_base->lock);
+	raw_spin_lock_irqsave(&cpu_base->lock, flags);
 	entry_time = now = hrtimer_update_base(cpu_base);
 retry:
 	cpu_base->in_hrtirq = 1;
@@ -1303,7 +1306,7 @@ void hrtimer_interrupt(struct clock_even
 	 */
 	cpu_base->expires_next = KTIME_MAX;
 
-	__hrtimer_run_queues(cpu_base, now);
+	__hrtimer_run_queues(cpu_base, now, flags);
 
 	/* Reevaluate the clock bases for the next expiry */
 	expires_next = __hrtimer_get_next_event(cpu_base);
@@ -1313,7 +1316,7 @@ void hrtimer_interrupt(struct clock_even
 	 */
 	cpu_base->expires_next = expires_next;
 	cpu_base->in_hrtirq = 0;
-	raw_spin_unlock(&cpu_base->lock);
+	raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
 
 	/* Reprogramming necessary ? */
 	if (!tick_program_event(expires_next, 0)) {
@@ -1334,7 +1337,7 @@ void hrtimer_interrupt(struct clock_even
 	 * Acquire base lock for updating the offsets and retrieving
 	 * the current time.
 	 */
-	raw_spin_lock(&cpu_base->lock);
+	raw_spin_lock_irqsave(&cpu_base->lock, flags);
 	now = hrtimer_update_base(cpu_base);
 	cpu_base->nr_retries++;
 	if (++retries < 3)
@@ -1347,7 +1350,8 @@ void hrtimer_interrupt(struct clock_even
 	 */
 	cpu_base->nr_hangs++;
 	cpu_base->hang_detected = 1;
-	raw_spin_unlock(&cpu_base->lock);
+	raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
+
 	delta = ktime_sub(now, entry_time);
 	if ((unsigned int)delta > cpu_base->max_hang_time)
 		cpu_base->max_hang_time = (unsigned int) delta;
@@ -1389,6 +1393,7 @@ static inline void __hrtimer_peek_ahead_
 void hrtimer_run_queues(void)
 {
 	struct hrtimer_cpu_base *cpu_base = this_cpu_ptr(&hrtimer_bases);
+	unsigned long flags;
 	ktime_t now;
 
 	if (__hrtimer_hres_active(cpu_base))
@@ -1406,10 +1411,10 @@ void hrtimer_run_queues(void)
 		return;
 	}
 
-	raw_spin_lock(&cpu_base->lock);
+	raw_spin_lock_irqsave(&cpu_base->lock, flags);
 	now = hrtimer_update_base(cpu_base);
-	__hrtimer_run_queues(cpu_base, now);
-	raw_spin_unlock(&cpu_base->lock);
+	__hrtimer_run_queues(cpu_base, now, flags);
+	raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
 }
 
 /*
author maintainer
#26 lore raw

PATCH v2 26/37 hrtimer: Add clock bases and hrtimer mode for soft irq context

In reply to Anna-Maria Gleixner

From: Anna-Maria Gleixner <[email protected]>

hrtimer callback functions are always executed in hard interrupt context. Users of hrtimer which need their timer function to be executed in soft interrupt context, make use of tasklets to get the proper context.

Add additional hrtimer clock bases for timers which must expire in softirq context, so the detour via the tasklet can be avoided. This is also required for RT, where the majority of hrtimer is moved into softirq hrtimer context.

The selection of the expiry mode happens via a mode bit. Introduce HRTIMER_MODE_SOFT and the matching combinations with the ABS/REL/PINNED bits and update the decoding of hrtimer_mode in tracepoints.

---
 include/linux/hrtimer.h      |   14 ++++++++++++++
 include/trace/events/timer.h |    6 +++++-
 kernel/time/hrtimer.c        |   20 ++++++++++++++++++++
 3 files changed, 39 insertions(+), 1 deletion(-)
include/linux/hrtimer.h +14 -0
@@ -33,14 +33,24 @@ struct hrtimer_cpu_base;
  * HRTIMER_MODE_REL		- Time value is relative to now
  * HRTIMER_MODE_PINNED		- Timer is bound to CPU (is only considered
  *				  when starting the timer)
+ * HRTIMER_MODE_SOFT		- Timer callback function will be executed in
+ *				  soft irq context
  */
 enum hrtimer_mode {
 	HRTIMER_MODE_ABS	= 0x00,
 	HRTIMER_MODE_REL	= 0x01,
 	HRTIMER_MODE_PINNED	= 0x02,
+	HRTIMER_MODE_SOFT	= 0x04,
 
 	HRTIMER_MODE_ABS_PINNED = HRTIMER_MODE_ABS | HRTIMER_MODE_PINNED,
 	HRTIMER_MODE_REL_PINNED = HRTIMER_MODE_REL | HRTIMER_MODE_PINNED,
+
+	HRTIMER_MODE_ABS_SOFT	= HRTIMER_MODE_ABS | HRTIMER_MODE_SOFT,
+	HRTIMER_MODE_REL_SOFT	= HRTIMER_MODE_REL | HRTIMER_MODE_SOFT,
+
+	HRTIMER_MODE_ABS_PINNED_SOFT = HRTIMER_MODE_ABS_PINNED | HRTIMER_MODE_SOFT,
+	HRTIMER_MODE_REL_PINNED_SOFT = HRTIMER_MODE_REL_PINNED | HRTIMER_MODE_SOFT,
+
 };
 
 /*
@@ -151,6 +161,10 @@ enum  hrtimer_base_type {
 	HRTIMER_BASE_REALTIME,
 	HRTIMER_BASE_BOOTTIME,
 	HRTIMER_BASE_TAI,
+	HRTIMER_BASE_MONOTONIC_SOFT,
+	HRTIMER_BASE_REALTIME_SOFT,
+	HRTIMER_BASE_BOOTTIME_SOFT,
+	HRTIMER_BASE_TAI_SOFT,
 	HRTIMER_MAX_CLOCK_BASES,
 };
 
include/trace/events/timer.h +5 -1
@@ -147,7 +147,11 @@ DEFINE_EVENT(timer_class, timer_cancel,
 		{ HRTIMER_MODE_ABS,		"ABS"		},	\
 		{ HRTIMER_MODE_REL,		"REL"		},	\
 		{ HRTIMER_MODE_ABS_PINNED,	"ABS|PINNED"	},	\
-		{ HRTIMER_MODE_REL_PINNED,	"REL|PINNED"	})
+		{ HRTIMER_MODE_REL_PINNED,	"REL|PINNED"	},	\
+		{ HRTIMER_MODE_ABS_SOFT,	"ABS|SOFT"	},	\
+		{ HRTIMER_MODE_REL_SOFT,	"REL|SOFT"	},	\
+		{ HRTIMER_MODE_ABS_PINNED_SOFT,	"ABS|PINNED|SOFT" },	\
+		{ HRTIMER_MODE_REL_PINNED_SOFT,	"REL|PINNED|SOFT" })
 
 /**
  * hrtimer_init - called when the hrtimer is initialized
kernel/time/hrtimer.c +20 -0
@@ -92,6 +92,26 @@ DEFINE_PER_CPU(struct hrtimer_cpu_base,
 			.clockid = CLOCK_TAI,
 			.get_time = &ktime_get_clocktai,
 		},
+		{
+			.index = HRTIMER_BASE_MONOTONIC_SOFT,
+			.clockid = CLOCK_MONOTONIC,
+			.get_time = &ktime_get,
+		},
+		{
+			.index = HRTIMER_BASE_REALTIME_SOFT,
+			.clockid = CLOCK_REALTIME,
+			.get_time = &ktime_get_real,
+		},
+		{
+			.index = HRTIMER_BASE_BOOTTIME_SOFT,
+			.clockid = CLOCK_BOOTTIME,
+			.get_time = &ktime_get_boottime,
+		},
+		{
+			.index = HRTIMER_BASE_TAI_SOFT,
+			.clockid = CLOCK_TAI,
+			.get_time = &ktime_get_clocktai,
+		},
 	}
 };
 
author maintainer
#27 lore raw

PATCH v2 27/37 hrtimer: Prepare handling of hard and softirq based hrtimers

In reply to Anna-Maria Gleixner

From: Anna-Maria Gleixner <[email protected]>

The softirq based hrtimer can utilize most of the existing hrtimers functions, but need to operate on a different data set.

Add an active_mask argument to various functions so the hard and soft bases can be selected. Fixup the existing callers and hand in the ACTIVE_HARD mask.

---
 kernel/time/hrtimer.c |   38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)
kernel/time/hrtimer.c +29 -9
@@ -60,6 +60,15 @@
 #include "tick-internal.h"
 
 /*
+ * Masks for selecting the soft and hard context timers from
+ * cpu_base->active
+ */
+#define MASK_SHIFT		(HRTIMER_BASE_MONOTONIC_SOFT)
+#define HRTIMER_ACTIVE_HARD	((1U << MASK_SHIFT) - 1)
+#define HRTIMER_ACTIVE_SOFT	(HRTIMER_ACTIVE_HARD << MASK_SHIFT)
+#define HRTIMER_ACTIVE_ALL	(HRTIMER_ACTIVE_SOFT | HRTIMER_ACTIVE_HARD)
+
+/*
  * The timer bases:
  *
  * There are more clockids than hrtimer bases. Thus, we index
@@ -515,13 +524,24 @@ static ktime_t __hrtimer_next_event_base
 	return expires_next;
 }
 
-static ktime_t __hrtimer_get_next_event(struct hrtimer_cpu_base *cpu_base)
+/*
+ * Recomputes cpu_base::*next_timer and returns the earliest expires_next but
+ * does not set cpu_base::*expires_next, that is done by hrtimer_reprogram.
+ *
+ * @active_mask must be one of:
+ *  - HRTIMER_ACTIVE,
+ *  - HRTIMER_ACTIVE_SOFT, or
+ *  - HRTIMER_ACTIVE_HARD.
+ */
+static ktime_t __hrtimer_get_next_event(struct hrtimer_cpu_base *cpu_base,
+					unsigned int active_mask)
 {
-	unsigned int active = cpu_base->active_bases;
+	unsigned int active;
 	ktime_t expires_next = KTIME_MAX;
 
 	cpu_base->next_timer = NULL;
 
+	active = cpu_base->active_bases & active_mask;
 	expires_next = __hrtimer_next_event_base(cpu_base, active, expires_next);
 
 	return expires_next;
@@ -562,7 +582,7 @@ hrtimer_force_reprogram(struct hrtimer_c
 {
 	ktime_t expires_next;
 
-	expires_next = __hrtimer_get_next_event(cpu_base);
+	expires_next = __hrtimer_get_next_event(cpu_base, HRTIMER_ACTIVE_HARD);
 
 	if (skip_equal && expires_next == cpu_base->expires_next)
 		return;
@@ -1086,7 +1106,7 @@ u64 hrtimer_get_next_event(void)
 	raw_spin_lock_irqsave(&cpu_base->lock, flags);
 
 	if (!__hrtimer_hres_active(cpu_base))
-		expires = __hrtimer_get_next_event(cpu_base);
+		expires = __hrtimer_get_next_event(cpu_base, HRTIMER_ACTIVE_HARD);
 
 	raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
 
@@ -1260,10 +1280,10 @@ static void __run_hrtimer(struct hrtimer
 }
 
 static void __hrtimer_run_queues(struct hrtimer_cpu_base *cpu_base, ktime_t now,
-				 unsigned long flags)
+				 unsigned long flags, unsigned int active_mask)
 {
 	struct hrtimer_clock_base *base;
-	unsigned int active = cpu_base->active_bases;
+	unsigned int active = cpu_base->active_bases & active_mask;
 
 	for_each_active_base(base, cpu_base, active) {
 		struct timerqueue_node *node;
@@ -1326,10 +1346,10 @@ void hrtimer_interrupt(struct clock_even
 	 */
 	cpu_base->expires_next = KTIME_MAX;
 
-	__hrtimer_run_queues(cpu_base, now, flags);
+	__hrtimer_run_queues(cpu_base, now, flags, HRTIMER_ACTIVE_HARD);
 
 	/* Reevaluate the clock bases for the next expiry */
-	expires_next = __hrtimer_get_next_event(cpu_base);
+	expires_next = __hrtimer_get_next_event(cpu_base, HRTIMER_ACTIVE_HARD);
 	/*
 	 * Store the new expiry value so the migration code can verify
 	 * against it.
@@ -1433,7 +1453,7 @@ void hrtimer_run_queues(void)
 
 	raw_spin_lock_irqsave(&cpu_base->lock, flags);
 	now = hrtimer_update_base(cpu_base);
-	__hrtimer_run_queues(cpu_base, now, flags);
+	__hrtimer_run_queues(cpu_base, now, flags, HRTIMER_ACTIVE_HARD);
 	raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
 }
 
author maintainer
#28 lore raw

PATCH v2 28/37 hrtimer: Implement support for softirq based hrtimers

In reply to Anna-Maria Gleixner

From: Anna-Maria Gleixner <[email protected]>

hrtimer callbacks are always invoked in hard interrupt context. Several users in tree require soft interrupt context for their callbacks and achieve this by combining a hrtimer with a tasklet. The hrtimer schedules the tasklet in hard interrupt context and the tasklet callback gets invoked in softirq context later.

That's suboptimal and aside of that the real-time patch moves most of the hrtimers into softirq context. So adding native support for hrtimers expiring in softirq context is a valuable extension for both mainline and the RT patch set.

Each valid hrtimer clock id has two associated hrtimer clock bases: one for timers expiring in hardirq context and one for timers expiring in softirq context.

Implement the functionality to associate a hrtimer with the hard or softirq related clock bases and update the relevant functions to take them into account when the next expiry time needs to be evaluated.

Add a check into the hard interrupt context handler functions to check whether the first expiring softirq based timer has expired. If it's expired the softirq is raised and the accounting of softirq based timers to evaluate the next expiry time for programming the timer hardware is skipped until the softirq processing has finished. At the end of the softirq processing the regular processing is resumed.

---
 include/linux/hrtimer.h |   14 +++
 kernel/time/hrtimer.c   |  191 ++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 174 insertions(+), 31 deletions(-)
include/linux/hrtimer.h +12 -2
@@ -113,6 +113,7 @@ struct hrtimer {
 	struct hrtimer_clock_base	*base;
 	u8				state;
 	u8				is_rel;
+	u8				is_soft;
 };
 
 /**
@@ -177,6 +178,8 @@ enum  hrtimer_base_type {
  * @clock_was_set_seq:	Sequence counter of clock was set events
  * @migration_enabled:	The migration of hrtimers to other cpus is enabled
  * @nohz_active:	The nohz functionality is enabled
+ * @softirq_activated:	displays, if the softirq is raised - update of softirq
+ *			related settings is not required then.
  * @in_hrtirq:		hrtimer_interrupt() is currently executing
  * @hres_active:	State of high resolution mode
  * @hang_detected:	The last hrtimer interrupt detected a hang
@@ -185,8 +188,11 @@ enum  hrtimer_base_type {
  * @nr_hangs:		Total number of hrtimer interrupt hangs
  * @max_hang_time:	Maximum time spent in hrtimer_interrupt
  * @expires_next:	absolute time of the next event, is required for remote
- *			hrtimer enqueue
+ *			hrtimer enqueue; it is the total first expiry time (hard
+ *			and soft hrtimer are taken into account)
  * @next_timer:		Pointer to the first expiring timer
+ * @softirq_expires_next: Time to check, if soft queues needs also to be expired
+ * @softirq_next_timer: Pointer to the first expiring softirq based timer
  * @clock_base:		array of clock bases for this cpu
  *
  * Note: next_timer is just an optimization for __remove_hrtimer().
@@ -200,6 +206,7 @@ struct hrtimer_cpu_base {
 	unsigned int			clock_was_set_seq;
 	unsigned int			migration_enabled	: 1,
 					nohz_active		: 1,
+					softirq_activated	: 1,
 					in_hrtirq		: 1,
 					hres_active		: 1,
 					hang_detected		: 1;
@@ -211,6 +218,8 @@ struct hrtimer_cpu_base {
 #endif
 	ktime_t				expires_next;
 	struct hrtimer			*next_timer;
+	ktime_t				softirq_expires_next;
+	struct hrtimer			*softirq_next_timer;
 	struct hrtimer_clock_base	clock_base[HRTIMER_MAX_CLOCK_BASES];
 } ____cacheline_aligned;
 
@@ -383,7 +392,8 @@ extern void hrtimer_start_range_ns(struc
  * @timer:	the timer to be added
  * @tim:	expiry time
  * @mode:	timer mode: absolute (HRTIMER_MODE_ABS) or
- *		relative (HRTIMER_MODE_REL), and pinned (HRTIMER_MODE_PINNED)
+ *		relative (HRTIMER_MODE_REL), and pinned (HRTIMER_MODE_PINNED);
+ *		softirq based mode is considered for debug purpose only!
  */
 static inline void hrtimer_start(struct hrtimer *timer, ktime_t tim,
 				 const enum hrtimer_mode mode)
kernel/time/hrtimer.c +162 -29

356 lines not on this page · raw message

#29 lore raw

Re: [PATCH v2 28/37] hrtimer: Implement support for softirq based hrtimers

Quoted 23 lines · Anna-Maria Gleixner
include/linux/hrtimer.h +10 -2
@@ -528,25 +546,42 @@ static ktime_t __hrtimer_next_event_base
  * Recomputes cpu_base::*next_timer and returns the earliest expires_next but
  * does not set cpu_base::*expires_next, that is done by hrtimer_reprogram.
  *
+ * When a softirq is pending, we can ignore the HRTIMER_ACTIVE_SOFT bases,
+ * those timers will get run whenever the softirq gets handled, at the end of
+ * hrtimer_run_softirq(), hrtimer_update_softirq_timer() will re-add these bases.
+ *
+ * Therefore softirq values are those from the HRTIMER_ACTIVE_SOFT clock bases.
+ * The !softirq values are the minima across HRTIMER_ACTIVE, unless an actual
+ * softirq is pending, in which case they're the minima of HRTIMER_ACTIVE_HARD.
+ *
  * @active_mask must be one of:
  *  - HRTIMER_ACTIVE,
  *  - HRTIMER_ACTIVE_SOFT, or
  *  - HRTIMER_ACTIVE_HARD.
  */
-static ktime_t __hrtimer_get_next_event(struct hrtimer_cpu_base *cpu_base,
-					unsigned int active_mask)
+static ktime_t
+__hrtimer_get_next_event(struct hrtimer_cpu_base *cpu_base, unsigned int active_mask)

include/linux/hrtimer.h static ktime_t __hrtimer_next_event_base · line 572

Quoted 10 lines
include/linux/hrtimer.h +5 -1
 {
 	unsigned int active;
+	struct hrtimer *next_timer = NULL;
 	ktime_t expires_next = KTIME_MAX;
 
-	cpu_base->next_timer = NULL;
+	if (!cpu_base->softirq_activated && (active_mask & HRTIMER_ACTIVE_SOFT)) {
+		active = cpu_base->active_bases & HRTIMER_ACTIVE_SOFT;
+		cpu_base->softirq_next_timer = next_timer;
+		expires_next = __hrtimer_next_event_base(cpu_base, active, expires_next);

doing

               cpu_base->softirq_next_timer = NULL;
               expires_next = __hrtimer_next_event_base(cpu_base, active, KTIME_MAX);

instead would make it more obvious I think. I wasn't sure if it is a typo and the timer assignment was meant to be after __hrtimer_next_event_base() was invoked or if NULL was indeed intended.

Quoted 21 lines
include/linux/hrtimer.h +8 -3
+
+		next_timer = cpu_base->softirq_next_timer;
+	}
 
-	active = cpu_base->active_bases & active_mask;
-	expires_next = __hrtimer_next_event_base(cpu_base, active, expires_next);
+	if (active_mask & HRTIMER_ACTIVE_HARD) {
+		active = cpu_base->active_bases & HRTIMER_ACTIVE_HARD;
+		cpu_base->next_timer = next_timer;
+		expires_next = __hrtimer_next_event_base(cpu_base, active, expires_next);
+	}
 
 	return expires_next;
 }
-#endif
 
 static inline ktime_t hrtimer_update_base(struct hrtimer_cpu_base *base)
 {
@@ -968,6 +1034,32 @@ static inline ktime_t hrtimer_update_low
 	return tim;
 }

include/linux/hrtimer.h static inline ktime_t hrtimer_update_low · line 1045

Quoted 10 lines
include/linux/hrtimer.h +9 -0
 
+static void
+hrtimer_update_softirq_timer(struct hrtimer_cpu_base *cpu_base, bool reprogram)
+{
+	ktime_t expires;
+
+	/*
+	 * Find the next SOFT expiration.
+	 */
+	expires = __hrtimer_get_next_event(cpu_base, HRTIMER_ACTIVE_SOFT);

If you replace the following block

include/linux/hrtimer.h static inline ktime_t hrtimer_update_low · line 1054

Quoted 9 lines
include/linux/hrtimer.h +9 -0
+
+	/*
+	 * reprogramming needs to be triggered, even if the next soft
+	 * hrtimer expires at the same time than the next hard
+	 * hrtimer. cpu_base->softirq_expires_next needs to be updated!
+	 */
+	if (!reprogram || expires == KTIME_MAX ||
+	    ktime_before(expires, cpu_base->expires_next))
+		return;

with

       if (expires == KTIME_MAX)
               return;
       if (!reprogram || !ktime_before(expires, cpu_base->expires_next)) {

               /*
                * ->softirq_next_timer was updated by __hrtimer_next_event_base()
                * and we need to make sure that ->softirq_expires_next matches.
                */
               cpu_base->softirq_expires_next = expires;
               return;
       }

then you have two bug less I *think*.

If *expires* is before ->expires_next then you don't want to return and do nothing but instead you want to reprogram timer for the soft-timer event.

And then even if *expires* is after ->expires_next then you need to ->softirq_expires_next. As the comment says, the next timer field has been already updated. At this point, ->softirq_expires_next is set to KTIME_MAX (due to the raise softirq part) so if this field is not udpated here, then the hr-irq won't see the pending timer and expire it. Even worse, if future soft-timer have a "later" expiry time then this timer now then this field won't be updated at all and all soft-timer processing will stall.

include/linux/hrtimer.h static inline ktime_t hrtimer_update_low · line 1062

Quoted 8 lines
include/linux/hrtimer.h +8 -0
+
+	/*
+	 * cpu_base->*next_timer is recomputed by __hrtimer_get_next_event()
+	 * cpu_base->*expires_next is only set by hrtimer_reprogram()
+	 */
+	hrtimer_reprogram(cpu_base->softirq_next_timer);
+}
+

Sebastian

author maintainer
#30 lore raw

Re: [PATCH v2 28/37] hrtimer: Implement support for softirq based hrtimers

Quoted 37 lines · Sebastian Andrzej Siewior
Quoted 23 lines · Anna-Maria Gleixner
include/linux/hrtimer.h +10 -2
@@ -528,25 +546,42 @@ static ktime_t __hrtimer_next_event_base
  * Recomputes cpu_base::*next_timer and returns the earliest expires_next but
  * does not set cpu_base::*expires_next, that is done by hrtimer_reprogram.
  *
+ * When a softirq is pending, we can ignore the HRTIMER_ACTIVE_SOFT bases,
+ * those timers will get run whenever the softirq gets handled, at the end of
+ * hrtimer_run_softirq(), hrtimer_update_softirq_timer() will re-add these bases.
+ *
+ * Therefore softirq values are those from the HRTIMER_ACTIVE_SOFT clock bases.
+ * The !softirq values are the minima across HRTIMER_ACTIVE, unless an actual
+ * softirq is pending, in which case they're the minima of HRTIMER_ACTIVE_HARD.
+ *
  * @active_mask must be one of:
  *  - HRTIMER_ACTIVE,
  *  - HRTIMER_ACTIVE_SOFT, or
  *  - HRTIMER_ACTIVE_HARD.
  */
-static ktime_t __hrtimer_get_next_event(struct hrtimer_cpu_base *cpu_base,
-					unsigned int active_mask)
+static ktime_t
+__hrtimer_get_next_event(struct hrtimer_cpu_base *cpu_base, unsigned int active_mask)

include/linux/hrtimer.h static ktime_t __hrtimer_next_event_base · line 572

Quoted 10 lines
include/linux/hrtimer.h +5 -1
 {
 	unsigned int active;
+	struct hrtimer *next_timer = NULL;
 	ktime_t expires_next = KTIME_MAX;
 
-	cpu_base->next_timer = NULL;
+	if (!cpu_base->softirq_activated && (active_mask & HRTIMER_ACTIVE_SOFT)) {
+		active = cpu_base->active_bases & HRTIMER_ACTIVE_SOFT;
+		cpu_base->softirq_next_timer = next_timer;
+		expires_next = __hrtimer_next_event_base(cpu_base, active, expires_next);

doing

Quoted 6 lines
               cpu_base->softirq_next_timer = NULL;
               expires_next = __hrtimer_next_event_base(cpu_base, active, KTIME_MAX);

instead would make it more obvious I think. I wasn't sure if it is a typo and the timer assignment was meant to be after __hrtimer_next_event_base() was invoked or if NULL was indeed intended.

will change it

Quoted 63 lines
Quoted 21 lines
include/linux/hrtimer.h +8 -3
+
+		next_timer = cpu_base->softirq_next_timer;
+	}
 
-	active = cpu_base->active_bases & active_mask;
-	expires_next = __hrtimer_next_event_base(cpu_base, active, expires_next);
+	if (active_mask & HRTIMER_ACTIVE_HARD) {
+		active = cpu_base->active_bases & HRTIMER_ACTIVE_HARD;
+		cpu_base->next_timer = next_timer;
+		expires_next = __hrtimer_next_event_base(cpu_base, active, expires_next);
+	}
 
 	return expires_next;
 }
-#endif
 
 static inline ktime_t hrtimer_update_base(struct hrtimer_cpu_base *base)
 {
@@ -968,6 +1034,32 @@ static inline ktime_t hrtimer_update_low
 	return tim;
 }

include/linux/hrtimer.h static inline ktime_t hrtimer_update_low · line 1045

Quoted 10 lines
include/linux/hrtimer.h +9 -0
 
+static void
+hrtimer_update_softirq_timer(struct hrtimer_cpu_base *cpu_base, bool reprogram)
+{
+	ktime_t expires;
+
+	/*
+	 * Find the next SOFT expiration.
+	 */
+	expires = __hrtimer_get_next_event(cpu_base, HRTIMER_ACTIVE_SOFT);

If you replace the following block

include/linux/hrtimer.h static inline ktime_t hrtimer_update_low · line 1054

Quoted 9 lines
include/linux/hrtimer.h +9 -0
+
+	/*
+	 * reprogramming needs to be triggered, even if the next soft
+	 * hrtimer expires at the same time than the next hard
+	 * hrtimer. cpu_base->softirq_expires_next needs to be updated!
+	 */
+	if (!reprogram || expires == KTIME_MAX ||
+	    ktime_before(expires, cpu_base->expires_next))
+		return;

with

       if (expires == KTIME_MAX)
               return;
       if (!reprogram || !ktime_before(expires, cpu_base->expires_next)) {

               /*
                * ->softirq_next_timer was updated by __hrtimer_next_event_base()
                * and we need to make sure that ->softirq_expires_next matches.
                */
               cpu_base->softirq_expires_next = expires;
               return;
       }

then you have two bug less I *think*.

If *expires* is before ->expires_next then you don't want to return and do nothing but instead you want to reprogram timer for the soft-timer event.

Quoted 9 lines

And then even if *expires* is after ->expires_next then you need to ->softirq_expires_next. As the comment says, the next timer field has been already updated. At this point, ->softirq_expires_next is set to KTIME_MAX (due to the raise softirq part) so if this field is not udpated here, then the hr-irq won't see the pending timer and expire it. Even worse, if future soft-timer have a "later" expiry time then this timer now then this field won't be updated at all and all soft-timer processing will stall.

I would propse another solution for this:

	if (expires == KTIME_MAX)
		return;

	hrtimer_reprogram(cpu_base->softirq_next_timer, reprogram);

Updating of cpu_base::*expires_next values is done in hrtimer_reprogram() only. I think this should not change. When expires equals KTIME_MAX, there is no soft hrtimer queued and updating of softirq_expires_next is not required. The hrtimer_reprogram() function gets the reprogram parameter handed in. If this parameter is set, the hrtimer_reprogram() returns after setting the softirq_expires_next value. In the migration case the hardware is not reprogrammed. If the first soft hrtimer expires after the first hard hrtimer, then hrtimer_reprogram() returns without reprogramming anyway.

Quoted 11 lines

include/linux/hrtimer.h static inline ktime_t hrtimer_update_low · line 1062

Quoted 8 lines
include/linux/hrtimer.h +8 -0
+
+	/*
+	 * cpu_base->*next_timer is recomputed by __hrtimer_get_next_event()
+	 * cpu_base->*expires_next is only set by hrtimer_reprogram()
+	 */
+	hrtimer_reprogram(cpu_base->softirq_next_timer);
+}
+

Sebastian

Anna-Maria

author maintainer
#31 lore raw

PATCH v2 29/37 hrtimer: Implement SOFT/HARD clock base selection

In reply to Anna-Maria Gleixner

From: Anna-Maria Gleixner <[email protected]>

All prerequisites to handle hrtimers for expiry in either hard or soft interrupt context are in place.

Add the missing bit in hrtimer_init() which associates the timer to the hard or the soft irq clock base.

---
 kernel/time/hrtimer.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
kernel/time/hrtimer.c +11 -4
@@ -1222,8 +1222,9 @@ static inline int hrtimer_clockid_to_bas
 static void __hrtimer_init(struct hrtimer *timer, clockid_t clock_id,
 			   enum hrtimer_mode mode)
 {
+	bool softtimer = !!(mode & HRTIMER_MODE_SOFT);
+	int base = softtimer ? HRTIMER_MAX_CLOCK_BASES / 2 : 0;
 	struct hrtimer_cpu_base *cpu_base;
-	int base;
 
 	memset(timer, 0, sizeof(struct hrtimer));
 
@@ -1237,7 +1238,8 @@ static void __hrtimer_init(struct hrtime
 	if (clock_id == CLOCK_REALTIME && mode & HRTIMER_MODE_REL)
 		clock_id = CLOCK_MONOTONIC;
 
-	base = hrtimer_clockid_to_base(clock_id);
+	base += hrtimer_clockid_to_base(clock_id);
+	timer->is_soft = softtimer;
 	timer->base = &cpu_base->clock_base[base];
 	timerqueue_init(&timer->node);
 }
@@ -1246,8 +1248,13 @@ static void __hrtimer_init(struct hrtime
  * hrtimer_init - initialize a timer to the given clock
  * @timer:	the timer to be initialized
  * @clock_id:	the clock to be used
- * @mode:	timer mode: absolute (HRTIMER_MODE_ABS) or
- *		relative (HRTIMER_MODE_REL); pinned is not considered here!
+ * @mode:       The modes which are relevant for intitialization:
+ *              HRTIMER_MODE_ABS, HRTIMER_MODE_REL, HRTIMER_MODE_ABS_SOFT,
+ *              HRTIMER_MODE_REL_SOFT
+ *
+ *              The PINNED variants of the above can be handed in,
+ *              but the PINNED bit is ignored as pinning happens
+ *              when the hrtimer is started
  */
 void hrtimer_init(struct hrtimer *timer, clockid_t clock_id,
 		  enum hrtimer_mode mode)
author maintainer
#32 lore raw

PATCH v2 30/37 can/bcm: Replace hrtimer_tasklet with softirq based hrtimer

In reply to Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

Switch the timer to HRTIMER_MODE_SOFT, which executed the timer callback in softirq context and remove the hrtimer_tasklet.

---
 net/can/bcm.c |  156 +++++++++++++++++++---------------------------------------
 1 file changed, 52 insertions(+), 104 deletions(-)
net/can/bcm.c +52 -104
@@ -102,7 +102,6 @@ struct bcm_op {
 	unsigned long frames_abs, frames_filtered;
 	struct bcm_timeval ival1, ival2;
 	struct hrtimer timer, thrtimer;
-	struct tasklet_struct tsklet, thrtsklet;
 	ktime_t rx_stamp, kt_ival1, kt_ival2, kt_lastmsg;
 	int rx_ifindex;
 	int cfsiz;
@@ -364,25 +363,34 @@ static void bcm_send_to_user(struct bcm_
 	}
 }
 
-static void bcm_tx_start_timer(struct bcm_op *op)
+static bool bcm_tx_set_expiry(struct bcm_op *op, struct hrtimer *hrt)
 {
+	ktime_t ival;
+
 	if (op->kt_ival1 && op->count)
-		hrtimer_start(&op->timer,
-			      ktime_add(ktime_get(), op->kt_ival1),
-			      HRTIMER_MODE_ABS);
+		ival = op->kt_ival1;
 	else if (op->kt_ival2)
-		hrtimer_start(&op->timer,
-			      ktime_add(ktime_get(), op->kt_ival2),
-			      HRTIMER_MODE_ABS);
+		ival = op->kt_ival2;
+	else
+		return false;
+
+	hrtimer_set_expires(hrt, ktime_add(ktime_get(), ival));
+	return true;
 }
 
-static void bcm_tx_timeout_tsklet(unsigned long data)
+static void bcm_tx_start_timer(struct bcm_op *op)
 {
-	struct bcm_op *op = (struct bcm_op *)data;
+	if (bcm_tx_set_expiry(op, &op->timer))
+		hrtimer_start_expires(&op->timer, HRTIMER_MODE_ABS_SOFT);
+}
+
+/* bcm_tx_timeout_handler - performs cyclic CAN frame transmissions */
+static enum hrtimer_restart bcm_tx_timeout_handler(struct hrtimer *hrtimer)
+{
+	struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
 	struct bcm_msg_head msg_head;
 
 	if (op->kt_ival1 && (op->count > 0)) {
-
 		op->count--;
 		if (!op->count && (op->flags & TX_COUNTEVT)) {
 
@@ -399,22 +407,12 @@ static void bcm_tx_timeout_tsklet(unsign
 		}
 		bcm_can_tx(op);
 
-	} else if (op->kt_ival2)
+	} else if (op->kt_ival2) {
 		bcm_can_tx(op);
+	}
 
-	bcm_tx_start_timer(op);
-}
-
-/*
- * bcm_tx_timeout_handler - performs cyclic CAN frame transmissions
- */
-static enum hrtimer_restart bcm_tx_timeout_handler(struct hrtimer *hrtimer)
-{
-	struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
-
-	tasklet_schedule(&op->tsklet);
-
-	return HRTIMER_NORESTART;
+	return bcm_tx_set_expiry(op, &op->timer) ?
+		HRTIMER_RESTART : HRTIMER_NORESTART;
 }
 
 /*
@@ -480,7 +478,7 @@ static void bcm_rx_update_and_send(struc
 		/* do not send the saved data - only start throttle timer */
 		hrtimer_start(&op->thrtimer,
 			      ktime_add(op->kt_lastmsg, op->kt_ival2),
-			      HRTIMER_MODE_ABS);
+			      HRTIMER_MODE_ABS_SOFT);
 		return;
 	}
 
@@ -539,14 +537,21 @@ static void bcm_rx_starttimer(struct bcm
 		return;
 
 	if (op->kt_ival1)
-		hrtimer_start(&op->timer, op->kt_ival1, HRTIMER_MODE_REL);
+		hrtimer_start(&op->timer, op->kt_ival1, HRTIMER_MODE_REL_SOFT);
 }
 
-static void bcm_rx_timeout_tsklet(unsigned long data)
+/* bcm_rx_timeout_handler - when the (cyclic) CAN frame reception timed out */
+static enum hrtimer_restart bcm_rx_timeout_handler(struct hrtimer *hrtimer)
 {
-	struct bcm_op *op = (struct bcm_op *)data;
+	struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
 	struct bcm_msg_head msg_head;
 
+	/* if user wants to be informed, when cyclic CAN-Messages come back */
+	if ((op->flags & RX_ANNOUNCE_RESUME) && op->last_frames) {
+		/* clear received CAN frames to indicate 'nothing received' */
+		memset(op->last_frames, 0, op->nframes * op->cfsiz);
+	}
+
 	/* create notification to user */
 	msg_head.opcode  = RX_TIMEOUT;
 	msg_head.flags   = op->flags;
@@ -557,25 +562,6 @@ static void bcm_rx_timeout_tsklet(unsign
 	msg_head.nframes = 0;
 
 	bcm_send_to_user(op, &msg_head, NULL, 0);
-}
-
-/*
- * bcm_rx_timeout_handler - when the (cyclic) CAN frame reception timed out
- */
-static enum hrtimer_restart bcm_rx_timeout_handler(struct hrtimer *hrtimer)
-{
-	struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
-
-	/* schedule before NET_RX_SOFTIRQ */
-	tasklet_hi_schedule(&op->tsklet);
-
-	/* no restart of the timer is done here! */
-
-	/* if user wants to be informed, when cyclic CAN-Messages come back */
-	if ((op->flags & RX_ANNOUNCE_RESUME) && op->last_frames) {
-		/* clear received CAN frames to indicate 'nothing received' */
-		memset(op->last_frames, 0, op->nframes * op->cfsiz);
-	}
 
 	return HRTIMER_NORESTART;
 }
@@ -583,14 +569,12 @@ static enum hrtimer_restart bcm_rx_timeo
 /*
  * bcm_rx_do_flush - helper for bcm_rx_thr_flush
  */
-static inline int bcm_rx_do_flush(struct bcm_op *op, int update,
-				  unsigned int index)
+static inline int bcm_rx_do_flush(struct bcm_op *op, unsigned int index)
 {
 	struct canfd_frame *lcf = op->last_frames + op->cfsiz * index;
 
 	if ((op->last_frames) && (lcf->flags & RX_THR)) {
-		if (update)
-			bcm_rx_changed(op, lcf);
+		bcm_rx_changed(op, lcf);
 		return 1;
 	}
 	return 0;
@@ -598,11 +582,8 @@ static inline int bcm_rx_do_flush(struct
 
 /*
  * bcm_rx_thr_flush - Check for throttled data and send it to the userspace
- *
- * update == 0 : just check if throttled data is available  (any irq context)
- * update == 1 : check and send throttled data to userspace (soft_irq context)
  */
-static int bcm_rx_thr_flush(struct bcm_op *op, int update)
+static int bcm_rx_thr_flush(struct bcm_op *op)
 {
 	int updated = 0;
 
@@ -611,24 +592,16 @@ static int bcm_rx_thr_flush(struct bcm_o
 
 		/* for MUX filter we start at index 1 */
 		for (i = 1; i < op->nframes; i++)
-			updated += bcm_rx_do_flush(op, update, i);
+			updated += bcm_rx_do_flush(op, i);
 
 	} else {
 		/* for RX_FILTER_ID and simple filter */
-		updated += bcm_rx_do_flush(op, update, 0);
+		updated += bcm_rx_do_flush(op, 0);
 	}
 
 	return updated;
 }
 
-static void bcm_rx_thr_tsklet(unsigned long data)
-{
-	struct bcm_op *op = (struct bcm_op *)data;
-
-	/* push the changed data to the userspace */
-	bcm_rx_thr_flush(op, 1);
-}
-
 /*
  * bcm_rx_thr_handler - the time for blocked content updates is over now:
  *                      Check for throttled data and send it to the userspace
@@ -637,9 +610,7 @@ static enum hrtimer_restart bcm_rx_thr_h
 {
 	struct bcm_op *op = container_of(hrtimer, struct bcm_op, thrtimer);
 
-	tasklet_schedule(&op->thrtsklet);
-
-	if (bcm_rx_thr_flush(op, 0)) {
+	if (bcm_rx_thr_flush(op)) {
 		hrtimer_forward(hrtimer, ktime_get(), op->kt_ival2);
 		return HRTIMER_RESTART;
 	} else {
@@ -735,23 +706,8 @@ static struct bcm_op *bcm_find_op(struct
 
 static void bcm_remove_op(struct bcm_op *op)
 {
-	if (op->tsklet.func) {
-		while (test_bit(TASKLET_STATE_SCHED, &op->tsklet.state) ||
-		       test_bit(TASKLET_STATE_RUN, &op->tsklet.state) ||
-		       hrtimer_active(&op->timer)) {
-			hrtimer_cancel(&op->timer);
-			tasklet_kill(&op->tsklet);
-		}
-	}
-
-	if (op->thrtsklet.func) {
-		while (test_bit(TASKLET_STATE_SCHED, &op->thrtsklet.state) ||
-		       test_bit(TASKLET_STATE_RUN, &op->thrtsklet.state) ||
-		       hrtimer_active(&op->thrtimer)) {
-			hrtimer_cancel(&op->thrtimer);
-			tasklet_kill(&op->thrtsklet);
-		}
-	}
+	hrtimer_cancel(&op->timer);
+	hrtimer_cancel(&op->thrtimer);
 
 	if ((op->frames) && (op->frames != &op->sframe))
 		kfree(op->frames);
@@ -979,15 +935,13 @@ static int bcm_tx_setup(struct bcm_msg_h
 		op->ifindex = ifindex;
 
 		/* initialize uninitialized (kzalloc) structure */
-		hrtimer_init(&op->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		hrtimer_init(&op->timer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_SOFT);
 		op->timer.function = bcm_tx_timeout_handler;
 
-		/* initialize tasklet for tx countevent notification */
-		tasklet_init(&op->tsklet, bcm_tx_timeout_tsklet,
-			     (unsigned long) op);
-
 		/* currently unused in tx_ops */
-		hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_SOFT);
 
 		/* add this bcm_op to the list of the tx_ops */
 		list_add(&op->list, &bo->tx_ops);
@@ -1150,20 +1104,14 @@ static int bcm_rx_setup(struct bcm_msg_h
 		op->rx_ifindex = ifindex;
 
 		/* initialize uninitialized (kzalloc) structure */
-		hrtimer_init(&op->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		hrtimer_init(&op->timer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_SOFT);
 		op->timer.function = bcm_rx_timeout_handler;
 
-		/* initialize tasklet for rx timeout notification */
-		tasklet_init(&op->tsklet, bcm_rx_timeout_tsklet,
-			     (unsigned long) op);
-
-		hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_SOFT);
 		op->thrtimer.function = bcm_rx_thr_handler;
 
-		/* initialize tasklet for rx throttle handling */
-		tasklet_init(&op->thrtsklet, bcm_rx_thr_tsklet,
-			     (unsigned long) op);
-
 		/* add this bcm_op to the list of the rx_ops */
 		list_add(&op->list, &bo->rx_ops);
 
@@ -1209,12 +1157,12 @@ static int bcm_rx_setup(struct bcm_msg_h
 			 */
 			op->kt_lastmsg = 0;
 			hrtimer_cancel(&op->thrtimer);
-			bcm_rx_thr_flush(op, 1);
+			bcm_rx_thr_flush(op);
 		}
 
 		if ((op->flags & STARTTIMER) && op->kt_ival1)
 			hrtimer_start(&op->timer, op->kt_ival1,
-				      HRTIMER_MODE_REL);
+				      HRTIMER_MODE_REL_SOFT);
 	}
 
 	/* now we can register for can_ids, if we added a new bcm_op */
maintainer Acked-by on v2 30/37
#33 lore raw

Re: [PATCH v2 30/37] can/bcm: Replace hrtimer_tasklet with softirq based hrtimer

Quoted 8 lines · Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

Switch the timer to HRTIMER_MODE_SOFT, which executed the timer callback in softirq context and remove the hrtimer_tasklet.

Quoted 304 lines
Cc Marc Kleine-Budde <[email protected]>
---
  net/can/bcm.c |  156 +++++++++++++++++++---------------------------------------
  1 file changed, 52 insertions(+), 104 deletions(-)
net/can/bcm.c +52 -104
@@ -102,7 +102,6 @@ struct bcm_op {
  	unsigned long frames_abs, frames_filtered;
  	struct bcm_timeval ival1, ival2;
  	struct hrtimer timer, thrtimer;
-	struct tasklet_struct tsklet, thrtsklet;
  	ktime_t rx_stamp, kt_ival1, kt_ival2, kt_lastmsg;
  	int rx_ifindex;
  	int cfsiz;
@@ -364,25 +363,34 @@ static void bcm_send_to_user(struct bcm_
  	}
  }
 
-static void bcm_tx_start_timer(struct bcm_op *op)
+static bool bcm_tx_set_expiry(struct bcm_op *op, struct hrtimer *hrt)
  {
+	ktime_t ival;
+
  	if (op->kt_ival1 && op->count)
-		hrtimer_start(&op->timer,
-			      ktime_add(ktime_get(), op->kt_ival1),
-			      HRTIMER_MODE_ABS);
+		ival = op->kt_ival1;
  	else if (op->kt_ival2)
-		hrtimer_start(&op->timer,
-			      ktime_add(ktime_get(), op->kt_ival2),
-			      HRTIMER_MODE_ABS);
+		ival = op->kt_ival2;
+	else
+		return false;
+
+	hrtimer_set_expires(hrt, ktime_add(ktime_get(), ival));
+	return true;
  }
 
-static void bcm_tx_timeout_tsklet(unsigned long data)
+static void bcm_tx_start_timer(struct bcm_op *op)
  {
-	struct bcm_op *op = (struct bcm_op *)data;
+	if (bcm_tx_set_expiry(op, &op->timer))
+		hrtimer_start_expires(&op->timer, HRTIMER_MODE_ABS_SOFT);
+}
+
+/* bcm_tx_timeout_handler - performs cyclic CAN frame transmissions */
+static enum hrtimer_restart bcm_tx_timeout_handler(struct hrtimer *hrtimer)
+{
+	struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
  	struct bcm_msg_head msg_head;
 
  	if (op->kt_ival1 && (op->count > 0)) {
-
  		op->count--;
  		if (!op->count && (op->flags & TX_COUNTEVT)) {
 
@@ -399,22 +407,12 @@ static void bcm_tx_timeout_tsklet(unsign
  		}
  		bcm_can_tx(op);
 
-	} else if (op->kt_ival2)
+	} else if (op->kt_ival2) {
  		bcm_can_tx(op);
+	}
 
-	bcm_tx_start_timer(op);
-}
-
-/*
- * bcm_tx_timeout_handler - performs cyclic CAN frame transmissions
- */
-static enum hrtimer_restart bcm_tx_timeout_handler(struct hrtimer *hrtimer)
-{
-	struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
-
-	tasklet_schedule(&op->tsklet);
-
-	return HRTIMER_NORESTART;
+	return bcm_tx_set_expiry(op, &op->timer) ?
+		HRTIMER_RESTART : HRTIMER_NORESTART;
  }
 
  /*
@@ -480,7 +478,7 @@ static void bcm_rx_update_and_send(struc
  		/* do not send the saved data - only start throttle timer */
  		hrtimer_start(&op->thrtimer,
  			      ktime_add(op->kt_lastmsg, op->kt_ival2),
-			      HRTIMER_MODE_ABS);
+			      HRTIMER_MODE_ABS_SOFT);
  		return;
  	}
 
@@ -539,14 +537,21 @@ static void bcm_rx_starttimer(struct bcm
  		return;
 
  	if (op->kt_ival1)
-		hrtimer_start(&op->timer, op->kt_ival1, HRTIMER_MODE_REL);
+		hrtimer_start(&op->timer, op->kt_ival1, HRTIMER_MODE_REL_SOFT);
  }
 
-static void bcm_rx_timeout_tsklet(unsigned long data)
+/* bcm_rx_timeout_handler - when the (cyclic) CAN frame reception timed out */
+static enum hrtimer_restart bcm_rx_timeout_handler(struct hrtimer *hrtimer)
  {
-	struct bcm_op *op = (struct bcm_op *)data;
+	struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
  	struct bcm_msg_head msg_head;
 
+	/* if user wants to be informed, when cyclic CAN-Messages come back */
+	if ((op->flags & RX_ANNOUNCE_RESUME) && op->last_frames) {
+		/* clear received CAN frames to indicate 'nothing received' */
+		memset(op->last_frames, 0, op->nframes * op->cfsiz);
+	}
+
  	/* create notification to user */
  	msg_head.opcode  = RX_TIMEOUT;
  	msg_head.flags   = op->flags;
@@ -557,25 +562,6 @@ static void bcm_rx_timeout_tsklet(unsign
  	msg_head.nframes = 0;
 
  	bcm_send_to_user(op, &msg_head, NULL, 0);
-}
-
-/*
- * bcm_rx_timeout_handler - when the (cyclic) CAN frame reception timed out
- */
-static enum hrtimer_restart bcm_rx_timeout_handler(struct hrtimer *hrtimer)
-{
-	struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
-
-	/* schedule before NET_RX_SOFTIRQ */
-	tasklet_hi_schedule(&op->tsklet);
-
-	/* no restart of the timer is done here! */
-
-	/* if user wants to be informed, when cyclic CAN-Messages come back */
-	if ((op->flags & RX_ANNOUNCE_RESUME) && op->last_frames) {
-		/* clear received CAN frames to indicate 'nothing received' */
-		memset(op->last_frames, 0, op->nframes * op->cfsiz);
-	}
 
  	return HRTIMER_NORESTART;
  }
@@ -583,14 +569,12 @@ static enum hrtimer_restart bcm_rx_timeo
  /*
   * bcm_rx_do_flush - helper for bcm_rx_thr_flush
   */
-static inline int bcm_rx_do_flush(struct bcm_op *op, int update,
-				  unsigned int index)
+static inline int bcm_rx_do_flush(struct bcm_op *op, unsigned int index)
  {
  	struct canfd_frame *lcf = op->last_frames + op->cfsiz * index;
 
  	if ((op->last_frames) && (lcf->flags & RX_THR)) {
-		if (update)
-			bcm_rx_changed(op, lcf);
+		bcm_rx_changed(op, lcf);
  		return 1;
  	}
  	return 0;
@@ -598,11 +582,8 @@ static inline int bcm_rx_do_flush(struct
 
  /*
   * bcm_rx_thr_flush - Check for throttled data and send it to the userspace
- *
- * update == 0 : just check if throttled data is available  (any irq context)
- * update == 1 : check and send throttled data to userspace (soft_irq context)
   */
-static int bcm_rx_thr_flush(struct bcm_op *op, int update)
+static int bcm_rx_thr_flush(struct bcm_op *op)
  {
  	int updated = 0;
 
@@ -611,24 +592,16 @@ static int bcm_rx_thr_flush(struct bcm_o
 
  		/* for MUX filter we start at index 1 */
  		for (i = 1; i < op->nframes; i++)
-			updated += bcm_rx_do_flush(op, update, i);
+			updated += bcm_rx_do_flush(op, i);
 
  	} else {
  		/* for RX_FILTER_ID and simple filter */
-		updated += bcm_rx_do_flush(op, update, 0);
+		updated += bcm_rx_do_flush(op, 0);
  	}
 
  	return updated;
  }
 
-static void bcm_rx_thr_tsklet(unsigned long data)
-{
-	struct bcm_op *op = (struct bcm_op *)data;
-
-	/* push the changed data to the userspace */
-	bcm_rx_thr_flush(op, 1);
-}
-
  /*
   * bcm_rx_thr_handler - the time for blocked content updates is over now:
   *                      Check for throttled data and send it to the userspace
@@ -637,9 +610,7 @@ static enum hrtimer_restart bcm_rx_thr_h
  {
  	struct bcm_op *op = container_of(hrtimer, struct bcm_op, thrtimer);
 
-	tasklet_schedule(&op->thrtsklet);
-
-	if (bcm_rx_thr_flush(op, 0)) {
+	if (bcm_rx_thr_flush(op)) {
  		hrtimer_forward(hrtimer, ktime_get(), op->kt_ival2);
  		return HRTIMER_RESTART;
  	} else {
@@ -735,23 +706,8 @@ static struct bcm_op *bcm_find_op(struct
 
  static void bcm_remove_op(struct bcm_op *op)
  {
-	if (op->tsklet.func) {
-		while (test_bit(TASKLET_STATE_SCHED, &op->tsklet.state) ||
-		       test_bit(TASKLET_STATE_RUN, &op->tsklet.state) ||
-		       hrtimer_active(&op->timer)) {
-			hrtimer_cancel(&op->timer);
-			tasklet_kill(&op->tsklet);
-		}
-	}
-
-	if (op->thrtsklet.func) {
-		while (test_bit(TASKLET_STATE_SCHED, &op->thrtsklet.state) ||
-		       test_bit(TASKLET_STATE_RUN, &op->thrtsklet.state) ||
-		       hrtimer_active(&op->thrtimer)) {
-			hrtimer_cancel(&op->thrtimer);
-			tasklet_kill(&op->thrtsklet);
-		}
-	}
+	hrtimer_cancel(&op->timer);
+	hrtimer_cancel(&op->thrtimer);
 
  	if ((op->frames) && (op->frames != &op->sframe))
  		kfree(op->frames);
@@ -979,15 +935,13 @@ static int bcm_tx_setup(struct bcm_msg_h
  		op->ifindex = ifindex;
 
  		/* initialize uninitialized (kzalloc) structure */
-		hrtimer_init(&op->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		hrtimer_init(&op->timer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_SOFT);
  		op->timer.function = bcm_tx_timeout_handler;
 
-		/* initialize tasklet for tx countevent notification */
-		tasklet_init(&op->tsklet, bcm_tx_timeout_tsklet,
-			     (unsigned long) op);
-
  		/* currently unused in tx_ops */
-		hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_SOFT);
 
  		/* add this bcm_op to the list of the tx_ops */
  		list_add(&op->list, &bo->tx_ops);
@@ -1150,20 +1104,14 @@ static int bcm_rx_setup(struct bcm_msg_h
  		op->rx_ifindex = ifindex;
 
  		/* initialize uninitialized (kzalloc) structure */
-		hrtimer_init(&op->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		hrtimer_init(&op->timer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_SOFT);
  		op->timer.function = bcm_rx_timeout_handler;
 
-		/* initialize tasklet for rx timeout notification */
-		tasklet_init(&op->tsklet, bcm_rx_timeout_tsklet,
-			     (unsigned long) op);
-
-		hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_SOFT);
  		op->thrtimer.function = bcm_rx_thr_handler;
 
-		/* initialize tasklet for rx throttle handling */
-		tasklet_init(&op->thrtsklet, bcm_rx_thr_tsklet,
-			     (unsigned long) op);
-
  		/* add this bcm_op to the list of the rx_ops */
  		list_add(&op->list, &bo->rx_ops);
 
@@ -1209,12 +1157,12 @@ static int bcm_rx_setup(struct bcm_msg_h
  			 */
  			op->kt_lastmsg = 0;
  			hrtimer_cancel(&op->thrtimer);
-			bcm_rx_thr_flush(op, 1);
+			bcm_rx_thr_flush(op);
  		}
 
  		if ((op->flags & STARTTIMER) && op->kt_ival1)
  			hrtimer_start(&op->timer, op->kt_ival1,
-				      HRTIMER_MODE_REL);
+				      HRTIMER_MODE_REL_SOFT);
  	}
 
  	/* now we can register for can_ids, if we added a new bcm_op */
author maintainer
#34 lore raw

PATCH v2 31/37 mac80211_hwsim: Replace hrtimer tasklet with softirq hrtimer

In reply to Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

Switch the timer to HRTIMER_MODE_SOFT, which executed the timer callback in softirq context and remove the hrtimer_tasklet.

---
 drivers/net/wireless/mac80211_hwsim.c |   44 +++++++++++++++-------------------
 1 file changed, 20 insertions(+), 24 deletions(-)
drivers/net/wireless/mac80211_hwsim.c +20 -24
@@ -537,7 +537,7 @@ struct mac80211_hwsim_data {
 	unsigned int rx_filter;
 	bool started, idle, scanning;
 	struct mutex mutex;
-	struct tasklet_hrtimer beacon_timer;
+	struct hrtimer beacon_timer;
 	enum ps_mode {
 		PS_DISABLED, PS_ENABLED, PS_AUTO_POLL, PS_MANUAL_POLL
 	} ps;
@@ -1418,7 +1418,7 @@ static void mac80211_hwsim_stop(struct i
 {
 	struct mac80211_hwsim_data *data = hw->priv;
 	data->started = false;
-	tasklet_hrtimer_cancel(&data->beacon_timer);
+	hrtimer_cancel(&data->beacon_timer);
 	wiphy_debug(hw->wiphy, "%s\n", __func__);
 }
 
@@ -1541,14 +1541,12 @@ static enum hrtimer_restart
 mac80211_hwsim_beacon(struct hrtimer *timer)
 {
 	struct mac80211_hwsim_data *data =
-		container_of(timer, struct mac80211_hwsim_data,
-			     beacon_timer.timer);
+		container_of(timer, struct mac80211_hwsim_data, beacon_timer);
 	struct ieee80211_hw *hw = data->hw;
 	u64 bcn_int = data->beacon_int;
-	ktime_t next_bcn;
 
 	if (!data->started)
-		goto out;
+		return HRTIMER_NORESTART;
 
 	ieee80211_iterate_active_interfaces_atomic(
 		hw, IEEE80211_IFACE_ITER_NORMAL,
@@ -1560,11 +1558,9 @@ mac80211_hwsim_beacon(struct hrtimer *ti
 		data->bcn_delta = 0;
 	}
 
-	next_bcn = ktime_add(hrtimer_get_expires(timer),
-			     ns_to_ktime(bcn_int * 1000));
-	tasklet_hrtimer_start(&data->beacon_timer, next_bcn, HRTIMER_MODE_ABS);
-out:
-	return HRTIMER_NORESTART;
+	hrtimer_forward(&data->beacon_timer, hrtimer_get_expires(timer),
+			ns_to_ktime(bcn_int * NSEC_PER_USEC));
+	return HRTIMER_RESTART;
 }
 
 static const char * const hwsim_chanwidths[] = {
@@ -1638,15 +1634,15 @@ static int mac80211_hwsim_config(struct
 	mutex_unlock(&data->mutex);
 
 	if (!data->started || !data->beacon_int)
-		tasklet_hrtimer_cancel(&data->beacon_timer);
-	else if (!hrtimer_is_queued(&data->beacon_timer.timer)) {
+		hrtimer_cancel(&data->beacon_timer);
+	else if (!hrtimer_is_queued(&data->beacon_timer)) {
 		u64 tsf = mac80211_hwsim_get_tsf(hw, NULL);
 		u32 bcn_int = data->beacon_int;
 		u64 until_tbtt = bcn_int - do_div(tsf, bcn_int);
 
-		tasklet_hrtimer_start(&data->beacon_timer,
-				      ns_to_ktime(until_tbtt * 1000),
-				      HRTIMER_MODE_REL);
+		hrtimer_start(&data->beacon_timer,
+			      ns_to_ktime(until_tbtt * 1000),
+			      HRTIMER_MODE_REL_SOFT);
 	}
 
 	return 0;
@@ -1709,7 +1705,7 @@ static void mac80211_hwsim_bss_info_chan
 			    info->enable_beacon, info->beacon_int);
 		vp->bcn_en = info->enable_beacon;
 		if (data->started &&
-		    !hrtimer_is_queued(&data->beacon_timer.timer) &&
+		    !hrtimer_is_queued(&data->beacon_timer) &&
 		    info->enable_beacon) {
 			u64 tsf, until_tbtt;
 			u32 bcn_int;
@@ -1717,9 +1713,9 @@ static void mac80211_hwsim_bss_info_chan
 			tsf = mac80211_hwsim_get_tsf(hw, vif);
 			bcn_int = data->beacon_int;
 			until_tbtt = bcn_int - do_div(tsf, bcn_int);
-			tasklet_hrtimer_start(&data->beacon_timer,
-					      ns_to_ktime(until_tbtt * 1000),
-					      HRTIMER_MODE_REL);
+			hrtimer_start(&data->beacon_timer,
+				      ns_to_ktime(until_tbtt * 1000),
+				      HRTIMER_MODE_REL_SOFT);
 		} else if (!info->enable_beacon) {
 			unsigned int count = 0;
 			ieee80211_iterate_active_interfaces_atomic(
@@ -1728,7 +1724,7 @@ static void mac80211_hwsim_bss_info_chan
 			wiphy_debug(hw->wiphy, "  beaconing vifs remaining: %u",
 				    count);
 			if (count == 0) {
-				tasklet_hrtimer_cancel(&data->beacon_timer);
+				hrtimer_cancel(&data->beacon_timer);
 				data->beacon_int = 0;
 			}
 		}
@@ -2720,9 +2716,9 @@ static int mac80211_hwsim_new_radio(stru
 				    data->debugfs,
 				    data, &hwsim_simulate_radar);
 
-	tasklet_hrtimer_init(&data->beacon_timer,
-			     mac80211_hwsim_beacon,
-			     CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
+	hrtimer_init(&data->beacon_timer, CLOCK_MONOTONIC,
+		     HRTIMER_MODE_ABS_SOFT);
+	data->beacon_timer.function = mac80211_hwsim_beacon;
 
 	spin_lock_bh(&hwsim_radio_lock);
 	list_add_tail(&data->list, &hwsim_radios);
maintainer
#35 lore raw

Re: [PATCH v2 31/37] mac80211_hwsim: Replace hrtimer tasklet with softirq hrtimer

Quoted 4 lines · Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

Switch the timer to HRTIMER_MODE_SOFT, which executed the timer callback in softirq context and remove the hrtimer_tasklet.

This doesn't build on my tree, due to HRTIMER_MODE_REL_SOFT not existing. Neither does HRTIMER_MODE_SOFT mentioned above, but that's just a commit log mistake.

(It also didn't apply on my tree, but that was a trivial context change.)

I don't understand why you initialize it with HRTIMER_MODE_ABS_SOFT either though:

drivers/net/wireless/mac80211_hwsim.c static int mac80211_hwsim_new_radio(stru · line 2720

Quoted 2 lines
drivers/net/wireless/mac80211_hwsim.c +2 -0
+       hrtimer_init(&data->beacon_timer, CLOCK_MONOTONIC,
+                    HRTIMER_MODE_ABS_SOFT);

shouldn't that already be REL?

johannes

#36 lore raw

Re: [PATCH v2 31/37] mac80211_hwsim: Replace hrtimer tasklet with softirq hrtimer

Quoted 5 lines · Johannes Berg
Quoted 4 lines · Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

Switch the timer to HRTIMER_MODE_SOFT, which executed the timer callback in softirq context and remove the hrtimer_tasklet.

I can't build as its part of the whole hrtimer rework series.

Quoted 3 lines

This doesn't build on my tree, due to HRTIMER_MODE_REL_SOFT not existing. Neither does HRTIMER_MODE_SOFT mentioned above, but that's just a commit log mistake.

Oops.

Quoted 8 lines

(It also didn't apply on my tree, but that was a trivial context change.)

I don't understand why you initialize it with HRTIMER_MODE_ABS_SOFT either though:

Quoted 2 lines
drivers/net/wireless/mac80211_hwsim.c +2 -0
+       hrtimer_init(&data->beacon_timer, CLOCK_MONOTONIC,
+                    HRTIMER_MODE_ABS_SOFT);

Sure, though it does not matter in that case. Will fix that up.

Thanks,

tglx

maintainer
#37 lore raw

Re: [PATCH v2 31/37] mac80211_hwsim: Replace hrtimer tasklet with softirq hrtimer

Quoted 9 lines · Thomas Gleixner
Quoted 5 lines · Johannes Berg
Quoted 4 lines · Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

Switch the timer to HRTIMER_MODE_SOFT, which executed the timer callback in softirq context and remove the hrtimer_tasklet.

I can't build as its part of the whole hrtimer rework series.

I guess you mean you *can* build it? Surely you're introducing the new HR timer modes in some patch that I didn't see? :-)

Quoted 4 lines
Quoted 2 lines

drivers/net/wireless/mac80211_hwsim.c static int mac80211_hwsim_new_radio(stru · line 2720

Quoted 2 lines
drivers/net/wireless/mac80211_hwsim.c +2 -0
+       hrtimer_init(&data->beacon_timer, CLOCK_MONOTONIC,
+                    HRTIMER_MODE_ABS_SOFT);

Sure, though it does not matter in that case. Will fix that up.

Right. Then again, why even pass it to init() and start()? Can you start without going through start()?

johannes

#38 lore raw

Re: [PATCH v2 31/37] mac80211_hwsim: Replace hrtimer tasklet with softirq hrtimer

Quoted 6 lines · Johannes Berg
Quoted 5 lines · Thomas Gleixner
Quoted 3 lines · Johannes Berg
Quoted 2 lines · Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines

Switch the timer to HRTIMER_MODE_SOFT, which executed the timer callback in softirq context and remove the hrtimer_tasklet.

I can't build as its part of the whole hrtimer rework series.

I guess you mean you *can* build it? Surely you're introducing the new HR timer modes in some patch that I didn't see? :-)

Sorry, we did not want to expose you to 30 patches fiddling with the core code. They are on LKML though.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

drivers/net/wireless/mac80211_hwsim.c static int mac80211_hwsim_new_radio(stru · line 2720

Quoted 2 lines
drivers/net/wireless/mac80211_hwsim.c +2 -0
+       hrtimer_init(&data->beacon_timer, CLOCK_MONOTONIC,
+                    HRTIMER_MODE_ABS_SOFT);

Sure, though it does not matter in that case. Will fix that up.

Right. Then again, why even pass it to init() and start()? Can you start without going through start()?

There is a subtle magic with CLOCK_REALTIME timers.

CLOCK_REALTIME timers differentiate between ABS and REL modes. ABS timers are exposed to clock modifications (settimeofday() ...), REL timers are not. We solve that by associating them to different clock bases, which has to be done at init time, but the start function needs the REL/ABS information as well.

For CLOCK_MONOTONIC this is not really required, but the function is used for all clock bases, so we require the mode bits for all.

Thanks,

tglx

maintainer
#39 lore raw

Re: [PATCH v2 31/37] mac80211_hwsim: Replace hrtimer tasklet with softirq hrtimer

Quoted 6 lines
Quoted 3 lines

I guess you mean you *can* build it? Surely you're introducing the new HR timer modes in some patch that I didn't see? :-)

Sorry, we did not want to expose you to 30 patches fiddling with the core code. They are on LKML though.

Sure, no worries. I just didn't even realize I shouldn't be applying the patch (myself) :-)

Quoted 5 lines
Quoted 2 lines

Right. Then again, why even pass it to init() and start()? Can you start without going through start()?

There is a subtle magic with CLOCK_REALTIME timers.

Quoted 8 lines

CLOCK_REALTIME timers differentiate between ABS and REL modes. ABS timers are exposed to clock modifications (settimeofday() ...), REL timers are not. We solve that by associating them to different clock bases, which has to be done at init time, but the start function needs the REL/ABS information as well.

For CLOCK_MONOTONIC this is not really required, but the function is used for all clock bases, so we require the mode bits for all.

Hmm. Couldn't you just store that then from init to use in start?

If you don't store it, yet don't verify that you passed the same thing, do you at least check that it's compatible? Sounds like something will totally go wrong if I pass CLOCK_REALTIME/ABS first and then use REL for start, or vice versa?

Also, in the code I see only checking

if (mode != HRTIMER_MODE_ABS) {
	// change clock ID:
	// realtime -> monotonic
	// realtime_soft -> monotonic_soft
}

but you're passing HRTIMER_MODE_ABS_SOFT too, isn't that considered an ABS mode?

But then, looking at the code again, I don't even see HRTIMER_MODE_ABS_SOFT existing, it sounds like it should be

hrtimer_init(..., CLOCK_REALTIME_SOFT, HRTIMER_MODE_REL);

then?

At least then the code I mentioned above makes sense, but it still feels pretty dangerous to not just store the mode and use it in start, but to require passing it again. You even just introduced the same bug, it just happened to not matter in this case since the clock isn't realtime.

Or am I completely confused now?

johannes

author maintainer
#40 lore raw

PATCH v2 32/37 xfrm: Replace hrtimer tasklet with softirq hrtimer

In reply to Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

Switch the timer to HRTIMER_MODE_SOFT, which executed the timer callback in softirq context and remove the hrtimer_tasklet.

---
 include/net/xfrm.h    |    2 +-
 net/xfrm/xfrm_state.c |   30 ++++++++++++++++++------------
 2 files changed, 19 insertions(+), 13 deletions(-)
include/net/xfrm.h +1 -1
@@ -216,7 +216,7 @@ struct xfrm_state {
 	struct xfrm_stats	stats;
 
 	struct xfrm_lifetime_cur curlft;
-	struct tasklet_hrtimer	mtimer;
+	struct hrtimer		mtimer;
 
 	struct xfrm_state_offload xso;
 
net/xfrm/xfrm_state.c +18 -12
@@ -426,7 +426,7 @@ static void xfrm_put_mode(struct xfrm_mo
 
 static void xfrm_state_gc_destroy(struct xfrm_state *x)
 {
-	tasklet_hrtimer_cancel(&x->mtimer);
+	hrtimer_cancel(&x->mtimer);
 	del_timer_sync(&x->rtimer);
 	kfree(x->aead);
 	kfree(x->aalg);
@@ -471,8 +471,8 @@ static void xfrm_state_gc_task(struct wo
 
 static enum hrtimer_restart xfrm_timer_handler(struct hrtimer *me)
 {
-	struct tasklet_hrtimer *thr = container_of(me, struct tasklet_hrtimer, timer);
-	struct xfrm_state *x = container_of(thr, struct xfrm_state, mtimer);
+	struct xfrm_state *x = container_of(me, struct xfrm_state, mtimer);
+	enum hrtimer_restart ret = HRTIMER_NORESTART;
 	unsigned long now = get_seconds();
 	long next = LONG_MAX;
 	int warn = 0;
@@ -536,7 +536,8 @@ static enum hrtimer_restart xfrm_timer_h
 		km_state_expired(x, 0, 0);
 resched:
 	if (next != LONG_MAX) {
-		tasklet_hrtimer_start(&x->mtimer, ktime_set(next, 0), HRTIMER_MODE_REL);
+		hrtimer_forward_now(&x->mtimer, ktime_set(next, 0));
+		ret = HRTIMER_RESTART;
 	}
 
 	goto out;
@@ -553,7 +554,7 @@ static enum hrtimer_restart xfrm_timer_h
 
 out:
 	spin_unlock(&x->lock);
-	return HRTIMER_NORESTART;
+	return ret;
 }
 
 static void xfrm_replay_timer_handler(unsigned long data);
@@ -572,8 +573,8 @@ struct xfrm_state *xfrm_state_alloc(stru
 		INIT_HLIST_NODE(&x->bydst);
 		INIT_HLIST_NODE(&x->bysrc);
 		INIT_HLIST_NODE(&x->byspi);
-		tasklet_hrtimer_init(&x->mtimer, xfrm_timer_handler,
-					CLOCK_BOOTTIME, HRTIMER_MODE_ABS);
+		hrtimer_init(&x->mtimer, CLOCK_BOOTTIME, HRTIMER_MODE_ABS_SOFT);
+		x->mtimer.function = xfrm_timer_handler;
 		setup_timer(&x->rtimer, xfrm_replay_timer_handler,
 				(unsigned long)x);
 		x->curlft.add_time = get_seconds();
@@ -1030,7 +1031,9 @@ xfrm_state_find(const xfrm_address_t *da
 				hlist_add_head_rcu(&x->byspi, net->xfrm.state_byspi + h);
 			}
 			x->lft.hard_add_expires_seconds = net->xfrm.sysctl_acq_expires;
-			tasklet_hrtimer_start(&x->mtimer, ktime_set(net->xfrm.sysctl_acq_expires, 0), HRTIMER_MODE_REL);
+			hrtimer_start(&x->mtimer,
+				      ktime_set(net->xfrm.sysctl_acq_expires, 0),
+				      HRTIMER_MODE_REL_SOFT);
 			net->xfrm.state_num++;
 			xfrm_hash_grow_check(net, x->bydst.next != NULL);
 			spin_unlock_bh(&net->xfrm.xfrm_state_lock);
@@ -1141,7 +1144,7 @@ static void __xfrm_state_insert(struct x
 		hlist_add_head_rcu(&x->byspi, net->xfrm.state_byspi + h);
 	}
 
-	tasklet_hrtimer_start(&x->mtimer, ktime_set(1, 0), HRTIMER_MODE_REL);
+	hrtimer_start(&x->mtimer, ktime_set(1, 0), HRTIMER_MODE_REL_SOFT);
 	if (x->replay_maxage)
 		mod_timer(&x->rtimer, jiffies + x->replay_maxage);
 
@@ -1245,7 +1248,9 @@ static struct xfrm_state *__find_acq_cor
 		x->mark.m = m->m;
 		x->lft.hard_add_expires_seconds = net->xfrm.sysctl_acq_expires;
 		xfrm_state_hold(x);
-		tasklet_hrtimer_start(&x->mtimer, ktime_set(net->xfrm.sysctl_acq_expires, 0), HRTIMER_MODE_REL);
+		hrtimer_start(&x->mtimer,
+			      ktime_set(net->xfrm.sysctl_acq_expires, 0),
+			      HRTIMER_MODE_REL_SOFT);
 		list_add(&x->km.all, &net->xfrm.state_all);
 		hlist_add_head_rcu(&x->bydst, net->xfrm.state_bydst + h);
 		h = xfrm_src_hash(net, daddr, saddr, family);
@@ -1544,7 +1549,8 @@ int xfrm_state_update(struct xfrm_state
 		memcpy(&x1->lft, &x->lft, sizeof(x1->lft));
 		x1->km.dying = 0;
 
-		tasklet_hrtimer_start(&x1->mtimer, ktime_set(1, 0), HRTIMER_MODE_REL);
+		hrtimer_start(&x1->mtimer, ktime_set(1, 0),
+			      HRTIMER_MODE_REL_SOFT);
 		if (x1->curlft.use_time)
 			xfrm_state_check_expire(x1);
 
@@ -1568,7 +1574,7 @@ int xfrm_state_check_expire(struct xfrm_
 	if (x->curlft.bytes >= x->lft.hard_byte_limit ||
 	    x->curlft.packets >= x->lft.hard_packet_limit) {
 		x->km.state = XFRM_STATE_EXPIRED;
-		tasklet_hrtimer_start(&x->mtimer, 0, HRTIMER_MODE_REL);
+		hrtimer_start(&x->mtimer, 0, HRTIMER_MODE_REL_SOFT);
 		return -EINVAL;
 	}
 
author maintainer
#41 lore raw

PATCH v2 33/37 softirq: Remove tasklet_hrtimer

In reply to Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

There are no more tasklet_hrtimer users of this interface. Remove it.

---
 include/linux/interrupt.h |   25 ----------------------
 kernel/softirq.c          |   51 ----------------------------------------------
 2 files changed, 76 deletions(-)
include/linux/interrupt.h +0 -25
@@ -632,31 +632,6 @@ extern void tasklet_kill_immediate(struc
 extern void tasklet_init(struct tasklet_struct *t,
 			 void (*func)(unsigned long), unsigned long data);
 
-struct tasklet_hrtimer {
-	struct hrtimer		timer;
-	struct tasklet_struct	tasklet;
-	enum hrtimer_restart	(*function)(struct hrtimer *);
-};
-
-extern void
-tasklet_hrtimer_init(struct tasklet_hrtimer *ttimer,
-		     enum hrtimer_restart (*function)(struct hrtimer *),
-		     clockid_t which_clock, enum hrtimer_mode mode);
-
-static inline
-void tasklet_hrtimer_start(struct tasklet_hrtimer *ttimer, ktime_t time,
-			   const enum hrtimer_mode mode)
-{
-	hrtimer_start(&ttimer->timer, time, mode);
-}
-
-static inline
-void tasklet_hrtimer_cancel(struct tasklet_hrtimer *ttimer)
-{
-	hrtimer_cancel(&ttimer->timer);
-	tasklet_kill(&ttimer->tasklet);
-}
-
 /*
  * Autoprobing for irqs:
  *
kernel/softirq.c +0 -51

57 lines not on this page · raw message

author maintainer
#42 lore raw

PATCH v2 34/37 ALSA/dummy: Replace tasklet with softirq hrtimer

In reply to Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

The tasklet is used to defer the execution of snd_pcm_period_elapsed() to the softirq context. Using the HRTIMER_MODE_SOFT mode invokes the timer callback in softirq context as well which renders the tasklet useless.

[o-takashi: avoid stall due to a call of hrtimer_cancel() on a callback
of hrtimer]

---

Quoted 8 lines · Takashi Sakamoto

As Iwai-san mentioned, in this function, .trigger can be called in two cases; XRUN occurs and draining is done. Thus, let you change the comment as 'In cases of XRUN and draining, this calls .trigger to stop PCM substream.'. I'm sorry to trouble you.

snd_pcm_period_elapsed()
->snd_pcm_update_hw_ptr0()
->snd_pcm_update_state()

Quoted 8 lines
    ->snd_pcm_drain_done()
      ...
      ->.trigger(TRIGGER_STOP)
    ->xrun()
      ->snd_pcm_stop()
        ...
        ->.trigger(TRIGGER_STOP)

I think you asked me just to update the comment. Did I do it right?

v2...v3: updated the comment as per Takashi Sakamoto's suggestion. v1...v2: merged Takashi Sakamoto fixup of the original patch into v2.

 sound/drivers/dummy.c |   27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)
sound/drivers/dummy.c +12 -15
@@ -376,17 +376,9 @@ struct dummy_hrtimer_pcm {
 	ktime_t period_time;
 	atomic_t running;
 	struct hrtimer timer;
-	struct tasklet_struct tasklet;
 	struct snd_pcm_substream *substream;
 };
 
-static void dummy_hrtimer_pcm_elapsed(unsigned long priv)
-{
-	struct dummy_hrtimer_pcm *dpcm = (struct dummy_hrtimer_pcm *)priv;
-	if (atomic_read(&dpcm->running))
-		snd_pcm_period_elapsed(dpcm->substream);
-}
-
 static enum hrtimer_restart dummy_hrtimer_callback(struct hrtimer *timer)
 {
 	struct dummy_hrtimer_pcm *dpcm;
@@ -394,7 +386,14 @@ static enum hrtimer_restart dummy_hrtime
 	dpcm = container_of(timer, struct dummy_hrtimer_pcm, timer);
 	if (!atomic_read(&dpcm->running))
 		return HRTIMER_NORESTART;
-	tasklet_schedule(&dpcm->tasklet);
+	/*
+	 * In cases of XRUN and draining, this calls .trigger to stop PCM
+	 * substream.
+	 */
+	snd_pcm_period_elapsed(dpcm->substream);
+	if (!atomic_read(&dpcm->running))
+		return HRTIMER_NORESTART;
+
 	hrtimer_forward_now(timer, dpcm->period_time);
 	return HRTIMER_RESTART;
 }
@@ -404,7 +403,7 @@ static int dummy_hrtimer_start(struct sn
 	struct dummy_hrtimer_pcm *dpcm = substream->runtime->private_data;
 
 	dpcm->base_time = hrtimer_cb_get_time(&dpcm->timer);
-	hrtimer_start(&dpcm->timer, dpcm->period_time, HRTIMER_MODE_REL);
+	hrtimer_start(&dpcm->timer, dpcm->period_time, HRTIMER_MODE_REL_SOFT);
 	atomic_set(&dpcm->running, 1);
 	return 0;
 }
@@ -414,14 +413,14 @@ static int dummy_hrtimer_stop(struct snd
 	struct dummy_hrtimer_pcm *dpcm = substream->runtime->private_data;
 
 	atomic_set(&dpcm->running, 0);
-	hrtimer_cancel(&dpcm->timer);
+	if (!hrtimer_callback_running(&dpcm->timer))
+		hrtimer_cancel(&dpcm->timer);
 	return 0;
 }
 
 static inline void dummy_hrtimer_sync(struct dummy_hrtimer_pcm *dpcm)
 {
 	hrtimer_cancel(&dpcm->timer);
-	tasklet_kill(&dpcm->tasklet);
 }
 
 static snd_pcm_uframes_t
@@ -466,12 +465,10 @@ static int dummy_hrtimer_create(struct s
 	if (!dpcm)
 		return -ENOMEM;
 	substream->runtime->private_data = dpcm;
-	hrtimer_init(&dpcm->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	hrtimer_init(&dpcm->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
 	dpcm->timer.function = dummy_hrtimer_callback;
 	dpcm->substream = substream;
 	atomic_set(&dpcm->running, 0);
-	tasklet_init(&dpcm->tasklet, dummy_hrtimer_pcm_elapsed,
-		     (unsigned long)dpcm);
 	return 0;
 }
 
Reviewed-by on v2 34/37
#43 lore raw

Re: [PATCH v2 34/37] ALSA/dummy: Replace tasklet with softirq hrtimer

Quoted 7 lines · Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

The tasklet is used to defer the execution of snd_pcm_period_elapsed() to the softirq context. Using the HRTIMER_MODE_SOFT mode invokes the timer callback in softirq context as well which renders the tasklet useless.

Quoted 6 lines

[o-takashi: avoid stall due to a call of hrtimer_cancel() on a callback
of hrtimer]

Feel free to take my ack:

thanks,

Takashi

Quoted 110 lines

---

Quoted 8 lines · Takashi Sakamoto

As Iwai-san mentioned, in this function, .trigger can be called in two cases; XRUN occurs and draining is done. Thus, let you change the comment as 'In cases of XRUN and draining, this calls .trigger to stop PCM substream.'. I'm sorry to trouble you.

snd_pcm_period_elapsed()
->snd_pcm_update_hw_ptr0()
->snd_pcm_update_state()

Quoted 8 lines
    ->snd_pcm_drain_done()
      ...
      ->.trigger(TRIGGER_STOP)
    ->xrun()
      ->snd_pcm_stop()
        ...
        ->.trigger(TRIGGER_STOP)

I think you asked me just to update the comment. Did I do it right?

v2...v3: updated the comment as per Takashi Sakamoto's suggestion. v1...v2: merged Takashi Sakamoto fixup of the original patch into v2.

 sound/drivers/dummy.c |   27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)
sound/drivers/dummy.c +12 -15
@@ -376,17 +376,9 @@ struct dummy_hrtimer_pcm {
 	ktime_t period_time;
 	atomic_t running;
 	struct hrtimer timer;
-	struct tasklet_struct tasklet;
 	struct snd_pcm_substream *substream;
 };
 
-static void dummy_hrtimer_pcm_elapsed(unsigned long priv)
-{
-	struct dummy_hrtimer_pcm *dpcm = (struct dummy_hrtimer_pcm *)priv;
-	if (atomic_read(&dpcm->running))
-		snd_pcm_period_elapsed(dpcm->substream);
-}
-
 static enum hrtimer_restart dummy_hrtimer_callback(struct hrtimer *timer)
 {
 	struct dummy_hrtimer_pcm *dpcm;
@@ -394,7 +386,14 @@ static enum hrtimer_restart dummy_hrtime
 	dpcm = container_of(timer, struct dummy_hrtimer_pcm, timer);
 	if (!atomic_read(&dpcm->running))
 		return HRTIMER_NORESTART;
-	tasklet_schedule(&dpcm->tasklet);
+	/*
+	 * In cases of XRUN and draining, this calls .trigger to stop PCM
+	 * substream.
+	 */
+	snd_pcm_period_elapsed(dpcm->substream);
+	if (!atomic_read(&dpcm->running))
+		return HRTIMER_NORESTART;
+
 	hrtimer_forward_now(timer, dpcm->period_time);
 	return HRTIMER_RESTART;
 }
@@ -404,7 +403,7 @@ static int dummy_hrtimer_start(struct sn
 	struct dummy_hrtimer_pcm *dpcm = substream->runtime->private_data;
 
 	dpcm->base_time = hrtimer_cb_get_time(&dpcm->timer);
-	hrtimer_start(&dpcm->timer, dpcm->period_time, HRTIMER_MODE_REL);
+	hrtimer_start(&dpcm->timer, dpcm->period_time, HRTIMER_MODE_REL_SOFT);
 	atomic_set(&dpcm->running, 1);
 	return 0;
 }
@@ -414,14 +413,14 @@ static int dummy_hrtimer_stop(struct snd
 	struct dummy_hrtimer_pcm *dpcm = substream->runtime->private_data;
 
 	atomic_set(&dpcm->running, 0);
-	hrtimer_cancel(&dpcm->timer);
+	if (!hrtimer_callback_running(&dpcm->timer))
+		hrtimer_cancel(&dpcm->timer);
 	return 0;
 }
 
 static inline void dummy_hrtimer_sync(struct dummy_hrtimer_pcm *dpcm)
 {
 	hrtimer_cancel(&dpcm->timer);
-	tasklet_kill(&dpcm->tasklet);
 }
 
 static snd_pcm_uframes_t
@@ -466,12 +465,10 @@ static int dummy_hrtimer_create(struct s
 	if (!dpcm)
 		return -ENOMEM;
 	substream->runtime->private_data = dpcm;
-	hrtimer_init(&dpcm->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	hrtimer_init(&dpcm->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
 	dpcm->timer.function = dummy_hrtimer_callback;
 	dpcm->substream = substream;
 	atomic_set(&dpcm->running, 0);
-	tasklet_init(&dpcm->tasklet, dummy_hrtimer_pcm_elapsed,
-		     (unsigned long)dpcm);
 	return 0;
 }
 
author maintainer
#44 lore raw

PATCH v2 36/37 usb/gadget/NCM: Replace tasklet with softirq hrtimer

In reply to Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

The tx_tasklet tasklet is used in invoke the hrtimer (task_timer) in softirq context. This can be also achieved without the tasklet but with HRTIMER_MODE_SOFT as hrtimer mode.

---
 drivers/usb/gadget/function/f_ncm.c |   30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)
drivers/usb/gadget/function/f_ncm.c +7 -23
@@ -77,9 +77,7 @@ struct f_ncm {
 	struct sk_buff			*skb_tx_ndp;
 	u16				ndp_dgram_count;
 	bool				timer_force_tx;
-	struct tasklet_struct		tx_tasklet;
 	struct hrtimer			task_timer;
-
 	bool				timer_stopping;
 };
 
@@ -1108,7 +1106,7 @@ static struct sk_buff *ncm_wrap_ntb(stru
 
 		/* Delay the timer. */
 		hrtimer_start(&ncm->task_timer, TX_TIMEOUT_NSECS,
-			      HRTIMER_MODE_REL);
+			      HRTIMER_MODE_REL_SOFT);
 
 		/* Add the datagram position entries */
 		ntb_ndp = skb_put_zero(ncm->skb_tx_ndp, dgram_idx_len);
@@ -1152,17 +1150,15 @@ static struct sk_buff *ncm_wrap_ntb(stru
 }
 
 /*
- * This transmits the NTB if there are frames waiting.
+ * The transmit should only be run if no skb data has been sent
+ * for a certain duration.
  */
-static void ncm_tx_tasklet(unsigned long data)
+static enum hrtimer_restart ncm_tx_timeout(struct hrtimer *data)
 {
-	struct f_ncm	*ncm = (void *)data;
-
-	if (ncm->timer_stopping)
-		return;
+	struct f_ncm *ncm = container_of(data, struct f_ncm, task_timer);
 
 	/* Only send if data is available. */
-	if (ncm->skb_tx_data) {
+	if (!ncm->timer_stopping && ncm->skb_tx_data) {
 		ncm->timer_force_tx = true;
 
 		/* XXX This allowance of a NULL skb argument to ndo_start_xmit
@@ -1175,16 +1171,6 @@ static void ncm_tx_tasklet(unsigned long
 
 		ncm->timer_force_tx = false;
 	}
-}
-
-/*
- * The transmit should only be run if no skb data has been sent
- * for a certain duration.
- */
-static enum hrtimer_restart ncm_tx_timeout(struct hrtimer *data)
-{
-	struct f_ncm *ncm = container_of(data, struct f_ncm, task_timer);
-	tasklet_schedule(&ncm->tx_tasklet);
 	return HRTIMER_NORESTART;
 }
 
@@ -1517,8 +1503,7 @@ static int ncm_bind(struct usb_configura
 	ncm->port.open = ncm_open;
 	ncm->port.close = ncm_close;
 
-	tasklet_init(&ncm->tx_tasklet, ncm_tx_tasklet, (unsigned long) ncm);
-	hrtimer_init(&ncm->task_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	hrtimer_init(&ncm->task_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
 	ncm->task_timer.function = ncm_tx_timeout;
 
 	DBG(cdev, "CDC Network: %s speed IN/%s OUT/%s NOTIFY/%s\n",
@@ -1627,7 +1612,6 @@ static void ncm_unbind(struct usb_config
 	DBG(c->cdev, "ncm unbind\n");
 
 	hrtimer_cancel(&ncm->task_timer);
-	tasklet_kill(&ncm->tx_tasklet);
 
 	ncm_string_defs[0].id = 0;
 	usb_free_all_descriptors(f);
author maintainer
#45 lore raw

PATCH v2 37/37 net/mvpp2: Replace tasklet with softirq hrtimer

In reply to Anna-Maria Gleixner

From: Thomas Gleixner <[email protected]>

The tx_done_tasklet tasklet is used in invoke the hrtimer (mvpp2_hr_timer_cb) in softirq context. This can be also achieved without the tasklet but with HRTIMER_MODE_SOFT as hrtimer mode.

---
 drivers/net/ethernet/marvell/mvpp2.c |   62 ++++++++++++++---------------------
 1 file changed, 25 insertions(+), 37 deletions(-)
drivers/net/ethernet/marvell/mvpp2.c +25 -37
@@ -830,9 +830,8 @@ struct mvpp2_pcpu_stats {
 /* Per-CPU port control */
 struct mvpp2_port_pcpu {
 	struct hrtimer tx_done_timer;
+	struct net_device *dev;
 	bool timer_scheduled;
-	/* Tasklet for egress finalization */
-	struct tasklet_struct tx_done_tasklet;
 };
 
 struct mvpp2_queue_vector {
@@ -5973,46 +5972,34 @@ static void mvpp2_link_event(struct net_
 	}
 }
 
-static void mvpp2_timer_set(struct mvpp2_port_pcpu *port_pcpu)
-{
-	ktime_t interval;
-
-	if (!port_pcpu->timer_scheduled) {
-		port_pcpu->timer_scheduled = true;
-		interval = MVPP2_TXDONE_HRTIMER_PERIOD_NS;
-		hrtimer_start(&port_pcpu->tx_done_timer, interval,
-			      HRTIMER_MODE_REL_PINNED);
-	}
-}
-
-static void mvpp2_tx_proc_cb(unsigned long data)
+static enum hrtimer_restart mvpp2_hr_timer_cb(struct hrtimer *timer)
 {
-	struct net_device *dev = (struct net_device *)data;
-	struct mvpp2_port *port = netdev_priv(dev);
-	struct mvpp2_port_pcpu *port_pcpu = this_cpu_ptr(port->pcpu);
+	struct net_device *dev;
+	struct mvpp2_port *port;
+	struct mvpp2_port_pcpu *port_pcpu;
 	unsigned int tx_todo, cause;
 
+	port_pcpu = container_of(timer, struct mvpp2_port_pcpu, tx_done_timer);
+	dev = port_pcpu->dev;
+
 	if (!netif_running(dev))
-		return;
+		return HRTIMER_NORESTART;
+
 	port_pcpu->timer_scheduled = false;
+	port = netdev_priv(dev);
 
 	/* Process all the Tx queues */
 	cause = (1 << port->ntxqs) - 1;
 	tx_todo = mvpp2_tx_done(port, cause, smp_processor_id());
 
 	/* Set the timer in case not all the packets were processed */
-	if (tx_todo)
-		mvpp2_timer_set(port_pcpu);
-}
-
-static enum hrtimer_restart mvpp2_hr_timer_cb(struct hrtimer *timer)
-{
-	struct mvpp2_port_pcpu *port_pcpu = container_of(timer,
-							 struct mvpp2_port_pcpu,
-							 tx_done_timer);
-
-	tasklet_schedule(&port_pcpu->tx_done_tasklet);
+	if (tx_todo && !port_pcpu->timer_scheduled) {
+		port_pcpu->timer_scheduled = true;
+		hrtimer_forward_now(&port_pcpu->tx_done_timer,
+				    MVPP2_TXDONE_HRTIMER_PERIOD_NS);
 
+		return HRTIMER_RESTART;
+	}
 	return HRTIMER_NORESTART;
 }
 
@@ -6498,7 +6485,12 @@ static int mvpp2_tx(struct sk_buff *skb,
 	    txq_pcpu->count > 0) {
 		struct mvpp2_port_pcpu *port_pcpu = this_cpu_ptr(port->pcpu);
 
-		mvpp2_timer_set(port_pcpu);
+		if (!port_pcpu->timer_scheduled) {
+			port_pcpu->timer_scheduled = true;
+			hrtimer_start(&port_pcpu->tx_done_timer,
+				      MVPP2_TXDONE_HRTIMER_PERIOD_NS,
+				      HRTIMER_MODE_REL_PINNED_SOFT);
+		}
 	}
 
 	return NETDEV_TX_OK;
@@ -6883,7 +6875,6 @@ static int mvpp2_stop(struct net_device
 
 			hrtimer_cancel(&port_pcpu->tx_done_timer);
 			port_pcpu->timer_scheduled = false;
-			tasklet_kill(&port_pcpu->tx_done_tasklet);
 		}
 	}
 	mvpp2_cleanup_rxqs(port);
@@ -7651,13 +7642,10 @@ static int mvpp2_port_probe(struct platf
 			port_pcpu = per_cpu_ptr(port->pcpu, cpu);
 
 			hrtimer_init(&port_pcpu->tx_done_timer, CLOCK_MONOTONIC,
-				     HRTIMER_MODE_REL_PINNED);
+				     HRTIMER_MODE_REL_PINNED_SOFT);
 			port_pcpu->tx_done_timer.function = mvpp2_hr_timer_cb;
 			port_pcpu->timer_scheduled = false;
-
-			tasklet_init(&port_pcpu->tx_done_tasklet,
-				     mvpp2_tx_proc_cb,
-				     (unsigned long)dev);
+			port_pcpu->dev = dev;
 		}
 	}
 
#46 lore raw
In reply to Anna-Maria Gleixner
Quoted 6 lines · Anna-Maria Gleixner

This series implements a new approach by adding SOFT hrtimer mode and instead of doing the list shuffle, timers started with this mode are put into separate soft expiry hrtimer queues. These queues are evaluated only when the hardirq context detects that the first expiring timer in the softirq queues has expired. That makes the overhead in the hardirq context minimal.

Thanks!

1-29,33: Acked-by: Peter Zijlstra (Intel) <[email protected]>