tasklet: ignore disabled tasklet in tasklet_action()

We met a ksoftirqd 100% issue, the perf top shows kernel is busy with
tasklet_action(), but no actual action is shown.  From dumped kernel,
there's only one disabled tasklet on the tasklet_vec.

tasklet_action might be handled after tasklet is disabled, this will make
disabled tasklet stayed on tasklet_vec.  tasklet_action will not handle
disabled tasklet, but place it on the tail of tasklet_vec, still raise
softirq for this tasklet.  Things will become worse if device driver uses
tasklet_disable on its device remove/close code.  The disabled tasklet
will stay on the vec, frequently __raise_softirq_off() and make ksoftirqd
wakeup even if no tasklets need to be handled.

This patch introduced a new TASKLET_STATE_HI bit to indicate HI_SOFTIRQ,
in tasklet_action(), simply ignore the disabled tasklet and don't raise
the softirq nr.  In my previous patch, I remove tasklet_hi_enable() since
it is the same as tasklet_enable().  So only tasklet_enable() needs to be
modified, if tasklet state is changed from disable to enable, use
__tasklet_schedule() to put it on the right vec.

Signed-off-by: Xiaotian Feng <dannyfeng@tencent.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/drivers/net/ethernet/jme.c b/drivers/net/ethernet/jme.c
index 92317e9c0f736821f605a4e3008b089b1ba9cb63..76a91f6c40cfda698ae8d17f8bd14fc7bb5e0e14 100644 (file)
--- a/drivers/net/ethernet/jme.c
+++ b/drivers/net/ethernet/jme.c
@@ -1364,8 +1364,8 @@ err_out_free_rx_resources:
        jme_free_rx_resources(jme);
 out_enable_tasklet:
        tasklet_enable(&jme->txclean_task);
-       tasklet_hi_enable(&jme->rxclean_task);
-       tasklet_hi_enable(&jme->rxempty_task);
+       tasklet_enable(&jme->rxclean_task);
+       tasklet_enable(&jme->rxempty_task);
 out:
        atomic_inc(&jme->link_changing);
 }
@@ -1862,8 +1862,8 @@ jme_open(struct net_device *netdev)
 
        tasklet_enable(&jme->linkch_task);
        tasklet_enable(&jme->txclean_task);
-       tasklet_hi_enable(&jme->rxclean_task);
-       tasklet_hi_enable(&jme->rxempty_task);
+       tasklet_enable(&jme->rxclean_task);
+       tasklet_enable(&jme->rxempty_task);
 
        rc = jme_request_irq(jme);
        if (rc)
@@ -2375,8 +2375,8 @@ static inline void jme_resume_rx(struct jme_adapter *jme)
        if (test_bit(JME_FLAG_POLL, &jme->flags)) {
                JME_NAPI_ENABLE(jme);
        } else {
-               tasklet_hi_enable(&jme->rxclean_task);
-               tasklet_hi_enable(&jme->rxempty_task);
+               tasklet_enable(&jme->rxclean_task);
+               tasklet_enable(&jme->rxempty_task);
        }
        dpi->cur                = PCC_P1;
        dpi->attempt            = PCC_P1;
@@ -3276,8 +3276,8 @@ jme_suspend(struct device *dev)
        }
 
        tasklet_enable(&jme->txclean_task);
-       tasklet_hi_enable(&jme->rxclean_task);
-       tasklet_hi_enable(&jme->rxempty_task);
+       tasklet_enable(&jme->rxclean_task);
+       tasklet_enable(&jme->rxempty_task);
 
        jme_powersave_phy(jme);
 

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 5e4e6170f43a5ef672ddd0d28694849669e310cb..f6178ee2983a122ebc1cfe568e5a42452a37103d 100644 (file)
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -521,7 +521,8 @@ struct tasklet_struct name = { NULL, 0, ATOMIC_INIT(1), func, data }
 enum
 {
        TASKLET_STATE_SCHED,    /* Tasklet is scheduled for execution */
-       TASKLET_STATE_RUN       /* Tasklet is running (SMP only) */
+       TASKLET_STATE_RUN,      /* Tasklet is running (SMP only) */
+       TASKLET_STATE_HI        /* Tasklet is HI_SOFTIRQ */
 };
 
 #ifdef CONFIG_SMP
@@ -593,13 +594,14 @@ static inline void tasklet_disable(struct tasklet_struct *t)
 static inline void tasklet_enable(struct tasklet_struct *t)
 {
        smp_mb__before_atomic_dec();
-       atomic_dec(&t->count);
-}
-
-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)) {
+               if (!test_bit(TASKLET_STATE_SCHED, &t->state))
+                       return;
+               if (test_bit(TASKLET_STATE_HI, &t->state))
+                       __tasklet_hi_schedule(t);
+               else
+                       __tasklet_schedule(t);
+       }
 }
 
 extern void tasklet_kill(struct tasklet_struct *t);

diff --git a/kernel/softirq.c b/kernel/softirq.c
index ed567babe789c10ef48a2db7b63e17e0a2192d65..b95202b51e0767b7a154c27a65578931146c9e22 100644 (file)
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -417,6 +417,7 @@ void __tasklet_schedule(struct tasklet_struct *t)
        *__this_cpu_read(tasklet_vec.tail) = t;
        __this_cpu_write(tasklet_vec.tail, &(t->next));
        raise_softirq_irqoff(TASKLET_SOFTIRQ);
+       clear_bit(TASKLET_STATE_HI, &t->state);
        local_irq_restore(flags);
 }
 
@@ -431,6 +432,7 @@ void __tasklet_hi_schedule(struct tasklet_struct *t)
        *__this_cpu_read(tasklet_hi_vec.tail) = t;
        __this_cpu_write(tasklet_hi_vec.tail,  &(t->next));
        raise_softirq_irqoff(HI_SOFTIRQ);
+       set_bit(TASKLET_STATE_HI, &t->state);
        local_irq_restore(flags);
 }
 
@@ -442,6 +444,7 @@ void __tasklet_hi_schedule_first(struct tasklet_struct *t)
 
        t->next = __this_cpu_read(tasklet_hi_vec.head);
        __this_cpu_write(tasklet_hi_vec.head, t);
+       set_bit(TASKLET_STATE_HI, &t->state);
        __raise_softirq_irqoff(HI_SOFTIRQ);
 }
 
@@ -467,10 +470,9 @@ static void tasklet_action(struct softirq_action *a)
                                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();
@@ -502,10 +504,9 @@ static void tasklet_hi_action(struct softirq_action *a)
                                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();