svn commit: r251422 - in head: contrib/bmake usr.bin/bmake

[Tijl Coosemans]

On 2013-06-24 20:57, Tijl Coosemans wrote:
> On 2013-06-05 18:12, Simon J. Gerraty wrote:
>> Author: sjg
>> Date: Wed Jun  5 16:12:50 2013
>> New Revision: 251422
>> URL: http://svnweb.freebsd.org/changeset/base/251422
>>
>> Log:
>>   Update to bmake-20130604 to fix file descriptor leak.
>>
>> Modified: head/contrib/bmake/job.c
>> ==============================================================================
>> --- head/contrib/bmake/job.c	Wed Jun  5 15:52:24 2013	(r251421)
>> +++ head/contrib/bmake/job.c	Wed Jun  5 16:12:50 2013	(r251422)
>> @@ -1,4 +1,4 @@
>> -/*	$NetBSD: job.c,v 1.172 2013/03/05 22:01:43 christos Exp $	*/
>> +/*	$NetBSD: job.c,v 1.173 2013/06/05 03:59:43 sjg Exp $	*/
>>  
>>  /*
>>   * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
>> @@ -70,14 +70,14 @@
>>   */
>>  
>>  #ifndef MAKE_NATIVE
>> -static char rcsid[] = "$NetBSD: job.c,v 1.172 2013/03/05 22:01:43 christos Exp $";
>> +static char rcsid[] = "$NetBSD: job.c,v 1.173 2013/06/05 03:59:43 sjg Exp $";
>>  #else
>>  #include <sys/cdefs.h>
>>  #ifndef lint
>>  #if 0
>>  static char sccsid[] = "@(#)job.c	8.2 (Berkeley) 3/19/94";
>>  #else
>> -__RCSID("$NetBSD: job.c,v 1.172 2013/03/05 22:01:43 christos Exp $");
>> +__RCSID("$NetBSD: job.c,v 1.173 2013/06/05 03:59:43 sjg Exp $");
>>  #endif
>>  #endif /* not lint */
>>  #endif
>> @@ -414,6 +414,15 @@ JobCreatePipe(Job *job, int minfd)
>>      if (pipe(job->jobPipe) == -1)
>>  	Punt("Cannot create pipe: %s", strerror(errno));
>>  
>> +    for (i = 0; i < 2; i++) {
>> +       /* Avoid using low numbered fds */
>> +       fd = fcntl(job->jobPipe[i], F_DUPFD, minfd);
>> +       if (fd != -1) {
>> +	   close(job->jobPipe[i]);
>> +	   job->jobPipe[i] = fd;
>> +       }
>> +    }
>> +    
>>      /* Set close-on-exec flag for both */
>>      (void)fcntl(job->jobPipe[0], F_SETFD, 1);
>>      (void)fcntl(job->jobPipe[1], F_SETFD, 1);
> 
> I've been noticing that bmake doesn't run parallel jobs as like fmake.
> I've attached a Makefile that I think shows what's going wrong.
> 
> If you run "make -j4" it outputs the following:
> 
> ===========================
> --- all ---
> -j 4 -i -J 15,16
> 4
> -j 4 -i
> 4
> --- sub_2 ---
> -j 4 -i -J 15,16
> 4
> -j 4 -i
> 4
> ===========================
> 
> Bmake outputs the target name in -j mode (e.g. "--- all ---"), but
> there's no "--- sub_1 ---" and "--- sub_3 ---" which suggests -j isn't
> working there. The -J flag also doesn't appear in .MAKEFLAGS in those
> targets.
> 
> I suspect the descriptors for the job server have to remain open so
> submakes can pick them up. At least, when I comment out the two fcntl
> calls above (and two more below), I do get the output I expect:
> 
> ===========================
> --- all ---
> -j 4 -J 15,16
> 4
> --- sub_1 ---
> -j 4 -J 15,16
> 4
> --- sub_2 ---
> -j 4 -J 15,16
> 4
> --- sub_3 ---
> -j 4 -J 15,16
> 4
> ===========================
> 
>> @@ -426,15 +435,6 @@ JobCreatePipe(Job *job, int minfd)
>>       */
>>      fcntl(job->jobPipe[0], F_SETFL, 
>>  	fcntl(job->jobPipe[0], F_GETFL, 0) | O_NONBLOCK);
>> -
>> -    for (i = 0; i < 2; i++) {
>> -       /* Avoid using low numbered fds */
>> -       fd = fcntl(job->jobPipe[i], F_DUPFD, minfd);
>> -       if (fd != -1) {
>> -	   close(job->jobPipe[i]);
>> -	   job->jobPipe[i] = fd;
>> -       }
>> -    }
>>  }
>>  
>>  /*-
>> @@ -2828,6 +2828,8 @@ Job_ServerStart(int max_tokens, int jp_0
>>  	/* Pipe passed in from parent */
>>  	tokenWaitJob.inPipe = jp_0;
>>  	tokenWaitJob.outPipe = jp_1;
>> +	(void)fcntl(jp_0, F_SETFD, 1);
>> +	(void)fcntl(jp_1, F_SETFD, 1);
> 
> These two fcntl calls have to be commented out too.

When those four lines are commented out buildworld with four jobs
runs 10% faster.

Now that I've taken a closer look at the code it seems bmake requires
targets that run make to be marked with .MAKE. Several targets in
Makefile and Makefile.inc1 already have this, but some don't, such as
bootstrap-tools. Can this be added there?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20130705/7604a230/attachment.sig>