[PATCH] fix loop with disabled tasklets

From: Mathijs Mohlmann (mathijs@knoware.nl)
Date: Sat Nov 10 2001 - 07:21:56 EST


        I have a questions about the current softirq implementation.

Looking at kernel/softirq.c:418 and futher. When the tasklet is disabled it
is rescheduled. This is fine, but __cpu_raise_softirq is also called. This
means that ksoftirqd will loop until the tasklet is enabled.
Normaly this is no biggy, because user processes still come through.
But during boot -when the tasklet_enable is yet to be called by "the idle
thread"- the system will lockup. This happens during the boot of my sun4m
with the keyboard tasklet.

To demonstrate, i wrote a module. Inserting this will result in 0% idle time.
Inserting this with the below patch, the system simply goes on.

If this really is an issue, this patch fixes it. It does change the current
behavior a bit though. Currently when a tasklet is run while it is already
running on a different cpu, the task is rescheduled. This results in the
tasklet being executed two times. With this patch applied, it will not. (this
is still good according to kernel/include/interrups.h:83 and futher).

        i'm not sure about the enable_tasklet bit. I think it will prevent
people from calling tasklet_enable from within an interrupt handler. But then
again, why do you want to do that? Thanx, velco and
        
        Any comments?

        me

#define MODULE
#include <linux/module.h>
#include <linux/sched.h>
#include <linux/interrupt.h>

MODULE_LICENSE("GPL");

void
dummy_lockit(unsigned long nill)
{
        printk("doing the lockit\n");
        return;
}

DECLARE_TASKLET_DISABLED(lockit, dummy_lockit, 0);
int
init_module(void)
{
        printk("going to lock\n");
        tasklet_schedule(&lockit);
        return(0);
}

void
cleanup_module(void)
{
        tasklet_enable(&lockit);
        set_current_state(TASK_INTERRUPTIBLE);
        schedule_timeout(HZ); /* some time to do dummy_lockit */

        printk("bye\n");
}

diff -ruN linux-2.4.14/include/linux/interrupt.h
linux/include/linux/interrupt.h
--- linux-2.4.14/include/linux/interrupt.h Sat Nov 3 11:20:41 2001
+++ linux/include/linux/interrupt.h Sat Nov 10 12:28:14 2001
@@ -185,13 +185,15 @@
 static inline void tasklet_enable(struct tasklet_struct *t)
 {
         smp_mb__before_atomic_dec();
- atomic_dec(&t->count);
+ if (atomic_dec_and_test(&t->count))
+ __cpu_raise_softirq(smp_processor_id(), TASKLET_SOFTIRQ);
 }
 
 static inline void tasklet_hi_enable(struct tasklet_struct *t)
 {
         smp_mb__before_atomic_dec();
- atomic_dec(&t->count);
+ if (atomic_dec_and_test(&t->count))
+ __cpu_raise_softirq(smp_processor_id(), HI_SOFTIRQ);
 }
 
 extern void tasklet_kill(struct tasklet_struct *t);
diff -ruN linux-2.4.14/kernel/softirq.c linux/kernel/softirq.c
--- linux-2.4.14/kernel/softirq.c Thu Nov 8 15:58:24 2001
+++ linux/kernel/softirq.c Sat Nov 10 12:27:24 2001
@@ -188,21 +188,19 @@
 
                 list = list->next;
 
- if (tasklet_trylock(t)) {
- if (!atomic_read(&t->count)) {
+ if (!atomic_read(&t->count)) {
+ if (tasklet_trylock(t)) {
                                 if (!test_and_clear_bit(TASKLET_STATE_SCHED, &t->state))
                                         BUG();
                                 t->func(t->data);
                                 tasklet_unlock(t);
- continue;
                         }
- tasklet_unlock(t);
+ continue;
                 }
 
                 local_irq_disable();
                 t->next = tasklet_vec[cpu].list;
                 tasklet_vec[cpu].list = t;
- __cpu_raise_softirq(cpu, TASKLET_SOFTIRQ);
                 local_irq_enable();
         }
 }
@@ -222,21 +220,19 @@
 
                 list = list->next;
 
- if (tasklet_trylock(t)) {
- if (!atomic_read(&t->count)) {
+ if (!atomic_read(&t->count)) {
+ if (tasklet_trylock(t)) {
                                 if (!test_and_clear_bit(TASKLET_STATE_SCHED, &t->state))
                                         BUG();
                                 t->func(t->data);
                                 tasklet_unlock(t);
- continue;
                         }
- tasklet_unlock(t);
+ continue;
                 }
 
                 local_irq_disable();
                 t->next = tasklet_hi_vec[cpu].list;
                 tasklet_hi_vec[cpu].list = t;
- __cpu_raise_softirq(cpu, HI_SOFTIRQ);
                 local_irq_enable();
         }
 }
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Nov 15 2001 - 21:00:24 EST