All of lore.kernel.org
 help / color / mirror / Atom feed
* Race condition in 2.4 tasklet handling
@ 2003-08-23  2:54 TeJun Huh
  2003-08-23  4:09 ` Race condition in 2.4 tasklet handling (cli() broken?) TeJun Huh
  2003-08-23 15:04 ` Race condition in 2.4 tasklet handling Anton Blanchard
  0 siblings, 2 replies; 14+ messages in thread
From: TeJun Huh @ 2003-08-23  2:54 UTC (permalink / raw)
  To: linux-kernel

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

 It's a similar to race condition spotted in i386 interrupt code.  The
race exists between tasklet_[hi_]action() and tasklet_disable().
Again, memory-ordered synchronization is used between
tasklet_struct.count and tasklet_struct.state.

 tasklet_disable() is find because there's an smp_mb() at the end of
tasklet_disable_nosync(); however, in tasklet_action(), there is no
mb() between tasklet_trylock(t) and atomic_read(&t->count).  This
won't cause any trouble on architectures which orders memory accesses
around atomic operations such (including x86), but on architectures
which don't, a tasklet can be executing on another cpu on return from
tasklet_disable().

 Adding smp_mb__after_test_and_set_bit() at the end of
tasklet_trylock() should remedy the situation.  As
smp_mb__{before|after}_test_and_set_bit() don't exist yet, I'm
attaching a patch which adds smp_mb__after_clear_bit().  The patch is
against 2.4.21.

P.S. Please comment on the addition of
smp_mb__{before|after}_test_and_set_bit().

P.P.S. One thing I don't really understand is the uses of smp_mb() at
the end of tasklet_disable() and smp_mb__before_atomic_dec() inside
tasklet_enable().  Can anybody tell me what those are for?

--
tejun

[-- Attachment #2: patch.taskletrace --]
[-- Type: text/plain, Size: 1020 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: linux
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.3     -> 1.4    
#	include/linux/interrupt.h	1.1     -> 1.2    
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/08/23	tj@atj.dyndns.org	1.4
# - tasklet race fix.
# --------------------------------------------
#
diff -Nru a/include/linux/interrupt.h b/include/linux/interrupt.h
--- a/include/linux/interrupt.h	Sat Aug 23 11:52:03 2003
+++ b/include/linux/interrupt.h	Sat Aug 23 11:52:03 2003
@@ -134,7 +134,10 @@
 #ifdef CONFIG_SMP
 static inline int tasklet_trylock(struct tasklet_struct *t)
 {
-	return !test_and_set_bit(TASKLET_STATE_RUN, &(t)->state);
+	int ret;
+	ret = !test_and_set_bit(TASKLET_STATE_RUN, &(t)->state);
+	smp_mb__after_clear_bit();
+	return ret;
 }
 
 static inline void tasklet_unlock(struct tasklet_struct *t)

^ permalink raw reply	[flat|nested] 14+ messages in thread
[parent not found: <nKwX.1yy.17@gated-at.bofh.it>]
end of thread, other threads:[~2003-08-26  0:25 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-08-23  2:54 Race condition in 2.4 tasklet handling TeJun Huh
2003-08-23  4:09 ` Race condition in 2.4 tasklet handling (cli() broken?) TeJun Huh
2003-08-23  5:26   ` TeJun Huh
2003-08-23 10:28     ` Stephan von Krawczynski
2003-08-23 15:13       ` TeJun Huh
2003-08-23 15:37         ` Stephan von Krawczynski
2003-08-25  6:31           ` TeJun Huh
2003-08-25  7:23             ` Stephan von Krawczynski
2003-08-26  0:27               ` TeJun Huh
2003-08-23 15:56         ` Stephan von Krawczynski
2003-08-23 16:36           ` TeJun Huh
2003-08-24  3:27           ` TeJun Huh
2003-08-23 15:04 ` Race condition in 2.4 tasklet handling Anton Blanchard
     [not found] <nKwX.1yy.17@gated-at.bofh.it>
2003-08-25  8:06 ` Race condition in 2.4 tasklet handling (cli() broken?) Peter T. Breuer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.