Consolidate hrtimer initialization - Part 5
Subsystems (10)
v2 cover Consolidate hrtimer initialization - Part 5
All previous users of hrtimer_init() have been converted to use hrtimer_setup() in the previous series:
https://lore.kernel.org/lkml/[email protected]
https://lore.kernel.org/lkml/[email protected]
https://lore.kernel.org/lkml/[email protected]
This series does the final cleanup:
- Delete hrtimer_init()
- Convert users who touch hrtimer::function to use hrtimer_update_function(). Make hrtimer::function private afterward.
- Rename the remaining *hrtimer_init*() to *hrtimer_setup*() to keep the names consistent
v1 -> v2 https://lore.kernel.org/lkml/[email protected]
- rebaes onto v6.14-rc1
---
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jens Axboe <[email protected]>
Cc: Kalle Valo <[email protected]>
Cc: Steven Rostedt <[email protected]>
--- Documentation/trace/ftrace.rst | 4 +- .../net/wireless/ralink/rt2x00/rt2800mmio.c | 2 +- .../net/wireless/ralink/rt2x00/rt2800usb.c | 2 +- drivers/tty/serial/xilinx_uartps.c | 4 +- include/linux/hrtimer.h | 4 +- include/linux/hrtimer_types.h | 4 +- include/trace/events/timer.h | 8 +-- io_uring/io_uring.c | 2 +- kernel/time/hrtimer.c | 69 +++++-------------- kernel/time/timer_list.c | 2 +- tools/perf/tests/shell/trace_btf_enum.sh | 2 +- 11 files changed, 35 insertions(+), 68 deletions(-)
Signature · 1 line
2.39.5
PATCH v2 1/12 hrtimers: Delete hrtimer_init()
hrtimer_init() is unused. Delete it.
--- include/linux/hrtimer.h | 2 -- include/linux/hrtimer_types.h | 2 +- kernel/time/hrtimer.c | 20 -------------------- 3 files changed, 1 insertion(+), 23 deletions(-)
include/linux/hrtimer.h +0 -2
@@ -231,8 +231,6 @@ static inline enum hrtimer_restart hrtimer_dummy_timeout(struct hrtimer *unused)/* Exported timer functions: */ /* 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_setup_on_stack(struct hrtimer *timer,
include/linux/hrtimer_types.h +1 -1
@@ -34,7 +34,7 @@ enum hrtimer_restart {* @is_hard: Set if hrtimer will be expired in hard interrupt context * even on RT. * -* The hrtimer structure must be initialized by hrtimer_init() +* The hrtimer structure must be initialized by hrtimer_setup() */ struct hrtimer {struct timerqueue_node node;
kernel/time/hrtimer.c +0 -20
@@ -1645,26 +1645,6 @@ static void __hrtimer_setup(struct hrtimer *timer, timer->function = function; } -/** -* hrtimer_init - initialize a timer to the given clock -* @timer: the timer to be initialized -* @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_init (struct hrtimer *timer,clockid_t clock_id, -enum hrtimer_mode mode) -{ -debug_init (timer, clock_id, mode); -__hrtimer_init (timer, clock_id, mode); -} -EXPORT_SYMBOL_GPL (hrtimer_init); -/** * hrtimer_setup - initialize a timer to the given clock * @timer: the timer to be initialized
Signature · 1 line
2.39.5
PATCH v2 2/12 hrtimers: Switch to use __htimer_setup()
In reply to Nam Cao__hrtimer_init_sleeper() calls __hrtimer_init() and also setups the callback function. But there is already __hrtimer_setup() which does both actions.
Switch to use __hrtimer_setup() to simplify the code.
--- kernel/time/hrtimer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
kernel/time/hrtimer.c +1 -2
@@ -2066,8 +2066,7 @@ static void __hrtimer_init_sleeper(struct hrtimer_sleeper *sl, mode |=HRTIMER_MODE_HARD ; } -__hrtimer_init (&sl->timer, clock_id, mode); - sl->timer.function = hrtimer_wakeup; +__hrtimer_setup (&sl->timer, hrtimer_wakeup, clock_id, mode); sl->task = current; }
Signature · 1 line
2.39.5
PATCH v2 3/12 hrtimers: Merge __hrtimer_init() into __hrtimer_setup()
In reply to Nam Cao__hrtimer_init() is only called by __hrtimer_setup(). Simplify by merging __hrtimer_init() into __hrtimer_setup().
--- kernel/time/hrtimer.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
kernel/time/hrtimer.c +3 -9
@@ -1597,8 +1597,9 @@ static inline int hrtimer_clockid_to_base(clockid_t clock_id)return HRTIMER_BASE_MONOTONIC ; } -static void __hrtimer_init (struct hrtimer *timer,clockid_t clock_id, -enum hrtimer_mode mode) +static void __hrtimer_setup (struct hrtimer *timer, +enum hrtimer_restart (*function)(struct hrtimer *), +clockid_t clock_id,enum hrtimer_mode mode) {bool softtimer = !!(mode &HRTIMER_MODE_SOFT );struct hrtimer_cpu_base *cpu_base;
@@ -1631,13 +1632,6 @@ static void __hrtimer_init(struct hrtimer *timer, clockid_t clock_id, timer->is_hard = !!(mode &HRTIMER_MODE_HARD ); timer->base = &cpu_base->clock_base[base];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;
Signature · 1 line
2.39.5
PATCH v2 4/12 serial: xilinx_uartps: Use helper function hrtimer_update_function()
In reply to Nam CaoThe field 'function' of struct hrtimer should not be changed directly, as the write is lockless and a concurrent timer expiry might end up using the wrong function pointer.
Switch to use hrtimer_update_function() which also performs runtime checks that it is safe to modify the callback.
--- drivers/tty/serial/xilinx_uartps.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/tty/serial/xilinx_uartps.c +2 -2
@@ -454,7 +454,7 @@ static void cdns_uart_handle_tx(void *dev_id)if (cdns_uart->port->rs485.flags &SER_RS485_ENABLED && (kfifo_is_empty (&tport->xmit_fifo) ||uart_tx_stopped (port))) { - cdns_uart->tx_timer.function = &cdns_rs485_rx_callback; +hrtimer_update_function (&cdns_uart->tx_timer, cdns_rs485_rx_callback);hrtimer_start (&cdns_uart->tx_timer,ns_to_ktime (cdns_calc_after_tx_delay (cdns_uart)),HRTIMER_MODE_REL ); }
@@ -734,7 +734,7 @@ static void cdns_uart_start_tx(struct uart_port *port)if (cdns_uart->port->rs485.flags &SER_RS485_ENABLED ) {if (!cdns_uart->rs485_tx_started) { - cdns_uart->tx_timer.function = &cdns_rs485_tx_callback; +hrtimer_update_function (&cdns_uart->tx_timer, cdns_rs485_tx_callback);cdns_rs485_tx_setup (cdns_uart); return hrtimer_start(&cdns_uart->tx_timer, ms_to_ktime(port->rs485.delay_rts_before_send),
Signature · 1 line
2.39.5
PATCH v2 5/12 io_uring: Use helper function hrtimer_update_function()
In reply to Nam CaoThe field 'function' of struct hrtimer should not be changed directly, as the write is lockless and a concurrent timer expiry might end up using the wrong function pointer.
Switch to use hrtimer_update_function() which also performs runtime checks that it is safe to modify the callback.
--- io_uring/io_uring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
io_uring/io_uring.c +1 -1
@@ -2421,7 +2421,7 @@ static enum hrtimer_restart io_cqring_min_timer_wakeup(struct hrtimer *timer) goto out_wake; } - iowq->t.function = io_cqring_timer_wakeup; +hrtimer_update_function (&iowq->t, io_cqring_timer_wakeup);hrtimer_set_expires (timer, iowq->timeout);return HRTIMER_RESTART ; out_wake:
Signature · 1 line
2.39.5
Jens Axboe
maintainerRe: [PATCH v2 05/12] io_uring: Use helper function hrtimer_update_function()
Quoted 18 lines · Nam Cao
The field 'function' of struct hrtimer should not be changed directly, as the write is lockless and a concurrent timer expiry might end up using the wrong function pointer.
Switch to use
hrtimer_update_function()which also performs runtime checks that it is safe to modify the callback.Signed-off-by Nam Cao <[email protected]>Cc Jens Axboe <[email protected]>--- io_uring/io_uring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)io_uring/io_uring.c +0
-0
io_uring/io_uring.c static enum hrtimer_restart io_cqring_min_timer_wakeup(struct hrtimer *timer) · line 2427
Quoted 8 lines
io_uring/io_uring.c +1
-1@@ -2421,7 +2421,7 @@ static enum hrtimer_restart io_cqring_min_timer_wakeup(struct hrtimer *timer) goto out_wake; } - iowq->t.function = io_cqring_timer_wakeup; +hrtimer_update_function (&iowq->t, io_cqring_timer_wakeup);hrtimer_set_expires (timer, iowq->timeout);return HRTIMER_RESTART ; out_wake:
The timer is known expired here, it's inside the callback. Is this really necessary or useful?
Signature · 1 line
Jens Axboe
Re: [PATCH v2 05/12] io_uring: Use helper function hrtimer_update_function()
Quoted 9 lines · Jens Axboe
Quoted 8 lines · Nam Cao
io_uring/io_uring.c +0
-0@@ -2421,7 +2421,7 @@ static enum hrtimer_restart io_cqring_min_timer_wakeup(struct hrtimer *timer) goto out_wake; }
Quoted 8 lines
io_uring/io_uring.c static enum hrtimer_restart io_cqring_min_timer_wakeup(struct hrtimer *timer) · line 2427
Quoted 5 lines
io_uring/io_uring.c +1
-1- iowq->t.function = io_cqring_timer_wakeup; +hrtimer_update_function (&iowq->t, io_cqring_timer_wakeup);hrtimer_set_expires (timer, iowq->timeout);return HRTIMER_RESTART ; out_wake:The timer is known expired here, it's inside the callback. Is this really necessary or useful?
It's not strictly required here, but in the end this allows to make the .function member private, which prevents stupid code fiddling with it without proper sanity checks in the debug case.
Thanks,
tglx
Jens Axboe
maintainerRe: [PATCH v2 05/12] io_uring: Use helper function hrtimer_update_function()
Quoted 19 lines · Thomas Gleixner
Quoted 9 lines · Jens Axboe
Quoted 8 lines · Nam Cao
io_uring/io_uring.c +0
-0@@ -2421,7 +2421,7 @@ static enum hrtimer_restart io_cqring_min_timer_wakeup(struct hrtimer *timer) goto out_wake; }Quoted 8 lines
io_uring/io_uring.c static enum hrtimer_restart io_cqring_min_timer_wakeup(struct hrtimer *timer) · line 2427
Quoted 5 lines
io_uring/io_uring.c +1
-1- iowq->t.function = io_cqring_timer_wakeup; +hrtimer_update_function (&iowq->t, io_cqring_timer_wakeup);hrtimer_set_expires (timer, iowq->timeout);return HRTIMER_RESTART ; out_wake:The timer is known expired here, it's inside the callback. Is this really necessary or useful?
Quoted 3 lines
It's not strictly required here, but in the end this allows to make the .function member private, which prevents stupid code fiddling with it without proper sanity checks in the debug case.
While that makes sense, this is also a potentially hot path for min timeout usage, which with small timeouts for batch/latency reasons can be run tens of thousand times per second. Adding a lock and IRQ dance would be counter productive.
How about we just add a comment on why this is fine, rather than slow down a case that's perfectly fine by wrapping it in something much more expensive than a simple memory write? Or perhaps have a basic helper to set it that doesn't do the unnecessary irq lock guard? That would still allow you to make .function private.
Signature · 1 line
Jens Axboe
Re: [PATCH v2 05/12] io_uring: Use helper function hrtimer_update_function()
Quoted 20 lines · Jens Axboe
Quoted 19 lines · Thomas Gleixner
Quoted 17 lines · Jens Axboe
io_uring/io_uring.c static enum hrtimer_restart io_cqring_min_timer_wakeup(struct hrtimer *timer) · line 2427
Quoted 13 lines · Nam Cao
io_uring/io_uring.c +1
-1@@ -2421,7 +2421,7 @@ static enum hrtimer_restart io_cqring_min_timer_wakeup(struct hrtimer *timer) goto out_wake; } - iowq->t.function = io_cqring_timer_wakeup; +hrtimer_update_function (&iowq->t, io_cqring_timer_wakeup);hrtimer_set_expires (timer, iowq->timeout);return HRTIMER_RESTART ; out_wake:The timer is known expired here, it's inside the callback. Is this really necessary or useful?
Quoted 8 lines
Quoted 3 lines
It's not strictly required here, but in the end this allows to make the .function member private, which prevents stupid code fiddling with it without proper sanity checks in the debug case.
While that makes sense, this is also a potentially hot path for min timeout usage, which with small timeouts for batch/latency reasons can be run tens of thousand times per second. Adding a lock and IRQ dance would be counter productive.
I fundamentally hate the fact that C does not enforce encapsulation in the first place. :)
Quoted 1 line
How about we just add a comment on why this is fine, rather than
Comments are the worst of a solution as you know.
Quoted 4 lines
slow down a case that's perfectly fine by wrapping it in something much more expensive than a simple memory write? Or perhaps have a basic helper to set it that doesn't do the unnecessary irq lock guard? That would still allow you to make .function private.
The right solution is to add a hotpath helper, which falls back to the more expensive variant with some anyway expensive debug option, or make the expensive part of hrtimer_update_function() depend on that debug option in the first place and otherwise fall back to a simple store.
The latter is the right thing to do. Let me fix that up in mainline.
Thanks,
tglx
Jens Axboe
maintainerRe: [PATCH v2 05/12] io_uring: Use helper function hrtimer_update_function()
Quoted 30 lines · Thomas Gleixner
Quoted 20 lines · Jens Axboe
Quoted 19 lines · Thomas Gleixner
Quoted 17 lines · Jens Axboe
io_uring/io_uring.c static enum hrtimer_restart io_cqring_min_timer_wakeup(struct hrtimer *timer) · line 2427
Quoted 13 lines · Nam Cao
io_uring/io_uring.c +1
-1@@ -2421,7 +2421,7 @@ static enum hrtimer_restart io_cqring_min_timer_wakeup(struct hrtimer *timer) goto out_wake; } - iowq->t.function = io_cqring_timer_wakeup; +hrtimer_update_function (&iowq->t, io_cqring_timer_wakeup);hrtimer_set_expires (timer, iowq->timeout);return HRTIMER_RESTART ; out_wake:The timer is known expired here, it's inside the callback. Is this really necessary or useful?
Quoted 8 lines
Quoted 3 lines
It's not strictly required here, but in the end this allows to make the .function member private, which prevents stupid code fiddling with it without proper sanity checks in the debug case.
While that makes sense, this is also a potentially hot path for min timeout usage, which with small timeouts for batch/latency reasons can be run tens of thousand times per second. Adding a lock and IRQ dance would be counter productive.
Quoted 6 lines
I fundamentally hate the fact that C does not enforce encapsulation in the first place. :)
Quoted 1 line
How about we just add a comment on why this is fine, rather than
Comments are the worst of a solution as you know.
Sure I don't disagree - this is just my way of attempting to be nice and suggest alternatives, as I don't like this patch.
Quoted 11 lines
Quoted 4 lines
slow down a case that's perfectly fine by wrapping it in something much more expensive than a simple memory write? Or perhaps have a basic helper to set it that doesn't do the unnecessary irq lock guard? That would still allow you to make .function private.
The right solution is to add a hotpath helper, which falls back to the more expensive variant with some anyway expensive debug option, or make the expensive part of
hrtimer_update_function()depend on that debug option in the first place and otherwise fall back to a simple store.The latter is the right thing to do. Let me fix that up in mainline.
Yep exactly, this is the way. Thanks!
Signature · 1 line
Jens Axboe
PATCH 1/1 hrtimers: Make hrtimer_update_function() less expensive
The sanity checks in hrtimer_update_function() are expensive for high frequency usage like in the io/uring code due to locking.
Hide the sanity checks behind CONFIG_PROVE_LOCKING, which has a decent chance to be enabled on a regular basis for testing.
--- include/linux/hrtimer.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
include/linux/hrtimer.h +2 -1
@@ -333,6 +333,7 @@ static inline int hrtimer_callback_runnistatic inline void hrtimer_update_function (struct hrtimer *timer,enum hrtimer_restart (*function)(struct hrtimer *)) { +#ifdef CONFIG_PROVE_LOCKING guard (raw_spinlock_irqsave)(&timer->base->cpu_base->lock);if (WARN_ON_ONCE (hrtimer_is_queued (timer)))
@@ -340,7 +341,7 @@ static inline void hrtimer_update_functiif (WARN_ON_ONCE (!function))return ; - +#endif timer->function = function; }
Jens Axboe
maintainerRe: [PATCH] hrtimers: Make hrtimer_update_function() less expensive
Quoted 5 lines · Thomas Gleixner
The sanity checks in
hrtimer_update_function()are expensive for high frequency usage like in the io/uring code due to locking.Hide the sanity checks behind
CONFIG_PROVE_LOCKING, which has a decent chance to be enabled on a regular basis for testing.
Looks good to me, thanks Thomas. On my side, I always have a debug run done with PROVE_LOCKING and KASAN, fwiw.
Signature · 1 line
Jens Axboe
Re: [PATCH] hrtimers: Make hrtimer_update_function() less expensive
Quoted 9 lines · Jens Axboe
Quoted 5 lines · Thomas Gleixner
The sanity checks in
hrtimer_update_function()are expensive for high frequency usage like in the io/uring code due to locking.Hide the sanity checks behind
CONFIG_PROVE_LOCKING, which has a decent chance to be enabled on a regular basis for testing.Looks good to me, thanks Thomas. On my side, I always have a debug run done with
PROVE_LOCKINGand KASAN, fwiw.
I assume that with that your objections against the conversion of ioring to hrtimer_update_function() is gone too.
Thanks,
tglx
Jens Axboe
maintainerRe: [PATCH] hrtimers: Make hrtimer_update_function() less expensive
Quoted 6 lines · Thomas Gleixner
Quoted 4 lines · Jens Axboe
Quoted 3 lines · Thomas Gleixner
The sanity checks in
hrtimer_update_function()are expensive for high frequency usage like in the io/uring code due to locking.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Hide the sanity checks behind
CONFIG_PROVE_LOCKING, which has a decent chance to be enabled on a regular basis for testing.Looks good to me, thanks Thomas. On my side, I always have a debug run done with
PROVE_LOCKINGand KASAN, fwiw.I assume that with that your objections against the conversion of ioring to
hrtimer_update_function()is gone too.
Certainly, with the expensive bits under PROVE_LOCKING, I have no objections to the patch.
Signature · 1 line
Jens Axboe
[tip: timers/core] hrtimers: Make hrtimer_update_function() less expensive
In reply to Thomas GleixnerThe following commit has been merged into the timers/core branch of tip:
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 2ea97b76d6712bfb0408e5b81ffd7bc4551d3153 Gitweb: https://git.kernel.org/tip/2ea97b76d6712bfb0408e5b81ffd7bc4551d3153 Author: Thomas Gleixner <[email protected]> AuthorDate: Fri, 07 Feb 2025 22:16:09 +01:00 Committer: Thomas Gleixner <[email protected]> CommitterDate: Mon, 10 Feb 2025 20:51:12 +01:00
hrtimers: Make hrtimer_update_function() less expensive
The sanity checks in hrtimer_update_function() are expensive for high frequency usage like in the io/uring code due to locking.
Hide the sanity checks behind CONFIG_PROVE_LOCKING, which has a decent chance to be enabled on a regular basis for testing.
--- include/linux/hrtimer.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
include/linux/hrtimer.h +2 -1
@@ -333,6 +333,7 @@ static inline int hrtimer_callback_running(struct hrtimer *timer)static inline void hrtimer_update_function (struct hrtimer *timer,enum hrtimer_restart (*function)(struct hrtimer *)) { +#ifdef CONFIG_PROVE_LOCKING guard (raw_spinlock_irqsave)(&timer->base->cpu_base->lock);if (WARN_ON_ONCE (hrtimer_is_queued (timer)))
@@ -340,7 +341,7 @@ static inline void hrtimer_update_function(struct hrtimer *timer,if (WARN_ON_ONCE (!function))return ; - +#endif timer->function = function; }
PATCH v2 6/12 wifi: rt2x00: Switch to use hrtimer_update_function()
In reply to Nam CaoThe field 'function' of struct hrtimer should not be changed directly, as the write is lockless and a concurrent timer expiry might end up using the wrong function pointer.
Switch to use hrtimer_update_function() which also performs runtime checks that it is safe to modify the callback.
--- drivers/net/wireless/ralink/rt2x00/rt2800mmio.c | 2 +- drivers/net/wireless/ralink/rt2x00/rt2800usb.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/net/wireless/ralink/rt2x00/rt2800mmio.c +1 -1
@@ -842,7 +842,7 @@ int rt2800mmio_probe_hw(struct rt2x00_dev *rt2x00dev)/* * Set txstatus timer function. */ - rt2x00dev->txstatus_timer.function = rt2800mmio_tx_sta_fifo_timeout; +hrtimer_update_function (&rt2x00dev->txstatus_timer, rt2800mmio_tx_sta_fifo_timeout);/* * Overwrite TX done handler
drivers/net/wireless/ralink/rt2x00/rt2800usb.c +1 -1
@@ -618,7 +618,7 @@ static int rt2800usb_probe_hw(struct rt2x00_dev *rt2x00dev)/* * Set txstatus timer function. */ - rt2x00dev->txstatus_timer.function = rt2800usb_tx_sta_fifo_timeout; +hrtimer_update_function (&rt2x00dev->txstatus_timer, rt2800usb_tx_sta_fifo_timeout);/* * Overwrite TX done handler
Signature · 1 line
2.39.5
PATCH v2 7/12 hrtimers: Make callback function pointer private
In reply to Nam CaoMake the field 'function' of struct hrtimer private, to prevent users from changing this field in an unsafe way. hrtimer_update_function() should be used if the callback function needs to be changed.
--- include/linux/hrtimer.h | 2 +- include/linux/hrtimer_types.h | 2 +- include/trace/events/timer.h | 4 ++-- kernel/time/hrtimer.c | 8 ++++---- kernel/time/timer_list.c | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-)
include/linux/hrtimer.h +1 -1
@@ -344,7 +344,7 @@ static inline void hrtimer_update_function(struct hrtimer *timer,if (WARN_ON_ONCE (!function))return ; - timer->function = function; +ACCESS_PRIVATE (timer, function) = function; }/* Forward a hrtimer so it expires after now: */
include/linux/hrtimer_types.h +1 -1
@@ -39,7 +39,7 @@ enum hrtimer_restart {struct hrtimer {struct timerqueue_node node;ktime_t _softexpires; -enum hrtimer_restart (*function)(struct hrtimer *); +enum hrtimer_restart (*__private function)(struct hrtimer *);struct hrtimer_clock_base *base;u8 state;u8 is_rel;
include/trace/events/timer.h +2 -2
@@ -235,7 +235,7 @@ TRACE_EVENT(hrtimer_start,TP_fast_assign ( __entry->hrtimer = hrtimer; - __entry->function = hrtimer->function; + __entry->function =ACCESS_PRIVATE (hrtimer, function); __entry->expires =hrtimer_get_expires (hrtimer); __entry->softexpires =hrtimer_get_softexpires (hrtimer); __entry->mode = mode;
@@ -271,7 +271,7 @@ TRACE_EVENT(hrtimer_expire_entry,TP_fast_assign ( __entry->hrtimer = hrtimer; __entry->now = *now; - __entry->function = hrtimer->function; + __entry->function =ACCESS_PRIVATE (hrtimer, function); ), TP_printk("hrtimer=%p function=%ps now=%llu",
kernel/time/hrtimer.c +4 -4
@@ -1326,7 +1326,7 @@ void hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,struct hrtimer_clock_base *base;unsigned long flags; -if (WARN_ON_ONCE (!timer->function)) +if (WARN_ON_ONCE (!ACCESS_PRIVATE (timer, function)))return ;/* * Check whether the HRTIMER_MODE_SOFT bit and hrtimer.is_soft
@@ -1634,9 +1634,9 @@ static void __hrtimer_setup(struct hrtimer *timer,timerqueue_init (&timer->node);if (WARN_ON_ONCE (!function)) - timer->function = hrtimer_dummy_timeout; +ACCESS_PRIVATE (timer, function) = hrtimer_dummy_timeout;else - timer->function = function; +ACCESS_PRIVATE (timer, function) = function; }/**
@@ -1748,7 +1748,7 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base,raw_write_seqcount_barrier (&base->seq);__remove_hrtimer (timer, base,HRTIMER_STATE_INACTIVE ,0 ); - fn = timer->function; + fn =ACCESS_PRIVATE (timer, function);/* * Clear the 'is relative' flag for the TIME_LOW_RES case. If the
kernel/time/timer_list.c +1 -1
@@ -46,7 +46,7 @@ static void print_timer(struct seq_file *m, struct hrtimer *taddr, struct hrtimer *timer, int idx, u64 now) { - SEQ_printf(m, " #%d: <%pK>, %ps", idx, taddr, timer->function); + SEQ_printf(m, " #%d: <%pK>, %ps", idx, taddr, ACCESS_PRIVATE(timer, function));SEQ_printf (m,", S:%02x" , timer->state);SEQ_printf (m,"\n" ); SEQ_printf(m, " # expires at %Lu-%Lu nsecs [in %Ld to %Ld nsecs]\n",
Signature · 1 line
2.39.5
PATCH v2 8/12 hrtimers: Remove unnecessary NULL check in hrtimer_start_range_ns()
In reply to Nam CaoThe 'function' field of struct hrtimer can only be changed using hrtimer_setup*() or hrtimer_update_function(), and both already null-check 'function'. Therefore, null-checking 'function' in hrtimer_start_range_ns() is not necessary.
--- kernel/time/hrtimer.c | 2 -- 1 file changed, 2 deletions(-)
kernel/time/hrtimer.c +0 -2
@@ -1326,8 +1326,6 @@ void hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,struct hrtimer_clock_base *base;unsigned long flags; -if (WARN_ON_ONCE (!ACCESS_PRIVATE (timer, function))) -return ;/* * Check whether the HRTIMER_MODE_SOFT bit and hrtimer.is_soft * match on CONFIG_PREEMPT_RT = n. With PREEMPT_RT check the hard
Signature · 1 line
2.39.5
PATCH v2 9/12 hrtimers: Rename __hrtimer_init_sleeper() to __hrtimer_setup_sleeper()
In reply to Nam CaoAll the hrtimer_init*() functions have been renamed to hrtimer_setup*(). Rename __hrtimer_init_sleeper() to __hrtimer_setup_sleeper() as well, to keep the names consistent.
--- kernel/time/hrtimer.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
kernel/time/hrtimer.c +4 -4
@@ -2021,7 +2021,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_setup_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)
@@ -2031,8 +2031,8 @@ void hrtimer_sleeper_start_expires(struct hrtimer_sleeper *sl, }EXPORT_SYMBOL_GPL (hrtimer_sleeper_start_expires); -static void __hrtimer_init_sleeper (struct hrtimer_sleeper *sl, -clockid_t clock_id,enum hrtimer_mode mode) +static void __hrtimer_setup_sleeper (struct hrtimer_sleeper *sl, +clockid_t clock_id,enum hrtimer_mode mode) {/* * On PREEMPT_RT enabled kernels hrtimers which are not explicitly
@@ -2072,7 +2072,7 @@ 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); +__hrtimer_setup_sleeper (sl, clock_id, mode); }EXPORT_SYMBOL_GPL (hrtimer_setup_sleeper_on_stack);
Signature · 1 line
2.39.5
PATCH v2 10/12 hrtimers: Rename debug_init() to debug_setup()
In reply to Nam CaoAll the hrtimer_init*() functions have been renamed to hrtimer_setup*(). Rename debug_init() to debug_setup() as well, to keep the names consistent.
--- kernel/time/hrtimer.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
kernel/time/hrtimer.c +2 -4
@@ -475,9 +475,7 @@ static inline void debug_hrtimer_activate(struct hrtimer *timer,static inline void debug_hrtimer_deactivate (struct hrtimer *timer) { }#endif -static inline void -debug_init (struct hrtimer *timer,clockid_t clockid, -enum hrtimer_mode mode) +static inline void debug_setup (struct hrtimer *timer,clockid_t clockid,enum hrtimer_mode mode) {debug_hrtimer_init (timer);trace_hrtimer_init (timer, clockid, mode);
@@ -1653,7 +1651,7 @@ static void __hrtimer_setup(struct hrtimer *timer,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); +debug_setup (timer, clock_id, mode);__hrtimer_setup (timer, function, clock_id, mode); }EXPORT_SYMBOL_GPL (hrtimer_setup);
Signature · 1 line
2.39.5
PATCH v2 11/12 hrtimers: Rename debug_init_on_stack() to debug_setup_on_stack()
In reply to Nam CaoAll the hrtimer_init*() functions have been renamed to hrtimer_setup*(). Rename debug_init_on_stack() to debug_setup_on_stack() as well, to keep the names consistent.
--- kernel/time/hrtimer.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
kernel/time/hrtimer.c +4 -4
@@ -481,8 +481,8 @@ static inline void debug_setup(struct hrtimer *timer, clockid_t clockid, enum hrtrace_hrtimer_init (timer, clockid, mode); } -static inline void debug_init_on_stack (struct hrtimer *timer,clockid_t clockid, -enum hrtimer_mode mode) +static inline void debug_setup_on_stack (struct hrtimer *timer,clockid_t clockid, +enum hrtimer_mode mode) {debug_hrtimer_init_on_stack (timer);trace_hrtimer_init (timer, clockid, mode);
@@ -1670,7 +1670,7 @@ 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 ); +debug_setup_on_stack (timer ,clock_id ,mode );__hrtimer_setup (timer, function, clock_id, mode); }EXPORT_SYMBOL_GPL (hrtimer_setup_on_stack);
@@ -2069,7 +2069,7 @@ static void __hrtimer_setup_sleeper(struct hrtimer_sleeper *sl,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); +debug_setup_on_stack (&sl->timer, clock_id, mode);__hrtimer_setup_sleeper (sl, clock_id, mode); }EXPORT_SYMBOL_GPL (hrtimer_setup_sleeper_on_stack);
Signature · 1 line
2.39.5
PATCH v2 12/12 tracing/timers: Rename hrtimer_init event to hrtimer_setup
In reply to Nam CaoThe function hrtimer_init() doesn't exist anymore. It was replaced by hrtimer_setup().
Thus, rename the hrtimer_init trace event to hrtimer_setup to keep it consistent.
--- Documentation/trace/ftrace.rst | 4 ++-- include/trace/events/timer.h | 4 ++-- kernel/time/hrtimer.c | 4 ++-- tools/perf/tests/shell/trace_btf_enum.sh | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-)
Documentation/trace/ftrace.rst +2 -2
@@ -3077,7 +3077,7 @@ Notice that we lost the sys_nanosleep. # cat set_ftrace_filter hrtimer_run_queues hrtimer_run_pending - hrtimer_init + hrtimer_setup hrtimer_cancel hrtimer_try_to_cancel hrtimer_forward
@@ -3115,7 +3115,7 @@ Again, now we want to append. # cat set_ftrace_filter hrtimer_run_queues hrtimer_run_pending - hrtimer_init + hrtimer_setup hrtimer_cancel hrtimer_try_to_cancel hrtimer_forward
include/trace/events/timer.h +2 -2
@@ -185,12 +185,12 @@ TRACE_EVENT(timer_base_idle, { HRTIMER_MODE_REL_PINNED_HARD, "REL|PINNED|HARD" })/** -* hrtimer_init - called when the hrtimer is initialized +* hrtimer_setup - called when the hrtimer is initialized * @hrtimer: pointer to struct hrtimer * @clockid: the hrtimers clock * @mode: the hrtimers mode */ -TRACE_EVENT(hrtimer_init, +TRACE_EVENT(hrtimer_setup, TP_PROTO(struct hrtimer *hrtimer, clockid_t clockid, enum hrtimer_mode mode),
kernel/time/hrtimer.c +2 -2
@@ -478,14 +478,14 @@ static inline void debug_hrtimer_deactivate(struct hrtimer *timer) { }static inline void debug_setup (struct hrtimer *timer,clockid_t clockid,enum hrtimer_mode mode) {debug_hrtimer_init (timer); -trace_hrtimer_init (timer, clockid, mode); +trace_hrtimer_setup (timer, clockid, mode); }static inline void debug_setup_on_stack (struct hrtimer *timer,clockid_t clockid,enum hrtimer_mode mode) {debug_hrtimer_init_on_stack (timer); -trace_hrtimer_init (timer, clockid, mode); +trace_hrtimer_setup (timer, clockid, mode); } static inline void debug_activate(struct hrtimer *timer,
tools/perf/tests/shell/trace_btf_enum.sh +1 -1
@@ -6,7 +6,7 @@ err=0 set -e syscall="landlock_add_rule" -non_syscall="timer:hrtimer_init,timer:hrtimer_start" +non_syscall="timer:hrtimer_setup,timer:hrtimer_start" TESTPROG="perf test -w landlock"
Signature · 1 line
2.39.5