Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 2 Oct 2002 23:40:28 +0200
From:      Stefan Farfeleder <e0026813@stud3.tuwien.ac.at>
To:        Poul-Henning Kamp <phk@FreeBSD.ORG>
Cc:        current@FreeBSD.ORG
Subject:   Re: Junior Kernel Hacker page updated...
Message-ID:  <20021002214028.GA94673@frog.fafoe>
In-Reply-To: <16372.1031998673@critter.freebsd.dk>
References:  <16372.1031998673@critter.freebsd.dk>

next in thread | previous in thread | raw e-mail | index | archive | help

--VS++wcV0S1rZb1Fb
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Sep 14, 2002 at 12:17:53PM +0200, Poul-Henning Kamp wrote:
> 
> This is just to note that I have updated the JKH page with a lot of new
> stuff, so if your coding-pencil itches:
> 
> 	http://people.freebsd.org/~phk/TODO/

|Make -j improvement
|
|make(1) with -j option uses a select loop to wait for events, and every
|100msec it drops out to look for processes exited etc.  A pure "make
|buildworld" on a single-CPU machine is up to 25% faster that the best
|"make -j N buildworld" time on the same hardware.  Changing to timeout
|to be 10msec improves things about 10%.
|I think that make(1) should use kqueue(2) instead, since that would
|eliminate the need for timeouts.

Ok, here's what I came up with.  However, with the patch applied, each
'make buildworld' on a SMP machine throws tons of

/freebsd/current/src/sys/vm/uma_core.c:1307: could sleep with "filedesc structure" locked from /freebsd/current/src/sys/kern/kern_event.c:959

at me and freezes badly at some point (no breaking into ddb possible).
This is totally repeatable.  Is anybody able to reproduce (and maybe
fix) this?

Regards,
Stefan Farfeleder

--VS++wcV0S1rZb1Fb
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="make.diff"

Index: job.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/make/job.c,v
retrieving revision 1.43
diff -u -r1.43 job.c
--- job.c	29 Sep 2002 00:20:28 -0000	1.43
+++ job.c	2 Oct 2002 21:34:51 -0000
@@ -64,9 +64,8 @@
  *	Job_CatchOutput	    	Print any output our children have produced.
  *	    	  	    	Should also be called fairly frequently to
  *	    	  	    	keep the user informed of what's going on.
- *	    	  	    	If no output is waiting, it will block for
- *	    	  	    	a time given by the SEL_* constants, below,
- *	    	  	    	or until output is ready.
+ *				If no output is waiting, it will block until
+ *				a child terminates or until output is ready.
  *
  *	Job_Init  	    	Called to intialize this module. in addition,
  *	    	  	    	any commands attached to the .BEGIN target
@@ -107,6 +106,8 @@
 #include <sys/file.h>
 #include <sys/time.h>
 #include <sys/wait.h>
+#include <sys/event.h>
+#include <sys/time.h>
 #include <err.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -237,8 +238,7 @@
 				 * (2) a job can only be run locally, but
 				 * nLocal equals maxLocal */
 #ifndef RMT_WILL_WATCH
-static fd_set  	outputs;    	/* Set of descriptors of pipes connected to
-				 * the output channels of children */
+static int	kqfd;		/* File descriptor obtained by kqueue() */
 #endif
 
 STATIC GNode   	*lastNode;	/* The node for which output was most recently
@@ -692,8 +692,6 @@
     if (usePipes) {
 #ifdef RMT_WILL_WATCH
 	Rmt_Ignore(job->inPipe);
-#else
-	FD_CLR(job->inPipe, &outputs);
 #endif
 	if (job->outPipe != job->inPipe) {
 	   (void) close(job->outPipe);
@@ -1265,14 +1263,25 @@
 	    /*
 	     * The first time a job is run for a node, we set the current
 	     * position in the buffer to the beginning and mark another
-	     * stream to watch in the outputs mask
+	     * stream to watch in the outputs mask.  The termination of this
+	     * job and the availability of new data in the pipe are
+	     * registered in the kqueue.
 	     */
