Messages in this thread Patch in this message | | | Date | Mon, 1 Feb 1999 19:24:01 +0100 (CET) | From | Andrea Arcangeli <> | Subject | Re: [patch] killed tqueue_lock spinlock, Re: [patch] race-fix for bottom-half-functions [Re: Subtle trouble in remove_bh().] |
| |
On Mon, 1 Feb 1999, Patrik Rak wrote:
> 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/
| |