hrtimers: Consolidate hrtimer initialization - Part 1

PATCH RESEND In review Nam Cao 3 participants 22 messages Oct 31, 2024 – Nov 4, 2024

Subsystems (15)

Networking DRM drivers x86 VFS +11 more subsystems

lore

Share & tools

lore

Nam Cao

author

2024-10-31 15:15 UTC #0 lore raw

v2 cover hrtimers: Consolidate hrtimer initialization - Part 1

(resend due to broken emails)

This is a follow up to version 1, which can be found here:

    https://lore.kernel.org/lkml/[email protected]/

hrtimers must be initialized with a hrtimer_init() variant, and after that the timer's callback function must be setup separately.

This separate initialization is error prone and awkward to use. The separate initialization is also problematic for a clean Rust abstraction.

A combined setup function like timer_setup() is less error prone and simpler to use.

This first part of the conversion provides:

- a set of hrtimer_setup*() variants, which take the function pointer as argument.

- hrtimer_update_function() which allows to change the callback function after initialization with the proper safety checks in place.

- conversion of the hrtimer_init*_on_stack() variants

- some minor cleanups

The remaining users will be converted in follow up series.

Most conversions were done with Coccinelle. See sematic patch below.

Changes versus v1:
- Open code kvm_xen_init_vcpu() (Sean)
- Drop the can/bcm patch (Oliver)
- Folded the removal of hrtimer_init_sleeper() (tglx)
- Update change logs and cover letter

The series applies on top of:

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

and is also available from git:

git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git hrtimer-setup-part1-v2

Best regards,

Nam

---
virtual patch
@@ expression timer, clock, mode, func; @@
- hrtimer_init(timer, clock, mode);
...
- timer->function = func;
+ hrtimer_setup(timer, func, clock, mode);

@@ expression timer, clock, mode, func; @@
- hrtimer_init(&timer, clock, mode);
...
- timer.function = func;
+ hrtimer_setup(&timer, func, clock, mode);

@@ expression timer, clock, mode, func; @@
- hrtimer_init_on_stack(&timer, clock, mode);
...
- timer.function = func;
+ hrtimer_setup_on_stack(&timer, func, clock, mode);

@@ expression timer, clock, mode; @@
- hrtimer_init_sleeper_on_stack(timer, clock, mode);
+ hrtimer_setup_sleeper_on_stack(timer, clock, mode);

---
 arch/x86/kvm/xen.c                             |  12 +--
 drivers/gpu/drm/i915/i915_request.c            |  17 ++--
 drivers/net/wireless/ralink/rt2x00/rt2x00usb.c |   2 -
 fs/aio.c                                       |   2 +-
 include/linux/hrtimer.h                        |  51 +++++++-----
 include/linux/wait.h                           |   4 +-
 io_uring/io_uring.c                            |   7 +-
 io_uring/timeout.c                             |   1 -
 kernel/futex/core.c                            |   6 +-
 kernel/sched/idle.c                            |   4 +-
 kernel/time/alarmtimer.c                       |   9 +-
 kernel/time/hrtimer.c                          | 110 ++++++++++++++++++-------
 kernel/time/sleep_timeout.c                    |   2 +-
 net/core/pktgen.c                              |   2 +-
 14 files changed, 136 insertions(+), 93 deletions(-)

Nam Cao

author

2024-10-31 15:15 UTC #1 lore raw

PATCH RESEND v2 1/19 hrtimers: Add missing hrtimer_init() trace points

hrtimer_init*_on_stack() is not covered by tracing when CONFIG_DEBUG_OBJECTS_TIMERS=y.

Rework the functions similar to hrtimer_init() and hrtimer_init_sleeper() so that the hrtimer_init() tracepoint is unconditionally available.

The rework makes hrtimer_init_sleeper() unused. Delete it.

Signed-off-by Nam Cao <[email protected]>

---
 include/linux/hrtimer.h | 19 +-----------
 kernel/time/hrtimer.c   | 65 +++++++++++++++++++++++------------------
 2 files changed, 37 insertions(+), 47 deletions(-)

include/linux/hrtimer.h +1 ~~-18~~

index aa1e65ccb615..5aa9d57528c4 100644
@@ -228,32 +228,15 @@ static inline void hrtimer_cancel_wait_running(struct hrtimer *timer)
 /* Initialize timers: */
 extern void hrtimer_init(struct hrtimer *timer, clockid_t which_clock,
 			 enum hrtimer_mode mode);
-extern void hrtimer_init_sleeper(struct hrtimer_sleeper *sl, clockid_t clock_id,
-				 enum hrtimer_mode mode);
-
-#ifdef CONFIG_DEBUG_OBJECTS_TIMERS
 extern void hrtimer_init_on_stack(struct hrtimer *timer, clockid_t which_clock,
 				  enum hrtimer_mode mode);
 extern void hrtimer_init_sleeper_on_stack(struct hrtimer_sleeper *sl,
 					  clockid_t clock_id,
 					  enum hrtimer_mode mode);
 
+#ifdef CONFIG_DEBUG_OBJECTS_TIMERS
 extern void destroy_hrtimer_on_stack(struct hrtimer *timer);
 #else
-static inline void hrtimer_init_on_stack(struct hrtimer *timer,
-					 clockid_t which_clock,
-					 enum hrtimer_mode mode)
-{
-	hrtimer_init(timer, which_clock, mode);
-}
-
-static inline void hrtimer_init_sleeper_on_stack(struct hrtimer_sleeper *sl,
-						 clockid_t clock_id,
-						 enum hrtimer_mode mode)
-{
-	hrtimer_init_sleeper(sl, clock_id, mode);
-}
-
 static inline void destroy_hrtimer_on_stack(struct hrtimer *timer) { }
 #endif

