Skip to contentSkip to conversation

getting rid of tqueue_lock

PATCH In review Manfred 2 participants 2 messages Jan 29, 2001 Char and misc drivers

lore
Share & tools
lore

Manfred

author
#0 lore raw

PATCH 1/1 getting rid of tqueue_lock

I noticed that the tasks queues still rely on the global tqueue_lock spinlock, instead of a per-taskqueue lock.

The patch is virtually transparent for task queue users: all users except ieee1394 use DECLARE_TASK_QUEUE.

I admit that the tqueue_lock isn't that often used (numbers from sgi's lockstat)

* 10000 users/min during 'find / -xdev -uid 4711' (1.2% of all spinlock calls)
* 60000 users/min during 'dd if=/dev/hda of=/dev/null' (~1.1% of all spinlock calls).

--
Manfred

// $Header$
// Kernel Version:

//  VERSION = 2
//  PATCHLEVEL = 4
//  SUBLEVEL = 1
//  EXTRAVERSION =-pre11
drivers/char/n_r3964.c +4 -2
@@ -1185,13 +1185,15 @@
     /*
      * Make sure that our task queue isn't activated.  If it
      * is, take it out of the linked list.
+     * FIXME: buggy, another cpu might be in the middle of
+     *		__run_task_queue().
      */
-    spin_lock_irqsave(&tqueue_lock, flags);
+    spin_lock_irqsave(&tq_timer.lock, flags);
     if (pInfo->bh_1.sync)
     	list_del(&pInfo->bh_1.list);
     if (pInfo->bh_2.sync)
     	list_del(&pInfo->bh_2.list);
-    spin_unlock_irqrestore(&tqueue_lock, flags);
+    spin_unlock_irqrestore(&tq_timer.lock, flags);
 
    /* Remove client-structs and message queues: */
     pClient=pInfo->firstClient;
include/linux/tqueue.h +21 -12
@@ -42,10 +42,21 @@
 	void *data;			/* argument to function */
 };
 
-typedef struct list_head task_queue;
+typedef struct {
+	struct list_head list;
+	spinlock_t lock;
+} task_queue;
 
-#define DECLARE_TASK_QUEUE(q)	LIST_HEAD(q)
-#define TQ_ACTIVE(q)		(!list_empty(&q))
+#define TASK_QUEUE_INIT(name) { LIST_HEAD_INIT(name.list), SPIN_LOCK_UNLOCKED }
+
+#define INIT_TASK_QUEUE(ptr) do { \
+	INIT_LIST_HEAD(&(ptr)->list); \
+	spin_lock_init(&(ptr)->lock); \
+} while (0)
+
+
+#define DECLARE_TASK_QUEUE(q)	task_queue q = TASK_QUEUE_INIT(q)
+#define TQ_ACTIVE(q)		(!list_empty(&(q).list))
 
 extern task_queue tq_timer, tq_immediate, tq_disk;
 
@@ -72,20 +83,18 @@
  * interrupt.
  */
 
-extern spinlock_t tqueue_lock;
-
 /*
  * Queue a task on a tq.  Return non-zero if it was successfully
  * added.
  */
-static inline int queue_task(struct tq_struct *bh_pointer, task_queue *bh_list)
+static inline int queue_task(struct tq_struct *bh_pointer, task_queue *tq)
 {
 	int ret = 0;
 	if (!test_and_set_bit(0,&bh_pointer->sync)) {
 		unsigned long flags;
-		spin_lock_irqsave(&tqueue_lock, flags);
-		list_add_tail(&bh_pointer->list, bh_list);
-		spin_unlock_irqrestore(&tqueue_lock, flags);
+		spin_lock_irqsave(&tq->lock, flags);
+		list_add_tail(&bh_pointer->list, &tq->list);
+		spin_unlock_irqrestore(&tq->lock, flags);
 		ret = 1;
 	}
 	return ret;
@@ -97,10 +106,10 @@
 
 extern void __run_task_queue(task_queue *list);
 
-static inline void run_task_queue(task_queue *list)
+static inline void run_task_queue(task_queue *tq)
 {
-	if (TQ_ACTIVE(*list))
-		__run_task_queue(list);
+	if (TQ_ACTIVE(*tq))
+		__run_task_queue(tq);
 }
 
 #endif /* _LINUX_TQUEUE_H */
kernel/timer.c +0 -2
@@ -323,8 +323,6 @@
 	spin_unlock_irq(&timerlist_lock);
 }
 
-spinlock_t tqueue_lock = SPIN_LOCK_UNLOCKED;
-
 void tqueue_bh(void)
 {
 	run_task_queue(&tq_timer);
kernel/ksyms.c +0 -1
@@ -372,7 +372,6 @@
 
 #ifdef CONFIG_SMP
 /* Various random spinlocks we want to export */
-EXPORT_SYMBOL(tqueue_lock);
 
 /* Big-Reader lock implementation */
 EXPORT_SYMBOL(__brlock_array);
kernel/softirq.c +5 -5
@@ -289,15 +289,15 @@
 	open_softirq(HI_SOFTIRQ, tasklet_hi_action, NULL);
 }
 
-void __run_task_queue(task_queue *list)
+void __run_task_queue(task_queue *tq)
 {
 	struct list_head head, *next;
 	unsigned long flags;
 
-	spin_lock_irqsave(&tqueue_lock, flags);
-	list_add(&head, list);
-	list_del_init(list);
-	spin_unlock_irqrestore(&tqueue_lock, flags);
+	spin_lock_irqsave(&tq->lock, flags);
+	list_add(&head, &tq->list);
+	list_del_init(&tq->list);
+	spin_unlock_irqrestore(&tq->lock, flags);
 
 	next = head.next;
 	while (next != &head) {
drivers/ieee1394/ieee1394_core.c +1 -0
@@ -100,6 +100,7 @@
 
         INIT_LIST_HEAD(&packet->list);
         sema_init(&packet->state_change, 0);
+        INIT_TASK_QUEUE(&packet->complete_tq);
         packet->state = unused;
         packet->generation = get_hpsb_generation();
         packet->data_be = 1;
#1 lore raw
Quoted 6 lines · Manfred

I noticed that the tasks queues still rely on the global tqueue_lock spinlock, instead of a per-taskqueue lock.

The patch is virtually transparent for task queue users: all users except ieee1394 use DECLARE_TASK_QUEUE.

Quoted 7 lines

I admit that the tqueue_lock isn't that often used (numbers from sgi's lockstat)

* 10000 users/min during 'find / -xdev -uid 4711' (1.2% of all spinlock calls)
* 60000 users/min during 'dd if=/dev/hda of=/dev/null' (~1.1% of all spinlock calls).

If there's no contention and nothing else changes, what is this task supposed to fix?

Rik
--
Virtual memory is like a game you can't win; However, without VM there's truly nothing to lose...

		http://www.surriel.com/

http://www.conectiva.com/ http://distro.conectiva.com.br/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/