> I did not have much time to think about it, but it could at least use
> some clean up, otherwise it causes unnecessary overhead on uniprocessor
> machines (they don't need the bit1 at all).
Done.
> If it's really an optimization depends IMHO on how much bhs are on the
> queue, as testing bit1 for each of them might actually make things
> worse...
I don't think so. A spin_lock() could lead to many CPU stuck on the same
spinlock even if they was adding task queue to different task queue *list.
Using a bit in the bh->sync will allow SMP to scale very better. And we
avoided a __cli() in every run_task_queue().
Here the latest update:
Index: tqueue.h
===================================================================
RCS file: /var/cvs/linux/include/linux/tqueue.h,v
retrieving revision 1.1.1.1
retrieving revision 1.1.2.6
diff -u -r1.1.1.1 -r1.1.2.6
--- tqueue.h 1999/01/18 01:27:10 1.1.1.1
+++ tqueue.h 1999/02/01 23:04:17 1.1.2.6
@@ -25,17 +25,9 @@
* - Bottom halfs are implemented as a linked list. You can have as many
* of them, as you want.
* - No more scanning of a bit field is required upon call of a bottom half.
- * - Support for chained bottom half lists. The run_task_queue() function can be
- * used as a bottom half handler. This is for example useful for bottom
- * halfs, which want to be delayed until the next clock tick.
- *
- * Problems:
- * - The queue_task_irq() inline function is only atomic with respect to itself.
- * Problems can occur, when queue_task_irq() is called from a normal system
- * call, and an interrupt comes in. No problems occur, when queue_task_irq()
- * is called from an interrupt or bottom half, and interrupted, as run_task_queue()
- * will not be executed/continued before the last interrupt returns. If in
- * doubt, use queue_task(), not queue_task_irq().
+ * - Support for chained bottom half lists. The run_task_queue() function
+ * can be used as a bottom half handler. This is for example useful for
+ * bottom halfs, which want to be delayed until the next clock tick.
* - Bottom halfs are called in the reverse order that they were linked into
* the list.
*/
@@ -75,20 +67,34 @@
* interrupt.
*/
-extern spinlock_t tqueue_lock;
-
/*
- * queue_task
+ * bh_pointer->sync description:
+ * - BIT0 set indicate that a bh function is just in the queue.
+ * - BIT1 set indicate that the bh_pointer->next is not ready to be read.
*/
+
extern __inline__ void queue_task(struct tq_struct *bh_pointer,
task_queue *bh_list)
{
if (!test_and_set_bit(0,&bh_pointer->sync)) {
unsigned long flags;
- spin_lock_irqsave(&tqueue_lock, flags);
- bh_pointer->next = *bh_list;
- *bh_list = bh_pointer;
- spin_unlock_irqrestore(&tqueue_lock, flags);
+
+#ifdef __SMP__
+ bh_pointer->sync |= 2;
+ wmb();
+#endif
+ /*
+ * Disable local interrupt to avoid the CPU to run a nested
+ * run_task_queue() while bit1 is set. -arca
+ */
+ __save_flags(flags);
+ __cli();
+ bh_pointer->next = xchg(bh_list,bh_pointer);
+#ifdef __SMP__
+ clear_bit(1,&bh_pointer->sync); /* see run_task */
+ wmb();
+#endif
+ __restore_flags(flags);
}
}
@@ -98,14 +104,9 @@
extern __inline__ void run_task_queue(task_queue *list)
{
if (*list) {
- unsigned long flags;
struct tq_struct *p;
- spin_lock_irqsave(&tqueue_lock, flags);
- p = *list;
- *list = NULL;
- spin_unlock_irqrestore(&tqueue_lock, flags);
-
+ p = xchg(list,NULL);
while (p) {
void *arg;
void (*f) (void *);
@@ -113,8 +114,19 @@
arg = p -> data;
f = p -> routine;
save_p = p;
+#ifdef __SMP__
+ /*
+ * queue task may have put p on the list
+ * but not yet glued the rest of the list
+ * back onto p
+ */
+ while(test_bit(1,&save_p->sync));
+ rmb();
+#endif
p = p -> next;
- mb();
+#ifdef __SMP__
+ wmb();
+#endif
save_p -> sync = 0;
(*f)(arg);
}
Andrea Arcangeli
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/