[rfc][patch] API for timer hooks

[rfc][patch] API for timer hooks

[Stas Sergeev]

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

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.


[-- Attachment #2: timer-hook-2.6.13-rc5-mm1.diff --]
[-- Type: text/x-patch, Size: 16405 bytes --]

diff -urN linux-2.6.13-rc5-pcsp-kern/arch/i386/kernel/time.c linux-2.6.13-rc5-pcsp/arch/i386/kernel/time.c
--- linux-2.6.13-rc5-pcsp-kern/arch/i386/kernel/time.c	2005-08-11 18:57:45.000000000 +0400
+++ linux-2.6.13-rc5-pcsp/arch/i386/kernel/time.c	2005-08-12 14:44:36.000000000 +0400
@@ -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);
 }
diff -urN linux-2.6.13-rc5-pcsp-kern/arch/i386/mach-visws/setup.c linux-2.6.13-rc5-pcsp/arch/i386/mach-visws/setup.c
--- linux-2.6.13-rc5-pcsp-kern/arch/i386/mach-visws/setup.c	2005-08-11 17:54:49.000000000 +0400
+++ linux-2.6.13-rc5-pcsp/arch/i386/mach-visws/setup.c	2005-08-11 18:31:40.000000000 +0400
@@ -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);
 }
diff -urN linux-2.6.13-rc5-pcsp-kern/arch/i386/mach-voyager/setup.c linux-2.6.13-rc5-pcsp/arch/i386/mach-voyager/setup.c
--- linux-2.6.13-rc5-pcsp-kern/arch/i386/mach-voyager/setup.c	2005-08-11 17:52:50.000000000 +0400
+++ linux-2.6.13-rc5-pcsp/arch/i386/mach-voyager/setup.c	2005-08-11 17:49:11.000000000 +0400
@@ -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);
 }
diff -urN linux-2.6.13-rc5-pcsp-kern/arch/i386/mach-voyager/voyager_basic.c linux-2.6.13-rc5-pcsp/arch/i386/mach-voyager/voyager_basic.c
--- linux-2.6.13-rc5-pcsp-kern/arch/i386/mach-voyager/voyager_basic.c	2005-08-11 17:54:49.000000000 +0400
+++ linux-2.6.13-rc5-pcsp/arch/i386/mach-voyager/voyager_basic.c	2005-08-11 18:31:22.000000000 +0400
@@ -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
diff -urN linux-2.6.13-rc5-pcsp-kern/drivers/oprofile/timer_int.c linux-2.6.13-rc5-pcsp/drivers/oprofile/timer_int.c
--- linux-2.6.13-rc5-pcsp-kern/drivers/oprofile/timer_int.c	2005-03-13 17:56:27.000000000 +0300
+++ linux-2.6.13-rc5-pcsp/drivers/oprofile/timer_int.c	2005-08-11 16:03:03.000000000 +0400
@@ -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);
 }
 
 
