mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mathijs Mohlmann <mathijs@knoware.nl>
To: andrea@suse.de, jgarzik@mandrakesoft.com
Cc: linux-kernel@vger.kernel.org, torvalds@transmeta.com
Subject: [PATCH] fix loop with disabled tasklets
Date: Sat, 10 Nov 2001 13:21:56 +0100	[thread overview]
Message-ID: <20011110122141.B2C68231A4@brand.mmohlmann.demon.nl> (raw)


	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();
 	}
 }

             reply	other threads:[~2001-11-10 12:21 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-11-10 12:21 Mathijs Mohlmann [this message]
2001-11-10 13:37 ` David S. Miller
2001-11-10 15:03   ` Andrea Arcangeli
2001-11-10 15:29     ` Mathijs Mohlmann
2001-11-10 16:02       ` Alan Cox
2001-11-10 16:37       ` Andrea Arcangeli
2001-11-12  1:11         ` Andrea Arcangeli
2001-11-12  7:42           ` Mathijs Mohlmann
2001-11-12 13:57             ` Andrea Arcangeli
2001-11-12  7:59           ` David S. Miller
2001-11-12 14:03             ` Andrea Arcangeli
2001-11-12  8:03         ` David S. Miller
2001-11-12 14:04           ` Andrea Arcangeli
2001-11-12 14:20             ` Andrea Arcangeli
2001-11-12 17:10               ` Thorsten Kukuk
2001-11-12 19:03               ` Mathijs Mohlmann
2001-11-11  2:32     ` Mathijs Mohlmann
2001-11-11 15:56 Momchil Velikov
2001-11-12  7:46 ` Mathijs Mohlmann
2001-11-12  8:07   ` Momchil Velikov
2001-11-12  9:11     ` Mathijs Mohlmann
2001-11-12  9:41       ` Momchil Velikov
2001-11-12  9:54         ` Mathijs Mohlmann
2001-11-12  8:00 ` David S. Miller
2001-11-12 15:33 Petr Vandrovec
2001-11-12 14:48 ` Andrea Arcangeli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20011110122141.B2C68231A4@brand.mmohlmann.demon.nl \
    --to=mathijs@knoware.nl \
    --cc=andrea@suse.de \
    --cc=jgarzik@mandrakesoft.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@transmeta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®