Seems to me that the timer code is attempting to replicate the softirq
characteristics at the tasklet level, which is a little pointless. This
patch converts timers to be a first-class softirq citizen.
Ingo, was there a reason you didn't do it this way to begin with?
diff -u linux-2.5.47-pci/include/linux/interrupt.h linux-2.5.47-pci/include/linux/interrupt.h
--- linux-2.5.47-pci/include/linux/interrupt.h 2002-11-16 22:28:40.000000000 -0500
+++ linux-2.5.47-pci/include/linux/interrupt.h 2002-11-17 11:23:25.000000000 -0500
@@ -45,6 +45,7 @@
enum
{
HI_SOFTIRQ=0,
+ TIMER_SOFTIRQ,
NET_TX_SOFTIRQ,
NET_RX_SOFTIRQ,
SCSI_SOFTIRQ,
--- linux-2.5.47/kernel/timer.c 2002-11-14 10:52:17.000000000 -0500
+++ linux-2.5.47-pci/kernel/timer.c 2002-11-17 11:27:30.000000000 -0500
@@ -66,9 +66,6 @@ typedef struct tvec_t_base_s tvec_base_t
/* Fake initialization */
static DEFINE_PER_CPU(tvec_base_t, tvec_bases) = { SPIN_LOCK_UNLOCKED };
-/* Fake initialization needed to avoid compiler breakage */
-static DEFINE_PER_CPU(struct tasklet_struct, timer_tasklet) = { NULL };
-
static void check_timer_failed(timer_t *timer)
{
static int whine_count;
@@ -766,9 +763,9 @@ rwlock_t xtime_lock __cacheline_aligned_
unsigned long last_time_offset;
/*
- * This function runs timers and the timer-tq in softirq context.
+ * This function runs timers and the timer-tq in bottom half context.
*/
-static void run_timer_tasklet(unsigned long data)
+static void run_timer_softirq(struct softirq_action *h)
{
tvec_base_t *base = &per_cpu(tvec_bases, smp_processor_id());
@@ -781,7 +778,7 @@ static void run_timer_tasklet(unsigned l
*/
void run_local_timers(void)
{
- tasklet_hi_schedule(&per_cpu(timer_tasklet, smp_processor_id()));
+ raise_softirq(TIMER_SOFTIRQ);
}
/*
@@ -1140,7 +1137,6 @@ static void __devinit init_timers_cpu(in
}
for (j = 0; j < TVR_SIZE; j++)
INIT_LIST_HEAD(base->tv1.vec + j);
- tasklet_init(&per_cpu(timer_tasklet, cpu), run_timer_tasklet, 0UL);
}
static int __devinit timer_cpu_notify(struct notifier_block *self,
@@ -1167,4 +1163,5 @@ void __init init_timers(void)
timer_cpu_notify(&timers_nb, (unsigned long)CPU_UP_PREPARE,
(void *)(long)smp_processor_id());
register_cpu_notifier(&timers_nb);
+ open_softirq(TIMER_SOFTIRQ, run_timer_softirq, NULL);
}
--
Revolutions do not require corporate support.
On Sun, 17 Nov 2002, Matthew Wilcox wrote:
> Seems to me that the timer code is attempting to replicate the softirq
> characteristics at the tasklet level, which is a little pointless.
> This patch converts timers to be a first-class softirq citizen.
i agree with your patch.
> Ingo, was there a reason you didn't do it this way to begin with?
because there was an interim state of the timer code in where we still had
a global timer context (ie. a timer tasklet). Only later did it get
converted to completely unsynchronized per-CPU tasklets. Which indeed is
what softirqs are :-)
Ingo
On Mon, 18 Nov 2002, Dipankar Sarma wrote:
> I wrote that part of smptimers to run the per-CPU lists from per-CPU
> tasklets while porting Ingo's code to 2.5 and Ingo just included it. At
> that time, it didn't seem necessary to use up a softirq vector when it
> could be easily done using tasklets.
i think a separate timer softirq is justified, timers are important
enough.
Ingo
On Sun, Nov 17, 2002 at 06:20:11PM +0100, Matthew Wilcox wrote:
>
> Seems to me that the timer code is attempting to replicate the softirq
> characteristics at the tasklet level, which is a little pointless. This
> patch converts timers to be a first-class softirq citizen.
>
> Ingo, was there a reason you didn't do it this way to begin with?
>
I wrote that part of smptimers to run the per-CPU lists from per-CPU
tasklets while porting Ingo's code to 2.5 and Ingo just included it.
At that time, it didn't seem necessary to use up a softirq vector
when it could be easily done using tasklets.
However it should work fine with softirqs too.
Thanks
Dipankar
On Mon, Nov 18, 2002 at 02:29:10PM -0800, george anzinger wrote:
> So then, is there any reason to not put them ahead of
> HI_SOFTIRQ? I.e.:
>
> enum
> {
> TIMER_SOFTIRQ=0,
> HI_SOFTIRQ
> NET_TX_SOFTIRQ,
> NET_RX_SOFTIRQ,
> SCSI_SOFTIRQ,
because then there would be no way to make a tasklet run before the timers?
turn it around. convince us that timers should run first.
--
Revolutions do not require corporate support.
Ingo Molnar wrote:
>
> On Mon, 18 Nov 2002, Dipankar Sarma wrote:
>
> > I wrote that part of smptimers to run the per-CPU lists from per-CPU
> > tasklets while porting Ingo's code to 2.5 and Ingo just included it. At
> > that time, it didn't seem necessary to use up a softirq vector when it
> > could be easily done using tasklets.
>
> i think a separate timer softirq is justified, timers are important
> enough.
>
> Ingo
>
So then, is there any reason to not put them ahead of
HI_SOFTIRQ? I.e.:
enum
{
TIMER_SOFTIRQ=0,
HI_SOFTIRQ
NET_TX_SOFTIRQ,
NET_RX_SOFTIRQ,
SCSI_SOFTIRQ,
--
George Anzinger [email protected]
High-res-timers:
http://sourceforge.net/projects/high-res-timers/
Preemption patch:
http://www.kernel.org/pub/linux/kernel/people/rml