Subject: Re: CVS commit: src/sys
To: Jason Thorpe <thorpej@wasabisystems.com>
From: Matthias Drochner <M.Drochner@fz-juelich.de>
List: tech-kern
Date: 06/30/2003 20:18:42
This is a multipart MIME message.

--==_Exmh_30377191993200
Content-Type: text/plain; charset=us-ascii


thorpej@wasabisystems.com said:
> One such example I've encountered is selrecord().  selrecord() used
> the  proc * argument only to record that "a process is selecting on
> this".   It does not care which lwp is selecting, just that "a
> process" is.   

While a number of the proc->lwp in the I/O area might be unnecessary,
selrecord() is probably a bad example. As implemented currently, it
doesn't record which LWP of a process called the select()/poll(), and
selwakeup() thus wakes up all LWPs of a process currently select()ing,
no matther whether this was for different file desrciptors.
I can't tell yet whether what effect this makes in real-world
applications -- if multiple LWPs are waiting for the same ressource
it might be even a gain, but otherwise it looks like a waste.
Needs some profiling to find out.

I'm running the appended patch for some weeks (or months) now, and
what I can tell so far is that through all the problems with SA
applications, it at least didn't make things worse.
(The patch is obviously a proof-of concept rather than ready for the
public - main reason is the missing lwp pointer...)

best regards
Matthias



--==_Exmh_30377191993200
Content-Type: text/plain ; name="select.txt"; charset=us-ascii
Content-Description: select.txt
Content-Disposition: attachment; filename="select.txt"

Index: sys_generic.c
===================================================================
RCS file: /cvsroot/src/sys/kern/sys_generic.c,v
retrieving revision 1.76
diff -u -r1.76 sys_generic.c
--- sys_generic.c	2003/06/29 22:31:25	1.76
+++ sys_generic.c	2003/06/30 18:16:39
@@ -975,12 +975,22 @@
 	struct lwp	*l;
 	struct proc	*p;
 	pid_t		mypid;
+	lwpid_t		mylid;
 
 	mypid = selector->p_pid;
-	if (sip->sel_pid == mypid)
-		return;
+	mylid = curlwp->l_lid; /* XXX */
+	if (sip->sel_pid == mypid) {
+		if (sip->sel_lid == mylid)
+			return;
+#if 0
+		printf("selrecord: pid %d collision: %d/%d\n",
+		       mypid, mylid, sip->sel_lid);
+#endif
+	}
 	if (sip->sel_pid && (p = pfind(sip->sel_pid))) {
 		LIST_FOREACH(l, &p->p_lwps, l_sibling) {
+			if (l->l_lid != sip->sel_lid)
+				continue;
 			if (l->l_wchan == (caddr_t)&selwait) {
 				sip->sel_collision = 1;
 				return;
@@ -989,6 +999,7 @@
 	}
 
 	sip->sel_pid = mypid;
+	sip->sel_lid = mylid;
 }
 
 /*
@@ -1015,6 +1026,8 @@
 	sip->sel_pid = 0;
 	if (p != NULL) {
 		LIST_FOREACH(l, &p->p_lwps, l_sibling) {
+			if (l->l_lid != sip->sel_lid)
+				continue;
 			SCHED_LOCK(s);
 			if (l->l_wchan == (caddr_t)&selwait) {
 				if (l->l_stat == LSSLEEP)

--==_Exmh_30377191993200--