Messages in this thread Patch in this message | | | Subject | Re: [patch 1/3] net: serialize hrtimer callback in sched_cbq | From | Peter Zijlstra <> | Date | Fri, 17 Jul 2009 14:14:52 +0200 |
| |
On Tue, 2009-07-14 at 09:42 -0700, Linus Torvalds wrote: > > On Tue, 14 Jul 2009, Peter Zijlstra wrote: > > > > Linus really hated the softirq mode, which is what prompted me to change > > that. > > > > Now, it might be he only hated the particular interface and the > > resulting code, but I think to remember he simply thought the whole > > thing daft. > > Yes. And I hated the bugs it had. > > Don't make something as core as timers any more complicated. Don't take > locks in timers and then complain about deadlocks. If your locking is > broken, don't make the core timers be idiotically broken. > > Because it was. The code was a total mess to follow, and had bugs.
How would something like the below work for people?
--- include/linux/hrtimer.h | 22 ++++++++++++++++++++-- kernel/hrtimer.c | 23 ++++++++++++++++++++++- 2 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h index 4759917..e7559fe 100644 --- a/include/linux/hrtimer.h +++ b/include/linux/hrtimer.h @@ -22,6 +22,7 @@ #include <linux/wait.h> #include <linux/percpu.h> #include <linux/timer.h> +#include <linux/interrupt.h> struct hrtimer_clock_base; @@ -91,7 +92,6 @@ enum hrtimer_restart { * @function: timer expiry callback function * @base: pointer to the timer base (per cpu and per clock) * @state: state information (See bit values above) - * @cb_entry: list head to enqueue an expired timer into the callback list * @start_site: timer statistics field to store the site where the timer * was started * @start_comm: timer statistics field to store the name of the process which @@ -108,7 +108,6 @@ struct hrtimer { enum hrtimer_restart (*function)(struct hrtimer *); struct hrtimer_clock_base *base; unsigned long state; - struct list_head cb_entry; #ifdef CONFIG_TIMER_STATS int start_pid; void *start_site; @@ -116,6 +115,12 @@ struct hrtimer { #endif }; +struct hrtimer_softirq { + struct hrtimer timer; + struct tasklet_struct tasklet; + enum hrtimer_restart (*function)(struct hrtimer *); +}; + /** * struct hrtimer_sleeper - simple sleeper structure * @timer: embedded timer structure @@ -335,6 +340,19 @@ static inline void hrtimer_init_on_stack(struct hrtimer *timer, static inline void destroy_hrtimer_on_stack(struct hrtimer *timer) { } #endif +enum hrtimer_restart __hrtimer_softirq_trampoline(struct hrtimer *timer); +void __hrtimer_tasklet_trampoline(unsigned long data); + +static inline void hrtimer_softirq_init(struct hrtimer_softirq *stimer, + enum hrtimer_restart (*func)(struct hrtimer *), + clockid_t which_clock, enum hrtimer_mode mode) +{ + hrtimer_init(&stimer->timer, which_clock, mode); + stimer->timer.function = __hrtimer_softirq_trampoline; + tasklet_init(&stimer->tasklet, __hrtimer_tasklet_trampoline, stimer); + stimer->function = func; +} + /* Basic timer operations: */ extern int hrtimer_start(struct hrtimer *timer, ktime_t tim, const enum hrtimer_mode mode); diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index ab5eb70..dae063c 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -1098,7 +1098,6 @@ static void __hrtimer_init(struct hrtimer *timer, clockid_t clock_id, clock_id = CLOCK_MONOTONIC; timer->base = &cpu_base->clock_base[clock_id]; - INIT_LIST_HEAD(&timer->cb_entry); hrtimer_init_timer_hres(timer); #ifdef CONFIG_TIMER_STATS @@ -1141,6 +1140,28 @@ int hrtimer_get_res(const clockid_t which_clock, struct timespec *tp) } EXPORT_SYMBOL_GPL(hrtimer_get_res); +enum hrtimer_restart __hrtimer_softirq_trampoline(struct hrtimer *timer) +{ + struct hrtimer_softirq *stimer = + container_of(timer, struct hrtimer_softirq, timer); + + tasklet_hi_schedule(&timer->tasklet); + + return HRTIMER_NORESTART; +} +EXPORT_SYMBOL_GPL(__hrtimer_softirq_trampoline); + +void __hrtimer_tasklet_trampoline(unsigned long data) +{ + struct hrtimer_softirq *stimer = (void *)data; + enum hrtimer_restart restart; + + restart = stimer->function(&stimer->timer); + if (restart != HRTIMER_NORESTART) + hrtimer_restart(&stimer->timer); +} +EXPORT_SYMBOL_GPL(__hrtimer_tasklet_trampoline); + static void __run_hrtimer(struct hrtimer *timer) { struct hrtimer_clock_base *base = timer->base;
| |