* [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5)
@ 2024-10-28 7:29 Nam Cao
2024-10-28 7:29 ` [PATCH 01/21] hrtimers: Add missing hrtimer_init event trace points Nam Cao
` (21 more replies)
0 siblings, 22 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, Jani Nikula, intel-gfx, Sean Christopherson,
Paolo Bonzini, x86, Jakub Kicinski, Oliver Hartkopp, Kalle Valo,
Jens Axboe, Christian Brauner, Peter Zijlstra, John Stultz
This is the first part of a 5-part series (split for convenience). All 5
parts are:
Part 1: https://lore.kernel.org/lkml/cover.1729864615.git.namcao@linutronix.de
Part 2: https://lore.kernel.org/lkml/cover.1729864823.git.namcao@linutronix.de
Part 3: https://lore.kernel.org/lkml/cover.1729865232.git.namcao@linutronix.de
Part 4: https://lore.kernel.org/lkml/cover.1729865485.git.namcao@linutronix.de
Part 5: https://lore.kernel.org/lkml/cover.1729865740.git.namcao@linutronix.de
To use hrtimer, hrtimer_init() (or one of its variant) must be called, and
also the timer's callfack function must be setup separately.
That can cause misuse of hrtimer. For example, because:
- The callback function is not setup
- The callback function is setup while it is not safe to do so
To prevent misuse of hrtimer, this series:
- Introduce new functions hrtimer_setup*(). These new functions are
similar to hrtimer_init*(), except that they also sanity-check and
initialize the callback function.
- Introduce hrtimer_update_function() which checks that it is safe to
change the callback function. The 'function' field of hrtimer is then
made private.
- Convert all users to use the new functions.
- Some minor cleanups on the way.
Most conversion patches were created using Coccinelle with the sematic
patch below; except for tricky cases that Coccinelle cannot handle, or for
some cases where a Coccinelle's bug regarding 100 column limit is
triggered. Any patches not mentioning Coccinelle were done manually.
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);
Signed-off-by: Nam Cao <namcao@linutronix.de>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: x86@kernel.org
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: John Stultz <jstultz@google.com>
Nam Cao (21):
hrtimers: Add missing hrtimer_init event trace points
hrtimers: Remove unused hrtimer_init_sleeper()
drm/i915/request: Remove unnecessary abuse of hrtimer::function
KVM: x86/xen: Initialize hrtimer in kvm_xen_init_vcpu()
can: bcm: Don't initialized an unused hrtimer
wifi: rt2x00: Remove redundant hrtimer_init()
io_uring: Remove redundant hrtimer's callback function setup
hrtimers: Introduce hrtimer_setup() to replace hrtimer_init()
hrtimers: Introduce hrtimer_setup_on_stack()
hrtimers: Introduce hrtimer_setup_sleeper_on_stack()
hrtimers: Introduce hrtimer_update_function()
fs/aio: Switch to use hrtimer_setup_sleeper_on_stack()
futex: Switch to use hrtimer_setup_sleeper_on_stack()
net: pktgen: Switch to use hrtimer_setup_sleeper_on_stack()
timers: Switch to use hrtimer_setup_sleeper_on_stack()
wait: Switch to use hrtimer_setup_sleeper_on_stack()
hrtimers: Delete hrtimer_init_sleeper_on_stack()
sched/idle: Switch to use hrtimer_setup_on_stack()
io_uring: Switch to use hrtimer_setup_on_stack()
alarmtimer: Switch to use hrtimer_setup() and hrtimer_setup_on_stack()
hrtimers: Delete hrtimer_init_on_stack()
arch/x86/kvm/xen.c | 4 +-
drivers/gpu/drm/i915/i915_request.c | 17 +--
.../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/can/bcm.c | 19 ++-
net/core/pktgen.c | 2 +-
15 files changed, 143 insertions(+), 97 deletions(-)
--
2.39.5
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 01/21] hrtimers: Add missing hrtimer_init event trace points
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 02/21] hrtimers: Remove unused hrtimer_init_sleeper() Nam Cao
` (20 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao
There is the "hrtimer_init" trace event when hrtimer gets initialized. This
event should also be triggered in hrtimer_init_on_stack() and
hrtimer_init_sleeper_on_stack(), which is not the case if
CONFIG_DEBUG_OBJECTS_TIMERS=y.
Make sure the "hrtimer_init" trace event is triggered properly by
introducing debug_init_on_stack() which is analogous to debug_init() and
call this function in hrtimer_init_on_stack() and
hrtimer_init_sleeper_on_stack(). These two functions now each have a single
implementation regardless of CONFIG_DEBUG_OBJECTS_TIMERS, similar to
hrtimer_init() and hrtimer_init_sleeper().
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
include/linux/hrtimer.h | 17 +----------
kernel/time/hrtimer.c | 66 +++++++++++++++++++++++++++--------------
2 files changed, 45 insertions(+), 38 deletions(-)
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index aa1e65ccb615..814d489cb7a2 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -230,30 +230,15 @@ 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
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 04f7d8a392c3..533769777ba4 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -417,6 +417,11 @@ static inline void debug_hrtimer_init(struct hrtimer *timer)
debug_object_init(timer, &hrtimer_debug_descr);
}
+static inline void debug_hrtimer_init_on_stack(struct hrtimer *timer)
+{
+ debug_object_init_on_stack(timer, &hrtimer_debug_descr);
+}
+
static inline void debug_hrtimer_activate(struct hrtimer *timer,
enum hrtimer_mode mode)
{
@@ -428,28 +433,6 @@ static inline void debug_hrtimer_deactivate(struct hrtimer *timer)
debug_object_deactivate(timer, &hrtimer_debug_descr);
}
-static void __hrtimer_init(struct hrtimer *timer, clockid_t clock_id,
- enum hrtimer_mode mode);
-
-void hrtimer_init_on_stack(struct hrtimer *timer, clockid_t clock_id,
- enum hrtimer_mode mode)
-{
- debug_object_init_on_stack(timer, &hrtimer_debug_descr);
- __hrtimer_init(timer, clock_id, mode);
-}
-EXPORT_SYMBOL_GPL(hrtimer_init_on_stack);
-
-static void __hrtimer_init_sleeper(struct hrtimer_sleeper *sl,
- clockid_t clock_id, enum hrtimer_mode mode);
-
-void hrtimer_init_sleeper_on_stack(struct hrtimer_sleeper *sl,
- clockid_t clock_id, enum hrtimer_mode mode)
-{
- debug_object_init_on_stack(&sl->timer, &hrtimer_debug_descr);
- __hrtimer_init_sleeper(sl, clock_id, mode);
-}
-EXPORT_SYMBOL_GPL(hrtimer_init_sleeper_on_stack);
-
void destroy_hrtimer_on_stack(struct hrtimer *timer)
{
debug_object_free(timer, &hrtimer_debug_descr);
@@ -459,6 +442,7 @@ EXPORT_SYMBOL_GPL(destroy_hrtimer_on_stack);
#else
static inline void debug_hrtimer_init(struct hrtimer *timer) { }
+static inline void debug_hrtimer_init_on_stack(struct hrtimer *timer) { }
static inline void debug_hrtimer_activate(struct hrtimer *timer,
enum hrtimer_mode mode) { }
static inline void debug_hrtimer_deactivate(struct hrtimer *timer) { }
@@ -472,6 +456,13 @@ debug_init(struct hrtimer *timer, clockid_t clockid,
trace_hrtimer_init(timer, clockid, mode);
}
+static inline void debug_init_on_stack(struct hrtimer *timer, clockid_t clockid,
+ enum hrtimer_mode mode)
+{
+ debug_hrtimer_init_on_stack(timer);
+ trace_hrtimer_init(timer, clockid, mode);
+}
+
static inline void debug_activate(struct hrtimer *timer,
enum hrtimer_mode mode)
{
@@ -1600,6 +1591,23 @@ void hrtimer_init(struct hrtimer *timer, clockid_t clock_id,
}
EXPORT_SYMBOL_GPL(hrtimer_init);
+/**
+ * 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);
+
/*
* 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
@@ -2001,6 +2009,20 @@ void hrtimer_init_sleeper(struct hrtimer_sleeper *sl, clockid_t clock_id,
}
EXPORT_SYMBOL_GPL(hrtimer_init_sleeper);
+/**
+ * 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);
+
int nanosleep_copyout(struct restart_block *restart, struct timespec64 *ts)
{
switch(restart->nanosleep.type) {
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 02/21] hrtimers: Remove unused hrtimer_init_sleeper()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
2024-10-28 7:29 ` [PATCH 01/21] hrtimers: Add missing hrtimer_init event trace points Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 03/21] drm/i915/request: Remove unnecessary abuse of hrtimer::function Nam Cao
` (19 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao
hrtimer_init_sleeper() is not used. Delete it.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
include/linux/hrtimer.h | 2 --
kernel/time/hrtimer.c | 17 +----------------
2 files changed, 1 insertion(+), 18 deletions(-)
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 814d489cb7a2..5aa9d57528c4 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -228,8 +228,6 @@ 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);
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,
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 533769777ba4..4b0507cf38ea 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1952,7 +1952,7 @@ void hrtimer_sleeper_start_expires(struct hrtimer_sleeper *sl,
* Make the enqueue delivery mode check work on RT. If the sleeper
* was initialized for hard interrupt delivery, force the mode bit.
* This is a special case for hrtimer_sleepers because
- * hrtimer_init_sleeper() determines the delivery mode on RT so the
+ * __hrtimer_init_sleeper() determines the delivery mode on RT so the
* fiddling with this decision is avoided at the call sites.
*/
if (IS_ENABLED(CONFIG_PREEMPT_RT) && sl->timer.is_hard)
@@ -1994,21 +1994,6 @@ static void __hrtimer_init_sleeper(struct hrtimer_sleeper *sl,
sl->task = current;
}
-/**
- * hrtimer_init_sleeper - initialize sleeper to the given clock
- * @sl: sleeper to be initialized
- * @clock_id: the clock to be used
- * @mode: timer mode abs/rel
- */
-void hrtimer_init_sleeper(struct hrtimer_sleeper *sl, clockid_t clock_id,
- enum hrtimer_mode mode)
-{
- debug_init(&sl->timer, clock_id, mode);
- __hrtimer_init_sleeper(sl, clock_id, mode);
-
-}
-EXPORT_SYMBOL_GPL(hrtimer_init_sleeper);
-
/**
* hrtimer_init_sleeper_on_stack - initialize a sleeper in stack memory
* @sl: sleeper to be initialized
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 03/21] drm/i915/request: Remove unnecessary abuse of hrtimer::function
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
2024-10-28 7:29 ` [PATCH 01/21] hrtimers: Add missing hrtimer_init event trace points Nam Cao
2024-10-28 7:29 ` [PATCH 02/21] hrtimers: Remove unused hrtimer_init_sleeper() Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 04/21] KVM: x86/xen: Initialize hrtimer in kvm_xen_init_vcpu() Nam Cao
` (18 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, Jani Nikula
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.
Remove the abuse of hrtimer::function and fully initialize the timer when
the request is created. Then, hrtimer_try_to_cancel() can be called without
touching the 'function' field.
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 <namcao@linutronix.de>
---
Cc: Jani Nikula <jani.nikula@linux.intel.com>
---
drivers/gpu/drm/i915/i915_request.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 519e096c607c..8f62cfa23fb7 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -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);
}
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 04/21] KVM: x86/xen: Initialize hrtimer in kvm_xen_init_vcpu()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (2 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 03/21] drm/i915/request: Remove unnecessary abuse of hrtimer::function Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 16:01 ` Sean Christopherson
2024-10-30 18:05 ` Sean Christopherson
2024-10-28 7:29 ` [PATCH 05/21] can: bcm: Don't initialized an unused hrtimer Nam Cao
` (17 subsequent siblings)
21 siblings, 2 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, Sean Christopherson, Paolo Bonzini, x86, kvm
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.
Instead of "abusing" the 'function' field, move the hrtimer initialization
into kvm_xen_init_vcpu() so that it will only be initialized once.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
Cc: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: x86@kernel.org
Cc: kvm@vger.kernel.org
---
arch/x86/kvm/xen.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 622fe24da910..c386fbe6b58d 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -1070,9 +1070,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 +2232,7 @@ 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);
+ kvm_xen_init_timer(vcpu);
kvm_gpc_init(&vcpu->arch.xen.runstate_cache, vcpu->kvm);
kvm_gpc_init(&vcpu->arch.xen.runstate2_cache, vcpu->kvm);
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 05/21] can: bcm: Don't initialized an unused hrtimer
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (3 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 04/21] KVM: x86/xen: Initialize hrtimer in kvm_xen_init_vcpu() Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-30 10:49 ` Oliver Hartkopp
2024-10-28 7:29 ` [PATCH 06/21] wifi: rt2x00: Remove redundant hrtimer_init() Nam Cao
` (16 subsequent siblings)
21 siblings, 1 reply; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, Oliver Hartkopp, Jakub Kicinski
The hrtimer "thrtimer" is not used for TX. But this timer is initialized
regardless.
Remove the hrtimer_init() for the unused hrtimer and change bcm_remove_op()
to make sure hrtimer_cancel() is not called with the uninitialized hrtimer.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Jakub Kicinski <kuba@kernel.org>
---
net/can/bcm.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 217049fa496e..792528f7bce2 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -780,10 +780,11 @@ static void bcm_free_op_rcu(struct rcu_head *rcu_head)
kfree(op);
}
-static void bcm_remove_op(struct bcm_op *op)
+static void bcm_remove_op(struct bcm_op *op, bool is_tx)
{
hrtimer_cancel(&op->timer);
- hrtimer_cancel(&op->thrtimer);
+ if (!is_tx)
+ hrtimer_cancel(&op->thrtimer);
call_rcu(&op->rcu, bcm_free_op_rcu);
}
@@ -844,7 +845,7 @@ static int bcm_delete_rx_op(struct list_head *ops, struct bcm_msg_head *mh,
bcm_rx_handler, op);
list_del(&op->list);
- bcm_remove_op(op);
+ bcm_remove_op(op, false);
return 1; /* done */
}
}
@@ -864,7 +865,7 @@ static int bcm_delete_tx_op(struct list_head *ops, struct bcm_msg_head *mh,
if ((op->can_id == mh->can_id) && (op->ifindex == ifindex) &&
(op->flags & CAN_FD_FRAME) == (mh->flags & CAN_FD_FRAME)) {
list_del(&op->list);
- bcm_remove_op(op);
+ bcm_remove_op(op, true);
return 1; /* done */
}
}
@@ -1015,10 +1016,6 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
HRTIMER_MODE_REL_SOFT);
op->timer.function = bcm_tx_timeout_handler;
- /* currently unused in tx_ops */
- 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);
@@ -1277,7 +1274,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
if (err) {
/* this bcm rx op is broken -> remove it */
list_del(&op->list);
- bcm_remove_op(op);
+ bcm_remove_op(op, false);
return err;
}
}
@@ -1581,7 +1578,7 @@ static int bcm_release(struct socket *sock)
#endif /* CONFIG_PROC_FS */
list_for_each_entry_safe(op, next, &bo->tx_ops, list)
- bcm_remove_op(op);
+ bcm_remove_op(op, true);
list_for_each_entry_safe(op, next, &bo->rx_ops, list) {
/*
@@ -1613,7 +1610,7 @@ static int bcm_release(struct socket *sock)
synchronize_rcu();
list_for_each_entry_safe(op, next, &bo->rx_ops, list)
- bcm_remove_op(op);
+ bcm_remove_op(op, false);
/* remove device reference */
if (bo->bound) {
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 06/21] wifi: rt2x00: Remove redundant hrtimer_init()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (4 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 05/21] can: bcm: Don't initialized an unused hrtimer Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-31 14:13 ` Kalle Valo
2024-10-28 7:29 ` [PATCH 07/21] io_uring: Remove redundant hrtimer's callback function setup Nam Cao
` (15 subsequent siblings)
21 siblings, 1 reply; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, Kalle Valo, linux-wireless
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 <namcao@linutronix.de>
---
Cc: Kalle Valo <kvalo@kernel.org>
Cc: linux-wireless@vger.kernel.org
---
drivers/net/wireless/ralink/rt2x00/rt2x00usb.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c b/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c
index 8fd22c69855f..a6d50149e0c3 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c
@@ -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)
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 07/21] io_uring: Remove redundant hrtimer's callback function setup
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (5 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 06/21] wifi: rt2x00: Remove redundant hrtimer_init() Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 08/21] hrtimers: Introduce hrtimer_setup() to replace hrtimer_init() Nam Cao
` (14 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, Jens Axboe
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 <namcao@linutronix.de>
---
Cc: Jens Axboe <axboe@kernel.dk>
---
io_uring/timeout.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/io_uring/timeout.c b/io_uring/timeout.c
index 9973876d91b0..2ffe5e1dc68a 100644
--- a/io_uring/timeout.c
+++ b/io_uring/timeout.c
@@ -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;
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 08/21] hrtimers: Introduce hrtimer_setup() to replace hrtimer_init()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (6 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 07/21] io_uring: Remove redundant hrtimer's callback function setup Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 09/21] hrtimers: Introduce hrtimer_setup_on_stack() Nam Cao
` (13 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao
To initialize hrtimer, hrtimer_init() needs to be called and also struct
hrtimer::function must be set. Introduce hrtimer_setup() which does both of
these things, so that users of hrtimer can be simplified.
The new function also does sanity check on the callback function pointer
from user. If the function pointer is invalid, initialize 'function' with a
dummy callback, so that the kernel doesn't blow up later on.
hrtimer_init() will be removed as soon as all of its users have been
converted to the new function.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
include/linux/hrtimer.h | 2 ++
kernel/time/hrtimer.c | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 5aa9d57528c4..bcc0715c59a8 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -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,
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 4b0507cf38ea..a5ef67edcda9 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -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
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 09/21] hrtimers: Introduce hrtimer_setup_on_stack()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (7 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 08/21] hrtimers: Introduce hrtimer_setup() to replace hrtimer_init() Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 10/21] hrtimers: Introduce hrtimer_setup_sleeper_on_stack() Nam Cao
` (12 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao
To initialize hrtimer on stack, hrtimer_init_on_stack() needs to be called
and also struct hrtimer::function must be set. Introduce
hrtimer_setup_on_stack() which does both of these things, so that users of
hrtimer can be simplified.
This function also does sanity check on the callback function pointer. If
the callback function pointer is invalid, set 'function' to a dummy
callback function, so that the kernel does not blow up later on.
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 <namcao@linutronix.de>
---
include/linux/hrtimer.h | 3 +++
kernel/time/hrtimer.c | 19 +++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index bcc0715c59a8..2da513f8d66a 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -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);
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index a5ef67edcda9..daee4e27f839 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -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
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 10/21] hrtimers: Introduce hrtimer_setup_sleeper_on_stack()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (8 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 09/21] hrtimers: Introduce hrtimer_setup_on_stack() Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 11/21] hrtimers: Introduce hrtimer_update_function() Nam Cao
` (11 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao
hrtimer_setup() and hrtimer_setup_on_stack() have been introduced to
replace hrtimer_init() and hrtimer_init_on_stack(). To keep the names
consistent, also introduce hrtimer_setup_sleeper_on_stack(). The old name
hrtimer_init_sleeper_on_stack() will be removed as soon as all of its users
have been updated.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
include/linux/hrtimer.h | 2 ++
kernel/time/hrtimer.c | 14 ++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 2da513f8d66a..48872a2b4071 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -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);
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index daee4e27f839..1d1f5c03673c 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -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) {
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 11/21] hrtimers: Introduce hrtimer_update_function()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (9 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 10/21] hrtimers: Introduce hrtimer_setup_sleeper_on_stack() Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 12/21] fs/aio: Switch to use hrtimer_setup_sleeper_on_stack() Nam Cao
` (10 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao
Some users of hrtimer need to change the callback function after the
initial setup, and they touch the 'function' field of struct hrtimer
directly. However, directly changing the 'function' field is not safe under
all circumstances. It's only safe when the hrtimer is not enqueued.
Introduce hrtimer_update_function() helper function, which also performs
runtime checks whether it is safe (i.e. the timer is not enqueued).
The field 'function' of struct hrtimer will be changed to private in the
future.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
include/linux/hrtimer.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 48872a2b4071..6e026730e803 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -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);
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 12/21] fs/aio: Switch to use hrtimer_setup_sleeper_on_stack()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (10 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 11/21] hrtimers: Introduce hrtimer_update_function() Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 13/21] futex: " Nam Cao
` (9 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, Christian Brauner
There is a newly introduced function hrtimer_setup_sleeper_on_stack() which
will replace hrtimer_init_sleeper_on_stack().
This function is the same as the old hrtimer_init_sleeper_on_stack(), but
it was introduced to keep the names consistent with other changes with
hrtimer.
Switch to use the new function.
Patch was created by using Coccinelle.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
Cc: Christian Brauner <brauner@kernel.org>
---
fs/aio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/aio.c b/fs/aio.c
index e8920178b50f..a5d331f29943 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -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);
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 13/21] futex: Switch to use hrtimer_setup_sleeper_on_stack()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (11 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 12/21] fs/aio: Switch to use hrtimer_setup_sleeper_on_stack() Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 14/21] net: pktgen: " Nam Cao
` (8 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, Peter Zijlstra
There is a newly introduced hrtimer_setup_sleeper_on_stack() which will
replace hrtimer_init_sleeper_on_stack(). Switch to use this new function.
This function is the same as the old hrtimer_init_sleeper_on_stack(), but
it was introduced to keep the names consistent with other changes with
hrtimer.
Patch was created by using Coccinelle.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
Cc: Peter Zijlstra <peterz@infradead.org>
---
kernel/futex/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index 136768ae2637..fb7214c7a36f 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -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().
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 14/21] net: pktgen: Switch to use hrtimer_setup_sleeper_on_stack()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (12 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 13/21] futex: " Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 15/21] timers: " Nam Cao
` (7 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, Jakub Kicinski
There is a newly introduced function hrtimer_setup_sleeper_on_stack() which
will replace hrtimer_init_sleeper_on_stack().
This function is the same as the old hrtimer_init_sleeper_on_stack(), but
it was introduced to keep the names consistent with other changes with
hrtimer.
Switch to use the new function.
Patch was created by using Coccinelle.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
Cc: Jakub Kicinski <kuba@kernel.org>
---
net/core/pktgen.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 34f68ef74b8f..7e23cacbe66e 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -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));
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 15/21] timers: Switch to use hrtimer_setup_sleeper_on_stack()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (13 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 14/21] net: pktgen: " Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 16/21] wait: " Nam Cao
` (6 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao
There is a newly introduced function hrtimer_setup_sleeper_on_stack() which
will replace hrtimer_init_sleeper_on_stack().
This function is the same as the old hrtimer_init_sleeper_on_stack(), but
it was introduced to keep the names consistent with other changes with
hrtimer.
Switch to use the new function.
Patch was created by using Coccinelle.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
kernel/time/hrtimer.c | 5 ++---
kernel/time/sleep_timeout.c | 2 +-
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 1d1f5c03673c..69430467a17d 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -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)
diff --git a/kernel/time/sleep_timeout.c b/kernel/time/sleep_timeout.c
index 3054e5232d20..dfe939f6e4ec 100644
--- a/kernel/time/sleep_timeout.c
+++ b/kernel/time/sleep_timeout.c
@@ -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);
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 16/21] wait: Switch to use hrtimer_setup_sleeper_on_stack()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (14 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 15/21] timers: " Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 17/21] hrtimers: Delete hrtimer_init_sleeper_on_stack() Nam Cao
` (5 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, Peter Zijlstra
There is a newly introduced function hrtimer_setup_sleeper_on_stack() which
will replace hrtimer_init_sleeper_on_stack().
This function is the same as the old hrtimer_init_sleeper_on_stack(), but
it was introduced to keep the names consistent with other changes with
hrtimer.
Switch to use the new function.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
Cc: Peter Zijlstra <peterz@infradead.org>
---
include/linux/wait.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 8aa3372f21a0..643b7c7bf376 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -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); \
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 17/21] hrtimers: Delete hrtimer_init_sleeper_on_stack()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (15 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 16/21] wait: " Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack() Nam Cao
` (4 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao
hrtimer_init_sleeper_on_stack() is unused. Delete it.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
include/linux/hrtimer.h | 3 ---
kernel/time/hrtimer.c | 14 --------------
2 files changed, 17 deletions(-)
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 6e026730e803..4e4f04b3c0c2 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -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);
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 69430467a17d..376b8182b72e 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -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
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (16 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 17/21] hrtimers: Delete hrtimer_init_sleeper_on_stack() Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 9:09 ` Peter Zijlstra
2024-10-28 7:29 ` [PATCH 19/21] io_uring: " Nam Cao
` (3 subsequent siblings)
21 siblings, 1 reply; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, Peter Zijlstra
There is a newly introduced function hrtimer_setup_on_stack(), which will
replace hrtimer_init_on_stack(). In addition to what
hrtimer_init_on_stack() does, this new function also sanity-checks and
initializes the callback function pointer.
Switch to use the new function.
Patch was created by using Coccinelle.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
Cc: Peter Zijlstra <peterz@infradead.org>
---
kernel/sched/idle.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index d2f096bb274c..631e42802925 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -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);
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 19/21] io_uring: Switch to use hrtimer_setup_on_stack()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (17 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack() Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 20/21] alarmtimer: Switch to use hrtimer_setup() and hrtimer_setup_on_stack() Nam Cao
` (2 subsequent siblings)
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, Jens Axboe
There is a newly introduced function hrtimer_setup_on_stack(), which will
replace hrtimer_init_on_stack(). In addition to what
hrtimer_init_on_stack() does, this new function also sanity-checks and
initializes the callback function pointer.
Switch to use the new function.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
Cc: Jens Axboe <axboe@kernel.dk>
---
io_uring/io_uring.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index feb61d68dca6..0842aa3f60e7 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -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);
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 20/21] alarmtimer: Switch to use hrtimer_setup() and hrtimer_setup_on_stack()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (18 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 19/21] io_uring: " Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 7:29 ` [PATCH 21/21] hrtimers: Delete hrtimer_init_on_stack() Nam Cao
2024-10-28 16:05 ` [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Sean Christopherson
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao, John Stultz
There are two newly introduced hrtimer_setup() and hrtimer_setup_on_stack()
which will replace hrtimer_init() and hrtimer_init_on_stack().
The new functions are mostly similar, except that they also sanity-check
and initialize the timer's callback function.
Switch to use the new functions.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
Cc: John Stultz <jstultz@google.com>
---
kernel/time/alarmtimer.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 8bf888641694..3c70a3abd6a2 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -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);
}
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 21/21] hrtimers: Delete hrtimer_init_on_stack()
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (19 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 20/21] alarmtimer: Switch to use hrtimer_setup() and hrtimer_setup_on_stack() Nam Cao
@ 2024-10-28 7:29 ` Nam Cao
2024-10-28 16:05 ` [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Sean Christopherson
21 siblings, 0 replies; 37+ messages in thread
From: Nam Cao @ 2024-10-28 7:29 UTC (permalink / raw)
To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Nam Cao
hrtimer_init_on_stack() is unused. Delete it.
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
include/linux/hrtimer.h | 2 --
kernel/time/hrtimer.c | 17 -----------------
2 files changed, 19 deletions(-)
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 4e4f04b3c0c2..7ef5f7ef31a9 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -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);
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 376b8182b72e..55e9ffbcd49a 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -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
--
2.39.5
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack()
2024-10-28 7:29 ` [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack() Nam Cao
@ 2024-10-28 9:09 ` Peter Zijlstra
2024-10-28 10:50 ` Thomas Gleixner
0 siblings, 1 reply; 37+ messages in thread
From: Peter Zijlstra @ 2024-10-28 9:09 UTC (permalink / raw)
To: Nam Cao
Cc: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
On Mon, Oct 28, 2024 at 08:29:37AM +0100, Nam Cao wrote:
> There is a newly introduced function hrtimer_setup_on_stack(), which will
> replace hrtimer_init_on_stack(). In addition to what
> hrtimer_init_on_stack() does, this new function also sanity-checks and
> initializes the callback function pointer.
>
> Switch to use the new function.
>
> Patch was created by using Coccinelle.
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
> Cc: Peter Zijlstra <peterz@infradead.org>
> ---
> kernel/sched/idle.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index d2f096bb274c..631e42802925 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -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);
WTF is hrtimer_setup_on_stack() ?
Do NOT send partial series. How the hell am I supposed to review things
if I don't even get to see the implementation of things,eh?
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack()
2024-10-28 9:09 ` Peter Zijlstra
@ 2024-10-28 10:50 ` Thomas Gleixner
2024-10-28 10:58 ` Peter Zijlstra
0 siblings, 1 reply; 37+ messages in thread
From: Thomas Gleixner @ 2024-10-28 10:50 UTC (permalink / raw)
To: Peter Zijlstra, Nam Cao
Cc: Anna-Maria Behnsen, Frederic Weisbecker, Andreas Hindborg,
Alice Ryhl, Miguel Ojeda, Kees Cook, linux-kernel
On Mon, Oct 28 2024 at 10:09, Peter Zijlstra wrote:
> On Mon, Oct 28, 2024 at 08:29:37AM +0100, Nam Cao wrote:
>> There is a newly introduced function hrtimer_setup_on_stack(), which will
>> replace hrtimer_init_on_stack(). In addition to what
>> hrtimer_init_on_stack() does, this new function also sanity-checks and
>> initializes the callback function pointer.
>>
>> Switch to use the new function.
>>
>> Patch was created by using Coccinelle.
>>
>> Signed-off-by: Nam Cao <namcao@linutronix.de>
>> ---
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> ---
>> kernel/sched/idle.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
>> index d2f096bb274c..631e42802925 100644
>> --- a/kernel/sched/idle.c
>> +++ b/kernel/sched/idle.c
>> @@ -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);
>
> WTF is hrtimer_setup_on_stack() ?
>
> Do NOT send partial series. How the hell am I supposed to review things
> if I don't even get to see the implementation of things,eh?
Can you tone down a bit? This was an oversight and I did not notice when
going over it. The full thread is in your LKML inbox, so can you just
move on?
Thanks,
tglx
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack()
2024-10-28 10:50 ` Thomas Gleixner
@ 2024-10-28 10:58 ` Peter Zijlstra
2024-10-28 22:33 ` Thomas Gleixner
0 siblings, 1 reply; 37+ messages in thread
From: Peter Zijlstra @ 2024-10-28 10:58 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Nam Cao, Anna-Maria Behnsen, Frederic Weisbecker,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
On Mon, Oct 28, 2024 at 11:50:44AM +0100, Thomas Gleixner wrote:
> On Mon, Oct 28 2024 at 10:09, Peter Zijlstra wrote:
> > On Mon, Oct 28, 2024 at 08:29:37AM +0100, Nam Cao wrote:
> >> There is a newly introduced function hrtimer_setup_on_stack(), which will
> >> replace hrtimer_init_on_stack(). In addition to what
> >> hrtimer_init_on_stack() does, this new function also sanity-checks and
> >> initializes the callback function pointer.
> >>
> >> Switch to use the new function.
> >>
> >> Patch was created by using Coccinelle.
> >>
> >> Signed-off-by: Nam Cao <namcao@linutronix.de>
> >> ---
> >> Cc: Peter Zijlstra <peterz@infradead.org>
> >> ---
> >> kernel/sched/idle.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> >> index d2f096bb274c..631e42802925 100644
> >> --- a/kernel/sched/idle.c
> >> +++ b/kernel/sched/idle.c
> >> @@ -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);
> >
> > WTF is hrtimer_setup_on_stack() ?
> >
> > Do NOT send partial series. How the hell am I supposed to review things
> > if I don't even get to see the implementation of things,eh?
>
> Can you tone down a bit? This was an oversight and I did not notice when
> going over it. The full thread is in your LKML inbox, so can you just
> move on?
*sigh*.. how am I supposed to know it's an over-sight? Some people are
actively pushing for this broken arse 'model' of posting.
Yes, I can dig out the remaining patches, but that's more work for me.
As you well know, I don't really need more work.
I suppose I'll see a new posting eventually or not, who knows.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 04/21] KVM: x86/xen: Initialize hrtimer in kvm_xen_init_vcpu()
2024-10-28 7:29 ` [PATCH 04/21] KVM: x86/xen: Initialize hrtimer in kvm_xen_init_vcpu() Nam Cao
@ 2024-10-28 16:01 ` Sean Christopherson
2024-10-28 22:19 ` Thomas Gleixner
2024-10-30 18:05 ` Sean Christopherson
1 sibling, 1 reply; 37+ messages in thread
From: Sean Christopherson @ 2024-10-28 16:01 UTC (permalink / raw)
To: Nam Cao
Cc: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel, Paolo Bonzini, x86, kvm
On Mon, Oct 28, 2024, Nam Cao wrote:
> 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.
>
> Instead of "abusing" the 'function' field, move the hrtimer initialization
> into kvm_xen_init_vcpu() so that it will only be initialized once.
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: x86@kernel.org
> Cc: kvm@vger.kernel.org
> ---
> arch/x86/kvm/xen.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 622fe24da910..c386fbe6b58d 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -1070,9 +1070,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 +2232,7 @@ 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);
> + kvm_xen_init_timer(vcpu);
I vote for opportunistically dropping kvm_xen_init_timer() and open coding its
contents here.
If the intent is for subsystems to grab their relevant patches, I can fixup when
applying.
> kvm_gpc_init(&vcpu->arch.xen.runstate_cache, vcpu->kvm);
> kvm_gpc_init(&vcpu->arch.xen.runstate2_cache, vcpu->kvm);
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5)
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
` (20 preceding siblings ...)
2024-10-28 7:29 ` [PATCH 21/21] hrtimers: Delete hrtimer_init_on_stack() Nam Cao
@ 2024-10-28 16:05 ` Sean Christopherson
2024-10-29 8:15 ` Thomas Gleixner
21 siblings, 1 reply; 37+ messages in thread
From: Sean Christopherson @ 2024-10-28 16:05 UTC (permalink / raw)
To: Nam Cao
Cc: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel, Jani Nikula, intel-gfx, Paolo Bonzini, x86,
Jakub Kicinski, Oliver Hartkopp, Kalle Valo, Jens Axboe,
Christian Brauner, Peter Zijlstra, John Stultz
On Mon, Oct 28, 2024, Nam Cao wrote:
> This is the first part of a 5-part series (split for convenience). All 5
> parts are:
>
> Part 1: https://lore.kernel.org/lkml/cover.1729864615.git.namcao@linutronix.de
> Part 2: https://lore.kernel.org/lkml/cover.1729864823.git.namcao@linutronix.de
> Part 3: https://lore.kernel.org/lkml/cover.1729865232.git.namcao@linutronix.de
> Part 4: https://lore.kernel.org/lkml/cover.1729865485.git.namcao@linutronix.de
> Part 5: https://lore.kernel.org/lkml/cover.1729865740.git.namcao@linutronix.de
How do y'all anticipate landing these patches? Is the plan/desire to get acks
from subsystems? Land the new helpers and then let subsystems grab their relevant
patches? Option C?
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 04/21] KVM: x86/xen: Initialize hrtimer in kvm_xen_init_vcpu()
2024-10-28 16:01 ` Sean Christopherson
@ 2024-10-28 22:19 ` Thomas Gleixner
0 siblings, 0 replies; 37+ messages in thread
From: Thomas Gleixner @ 2024-10-28 22:19 UTC (permalink / raw)
To: Sean Christopherson, Nam Cao
Cc: Anna-Maria Behnsen, Frederic Weisbecker, Andreas Hindborg,
Alice Ryhl, Miguel Ojeda, Kees Cook, linux-kernel, Paolo Bonzini,
x86, kvm
On Mon, Oct 28 2024 at 09:01, Sean Christopherson wrote:
> On Mon, Oct 28, 2024, Nam Cao wrote:
>> 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.
>>
>> Instead of "abusing" the 'function' field, move the hrtimer initialization
>> into kvm_xen_init_vcpu() so that it will only be initialized once.
>>
>> Signed-off-by: Nam Cao <namcao@linutronix.de>
>> ---
>> Cc: Sean Christopherson <seanjc@google.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: x86@kernel.org
>> Cc: kvm@vger.kernel.org
>> ---
>> arch/x86/kvm/xen.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
>> index 622fe24da910..c386fbe6b58d 100644
>> --- a/arch/x86/kvm/xen.c
>> +++ b/arch/x86/kvm/xen.c
>> @@ -1070,9 +1070,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 +2232,7 @@ 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);
>> + kvm_xen_init_timer(vcpu);
>
> I vote for opportunistically dropping kvm_xen_init_timer() and open coding its
> contents here.
Makes sense.
> If the intent is for subsystems to grab their relevant patches, I can fixup when
> applying.
Ideally I can route it through my tree to avoid a full release delay as
the subsequent changes depend on this and the core hrtimer API changes.
Thanks,
tglx
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack()
2024-10-28 10:58 ` Peter Zijlstra
@ 2024-10-28 22:33 ` Thomas Gleixner
0 siblings, 0 replies; 37+ messages in thread
From: Thomas Gleixner @ 2024-10-28 22:33 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Nam Cao, Anna-Maria Behnsen, Frederic Weisbecker,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
On Mon, Oct 28 2024 at 11:58, Peter Zijlstra wrote:
> On Mon, Oct 28, 2024 at 11:50:44AM +0100, Thomas Gleixner wrote:
>> > Do NOT send partial series. How the hell am I supposed to review things
>> > if I don't even get to see the implementation of things,eh?
>>
>> Can you tone down a bit? This was an oversight and I did not notice when
>> going over it. The full thread is in your LKML inbox, so can you just
>> move on?
>
> *sigh*.. how am I supposed to know it's an over-sight? Some people are
> actively pushing for this broken arse 'model' of posting.
>
> Yes, I can dig out the remaining patches, but that's more work for me.
> As you well know, I don't really need more work.
Nobody needs more work. But as we (as a community) can't agree on how to
post a 150+ patch series with a potential cc list of 200+ people and a
gazillion of mailing lists, there are only a few options left. And yes,
any model will annoy some people...
You at least got the cover letter, no?
I'm tired of posting a "Add new API/infrastructure" patch and then
finally 5 years later having the last newly added offender converted
over. I rather inflict the pain once, but obviously there is no way to
please everyone.
> I suppose I'll see a new posting eventually or not, who knows.
I'm happy to personally bounce you the pile if you insist or
alternatively annoy everyone with a resend of the full glory as well.
Not sure what that buys, but one thing is sure that we both wasted 10
times more time debating this nonsense than what it would have cost you
to look at the full mail thread which is in your LKML folder anyway.
Seriously?
Thanks,
tglx
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5)
2024-10-28 16:05 ` [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Sean Christopherson
@ 2024-10-29 8:15 ` Thomas Gleixner
0 siblings, 0 replies; 37+ messages in thread
From: Thomas Gleixner @ 2024-10-29 8:15 UTC (permalink / raw)
To: Sean Christopherson, Nam Cao
Cc: Anna-Maria Behnsen, Frederic Weisbecker, Andreas Hindborg,
Alice Ryhl, Miguel Ojeda, Kees Cook, linux-kernel, Jani Nikula,
intel-gfx, Paolo Bonzini, x86, Jakub Kicinski, Oliver Hartkopp,
Kalle Valo, Jens Axboe, Christian Brauner, Peter Zijlstra,
John Stultz
On Mon, Oct 28 2024 at 09:05, Sean Christopherson wrote:
> On Mon, Oct 28, 2024, Nam Cao wrote:
>> This is the first part of a 5-part series (split for convenience). All 5
>> parts are:
>>
>> Part 1: https://lore.kernel.org/lkml/cover.1729864615.git.namcao@linutronix.de
>> Part 2: https://lore.kernel.org/lkml/cover.1729864823.git.namcao@linutronix.de
>> Part 3: https://lore.kernel.org/lkml/cover.1729865232.git.namcao@linutronix.de
>> Part 4: https://lore.kernel.org/lkml/cover.1729865485.git.namcao@linutronix.de
>> Part 5: https://lore.kernel.org/lkml/cover.1729865740.git.namcao@linutronix.de
>
> How do y'all anticipate landing these patches? Is the plan/desire to get acks
> from subsystems? Land the new helpers and then let subsystems grab their relevant
> patches? Option C?
Ideally I can just collect acks and route them through my tree. Last
time we did this helper first and subsystems grab it took ages and lots
of chasing to get it done.
Thanks,
tglx
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 05/21] can: bcm: Don't initialized an unused hrtimer
2024-10-28 7:29 ` [PATCH 05/21] can: bcm: Don't initialized an unused hrtimer Nam Cao
@ 2024-10-30 10:49 ` Oliver Hartkopp
2024-10-30 12:15 ` Nam Cao
0 siblings, 1 reply; 37+ messages in thread
From: Oliver Hartkopp @ 2024-10-30 10:49 UTC (permalink / raw)
To: Nam Cao, Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel
Cc: Jakub Kicinski
On 28.10.24 08:29, Nam Cao wrote:
> The hrtimer "thrtimer" is not used for TX. But this timer is initialized
> regardless.
>
> Remove the hrtimer_init() for the unused hrtimer and change bcm_remove_op()
> to make sure hrtimer_cancel() is not called with the uninitialized hrtimer.
NAK.
There are several other occurrences of thrtimer that are not covered by
RX/TX distinction, where the second timer is canceled.
This one-time init and cancel of an unused hrtimer costs nearly nothing
and is not even in any hot path.
So this incomplete patch only adds complexity and potential error cases
in some 20 y/o code for nothing.
Therefore I would like to skip it.
Thanks,
Oliver
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
> Cc: Oliver Hartkopp <socketcan@hartkopp.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> ---
> net/can/bcm.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 217049fa496e..792528f7bce2 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -780,10 +780,11 @@ static void bcm_free_op_rcu(struct rcu_head *rcu_head)
> kfree(op);
> }
>
> -static void bcm_remove_op(struct bcm_op *op)
> +static void bcm_remove_op(struct bcm_op *op, bool is_tx)
> {
> hrtimer_cancel(&op->timer);
> - hrtimer_cancel(&op->thrtimer);
> + if (!is_tx)
> + hrtimer_cancel(&op->thrtimer);
>
> call_rcu(&op->rcu, bcm_free_op_rcu);
> }
> @@ -844,7 +845,7 @@ static int bcm_delete_rx_op(struct list_head *ops, struct bcm_msg_head *mh,
> bcm_rx_handler, op);
>
> list_del(&op->list);
> - bcm_remove_op(op);
> + bcm_remove_op(op, false);
> return 1; /* done */
> }
> }
> @@ -864,7 +865,7 @@ static int bcm_delete_tx_op(struct list_head *ops, struct bcm_msg_head *mh,
> if ((op->can_id == mh->can_id) && (op->ifindex == ifindex) &&
> (op->flags & CAN_FD_FRAME) == (mh->flags & CAN_FD_FRAME)) {
> list_del(&op->list);
> - bcm_remove_op(op);
> + bcm_remove_op(op, true);
> return 1; /* done */
> }
> }
> @@ -1015,10 +1016,6 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> HRTIMER_MODE_REL_SOFT);
> op->timer.function = bcm_tx_timeout_handler;
>
> - /* currently unused in tx_ops */
> - 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);
>
> @@ -1277,7 +1274,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> if (err) {
> /* this bcm rx op is broken -> remove it */
> list_del(&op->list);
> - bcm_remove_op(op);
> + bcm_remove_op(op, false);
> return err;
> }
> }
> @@ -1581,7 +1578,7 @@ static int bcm_release(struct socket *sock)
> #endif /* CONFIG_PROC_FS */
>
> list_for_each_entry_safe(op, next, &bo->tx_ops, list)
> - bcm_remove_op(op);
> + bcm_remove_op(op, true);
>
> list_for_each_entry_safe(op, next, &bo->rx_ops, list) {
> /*
> @@ -1613,7 +1610,7 @@ static int bcm_release(struct socket *sock)
> synchronize_rcu();
>
> list_for_each_entry_safe(op, next, &bo->rx_ops, list)
> - bcm_remove_op(op);
> + bcm_remove_op(op, false);
>
> /* remove device reference */
> if (bo->bound) {
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 05/21] can: bcm: Don't initialized an unused hrtimer
2024-10-30 10:49 ` Oliver Hartkopp
@ 2024-10-30 12:15 ` Nam Cao
2024-10-30 14:51 ` Oliver Hartkopp
0 siblings, 1 reply; 37+ messages in thread
From: Nam Cao @ 2024-10-30 12:15 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel, Jakub Kicinski
On Wed, Oct 30, 2024 at 11:49:49AM +0100, Oliver Hartkopp wrote:
> On 28.10.24 08:29, Nam Cao wrote:
> > The hrtimer "thrtimer" is not used for TX. But this timer is initialized
> > regardless.
> >
> > Remove the hrtimer_init() for the unused hrtimer and change bcm_remove_op()
> > to make sure hrtimer_cancel() is not called with the uninitialized hrtimer.
>
> NAK.
>
> There are several other occurrences of thrtimer that are not covered by
> RX/TX distinction, where the second timer is canceled.
>
> This one-time init and cancel of an unused hrtimer costs nearly nothing and
> is not even in any hot path.
>
> So this incomplete patch only adds complexity and potential error cases in
> some 20 y/o code for nothing.
The "real" motivation is preparing to use hrtimer_setup() instead of
hrtimer_init() [1] and deleting hrtimer_init() [2]. The new function
mandates a callback function, and since the TX thrtimer doesn't have a
callback function, hrtimer_setup() cannot be used.
Your concerns are also valid. So I can drop this patch, and use a dummy
function to make hrtimer_setup() happy, like how it's done for the rt2x00
driver [3]. It will make the driver a bit ugly, but it's obvious that it
won't cause any regression.
Best regards,
Nam
[1] https://lore.kernel.org/lkml/e4ce3a3a28625d54ef93e47bfb02f7ffb741758a.1729865232.git.namcao@linutronix.de/
[2] https://lore.kernel.org/lkml/7bde2762d82d30dab184c7a747e76afc41208da0.1729865740.git.namcao@linutronix.de/
[3] https://lore.kernel.org/lkml/49f2bce487f56eb2a3ff572ea6d7de0a43560c0f.1729865232.git.namcao@linutronix.de/
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 05/21] can: bcm: Don't initialized an unused hrtimer
2024-10-30 12:15 ` Nam Cao
@ 2024-10-30 14:51 ` Oliver Hartkopp
2024-10-30 15:01 ` Oliver Hartkopp
0 siblings, 1 reply; 37+ messages in thread
From: Oliver Hartkopp @ 2024-10-30 14:51 UTC (permalink / raw)
To: Nam Cao
Cc: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel, Jakub Kicinski
On 30.10.24 13:15, Nam Cao wrote:
> On Wed, Oct 30, 2024 at 11:49:49AM +0100, Oliver Hartkopp wrote:
>> On 28.10.24 08:29, Nam Cao wrote:
>>> The hrtimer "thrtimer" is not used for TX. But this timer is initialized
>>> regardless.
>>>
>>> Remove the hrtimer_init() for the unused hrtimer and change bcm_remove_op()
>>> to make sure hrtimer_cancel() is not called with the uninitialized hrtimer.
>>
>> NAK.
>>
>> There are several other occurrences of thrtimer that are not covered by
>> RX/TX distinction, where the second timer is canceled.
>>
>> This one-time init and cancel of an unused hrtimer costs nearly nothing and
>> is not even in any hot path.
>>
>> So this incomplete patch only adds complexity and potential error cases in
>> some 20 y/o code for nothing.
>
> The "real" motivation is preparing to use hrtimer_setup() instead of
> hrtimer_init() [1] and deleting hrtimer_init() [2]. The new function
> mandates a callback function, and since the TX thrtimer doesn't have a
> callback function, hrtimer_setup() cannot be used.
>
> Your concerns are also valid. So I can drop this patch, and use a dummy
> function to make hrtimer_setup() happy, like how it's done for the rt2x00
> driver [3]. It will make the driver a bit ugly, but it's obvious that it
> won't cause any regression.
Many thanks for your efforts!
That's a good approach!
Best regards,
Oliver
>
> Best regards,
> Nam
>
> [1] https://lore.kernel.org/lkml/e4ce3a3a28625d54ef93e47bfb02f7ffb741758a.1729865232.git.namcao@linutronix.de/
> [2] https://lore.kernel.org/lkml/7bde2762d82d30dab184c7a747e76afc41208da0.1729865740.git.namcao@linutronix.de/
> [3] https://lore.kernel.org/lkml/49f2bce487f56eb2a3ff572ea6d7de0a43560c0f.1729865232.git.namcao@linutronix.de/
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 05/21] can: bcm: Don't initialized an unused hrtimer
2024-10-30 14:51 ` Oliver Hartkopp
@ 2024-10-30 15:01 ` Oliver Hartkopp
2024-10-30 15:18 ` Thomas Gleixner
0 siblings, 1 reply; 37+ messages in thread
From: Oliver Hartkopp @ 2024-10-30 15:01 UTC (permalink / raw)
To: Nam Cao
Cc: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel, Jakub Kicinski
Just an additional idea:
Would it probably make sense to create a generic dummy function for all
those cases like in the referenced rt2x00 driver?
E.g. as (inline?) function in hrtimer.h
enum hrtimer_restart hrtimer_nop_callback(struct hrtimer *timer)
{
return HRTIMER_NORESTART;
}
??
Best regards,
Oliver
On 30.10.24 15:51, Oliver Hartkopp wrote:
>
>
> On 30.10.24 13:15, Nam Cao wrote:
>> On Wed, Oct 30, 2024 at 11:49:49AM +0100, Oliver Hartkopp wrote:
>>> On 28.10.24 08:29, Nam Cao wrote:
>>>> The hrtimer "thrtimer" is not used for TX. But this timer is
>>>> initialized
>>>> regardless.
>>>>
>>>> Remove the hrtimer_init() for the unused hrtimer and change
>>>> bcm_remove_op()
>>>> to make sure hrtimer_cancel() is not called with the uninitialized
>>>> hrtimer.
>>>
>>> NAK.
>>>
>>> There are several other occurrences of thrtimer that are not covered by
>>> RX/TX distinction, where the second timer is canceled.
>>>
>>> This one-time init and cancel of an unused hrtimer costs nearly
>>> nothing and
>>> is not even in any hot path.
>>>
>>> So this incomplete patch only adds complexity and potential error
>>> cases in
>>> some 20 y/o code for nothing.
>>
>> The "real" motivation is preparing to use hrtimer_setup() instead of
>> hrtimer_init() [1] and deleting hrtimer_init() [2]. The new function
>> mandates a callback function, and since the TX thrtimer doesn't have a
>> callback function, hrtimer_setup() cannot be used.
>>
>> Your concerns are also valid. So I can drop this patch, and use a dummy
>> function to make hrtimer_setup() happy, like how it's done for the rt2x00
>> driver [3]. It will make the driver a bit ugly, but it's obvious that it
>> won't cause any regression.
>
> Many thanks for your efforts!
> That's a good approach!
>
> Best regards,
> Oliver
>
>>
>> Best regards,
>> Nam
>>
>> [1] https://lore.kernel.org/lkml/
>> e4ce3a3a28625d54ef93e47bfb02f7ffb741758a.1729865232.git.namcao@linutronix.de/
>> [2] https://lore.kernel.org/
>> lkml/7bde2762d82d30dab184c7a747e76afc41208da0.1729865740.git.namcao@linutronix.de/
>> [3] https://lore.kernel.org/
>> lkml/49f2bce487f56eb2a3ff572ea6d7de0a43560c0f.1729865232.git.namcao@linutronix.de/
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 05/21] can: bcm: Don't initialized an unused hrtimer
2024-10-30 15:01 ` Oliver Hartkopp
@ 2024-10-30 15:18 ` Thomas Gleixner
0 siblings, 0 replies; 37+ messages in thread
From: Thomas Gleixner @ 2024-10-30 15:18 UTC (permalink / raw)
To: Oliver Hartkopp, Nam Cao
Cc: Anna-Maria Behnsen, Frederic Weisbecker, Andreas Hindborg,
Alice Ryhl, Miguel Ojeda, Kees Cook, linux-kernel, Jakub Kicinski
On Wed, Oct 30 2024 at 16:01, Oliver Hartkopp wrote:
> Just an additional idea:
>
> Would it probably make sense to create a generic dummy function for all
> those cases like in the referenced rt2x00 driver?
>
> E.g. as (inline?) function in hrtimer.h
>
> enum hrtimer_restart hrtimer_nop_callback(struct hrtimer *timer)
> {
> return HRTIMER_NORESTART;
> }
Yes.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 04/21] KVM: x86/xen: Initialize hrtimer in kvm_xen_init_vcpu()
2024-10-28 7:29 ` [PATCH 04/21] KVM: x86/xen: Initialize hrtimer in kvm_xen_init_vcpu() Nam Cao
2024-10-28 16:01 ` Sean Christopherson
@ 2024-10-30 18:05 ` Sean Christopherson
1 sibling, 0 replies; 37+ messages in thread
From: Sean Christopherson @ 2024-10-30 18:05 UTC (permalink / raw)
To: Nam Cao
Cc: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel, Paolo Bonzini, x86, kvm
On Mon, Oct 28, 2024, Nam Cao wrote:
> 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.
>
> Instead of "abusing" the 'function' field, move the hrtimer initialization
> into kvm_xen_init_vcpu() so that it will only be initialized once.
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
With or without the helper dropped, which can easily go on top at our leisure:
Acked-by: Sean Christopherson <seanjc@google.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 06/21] wifi: rt2x00: Remove redundant hrtimer_init()
2024-10-28 7:29 ` [PATCH 06/21] wifi: rt2x00: Remove redundant hrtimer_init() Nam Cao
@ 2024-10-31 14:13 ` Kalle Valo
0 siblings, 0 replies; 37+ messages in thread
From: Kalle Valo @ 2024-10-31 14:13 UTC (permalink / raw)
To: Nam Cao
Cc: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Kees Cook,
linux-kernel, Nam Cao, linux-wireless
Nam Cao <namcao@linutronix.de> wrote:
> 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 <namcao@linutronix.de>
Feel free to this via another tree:
Acked-by: Kalle Valo <kvalo@kernel.org>
--
https://patchwork.kernel.org/project/linux-wireless/patch/b1279cd5bf0c77a9434a70a025701ec251a30f8e.1729864615.git.namcao@linutronix.de/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2024-10-31 14:13 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-28 7:29 [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Nam Cao
2024-10-28 7:29 ` [PATCH 01/21] hrtimers: Add missing hrtimer_init event trace points Nam Cao
2024-10-28 7:29 ` [PATCH 02/21] hrtimers: Remove unused hrtimer_init_sleeper() Nam Cao
2024-10-28 7:29 ` [PATCH 03/21] drm/i915/request: Remove unnecessary abuse of hrtimer::function Nam Cao
2024-10-28 7:29 ` [PATCH 04/21] KVM: x86/xen: Initialize hrtimer in kvm_xen_init_vcpu() Nam Cao
2024-10-28 16:01 ` Sean Christopherson
2024-10-28 22:19 ` Thomas Gleixner
2024-10-30 18:05 ` Sean Christopherson
2024-10-28 7:29 ` [PATCH 05/21] can: bcm: Don't initialized an unused hrtimer Nam Cao
2024-10-30 10:49 ` Oliver Hartkopp
2024-10-30 12:15 ` Nam Cao
2024-10-30 14:51 ` Oliver Hartkopp
2024-10-30 15:01 ` Oliver Hartkopp
2024-10-30 15:18 ` Thomas Gleixner
2024-10-28 7:29 ` [PATCH 06/21] wifi: rt2x00: Remove redundant hrtimer_init() Nam Cao
2024-10-31 14:13 ` Kalle Valo
2024-10-28 7:29 ` [PATCH 07/21] io_uring: Remove redundant hrtimer's callback function setup Nam Cao
2024-10-28 7:29 ` [PATCH 08/21] hrtimers: Introduce hrtimer_setup() to replace hrtimer_init() Nam Cao
2024-10-28 7:29 ` [PATCH 09/21] hrtimers: Introduce hrtimer_setup_on_stack() Nam Cao
2024-10-28 7:29 ` [PATCH 10/21] hrtimers: Introduce hrtimer_setup_sleeper_on_stack() Nam Cao
2024-10-28 7:29 ` [PATCH 11/21] hrtimers: Introduce hrtimer_update_function() Nam Cao
2024-10-28 7:29 ` [PATCH 12/21] fs/aio: Switch to use hrtimer_setup_sleeper_on_stack() Nam Cao
2024-10-28 7:29 ` [PATCH 13/21] futex: " Nam Cao
2024-10-28 7:29 ` [PATCH 14/21] net: pktgen: " Nam Cao
2024-10-28 7:29 ` [PATCH 15/21] timers: " Nam Cao
2024-10-28 7:29 ` [PATCH 16/21] wait: " Nam Cao
2024-10-28 7:29 ` [PATCH 17/21] hrtimers: Delete hrtimer_init_sleeper_on_stack() Nam Cao
2024-10-28 7:29 ` [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack() Nam Cao
2024-10-28 9:09 ` Peter Zijlstra
2024-10-28 10:50 ` Thomas Gleixner
2024-10-28 10:58 ` Peter Zijlstra
2024-10-28 22:33 ` Thomas Gleixner
2024-10-28 7:29 ` [PATCH 19/21] io_uring: " Nam Cao
2024-10-28 7:29 ` [PATCH 20/21] alarmtimer: Switch to use hrtimer_setup() and hrtimer_setup_on_stack() Nam Cao
2024-10-28 7:29 ` [PATCH 21/21] hrtimers: Delete hrtimer_init_on_stack() Nam Cao
2024-10-28 16:05 ` [PATCH 00/21] hrtimers: Switch to new hrtimer interface functions (1/5) Sean Christopherson
2024-10-29 8:15 ` Thomas Gleixner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox