Subject: Re: CVS commit: src/sys
To: None <tech-kern@netbsd.org>
From: Mindaugas R. <rmind@NetBSD.org>
List: tech-kern
Date: 07/13/2007 15:53:25
This is a multi-part message in MIME format.

--Multipart=_Fri__13_Jul_2007_15_53_25_+0300_gCJ6V3c8s7=8MWjd
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

Andrew Doran <ad@netbsd.org> wrote:
> Thanks for working on this, however, you didn't note that it was also
> reviewed by me, that I objected to traversing the linked list, and that I
> was unhappy about the code being committed until that was fixed.
> <...>

I am sorry for this (thought <tech-kern> would be OK..). Actually, we all
agreed about using CPU ID as an index here, I just have followed yamt's OK
*for now*. While I am working to improve this, my intention was to commit a
checkpoint.

> Also, I note that you ignored my suggestion that workqueue_enqueue() should
> default to handing work to the local CPU. With your changes it defaults to
> the last kthread created, which seems completely arbitrary.

I have missed it, somehow. Please take a look at the patch (unfortunately, I
cannot compile-test it right now) - would you be OK with it *for now*, or you
do not want to have linked list in the tree?

-- 
Best regards,
Mindaugas
www.NetBSD.org

--Multipart=_Fri__13_Jul_2007_15_53_25_+0300_gCJ6V3c8s7=8MWjd
Content-Type: text/plain;
 name="wq_fix.diff"
Content-Disposition: attachment;
 filename="wq_fix.diff"
Content-Transfer-Encoding: 7bit

Index: subr_workqueue.c
===================================================================
RCS file: /cvsroot/src/sys/kern/subr_workqueue.c,v
retrieving revision 1.15
diff -u -p -r1.15 subr_workqueue.c
--- subr_workqueue.c	13 Jul 2007 07:21:31 -0000	1.15
+++ subr_workqueue.c	13 Jul 2007 12:45:08 -0000
@@ -65,6 +65,10 @@ workqueue_queue_lookup(struct workqueue 
 {
 	struct workqueue_queue *q;
 
+	if (ci == NULL)
+		ci = curcpu();
+
+	/* XXX */
 	SLIST_FOREACH(q, &wq->wq_queue, q_list)
 		if (q->q_ci == ci)
 			return q;
@@ -157,7 +161,7 @@ workqueue_initqueue(struct workqueue *wq
 	cpuid_t cpuid;
 
 #ifdef MULTIPROCESSOR
-	cpuid = ci->ci_cpuid;
+	cpuid = ci ? ci->ci_cpuid : 0;
 #else
 	cpuid = 0;
 #endif
@@ -250,14 +254,14 @@ workqueue_create(struct workqueue **wqp,
 			workqueue_destroy(wq);
 
 	} else {
-		error = workqueue_initqueue(wq, ipl, flags, curcpu());
+		error = workqueue_initqueue(wq, ipl, flags, NULL);
 		if (error) {
 			kmem_free(wq, sizeof(*wq));
 			return error;
 		}
 	}
 #else
-	error = workqueue_initqueue(wq, ipl, flags, curcpu());
+	error = workqueue_initqueue(wq, ipl, flags, NULL);
 	if (error) {
 		kmem_free(wq, sizeof(*wq));
 		return error;

--Multipart=_Fri__13_Jul_2007_15_53_25_+0300_gCJ6V3c8s7=8MWjd--