Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 03 Oct 2002 16:41:46 +0200
From:      Poul-Henning Kamp <phk@critter.freebsd.dk>
To:        Stefan Farfeleder <e0026813@stud3.tuwien.ac.at>
Cc:        current@FreeBSD.ORG
Subject:   Re: Junior Kernel Hacker page updated... 
Message-ID:  <4860.1033656106@critter.freebsd.dk>
In-Reply-To: Your message of "Wed, 02 Oct 2002 23:40:28 %2B0200." <20021002214028.GA94673@frog.fafoe> 

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

Hi Stefan,

I tried this patch and it paniced my (almost-) current machine with
a pagefault in the kqueue code:  Bravo!

I can see that there is some amount of #ifdef stuff in your patch,
in light of that, would it be possible to make an #ifdef'ed version
of your patch which we could commit ?

That way we give the kqueue hackers a good testcase, and we can
easily enable when they have solved the problem.

Poul-Henning



In message <20021002214028.GA94673@frog.fafoe>, Stefan Farfeleder writes:
>
>--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--
>

-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk@FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.

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?4860.1033656106>