kernel/time/hrtimer.c +36 ~~-29~~

114 lines not on this page · raw message

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #2 lore raw

PATCH RESEND v2 2/19 drm/i915/request: Remove unnecessary modification of hrtimer::function

In reply to Nam Cao

When a request is created, the hrtimer is not initialized and only its 'function' field is set to NULL. The hrtimer is only initialized when the request is enqueued. The point of setting 'function' to NULL is that, it can be used to check whether hrtimer_try_to_cancel() should be called while retiring the request.

This "trick" is unnecessary, because hrtimer_try_to_cancel() already does its own check whether the timer is armed. If the timer is not armed, hrtimer_try_to_cancel() returns 0.

Fully initialize the timer when the request is created, which allows to make the hrtimer::function field private once all users of hrtimer_init() are converted to hrtimer_setup(), which requires a valid callback function to be set.

Because hrtimer_try_to_cancel() returns 0 if the timer is not armed, the logic to check whether to call i915_request_put() remains equivalent.

Signed-off-by Nam Cao <[email protected]>

Cc Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/i915_request.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

drivers/gpu/drm/i915/i915_request.c +9 -8

index 519e096c607c..8f62cfa23fb7 100644
@@ -273,11 +273,6 @@ i915_request_active_engine(struct i915_request *rq,
 	return ret;
 }
 
-static void __rq_init_watchdog(struct i915_request *rq)
-{
-	rq->watchdog.timer.function = NULL;
-}
-
 static enum hrtimer_restart __rq_watchdog_expired(struct hrtimer *hrtimer)
 {
 	struct i915_request *rq =

@@ -294,6 +289,14 @@ static enum hrtimer_restart __rq_watchdog_expired(struct hrtimer *hrtimer)
 	return HRTIMER_NORESTART;
 }
 