+	    struct kevent	kev[2];
+
 	    job->curPos = 0;
 
 #ifdef RMT_WILL_WATCH
 	    Rmt_Watch(job->inPipe, JobLocalInput, job);
 #else
-	    FD_SET(job->inPipe, &outputs);
+	    EV_SET(&kev[0], job->inPipe, EVFILT_READ, EV_ADD, 0, 0, job);
+	    EV_SET(&kev[1], job->pid, EVFILT_PROC, EV_ADD | EV_ONESHOT,
+		NOTE_EXIT, 0, NULL);
+	    if (kevent(kqfd, kev, 2, NULL, 0, NULL) != 0) {
+		/* kevent() will fail if the job is already finished */
+		if (errno != EBADF && errno != ESRCH)
+		    Punt("kevent: %s", strerror(errno));
+	    }
 #endif /* RMT_WILL_WATCH */
 	}
 
@@ -2229,12 +2238,12 @@
 Job_CatchOutput()
 {
     int           	  nfds;
-    struct timeval	  timeout;
-    fd_set           	  readfds;
-    LstNode		  ln;
-    Job		   	  *job;
 #ifdef RMT_WILL_WATCH
     int	    	  	  pnJobs;   	/* Previous nJobs */
+#else
+    struct kevent	  kev[4];	/* 4 is somewhat arbitrary */
+    size_t		  kevsize = sizeof(kev) / sizeof(kev[0]);
+    int			  i;
 #endif
 
     (void) fflush(stdout);
@@ -2262,25 +2271,20 @@
     }
 #else
     if (usePipes) {
-	readfds = outputs;
-	timeout.tv_sec = SEL_SEC;
-	timeout.tv_usec = SEL_USEC;
-
-	if ((nfds = select(FD_SETSIZE, &readfds, (fd_set *) 0,
-			   (fd_set *) 0, &timeout)) <= 0)
-	    return;
-	else {
-	    if (Lst_Open(jobs) == FAILURE) {
-		Punt("Cannot open job table");
-	    }
-	    while (nfds && (ln = Lst_Next(jobs)) != NULL) {
-		job = (Job *) Lst_Datum(ln);
-		if (FD_ISSET(job->inPipe, &readfds)) {
-		    JobDoOutput(job, FALSE);
-		    nfds -= 1;
+	if ((nfds = kevent(kqfd, NULL, 0, kev, kevsize, NULL)) == -1) {
+	    Punt("kevent: %s", strerror(errno));
+	} else {
+	    for (i = 0; i < nfds; i++) {
+		switch (kev[i].filter) {
+		case EVFILT_READ:
+		    JobDoOutput(kev[i].udata, FALSE);
+		    break;
+		case EVFILT_PROC:
+		    /* Just wake up and let Job_CatchChildren() collect the
+		     * terminated job. */
+		    break;
 		}
 	    }
-	    Lst_Close(jobs);
 	}
     }
 #endif /* RMT_WILL_WATCH */
@@ -2408,6 +2412,13 @@
     }
     if (signal(SIGWINCH, SIG_IGN) != SIG_IGN) {
 	(void) signal(SIGWINCH, JobPassSig);
+    }
+#endif
+
+#ifndef RMT_WILL_WATCH
+    
+    if ((kqfd = kqueue()) == -1) {
+	Punt("kqueue: %s", strerror(errno));
     }
 #endif
 
Index: job.h
===================================================================
RCS file: /home/ncvs/src/usr.bin/make/job.h,v
retrieving revision 1.18
diff -u -r1.18 job.h
--- job.h	26 Sep 2002 01:39:22 -0000	1.18
+++ job.h	1 Oct 2002 16:31:10 -0000
@@ -50,14 +50,6 @@
 
 #define	TMPPAT	"/tmp/makeXXXXXXXXXX"
 
-/*
- * The SEL_ constants determine the maximum amount of time spent in select
- * before coming out to see if a child has finished. SEL_SEC is the number of
- * seconds and SEL_USEC is the number of micro-seconds
- */
-#define	SEL_SEC		0
-#define	SEL_USEC	100000
-
 
 /*-
  * Job Table definitions.

--VS++wcV0S1rZb1Fb--

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021002214028.GA94673>