diff -urN linux-2.6.13-rc5-pcsp-kern/include/asm-i386/mach-default/do_timer.h linux-2.6.13-rc5-pcsp/include/asm-i386/mach-default/do_timer.h
--- linux-2.6.13-rc5-pcsp-kern/include/asm-i386/mach-default/do_timer.h	2005-08-10 10:12:12.000000000 +0400
+++ linux-2.6.13-rc5-pcsp/include/asm-i386/mach-default/do_timer.h	2005-08-11 11:49:51.000000000 +0400
@@ -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
diff -urN linux-2.6.13-rc5-pcsp-kern/include/asm-i386/mach-visws/do_timer.h linux-2.6.13-rc5-pcsp/include/asm-i386/mach-visws/do_timer.h
--- linux-2.6.13-rc5-pcsp-kern/include/asm-i386/mach-visws/do_timer.h	2005-08-11 17:55:10.000000000 +0400
+++ linux-2.6.13-rc5-pcsp/include/asm-i386/mach-visws/do_timer.h	2005-08-10 16:11:13.000000000 +0400
@@ -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;
diff -urN linux-2.6.13-rc5-pcsp-kern/include/asm-i386/mach-voyager/do_timer.h linux-2.6.13-rc5-pcsp/include/asm-i386/mach-voyager/do_timer.h
--- linux-2.6.13-rc5-pcsp-kern/include/asm-i386/mach-voyager/do_timer.h	2004-10-30 18:30:51.000000000 +0400
+++ linux-2.6.13-rc5-pcsp/include/asm-i386/mach-voyager/do_timer.h	2005-08-11 11:48:56.000000000 +0400
@@ -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
diff -urN linux-2.6.13-rc5-pcsp-kern/include/asm-i386/voyager.h linux-2.6.13-rc5-pcsp/include/asm-i386/voyager.h
--- linux-2.6.13-rc5-pcsp-kern/include/asm-i386/voyager.h	2004-01-09 10:00:02.000000000 +0300
+++ linux-2.6.13-rc5-pcsp/include/asm-i386/voyager.h	2005-08-11 16:11:01.000000000 +0400
@@ -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);
diff -urN linux-2.6.13-rc5-pcsp-kern/include/linux/profile.h linux-2.6.13-rc5-pcsp/include/linux/profile.h
--- linux-2.6.13-rc5-pcsp-kern/include/linux/profile.h	2004-12-26 00:37:13.000000000 +0300
+++ linux-2.6.13-rc5-pcsp/include/linux/profile.h	2005-08-11 11:19:45.000000000 +0400
@@ -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__ */
diff -urN linux-2.6.13-rc5-pcsp-kern/include/linux/sched.h linux-2.6.13-rc5-pcsp/include/linux/sched.h
--- linux-2.6.13-rc5-pcsp-kern/include/linux/sched.h	2005-08-11 17:55:13.000000000 +0400
+++ linux-2.6.13-rc5-pcsp/include/linux/sched.h	2005-08-11 16:09:00.000000000 +0400
@@ -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));
diff -urN linux-2.6.13-rc5-pcsp-kern/include/linux/timer.h linux-2.6.13-rc5-pcsp/include/linux/timer.h
--- linux-2.6.13-rc5-pcsp-kern/include/linux/timer.h	2005-08-11 17:55:14.000000000 +0400
+++ linux-2.6.13-rc5-pcsp/include/linux/timer.h	2005-08-11 16:14:58.000000000 +0400
@@ -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
diff -urN linux-2.6.13-rc5-pcsp-kern/kernel/profile.c linux-2.6.13-rc5-pcsp/kernel/profile.c
--- linux-2.6.13-rc5-pcsp-kern/kernel/profile.c	2005-08-10 10:12:17.000000000 +0400
+++ linux-2.6.13-rc5-pcsp/kernel/profile.c	2005-08-11 11:23:23.000000000 +0400
@@ -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));
 }
diff -urN linux-2.6.13-rc5-pcsp-kern/kernel/timer.c linux-2.6.13-rc5-pcsp/kernel/timer.c
--- linux-2.6.13-rc5-pcsp-kern/kernel/timer.c	2005-08-11 17:55:14.000000000 +0400
+++ linux-2.6.13-rc5-pcsp/kernel/timer.c	2005-08-11 16:50:50.000000000 +0400
@@ -93,6 +93,13 @@
 #endif
 }
 