+static void __rq_init_watchdog(struct i915_request *rq)
+{
+	struct i915_request_watchdog *wdg = &rq->watchdog;
+
+	hrtimer_init(&wdg->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	wdg->timer.function = __rq_watchdog_expired;
+}
+
 static void __rq_arm_watchdog(struct i915_request *rq)
 {
 	struct i915_request_watchdog *wdg = &rq->watchdog;

@@ -304,8 +307,6 @@ static void __rq_arm_watchdog(struct i915_request *rq)
 
 	i915_request_get(rq);
 
-	hrtimer_init(&wdg->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
-	wdg->timer.function = __rq_watchdog_expired;
 	hrtimer_start_range_ns(&wdg->timer,
 			       ns_to_ktime(ce->watchdog.timeout_us *
 					   NSEC_PER_USEC),

@@ -317,7 +318,7 @@ static void __rq_cancel_watchdog(struct i915_request *rq)
 {
 	struct i915_request_watchdog *wdg = &rq->watchdog;
 
-	if (wdg->timer.function && hrtimer_try_to_cancel(&wdg->timer) > 0)
+	if (hrtimer_try_to_cancel(&wdg->timer) > 0)
 		i915_request_put(rq);
 }

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #3 lore raw

PATCH RESEND v2 3/19 KVM: x86/xen: Initialize hrtimer in kvm_xen_init_vcpu()

In reply to Nam Cao

The hrtimer is initialized in the KVM_XEN_VCPU_SET_ATTR ioctl. That caused problem in the past, because the hrtimer can be initialized multiple times, which was fixed by commit af735db31285 ("KVM: x86/xen: Initialize Xen timer only once"). This commit avoids initializing the timer multiple times by checking the field 'function' of struct hrtimer to determine if it has already been initialized.

This is not required and in the way to make the function field private.

Move the hrtimer initialization into kvm_xen_init_vcpu() so that it will only be initialized once.

Signed-off-by Nam Cao <[email protected]>

Acked-by Sean Christopherson <[email protected]>

Cc Paolo Bonzini <[email protected]>

Cc [email protected]

---
 arch/x86/kvm/xen.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

arch/x86/kvm/xen.c +2 ~~-10~~

index 622fe24da910..a909b817b9c0 100644
@@ -263,13 +263,6 @@ static void kvm_xen_stop_timer(struct kvm_vcpu *vcpu)
 	atomic_set(&vcpu->arch.xen.timer_pending, 0);
 }
 
-static void kvm_xen_init_timer(struct kvm_vcpu *vcpu)
-{
-	hrtimer_init(&vcpu->arch.xen.timer, CLOCK_MONOTONIC,
-		     HRTIMER_MODE_ABS_HARD);
-	vcpu->arch.xen.timer.function = xen_timer_callback;
-}
-
 static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
 {
 	struct kvm_vcpu_xen *vx = &v->arch.xen;

@@ -1070,9 +1063,6 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 			break;
 		}
 
-		if (!vcpu->arch.xen.timer.function)
-			kvm_xen_init_timer(vcpu);
-
 		/* Stop the timer (if it's running) before changing the vector */
 		kvm_xen_stop_timer(vcpu);
 		vcpu->arch.xen.timer_virq = data->u.timer.port;

@@ -2235,6 +2225,8 @@ void kvm_xen_init_vcpu(struct kvm_vcpu *vcpu)
 	vcpu->arch.xen.poll_evtchn = 0;
 
 	timer_setup(&vcpu->arch.xen.poll_timer, cancel_evtchn_poll, 0);
+	hrtimer_init(&vcpu->arch.xen.timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_HARD);
+	vcpu->arch.xen.timer.function = xen_timer_callback;
 
 	kvm_gpc_init(&vcpu->arch.xen.runstate_cache, vcpu->kvm);
 	kvm_gpc_init(&vcpu->arch.xen.runstate2_cache, vcpu->kvm);

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #4 lore raw

PATCH RESEND v2 4/19 wifi: rt2x00: Remove redundant hrtimer_init()

In reply to Nam Cao

rt2x00usb_probe() executes a hrtimer_init() for txstatus_timer. Afterwards, rt2x00lib_probe_dev() is called which also initializes this txstatus_timer with the same settings.

Remove the redundant hrtimer_init() call in rt2x00usb_probe().

Signed-off-by Nam Cao <[email protected]>

Acked-by Kalle Valo <[email protected]>

Cc [email protected]

---
 drivers/net/wireless/ralink/rt2x00/rt2x00usb.c | 2 --
 1 file changed, 2 deletions(-)

drivers/net/wireless/ralink/rt2x00/rt2x00usb.c +0 -2

index 8fd22c69855f..a6d50149e0c3 100644
@@ -823,8 +823,6 @@ int rt2x00usb_probe(struct usb_interface *usb_intf,
 
 	INIT_WORK(&rt2x00dev->rxdone_work, rt2x00usb_work_rxdone);
 	INIT_WORK(&rt2x00dev->txdone_work, rt2x00usb_work_txdone);
-	hrtimer_init(&rt2x00dev->txstatus_timer, CLOCK_MONOTONIC,
-		     HRTIMER_MODE_REL);
 
 	retval = rt2x00usb_alloc_reg(rt2x00dev);
 	if (retval)

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #5 lore raw

PATCH RESEND v2 5/19 io_uring: Remove redundant hrtimer's callback function setup

In reply to Nam Cao

The IORING_OP_TIMEOUT command uses hrtimer underneath. The timer's callback function is setup in io_timeout(), and then the callback function is setup again when the timer is rearmed.

Since the callback function is the same for both cases, the latter setup is redundant, therefore remove it.

Signed-off-by Nam Cao <[email protected]>

Cc Jens Axboe <[email protected]>

---
 io_uring/timeout.c | 1 -
 1 file changed, 1 deletion(-)

io_uring/timeout.c +0 -1

index 9973876d91b0..2ffe5e1dc68a 100644
@@ -76,7 +76,6 @@ static void io_timeout_complete(struct io_kiocb *req, struct io_tw_state *ts)
 			/* re-arm timer */
 			spin_lock_irq(&ctx->timeout_lock);
 			list_add(&timeout->list, ctx->timeout_list.prev);
-			data->timer.function = io_timeout_fn;
 			hrtimer_start(&data->timer, timespec64_to_ktime(data->ts), data->mode);
 			spin_unlock_irq(&ctx->timeout_lock);
 			return;

Signature · 1 line

2.39.5

Jens Axboe

maintainer Reviewed-by on v2 5/19

2024-10-31 15:21 UTC #6 lore raw

Re: [RESEND PATCH v2 05/19] io_uring: Remove redundant hrtimer's callback function setup

Quoted 18 lines · Nam Cao

The IORING_OP_TIMEOUT command uses hrtimer underneath. The timer's callback function is setup in io_timeout(), and then the callback function is setup again when the timer is rearmed.
Since the callback function is the same for both cases, the latter setup is redundant, therefore remove it.
Signed-off-by Nam Cao <[email protected]>
Cc Jens Axboe <[email protected]>
---
 io_uring/timeout.c | 1 -
 1 file changed, 1 deletion(-)
io_uring/timeout.c +0 -0
index 9973876d91b0..2ffe5e1dc68a 100644

io_uring/timeout.c static void io_timeout_complete(struct io_kiocb *req, struct io_tw_state *ts) · line 81

Quoted 7 lines

io_uring/timeout.c +0 -1

@@ -76,7 +76,6 @@ static void io_timeout_complete(struct io_kiocb *req, struct io_tw_state *ts)
 			/* re-arm timer */
 			spin_lock_irq(&ctx->timeout_lock);
 			list_add(&timeout->list, ctx->timeout_list.prev);
-			data->timer.function = io_timeout_fn;
 			hrtimer_start(&data->timer, timespec64_to_ktime(data->ts), data->mode);
 			spin_unlock_irq(&ctx->timeout_lock);
 			return;

Should be fine, io_timeout_complete() is only used for non-linked timeouts.

Reviewed-by Jens Axboe <[email protected]:

Signature · 1 line

Jens Axboe

Nam Cao

author

2024-10-31 15:15 UTC #7 lore raw

PATCH RESEND v2 6/19 hrtimers: Introduce hrtimer_setup() to replace hrtimer_init()

In reply to Nam Cao

To initialize hrtimer, hrtimer_init() needs to be called and also hrtimer::function must be set. This is error-prone and awkward to use.

Introduce hrtimer_setup() which does both of these things, so that users of hrtimer can be simplified.

The new setup function also has a sanity check for the provided function pointer. If NULL, a warning is emitted and a dummy callback installed.

hrtimer_init() will be removed as soon as all of its users have been converted to the new function.

Signed-off-by Nam Cao <[email protected]>

---
 include/linux/hrtimer.h |  2 ++
 kernel/time/hrtimer.c   | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

include/linux/hrtimer.h +2 -0

index 5aa9d57528c4..bcc0715c59a8 100644
@@ -228,6 +228,8 @@ static inline void hrtimer_cancel_wait_running(struct hrtimer *timer)
 /* Initialize timers: */
 extern void hrtimer_init(struct hrtimer *timer, clockid_t which_clock,
 			 enum hrtimer_mode mode);
+extern void hrtimer_setup(struct hrtimer *timer, enum hrtimer_restart (*function)(struct hrtimer *),
+			  clockid_t clock_id, enum hrtimer_mode mode);
 extern void hrtimer_init_on_stack(struct hrtimer *timer, clockid_t which_clock,
 				  enum hrtimer_mode mode);
 extern void hrtimer_init_sleeper_on_stack(struct hrtimer_sleeper *sl,

kernel/time/hrtimer.c +38 -0

index 4b0507cf38ea..a5ef67edcda9 100644
@@ -1535,6 +1535,11 @@ static inline int hrtimer_clockid_to_base(clockid_t clock_id)
 	return HRTIMER_BASE_MONOTONIC;
 }
 
+static enum hrtimer_restart hrtimer_dummy_timeout(struct hrtimer *unused)
+{
+	return HRTIMER_NORESTART;
+}
+
 static void __hrtimer_init(struct hrtimer *timer, clockid_t clock_id,
 			   enum hrtimer_mode mode)
 {

@@ -1571,6 +1576,18 @@ static void __hrtimer_init(struct hrtimer *timer, clockid_t clock_id,
 	timerqueue_init(&timer->node);
 }
 
+static void __hrtimer_setup(struct hrtimer *timer,
+			    enum hrtimer_restart (*function)(struct hrtimer *),
+			    clockid_t clock_id, enum hrtimer_mode mode)
+{
+	__hrtimer_init(timer, clock_id, mode);
+
+	if (WARN_ON_ONCE(!function))
+		timer->function = hrtimer_dummy_timeout;
+	else
+		timer->function = function;
+}
+
 /**
  * hrtimer_init - initialize a timer to the given clock
  * @timer:	the timer to be initialized

@@ -1591,6 +1608,27 @@ void hrtimer_init(struct hrtimer *timer, clockid_t clock_id,
 }
 EXPORT_SYMBOL_GPL(hrtimer_init);
 
+/**
+ * hrtimer_setup - initialize a timer to the given clock
+ * @timer:	the timer to be initialized
+ * @function:	the callback function
+ * @clock_id:	the clock to be used
+ * @mode:       The modes which are relevant for initialization:
+ *              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_setup(struct hrtimer *timer, enum hrtimer_restart (*function)(struct hrtimer *),
+		   clockid_t clock_id, enum hrtimer_mode mode)
+{
+	debug_init(timer, clock_id, mode);
+	__hrtimer_setup(timer, function, clock_id, mode);
+}
+EXPORT_SYMBOL_GPL(hrtimer_setup);
+
 /**
  * hrtimer_init_on_stack - initialize a timer in stack memory
  * @timer:	The timer to be initialized

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #8 lore raw

PATCH RESEND v2 7/19 hrtimers: Introduce hrtimer_setup_on_stack()

In reply to Nam Cao

To initialize hrtimer on stack, hrtimer_init_on_stack() needs to be called and also hrtimer::function must be set. This is error-prone and awkward to use.

Introduce hrtimer_setup_on_stack() which does both of these things, so that users of hrtimer can be simplified.

The new setup function also has a sanity check for the provided function pointer. If NULL, a warning is emitted and a dummy callback installed.

hrtimer_init_on_stack() will be removed as soon as all of its users have been converted to the new function.

Signed-off-by Nam Cao <[email protected]>

---
 include/linux/hrtimer.h |  3 +++
 kernel/time/hrtimer.c   | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+)

include/linux/hrtimer.h +3 -0

index bcc0715c59a8..2da513f8d66a 100644
@@ -232,6 +232,9 @@ extern void hrtimer_setup(struct hrtimer *timer, enum hrtimer_restart (*function
 			  clockid_t clock_id, enum hrtimer_mode mode);
 extern void hrtimer_init_on_stack(struct hrtimer *timer, clockid_t which_clock,
 				  enum hrtimer_mode mode);
+extern void hrtimer_setup_on_stack(struct hrtimer *timer,
+				   enum hrtimer_restart (*function)(struct hrtimer *),
+				   clockid_t clock_id, enum hrtimer_mode mode);
 extern void hrtimer_init_sleeper_on_stack(struct hrtimer_sleeper *sl,
 					  clockid_t clock_id,
 					  enum hrtimer_mode mode);

kernel/time/hrtimer.c +19 -0

index a5ef67edcda9..daee4e27f839 100644
@@ -1646,6 +1646,25 @@ void hrtimer_init_on_stack(struct hrtimer *timer, clockid_t clock_id,
 }
 EXPORT_SYMBOL_GPL(hrtimer_init_on_stack);
 
+/**
+ * hrtimer_setup_on_stack - initialize a timer on stack memory
+ * @timer:	The timer to be initialized
+ * @function:	the callback function
+ * @clock_id:	The clock to be used
+ * @mode:       The timer mode
+ *
+ * Similar to hrtimer_setup(), except that this one must be used if struct hrtimer is in stack
+ * memory.
+ */
+void hrtimer_setup_on_stack(struct hrtimer *timer,
+			    enum hrtimer_restart (*function)(struct hrtimer *),
+			    clockid_t clock_id, enum hrtimer_mode mode)
+{
+	debug_init_on_stack(timer, clock_id, mode);
+	__hrtimer_setup(timer, function, clock_id, mode);
+}
+EXPORT_SYMBOL_GPL(hrtimer_setup_on_stack);
+
 /*
  * A timer is active, when it is enqueued into the rbtree or the
  * callback function is running or it's in the state of being migrated

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #9 lore raw

PATCH RESEND v2 8/19 hrtimers: Introduce hrtimer_setup_sleeper_on_stack()

In reply to Nam Cao

The hrtimer_init*() API is replaced by hrtimer_setup*() variants to initialize the timer including the callback function at once.

hrtimer_init_sleeper_on_stack() does not need user to setup the callback function separately, so a new variant would not be strictly necessary.

Nonetheless, to keep the naming convention consistent, introduce hrtimer_setup_sleeper_on_stack(). hrtimer_init_on_stack() will be removed once all users are converted.

Signed-off-by Nam Cao <[email protected]>

---
 include/linux/hrtimer.h |  2 ++
 kernel/time/hrtimer.c   | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

include/linux/hrtimer.h +2 -0

index 2da513f8d66a..48872a2b4071 100644
@@ -238,6 +238,8 @@ extern void hrtimer_setup_on_stack(struct hrtimer *timer,
 extern void hrtimer_init_sleeper_on_stack(struct hrtimer_sleeper *sl,
 					  clockid_t clock_id,
 					  enum hrtimer_mode mode);
+extern void hrtimer_setup_sleeper_on_stack(struct hrtimer_sleeper *sl, clockid_t clock_id,
+					   enum hrtimer_mode mode);
 
 #ifdef CONFIG_DEBUG_OBJECTS_TIMERS
 extern void destroy_hrtimer_on_stack(struct hrtimer *timer);

kernel/time/hrtimer.c +14 -0

index daee4e27f839..1d1f5c03673c 100644
@@ -2065,6 +2065,20 @@ void hrtimer_init_sleeper_on_stack(struct hrtimer_sleeper *sl,
 }
 EXPORT_SYMBOL_GPL(hrtimer_init_sleeper_on_stack);
 
+/**
+ * hrtimer_setup_sleeper_on_stack - initialize a sleeper in stack memory
+ * @sl:		sleeper to be initialized
+ * @clock_id:	the clock to be used
+ * @mode:	timer mode abs/rel
+ */
+void hrtimer_setup_sleeper_on_stack(struct hrtimer_sleeper *sl,
+				    clockid_t clock_id, enum hrtimer_mode mode)
+{
+	debug_init_on_stack(&sl->timer, clock_id, mode);
+	__hrtimer_init_sleeper(sl, clock_id, mode);
+}
+EXPORT_SYMBOL_GPL(hrtimer_setup_sleeper_on_stack);
+
 int nanosleep_copyout(struct restart_block *restart, struct timespec64 *ts)
 {
 	switch(restart->nanosleep.type) {

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #10 lore raw

PATCH RESEND v2 9/19 hrtimers: Introduce hrtimer_update_function()

In reply to Nam Cao

Some users of hrtimer need to change the callback function after the initial setup. They write to hrtimer::function directly.

That's not safe under all circumstances as the write is lockless and a concurrent timer expiry might end up using the wrong function pointer.

Introduce hrtimer_update_function(), which also performs runtime checks whether it is safe to modify the callback.

This allows to make hrtimer::function private once all users are converted.

Signed-off-by Nam Cao <[email protected]>

---
 include/linux/hrtimer.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

include/linux/hrtimer.h +22 -0

index 48872a2b4071..6e026730e803 100644
@@ -327,6 +327,28 @@ static inline int hrtimer_callback_running(struct hrtimer *timer)
 	return timer->base->running == timer;
 }
 
+/**
+ * hrtimer_update_function - Update the timer's callback function
+ * @timer:	Timer to update
+ * @function:	New callback function
+ *
+ * Only safe to call if the timer is not enqueued. Can be called in the callback function if the
+ * timer is not enqueued at the same time (see the comments above HRTIMER_STATE_ENQUEUED).
+ */
+static inline void hrtimer_update_function(struct hrtimer *timer,
+					   enum hrtimer_restart (*function)(struct hrtimer *))
+{
+	guard(raw_spinlock_irqsave)(&timer->base->cpu_base->lock);
+
+	if (WARN_ON_ONCE(hrtimer_is_queued(timer)))
+		return;
+
+	if (WARN_ON_ONCE(!function))
+		return;
+
+	timer->function = function;
+}
+
 /* Forward a hrtimer so it expires after now: */
 extern u64
 hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval);

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #11 lore raw

PATCH RESEND v2 10/19 fs/aio: Switch to use hrtimer_setup_sleeper_on_stack()

In reply to Nam Cao

hrtimer_setup_sleeper_on_stack() replaces hrtimer_init_sleeper_on_stack() to keep the naming convention consistent.

Convert the usage site over to it. The conversion was done with Coccinelle.

Signed-off-by Nam Cao <[email protected]>

Cc Christian Brauner <[email protected]>

---
 fs/aio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/aio.c +1 -1

index e8920178b50f..a5d331f29943 100644
@@ -1335,7 +1335,7 @@ static long read_events(struct kioctx *ctx, long min_nr, long nr,
 	if (until == 0 || ret < 0 || ret >= min_nr)
 		return ret;
 
-	hrtimer_init_sleeper_on_stack(&t, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	hrtimer_setup_sleeper_on_stack(&t, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	if (until != KTIME_MAX) {
 		hrtimer_set_expires_range_ns(&t.timer, until, current->timer_slack_ns);
 		hrtimer_sleeper_start_expires(&t, HRTIMER_MODE_REL);

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #12 lore raw

PATCH RESEND v2 11/19 futex: Switch to use hrtimer_setup_sleeper_on_stack()

In reply to Nam Cao

hrtimer_setup_sleeper_on_stack() replaces hrtimer_init_sleeper_on_stack() to keep the naming convention consistent.

Convert the usage site over to it. The conversion was done with Coccinelle.

Signed-off-by Nam Cao <[email protected]>

Cc Peter Zijlstra <[email protected]>

---
 kernel/futex/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

kernel/futex/core.c +3 -3

index 136768ae2637..fb7214c7a36f 100644
@@ -140,9 +140,9 @@ futex_setup_timer(ktime_t *time, struct hrtimer_sleeper *timeout,
 	if (!time)
 		return NULL;
 
-	hrtimer_init_sleeper_on_stack(timeout, (flags & FLAGS_CLOCKRT) ?
-				      CLOCK_REALTIME : CLOCK_MONOTONIC,
-				      HRTIMER_MODE_ABS);
+	hrtimer_setup_sleeper_on_stack(timeout,
+				       (flags & FLAGS_CLOCKRT) ? CLOCK_REALTIME : CLOCK_MONOTONIC,
+				       HRTIMER_MODE_ABS);
 	/*
 	 * If range_ns is 0, calling hrtimer_set_expires_range_ns() is
 	 * effectively the same as calling hrtimer_set_expires().

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #13 lore raw

PATCH RESEND v2 12/19 net: pktgen: Switch to use hrtimer_setup_sleeper_on_stack()

In reply to Nam Cao

hrtimer_setup_sleeper_on_stack() replaces hrtimer_init_sleeper_on_stack() to keep the naming convention consistent.

Convert the usage site over to it. The conversion was done with Coccinelle.

Signed-off-by Nam Cao <[email protected]>

Cc Jakub Kicinski <[email protected]>

---
 net/core/pktgen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

net/core/pktgen.c +1 -1

index 34f68ef74b8f..7e23cacbe66e 100644
@@ -2285,7 +2285,7 @@ static void spin(struct pktgen_dev *pkt_dev, ktime_t spin_until)
 	s64 remaining;
 	struct hrtimer_sleeper t;
 
-	hrtimer_init_sleeper_on_stack(&t, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
+	hrtimer_setup_sleeper_on_stack(&t, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
 	hrtimer_set_expires(&t.timer, spin_until);
 
 	remaining = ktime_to_ns(hrtimer_expires_remaining(&t.timer));

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #14 lore raw

PATCH RESEND v2 13/19 timers: Switch to use hrtimer_setup_sleeper_on_stack()

In reply to Nam Cao

hrtimer_setup_sleeper_on_stack() replaces hrtimer_init_sleeper_on_stack() to keep the naming convention consistent.

Convert the usage sites over to it. The conversion was done with Coccinelle.

Signed-off-by Nam Cao <[email protected]>

---
 kernel/time/hrtimer.c       | 5 ++---
 kernel/time/sleep_timeout.c | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

kernel/time/hrtimer.c +2 -3

index 1d1f5c03673c..69430467a17d 100644
@@ -2138,8 +2138,7 @@ static long __sched hrtimer_nanosleep_restart(struct restart_block *restart)
 	struct hrtimer_sleeper t;
 	int ret;
 
-	hrtimer_init_sleeper_on_stack(&t, restart->nanosleep.clockid,
-				      HRTIMER_MODE_ABS);
+	hrtimer_setup_sleeper_on_stack(&t, restart->nanosleep.clockid, HRTIMER_MODE_ABS);
 	hrtimer_set_expires_tv64(&t.timer, restart->nanosleep.expires);
 	ret = do_nanosleep(&t, HRTIMER_MODE_ABS);
 	destroy_hrtimer_on_stack(&t.timer);

@@ -2153,7 +2152,7 @@ long hrtimer_nanosleep(ktime_t rqtp, const enum hrtimer_mode mode,
 	struct hrtimer_sleeper t;
 	int ret = 0;
 
-	hrtimer_init_sleeper_on_stack(&t, clockid, mode);
+	hrtimer_setup_sleeper_on_stack(&t, clockid, mode);
 	hrtimer_set_expires_range_ns(&t.timer, rqtp, current->timer_slack_ns);
 	ret = do_nanosleep(&t, mode);
 	if (ret != -ERESTART_RESTARTBLOCK)

kernel/time/sleep_timeout.c +1 -1

index 3054e5232d20..dfe939f6e4ec 100644
@@ -208,7 +208,7 @@ int __sched schedule_hrtimeout_range_clock(ktime_t *expires, u64 delta,
 		return -EINTR;
 	}
 
-	hrtimer_init_sleeper_on_stack(&t, clock_id, mode);
+	hrtimer_setup_sleeper_on_stack(&t, clock_id, mode);
 	hrtimer_set_expires_range_ns(&t.timer, *expires, delta);
 	hrtimer_sleeper_start_expires(&t, mode);

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #15 lore raw

PATCH RESEND v2 14/19 wait: Switch to use hrtimer_setup_sleeper_on_stack()

In reply to Nam Cao

hrtimer_setup_sleeper_on_stack() replaces hrtimer_init_sleeper_on_stack() to keep the naming convention consistent.

Convert the usage site over to it.

Signed-off-by Nam Cao <[email protected]>

Cc Peter Zijlstra <[email protected]>

---
 include/linux/wait.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

include/linux/wait.h +2 -2

index 8aa3372f21a0..643b7c7bf376 100644
@@ -541,8 +541,8 @@ do {										\
 	int __ret = 0;								\
 	struct hrtimer_sleeper __t;						\
 										\
-	hrtimer_init_sleeper_on_stack(&__t, CLOCK_MONOTONIC,			\
-				      HRTIMER_MODE_REL);			\
+	hrtimer_setup_sleeper_on_stack(&__t, CLOCK_MONOTONIC,			\
+				       HRTIMER_MODE_REL);			\
 	if ((timeout) != KTIME_MAX) {						\
 		hrtimer_set_expires_range_ns(&__t.timer, timeout,		\
 					current->timer_slack_ns);		\

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #16 lore raw

PATCH RESEND v2 15/19 hrtimers: Delete hrtimer_init_sleeper_on_stack()

In reply to Nam Cao

hrtimer_init_sleeper_on_stack() is now unused. Delete it.

Signed-off-by Nam Cao <[email protected]>

---
 include/linux/hrtimer.h |  3 ---
 kernel/time/hrtimer.c   | 14 --------------
 2 files changed, 17 deletions(-)

include/linux/hrtimer.h +0 -3

index 6e026730e803..4e4f04b3c0c2 100644
@@ -235,9 +235,6 @@ extern void hrtimer_init_on_stack(struct hrtimer *timer, clockid_t which_clock,
 extern void hrtimer_setup_on_stack(struct hrtimer *timer,
 				   enum hrtimer_restart (*function)(struct hrtimer *),
 				   clockid_t clock_id, enum hrtimer_mode mode);
-extern void hrtimer_init_sleeper_on_stack(struct hrtimer_sleeper *sl,
-					  clockid_t clock_id,
-					  enum hrtimer_mode mode);
 extern void hrtimer_setup_sleeper_on_stack(struct hrtimer_sleeper *sl, clockid_t clock_id,
 					   enum hrtimer_mode mode);

kernel/time/hrtimer.c +0 ~~-14~~

index 69430467a17d..376b8182b72e 100644
@@ -2051,20 +2051,6 @@ static void __hrtimer_init_sleeper(struct hrtimer_sleeper *sl,
 	sl->task = current;
 }
 
-/**
- * hrtimer_init_sleeper_on_stack - initialize a sleeper in stack memory
- * @sl:		sleeper to be initialized
- * @clock_id:	the clock to be used
- * @mode:	timer mode abs/rel
- */
-void hrtimer_init_sleeper_on_stack(struct hrtimer_sleeper *sl,
-				   clockid_t clock_id, enum hrtimer_mode mode)
-{
-	debug_init_on_stack(&sl->timer, clock_id, mode);
-	__hrtimer_init_sleeper(sl, clock_id, mode);
-}
-EXPORT_SYMBOL_GPL(hrtimer_init_sleeper_on_stack);
-
 /**
  * hrtimer_setup_sleeper_on_stack - initialize a sleeper in stack memory
  * @sl:		sleeper to be initialized

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #17 lore raw

PATCH RESEND v2 16/19 sched/idle: Switch to use hrtimer_setup_on_stack()

In reply to Nam Cao

hrtimer_setup_on_stack() takes the callback function pointer as argument and initializes the timer completely.

Replace hrtimer_init_on_stack() and the open coded initialization of hrtimer::function with the new setup mechanism.

The conversion was done with Coccinelle.

Signed-off-by Nam Cao <[email protected]>

Cc Peter Zijlstra <[email protected]>

---
 kernel/sched/idle.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

kernel/sched/idle.c +2 -2

index d2f096bb274c..631e42802925 100644
@@ -399,8 +399,8 @@ void play_idle_precise(u64 duration_ns, u64 latency_ns)
 	cpuidle_use_deepest_state(latency_ns);
 
 	it.done = 0;
-	hrtimer_init_on_stack(&it.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
-	it.timer.function = idle_inject_timer_fn;
+	hrtimer_setup_on_stack(&it.timer, idle_inject_timer_fn, CLOCK_MONOTONIC,
+			       HRTIMER_MODE_REL_HARD);
 	hrtimer_start(&it.timer, ns_to_ktime(duration_ns),
 		      HRTIMER_MODE_REL_PINNED_HARD);

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #18 lore raw

PATCH RESEND v2 17/19 io_uring: Switch to use hrtimer_setup_on_stack()

In reply to Nam Cao

hrtimer_setup_on_stack() takes the callback function pointer as argument and initializes the timer completely.

Replace hrtimer_init_on_stack() and the open coded initialization of hrtimer::function with the new setup mechanism.

Signed-off-by Nam Cao <[email protected]>

Cc Jens Axboe <[email protected]>

---
 io_uring/io_uring.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

io_uring/io_uring.c +4 -3

index feb61d68dca6..0842aa3f60e7 100644
@@ -2435,13 +2435,14 @@ static int io_cqring_schedule_timeout(struct io_wait_queue *iowq,
 {
 	ktime_t timeout;
 
-	hrtimer_init_on_stack(&iowq->t, clock_id, HRTIMER_MODE_ABS);
 	if (iowq->min_timeout) {
 		timeout = ktime_add_ns(iowq->min_timeout, start_time);
-		iowq->t.function = io_cqring_min_timer_wakeup;
+		hrtimer_setup_on_stack(&iowq->t, io_cqring_min_timer_wakeup, clock_id,
+				       HRTIMER_MODE_ABS);
 	} else {
 		timeout = iowq->timeout;
-		iowq->t.function = io_cqring_timer_wakeup;
+		hrtimer_setup_on_stack(&iowq->t, io_cqring_timer_wakeup, clock_id,
+				       HRTIMER_MODE_ABS);
 	}
 
 	hrtimer_set_expires_range_ns(&iowq->t, timeout, 0);

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #19 lore raw

PATCH RESEND v2 18/19 alarmtimer: Switch to use hrtimer_setup() and hrtimer_setup_on_stack()

In reply to Nam Cao

hrtimer_setup() and hrtimer_setup_on_stack() take the callback function pointer as argument and initialize the timer completely.

Replace the hrtimer_init*() variants and the open coded initialization of hrtimer::function with the new setup mechanism.

Switch to use the new functions.

Signed-off-by Nam Cao <[email protected]>

Cc John Stultz <[email protected]>

---
 kernel/time/alarmtimer.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

kernel/time/alarmtimer.c +4 -5

index 452d8aa2f6e0..f581868e1b3e 100644
@@ -337,7 +337,6 @@ __alarm_init(struct alarm *alarm, enum alarmtimer_type type,
 	     enum alarmtimer_restart (*function)(struct alarm *, ktime_t))
 {
 	timerqueue_init(&alarm->node);
-	alarm->timer.function = alarmtimer_fired;
 	alarm->function = function;
 	alarm->type = type;
 	alarm->state = ALARMTIMER_STATE_INACTIVE;

@@ -352,8 +351,8 @@ __alarm_init(struct alarm *alarm, enum alarmtimer_type type,
 void alarm_init(struct alarm *alarm, enum alarmtimer_type type,
 		enum alarmtimer_restart (*function)(struct alarm *, ktime_t))
 {
-	hrtimer_init(&alarm->timer, alarm_bases[type].base_clockid,
-		     HRTIMER_MODE_ABS);
+	hrtimer_setup(&alarm->timer, alarmtimer_fired, alarm_bases[type].base_clockid,
+		      HRTIMER_MODE_ABS);
 	__alarm_init(alarm, type, function);
 }
 EXPORT_SYMBOL_GPL(alarm_init);

@@ -816,8 +815,8 @@ static void
 alarm_init_on_stack(struct alarm *alarm, enum alarmtimer_type type,
 		    enum alarmtimer_restart (*function)(struct alarm *, ktime_t))
 {
-	hrtimer_init_on_stack(&alarm->timer, alarm_bases[type].base_clockid,
-			      HRTIMER_MODE_ABS);
+	hrtimer_setup_on_stack(&alarm->timer, alarmtimer_fired, alarm_bases[type].base_clockid,
+			       HRTIMER_MODE_ABS);
 	__alarm_init(alarm, type, function);
 }

Signature · 1 line

2.39.5

Nam Cao

author

2024-10-31 15:15 UTC #20 lore raw

PATCH RESEND v2 19/19 hrtimers: Delete hrtimer_init_on_stack()

In reply to Nam Cao

hrtimer_init_on_stack() is now unused. Delete it.

Signed-off-by Nam Cao <[email protected]>

---
 include/linux/hrtimer.h |  2 --
 kernel/time/hrtimer.c   | 17 -----------------
 2 files changed, 19 deletions(-)

include/linux/hrtimer.h +0 -2

index 4e4f04b3c0c2..7ef5f7ef31a9 100644
@@ -230,8 +230,6 @@ extern void hrtimer_init(struct hrtimer *timer, clockid_t which_clock,
 			 enum hrtimer_mode mode);
 extern void hrtimer_setup(struct hrtimer *timer, enum hrtimer_restart (*function)(struct hrtimer *),
 			  clockid_t clock_id, enum hrtimer_mode mode);
-extern void hrtimer_init_on_stack(struct hrtimer *timer, clockid_t which_clock,
-				  enum hrtimer_mode mode);
 extern void hrtimer_setup_on_stack(struct hrtimer *timer,
 				   enum hrtimer_restart (*function)(struct hrtimer *),
 				   clockid_t clock_id, enum hrtimer_mode mode);

kernel/time/hrtimer.c +0 ~~-17~~

index 376b8182b72e..55e9ffbcd49a 100644
@@ -1629,23 +1629,6 @@ void hrtimer_setup(struct hrtimer *timer, enum hrtimer_restart (*function)(struc
 }
 EXPORT_SYMBOL_GPL(hrtimer_setup);
 
-/**
- * hrtimer_init_on_stack - initialize a timer in stack memory
- * @timer:	The timer to be initialized
- * @clock_id:	The clock to be used
- * @mode:       The timer mode
- *
- * Similar to hrtimer_init(), except that this one must be used if struct hrtimer is in stack
- * memory.
- */
-void hrtimer_init_on_stack(struct hrtimer *timer, clockid_t clock_id,
-			   enum hrtimer_mode mode)
-{
-	debug_init_on_stack(timer, clock_id, mode);
-	__hrtimer_init(timer, clock_id, mode);
-}
-EXPORT_SYMBOL_GPL(hrtimer_init_on_stack);
-
 /**
  * hrtimer_setup_on_stack - initialize a timer on stack memory
  * @timer:	The timer to be initialized

Signature · 1 line

2.39.5

Patchwork

2024-11-04 10:56 UTC #21 lore raw

✗ Fi.CI.BUILD: failure for hrtimers: Consolidate hrtimer initialization - Part 1 (rev2)

In reply to Nam Cao

== Series Details ==

Series: hrtimers: Consolidate hrtimer initialization - Part 1 (rev2)
URL : https://patchwork.freedesktop.org/series/140806/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/140806/revisions/2/mbox/ not applied
Applying: hrtimers: Add missing hrtimer_init() trace points
Applying: drm/i915/request: Remove unnecessary modification of hrtimer::function
Applying: KVM: x86/xen: Initialize hrtimer in kvm_xen_init_vcpu()
Applying: wifi: rt2x00: Remove redundant hrtimer_init()
Applying: io_uring: Remove redundant hrtimer's callback function setup
Applying: hrtimers: Introduce hrtimer_setup() to replace hrtimer_init()
Applying: hrtimers: Introduce hrtimer_setup_on_stack()
Applying: hrtimers: Introduce hrtimer_setup_sleeper_on_stack()
Applying: hrtimers: Introduce hrtimer_update_function()
Applying: fs/aio: Switch to use hrtimer_setup_sleeper_on_stack()
Applying: futex: Switch to use hrtimer_setup_sleeper_on_stack()
Applying: net: pktgen: Switch to use hrtimer_setup_sleeper_on_stack()
Applying: timers: Switch to use hrtimer_setup_sleeper_on_stack()
error: sha1 information is lacking or useless (kernel/time/hrtimer.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0013 timers: Switch to use hrtimer_setup_sleeper_on_stack()
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Build failed, no error log produced