API for timer hooks
Subsystems (2)
RFC PATCH 1/1 API for timer hooks
Hello.
Right now it seems like the only interface for registering the timer hooks is that one of kernel/profile.c, and it is very limited. The arch-specific timer hooks are provided in an arch-specific headers as a static functions.
Since my driver needs the timer hook, I thought it might be a good idea to add an API for registering the timer hooks. The attached patch adds such an API and converts all the relevant places to use it. I changed oprofile to use it, and also converted the arch-specific hooks, which looks like a fair cleanup.
The API allows to register, unregister and grab the timer hook. The grabbing hook will always be executed first, and can decide to prevent an execution of the rest ones. The hook can have the "run_always" flag set, in which case it won't be bypassed, regardless of the grabbing hook.
Does such an API look viable?
As usual, it is needed for the PC-Speaker PCM driver that is currently in an ALSA CVS, awaiting for the proper interface to appear in the kernel.
arch/i386/kernel/time.c +38 -2
@@ -251,6 +251,32 @@EXPORT_SYMBOL (profile_pc);#endif +static int timer_mark_offset (struct pt_regs *regs) +{ + cur_timer->mark_offset (); +return 0 ; +} + +static int do_process_time (struct pt_regs *regs) +{ +#ifndef CONFIG_SMP +update_process_times (user_mode (regs)); +#endif +/* +* In the SMP case we use the local APIC timer interrupt to do the +* profiling, except when we simulate SMP mode on a uniprocessor +* system, in that case we have to call the local interrupt handler. +*/ +#ifndef CONFIG_X86_LOCAL_APIC +profile_tick (CPU_PROFILING , regs); +#else +if (!using_apic_timer) +smp_local_timer_interrupt (regs); +#endif +return 0 ; +} + +/* * timer_interrupt() needs to keep up the real-time clock, * as well as call the "do_timer()" routine every clocktick
@@ -308,8 +334,6 @@*/ write_seqlock (&xtime_lock); - cur_timer->mark_offset (); -do_timer_interrupt (irq,NULL , regs);write_sequnlock (&xtime_lock);
@@ -451,6 +475,10 @@device_initcall (time_init_device); +static struct timer_hook hook0 = { .hook_fn = timer_mark_offset, .run_always =0 }; +static struct timer_hook hook1 = { .hook_fn = do_timer, .run_always =0 }; +static struct timer_hook hook2 = { .hook_fn = do_process_time, .run_always =0 }; +#ifdef CONFIG_HPET_TIMER extern void (*late_time_init)(void );/* Duplicate of time_init() below, with hpet_enable part added */
@@ -468,7 +496,11 @@ cur_timer =select_timer ();printk (KERN_INFO "Using %s for high-res timesource\n" ,cur_timer->name); +/* register timer hooks in reverse order */ time_init_hook (); +setup_timer_hook (&hook2); +setup_timer_hook (&hook1); + setup_timer_hook(&hook0); }#endif
@@ -492,5 +524,9 @@ cur_timer =select_timer ();printk (KERN_INFO "Using %s for high-res timesource\n" ,cur_timer->name); +/* register timer hooks in reverse order */ time_init_hook (); +setup_timer_hook (&hook2); +setup_timer_hook (&hook1); +setup_timer_hook (&hook0); }
arch/i386/mach-visws/setup.c +11 -0
@@ -118,6 +118,15 @@ .name = "timer", }; +static int co_clear_timerint (struct pt_regs *regs) +{ +/* Clear the interrupt */ +co_cpu_write (CO_CPU_STAT ,co_cpu_read (CO_CPU_STAT ) & ~CO_STAT_TIMEINTR ); +return 0 ; +} + + +static struct timer_hook hook = { .hook_fn = co_clear_timerint, .run_always =1 };void __init time_init_hook (void ) {printk (KERN_INFO "Starting Cobalt Timer system clock\n" );
@@ -131,6 +140,8 @@/* Enable (unmask) the timer interrupt */ co_cpu_write (CO_CPU_CTRL ,co_cpu_read (CO_CPU_CTRL ) & ~CO_CTRL_TIMEMASK ); +setup_timer_hook (&hook); +/* Wire cpu IDT entry to s/w handler (and Cobalt APIC to IDT) */ setup_irq (0 , &irq0); }
arch/i386/mach-voyager/setup.c +3 -1
@@ -8,6 +8,7 @@#include <linux/interrupt.h> #include <asm/acpi.h> #include <asm/arch_hooks.h> +#include <asm/voyager.h> void __init pre_intr_init_hook (void ) {
@@ -41,8 +42,9 @@ }static struct irqaction irq0 = { timer_interrupt,SA_INTERRUPT ,CPU_MASK_NONE ,"timer" ,NULL ,NULL }; - +static struct timer_hook hook = { .hook_fn = voyager_timer_interrupt, .run_always =1 };void __init time_init_hook (void ) { +setup_timer_hook (&hook);setup_irq (0 , &irq0); }
arch/i386/mach-voyager/voyager_basic.c +2 -2
@@ -165,8 +165,7 @@/* voyager specific handling code for timer interrupts. Used to hand * off the timer tick to the SMP code, since the VIC doesn't have an * internal timer (The QIC does, but that's another story). */ -void -voyager_timer_interrupt (struct pt_regs *regs) +int voyager_timer_interrupt (struct pt_regs *regs) {if ((jiffies &0x3ff ) ==0 ) {
@@ -204,6 +203,7 @@#ifdef CONFIG_SMP smp_vic_timer_interrupt (regs);#endif +return 0 ; }void
drivers/oprofile/timer_int.c +6 -2
@@ -12,11 +12,14 @@#include <linux/smp.h> #include <linux/oprofile.h> #include <linux/profile.h> +#include <linux/timer.h> #include <linux/init.h> #include <asm/ptrace.h> #include "oprof.h" +static void *hook_ptr; +static int timer_notify (struct pt_regs *regs) {oprofile_add_sample (regs,0 );
@@ -25,13 +28,14 @@static int timer_start (void ) { -return register_timer_hook (timer_notify); + hook_ptr =register_timer_hook (timer_notify); +return !hook_ptr; }static void timer_stop (void ) { -unregister_timer_hook (timer_notify); +unregister_timer_hook (hook_ptr); }
include/asm-i386/mach-default/do_timer.h +0 -31
@@ -3,37 +3,6 @@#include <asm/apic.h> #include <asm/i8259.h> -/** -* do_timer_interrupt_hook - hook into timer tick -* @regs: standard registers from interrupt -* -* Description: -* This hook is called immediately after the timer interrupt is ack'd. -* It's primary purpose is to allow architectures that don't possess -* individual per CPU clocks (like the CPU APICs supply) to broadcast the -* timer interrupt as a means of triggering reschedules etc. -**/ - -static inline void do_timer_interrupt_hook (struct pt_regs *regs) -{ -do_timer (regs); -#ifndef CONFIG_SMP -update_process_times (user_mode (regs)); -#endif -/* -* In the SMP case we use the local APIC timer interrupt to do the -* profiling, except when we simulate SMP mode on a uniprocessor -* system, in that case we have to call the local interrupt handler. -*/ -#ifndef CONFIG_X86_LOCAL_APIC -profile_tick (CPU_PROFILING , regs); -#else -if (!using_apic_timer) -smp_local_timer_interrupt (regs); -#endif -} - -/* you can safely undefine this if you don't have the Neptune chipset */ #define BUGGY_NEPTUN_TIMER
include/asm-i386/mach-visws/do_timer.h +0 -22
@@ -4,28 +4,6 @@#include <asm/i8259.h> #include "cobalt.h" -static inline void do_timer_interrupt_hook (struct pt_regs *regs) -{ -/* Clear the interrupt */ -co_cpu_write (CO_CPU_STAT ,co_cpu_read (CO_CPU_STAT ) & ~CO_STAT_TIMEINTR ); - -do_timer (regs); -#ifndef CONFIG_SMP -update_process_times (user_mode (regs)); -#endif -/* -* In the SMP case we use the local APIC timer interrupt to do the -* profiling, except when we simulate SMP mode on a uniprocessor -* system, in that case we have to call the local interrupt handler. -*/ -#ifndef CONFIG_X86_LOCAL_APIC -profile_tick (CPU_PROFILING , regs); -#else -if (!using_apic_timer) -smp_local_timer_interrupt (regs); -#endif -} -static inline int do_timer_overflow (int count) {int i;
include/asm-i386/mach-voyager/do_timer.h +0 -13
@@ -1,16 +1,3 @@ -/* defines for inline arch setup functions */ -#include <asm/voyager.h> - -static inline void do_timer_interrupt_hook (struct pt_regs *regs) -{ -do_timer (regs); -#ifndef CONFIG_SMP -update_process_times (user_mode (regs)); -#endif - -voyager_timer_interrupt (regs); -} -static inline int do_timer_overflow (int count) {/* can't read the ISR, just assume 1 tick
include/asm-i386/voyager.h +1 -1
@@ -505,7 +505,7 @@extern void voyager_smp_intr_init (void );extern __u8 voyager_extended_cmos_read (__u16 cmos_address);extern void voyager_smp_dump (void ); -extern void voyager_timer_interrupt (struct pt_regs *regs); +extern int voyager_timer_interrupt (struct pt_regs *regs);extern void smp_local_timer_interrupt (struct pt_regs * regs);extern void voyager_power_off (void );extern void smp_voyager_power_off (void *dummy);
include/linux/profile.h +0 -16
@@ -53,12 +53,6 @@int profile_event_register (enum profile_type ,struct notifier_block * n);int profile_event_unregister (enum profile_type ,struct notifier_block * n); -int register_timer_hook (int (*hook)(struct pt_regs *)); -void unregister_timer_hook (int (*hook)(struct pt_regs *)); - -/* Timer based profiling hook */ -extern int (*timer_hook)(struct pt_regs *); -struct pt_regs ;#else
@@ -87,16 +81,6 @@#define profile_handoff_task (a) (0)#define profile_munmap (a) do { } while (0) -static inline int register_timer_hook (int (*hook)(struct pt_regs *)) -{ -return -ENOSYS ; -} - -static inline void unregister_timer_hook (int (*hook)(struct pt_regs *)) -{ -return ; -} -#endif /* CONFIG_PROFILING */ #endif /* __KERNEL__ */
include/linux/sched.h +1 -1
@@ -1005,7 +1005,7 @@#include <asm/current.h> -extern void do_timer (struct pt_regs *); +extern int do_timer (struct pt_regs *);extern int FASTCALL (wake_up_state (struct task_struct * tsk,unsigned int state));extern int FASTCALL (wake_up_process (struct task_struct * tsk));
include/linux/timer.h +13 -0
@@ -93,4 +93,17 @@extern void run_local_timers (void );extern void it_real_fn (unsigned long ); +struct timer_hook { +int (*hook_fn)(struct pt_regs *regs); +int run_always; +struct list_head list; +}; +extern void do_timer_interrupt_hook (struct pt_regs *regs); +extern void setup_timer_hook (struct timer_hook *hook); +extern void remove_timer_hook (struct timer_hook *hook); +extern void *register_timer_hook (int (*hook)(struct pt_regs *)); +extern void unregister_timer_hook (void *hook_ptr); +extern int grab_timer_hook (void *hook_ptr); +extern void ungrab_timer_hook (void *hook_ptr); +#endif
kernel/profile.c +0 -23
@@ -34,9 +34,6 @@#define NR_PROFILE_HIT (PAGE_SIZE/sizeof(struct profile_hit))#define NR_PROFILE_GRP (NR_PROFILE_HIT/PROFILE_GRPSZ) -/* Oprofile timer tick hook */ -int (*timer_hook)(struct pt_regs *)__read_mostly ; -static atomic_t *prof_buffer;static unsigned long prof_len, prof_shift;static int prof_on__read_mostly ;
@@ -175,24 +172,6 @@return err; } -int register_timer_hook (int (*hook)(struct pt_regs *)) -{ -if (timer_hook) -return -EBUSY ; - timer_hook = hook; -return 0 ; -} - -void unregister_timer_hook (int (*hook)(struct pt_regs *)) -{ -WARN_ON (hook != timer_hook); - timer_hook =NULL ; -/* make sure all CPUs see the NULL hook */ -synchronize_sched ();/* Allow ongoing interrupts to complete. */ -} - -EXPORT_SYMBOL_GPL (register_timer_hook); -EXPORT_SYMBOL_GPL (unregister_timer_hook);EXPORT_SYMBOL_GPL (task_handoff_register);EXPORT_SYMBOL_GPL (task_handoff_unregister);
@@ -385,8 +364,6 @@void profile_tick (int type,struct pt_regs *regs) { -if (type ==CPU_PROFILING && timer_hook) -timer_hook (regs);if (!user_mode (regs) &&cpu_isset (smp_processor_id (), prof_cpu_mask))profile_hit (type, (void *)profile_pc (regs)); }
kernel/timer.c +80 -1
106 lines not on this page · raw message
Quoted 26 lines · Stas Sergeev
Hello.
Right now it seems like the only interface for registering the timer hooks is that one of
kernel/profile.c, and it is very limited. The arch-specific timer hooks are provided in an arch-specific headers as a static functions.
Since my driver needs the timer hook, I thought it might be a good idea to add an API for registering the timer hooks. The attached patch adds such an API and converts all the relevant places to use it. I changed oprofile to use it, and also converted the arch-specific hooks, which looks like a fair cleanup.The API allows to register, unregister and grab the timer hook. The grabbing hook will always be executed first, and can decide to prevent an execution of the rest ones. The hook can have the "
run_always" flag set, in which case it won't be bypassed, regardless of the grabbing hook.
Quoted 5 lines
Does such an API look viable?
As usual, it is needed for the PC-Speaker PCM driver that is currently in an ALSA CVS, awaiting for the proper interface to appear in the kernel.
Interesting. Could you explain why the soft-timer interface doesn't suffice?
thanks
-john
Hello.
Quoted 2 lines · john stultz
Interesting. Could you explain why the soft-timer interface doesn't<> suffice?
I'll try to explain why *I think*
it doesn't suffice, please correct me if my assumptions are wrong.
There are two (bad) things about the
PC-Speaker driver:
1. It needs the higher interrupt frequency. Since there seem to be no API to change
the timer frequency at runtime, the driver
does this itself. Now I have googled out
the thread
[PATCH] i386: Selectable Frequency of the Timer Interrupt but it doesn't look like it ended up
with some patch applied, or where is it?
2. It needs its handler to be executed
first in the chain. Otherwise the quality
is poor because of the latency.
My approach solves both problems by introducing the grabbing ability. This is a rather simple patch, and since it allows to do some cleanup, I though it could be usefull not only for the speaker driver.
But if you can tell me how to achieve at least the point 1 (that is, speed up the timer at run-time quite arbitrary) without the kernel mods, then it would be a real salvation.
Any hints/pointers are appreciated.
Quoted 9 lines · Stas Sergeev
Hello.
Quoted 2 lines · john stultz
Interesting. Could you explain why the soft-timer interface doesn't<> suffice?
I'll try to explain why *I think*
it doesn't suffice, please correct me if my assumptions are wrong.
Quoted 10 lines
There are two (bad) things about the
PC-Speaker driver:
1. It needs the higher interrupt frequency. Since there seem to be no API to change
the timer frequency at runtime, the driver
does this itself. Now I have googled out
the thread
[PATCH] i386: Selectable Frequency of the Timer Interrupt but it doesn't look like it ended up
with some patch applied, or where is it?
This thread resulted in CONFIG_HZ. You get to choose between 100, 250 or 1000. It was not meant to allow runtime HZ modifications.
Quoted 3 lines
2. It needs its handler to be executed first in the chain. Otherwise the quality is poor because of the latency.
Yeah, that's a tougher one :)
Quoted 11 lines
My approach solves both problems by introducing the grabbing ability. This is a rather simple patch, and since it allows to do some cleanup, I though it could be usefull not only for the speaker driver.
But if you can tell me how to achieve at least the point 1 (that is, speed up the timer at run-time quite arbitrary) without the kernel mods, then it would be a real salvation.
Does the dynamic-tick patch help you at all? I'm not sure if it's meant to, admittedly. I'm also not sure if it has any cap on the maximum HZ it attempts to reprogram the hardware to... Mucking with HZ at run-time is going to break lots of stuff, though...well, not necessarily, depends on how you muck with jiffies :)
Thanks,
Nish
Quoted 5 lines · Stas Sergeev
1. It needs the higher interrupt frequency. Since there seem to be no API to change the timer frequency at runtime, the driver does this itself. Now I have googled out the thread
Wow, your driver implements bass and treble controls by varying the frequency of the timer interrupt. That's a neat hack, but I'd expect it to raise a few eyebrows if it's submitted for mainline...
Lee
Hello.
Quoted 5 lines · Nish Aravamudan
Quoted 3 lines
[PATCH] i386: Selectable Frequency of the Timer Interrupt but it doesn't look like it ended up
with some patch applied, or where is it?This thread resulted in
CONFIG_HZ. You get to choose between 100, 250 or 1000. It was not meant to allow runtime HZ modifications.
I see, but the thread was long, at one point a lot have been said about setting HZ at boot - that's something that most likely could be usefull for me.
Quoted 4 lines
Quoted 3 lines
2. It needs its handler to be executed
first in the chain. Otherwise the quality
is poor because of the latency.Yeah, that's a tougher one :)
Yes, but this can wait. Only the ability to set the higher interrupt rates is vital. But in fact, what is the problem to introduce the grabbing timer within the current soft-timer API? Or some priority scheme? I think it is actually much easier to achieve than the variable freq.
Quoted 2 lines
Does the dynamic-tick patch help you at all? I'm not sure if it's meant to, admittedly.
I don't think it helps. It seems to be self- contained. It doesn't add the generic API, all its functions are using the specific global structure, AFAICS.
Quoted 4 lines
I'm also not sure if it has any cap on the
maximum HZ it attempts to reprogram the hardware to... Mucking with HZ at run-time is going to break lots of stuff, though...well, not necessarily, depends on how you muck with jiffies :)
That's why I thought I should avoid this alltogether. My patch introduces the grabbing hook, that can decide to bypass the rest of the chain. The idea is that it is now up to that hook
to make sure the rest of the chain is being executed with the *old* frequency. Surely this is a kind of an ad-hoc, but the change is very small and seemingly innocent, plus the cleanup - it may not be that bad after all, I think:)
Hello.
Quoted 3 lines · Lee Revell
Wow, your driver implements bass and treble controls by varying the frequency of the timer interrupt. That's a neat hack, but I'd expect it to raise a few eyebrows if it's submitted for mainline...
I realized that some time ago, and now, even though the code it still there, the treble/bass controls are no longer exported to the mixer. The driver now works on a fixed frequency. Well, you can still select one of the two base frequencies, but if need be, I can disallow also this. I am willing to reduce the requirements as much as possible, as long as it will help getting the thing in, but perhaps allowing a single higher frequency, or allowing just any frequency, is pretty much the same task, and doesn't look achievable within the currently existing timer API anyway.
Quoted 4 lines · Stas Sergeev
perhaps allowing a single higher frequency, or allowing just any frequency, is pretty much the same task, and doesn't look achievable within the currently existing
timer API anyway
Lots of things aren't doable with the current timer API, hence all the recent work on dynamic tick. This driver would actually be a perfect test for the dynamic tick system as your PC speaker driver basically implements its own dynamic tick mechanism. Just replace all the PIT reprogramming with itimers.
Lee
Hello.
Quoted 2 lines · Lee Revell
Lots of things aren't doable with the current timer API, hence all the recent work on dynamic tick.
I've found only this about the dynamic tick:
http://lwn.net/Articles/138969/
and it seems that it is intended only to slow down the interrupts when there is no work to do, rather than to allow setting an arbitrary frequencies or something like that.
I guess now I realized how you (and Nish) assume I could use it: is it that I should set CONFIG_HZ to the value I need at compile-time, and just remove all the timer reprogramming from the driver in a hope the dynamic-tick patch will slow it down itself when necessary? Or am I misunderstanding the suggestion? That would be really excellent, but
it there a patch around that allows to set an arbitrary CONFIG_HZ values, or should I try to code up one myself? I think
I tried that a few years ago, and the code all around the kernel was resisting to work with HZ>1000, but I guess now it was all changed.
Quoted 7 lines · Stas Sergeev
I guess now I realized how you (and Nish) assume I could use it: is it that I should set
CONFIG_HZto the value I need at compile-time, and just remove all the timer reprogramming from the driver in a hope the dynamic-tick patch will slow it down itself when necessary?
The current implementations don't allow HZ to go higher than CONFIG_HZ but that's the next logical step.
Lee
Hello.
Quoted 7 lines · Lee Revell
Quoted 5 lines
should set
CONFIG_HZto the value I need at compile-time, and just remove all the timer reprogramming from the driver in a hope the dynamic-tick patch will slow it down itself when necessary?The current implementations don't allow HZ to go higher than
CONFIG_HZbut that's the next logical step.
What I was thinking about, is that I can just set CONFIG_HZ to the value I need. It would be a very high value, but with the dynamic-tick patch it shouldn't hurt. I don't see how can I use the dynamic-tick patch otherwise, I actually though this is how you implied I should use it.
The question with that approach is just how to set CONFIG_HZ to an arbitrary values rather than to the 3 pre-defined constants (shouldn't be difficult), and whether or not the dynamic-tick patch will be able to slow the timer down _that_ much:)
That would actually probably be an ideal solution for my problem - suddenly I don't need to change the timer speed at all. The only limitation would be that when the speaker driver is enabled in the config, the ability to manually select the CONFIG_HZ will be lost, but maybe it is not that bad at all...
Quoted 5 lines · Stas Sergeev
The only limitation would be that when the speaker driver is enabled in the config, the ability to manually select the
CONFIG_HZwill be lost, but maybe it is not that bad at all
CONFIG_HZ is just a short term hack to placate people who insist on a tick rate lower than 1000 but can't wait for dynamic tick to be ready. Once dynamic tick is merged then CONFIG_HZ will need to go away. We don't impose arbitrary restrictions on the period of the soundcard interrupt, I don't see why the PIT should be any different.
I would be quite disappointed if dynamic tick does not get merged for 2.6.14.
Lee