+struct timer_hook_list {
+	struct list_head head;
+	struct timer_hook *grab;
+	spinlock_t lock;
+};
+static struct timer_hook_list timer_hook_list;
+
 static void check_timer_failed(struct timer_list *timer)
 {
 	static int whine_count;
@@ -961,11 +968,12 @@
  * jiffies is defined in the linker script...
  */
 
-void do_timer(struct pt_regs *regs)
+int do_timer(struct pt_regs *regs)
 {
 	jiffies_64++;
 	update_times();
 	softlockup_tick(regs);
+	return 0;
 }
 
 #ifdef __ARCH_WANT_SYS_ALARM
@@ -1426,6 +1434,10 @@
 
 void __init init_timers(void)
 {
+	INIT_LIST_HEAD(&timer_hook_list.head);
+	spin_lock_init(&timer_hook_list.lock);
+	timer_hook_list.grab = NULL;
+
 	timer_cpu_notify(&timers_nb, (unsigned long)CPU_UP_PREPARE,
 				(void *)(long)smp_processor_id());
 	register_cpu_notifier(&timers_nb);
@@ -1651,3 +1663,70 @@
 }
 
 EXPORT_SYMBOL(msleep_interruptible);
+
+
+
+void do_timer_interrupt_hook(struct pt_regs *regs)
+{
+	struct timer_hook *ptr;
+	int done = 0;
+	if (unlikely(timer_hook_list.grab))
+		done = timer_hook_list.grab->hook_fn(regs);
+	/* called within IRQ context, rcu_read_lock not needed? */
+	list_for_each_entry_rcu(ptr, &timer_hook_list.head, list) {
+		if (!done || ptr->run_always)
+			ptr->hook_fn(regs);
+	}
+}
+
+void setup_timer_hook(struct timer_hook *hook)
+{
+	spin_lock(&timer_hook_list.lock);
+	list_add_rcu(&hook->list, &timer_hook_list.head);
+	spin_unlock(&timer_hook_list.lock);
+}
+
+void remove_timer_hook(struct timer_hook *hook)
+{
+	spin_lock(&timer_hook_list.lock);
+	list_del_rcu(&hook->list);
+	spin_unlock(&timer_hook_list.lock);
+}
+
+void *register_timer_hook(int (*func)(struct pt_regs *))
+{
+	struct timer_hook *ptr;
+	ptr = kmalloc(sizeof(struct timer_hook), GFP_ATOMIC);
+	ptr->hook_fn = func;
+	ptr->run_always = 0;
+	setup_timer_hook(ptr);
+	return ptr;
+}
+
+void unregister_timer_hook(void *hook_ptr)
+{
+	struct timer_hook *ptr = (struct timer_hook *)hook_ptr;
+	remove_timer_hook(ptr);
+	kfree(ptr);
+}
+
+int grab_timer_hook(void *hook_ptr)
+{
+	if (timer_hook_list.grab)
+		return -EBUSY;
+	timer_hook_list.grab = (struct timer_hook *)hook_ptr;
+	return 0;
+}
+
+void ungrab_timer_hook(void *hook_ptr)
+{
+	WARN_ON(timer_hook_list.grab != (struct timer_hook *)hook_ptr);
+	timer_hook_list.grab = NULL;
+}
+
+EXPORT_SYMBOL_GPL(setup_timer_hook);
+EXPORT_SYMBOL_GPL(remove_timer_hook);
+EXPORT_SYMBOL_GPL(register_timer_hook);
+EXPORT_SYMBOL_GPL(unregister_timer_hook);
+EXPORT_SYMBOL_GPL(grab_timer_hook);
+EXPORT_SYMBOL_GPL(ungrab_timer_hook);

Re: [rfc][patch] API for timer hooks

[john stultz]

On Sat, 2005-08-13 at 17:36 +0400, Stas Sergeev wrote:
> 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.


Interesting. Could you explain why the soft-timer interface doesn't
suffice?

thanks
-john

Re: [rfc][patch] API for timer hooks

[Stas Sergeev]

Hello.

john stultz wrote:
> 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.

Re: [rfc][patch] API for timer hooks

[Nish Aravamudan]

On 8/16/05, Stas Sergeev <stsp@aknet.ru> wrote:
> Hello.
> 
> john stultz wrote:
> > 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?

This thread resulted in CONFIG_HZ. You get to choose between 100, 250
or 1000. It was not meant to allow runtime HZ modifications.

> 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 :)

> 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

Re: [rfc][patch] API for timer hooks

[Lee Revell]

On Wed, 2005-08-17 at 00:21 +0400, Stas Sergeev wrote:
> 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

Re: [rfc][patch] API for timer hooks

[Stas Sergeev]

Hello.

Nish Aravamudan wrote:
>> [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.

>> 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.

> 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.

> 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:)

Re: [rfc][patch] API for timer hooks

[Stas Sergeev]

Hello.

Lee Revell wrote:
> 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.

Re: [rfc][patch] API for timer hooks

[Lee Revell]

On Wed, 2005-08-17 at 20:21 +0400, Stas Sergeev wrote:
> 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 
 

Re: [rfc][patch] API for timer hooks

[Stas Sergeev]

Hello.

Lee Revell wrote:
> 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.

Re: [rfc][patch] API for timer hooks

[Lee Revell]

On Wed, 2005-08-17 at 21:41 +0400, Stas Sergeev wrote:
> 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? 

The current implementations don't allow HZ to go higher than CONFIG_HZ
but that's the next logical step.

Lee

Re: [rfc][patch] API for timer hooks

[Stas Sergeev]

Hello.

Lee Revell wrote:
>> 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?
> The current implementations don't allow HZ to go higher than CONFIG_HZ
> but 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...

Re: [rfc][patch] API for timer hooks

[Lee Revell]

On Thu, 2005-08-18 at 20:59 +0400, Stas Sergeev wrote:
> 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 

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