Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

NeilBrown

Hi,
One of my Makefiles contains
  MAKEFLAGS += -j

Prior to the above commit, this would cause make to run as if I had run
   make -j

Since that commit, it appears to be ignored.  If I run
  make -j
explicitly I get parallel makes, but setting MAKEFLAGS in the Makefile
doesn't achieve that result.

This usage is still described as supported by make.texi:

The @code{MAKEFLAGS} variable can also be useful if you want to have
certain options, such as @samp{-k} (@pxref{Options Summary, ,Summary of
Options}), set each time you run @code{make}.  You simply put a value for
@code{MAKEFLAGS} in your environment.  You can also set @code{MAKEFLAGS} in
a makefile, to specify additional flags that should also be in effect for
that makefile.  (Note that you cannot use @code{MFLAGS} this way.  That
variable is set only for compatibility; @code{make} does not interpret a
value you set for it in any way.)


Thanks,
NeilBrown

_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make

signature.asc (847 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

NeilBrown
On Thu, Sep 28 2017, NeilBrown wrote:

> Hi,
> One of my Makefiles contains
>   MAKEFLAGS += -j
>
> Prior to the above commit, this would cause make to run as if I had run
>    make -j
>
> Since that commit, it appears to be ignored.  If I run
>   make -j
> explicitly I get parallel makes, but setting MAKEFLAGS in the Makefile
> doesn't achieve that result.
>
> This usage is still described as supported by make.texi:
>
> The @code{MAKEFLAGS} variable can also be useful if you want to have
> certain options, such as @samp{-k} (@pxref{Options Summary, ,Summary of
> Options}), set each time you run @code{make}.  You simply put a value for
> @code{MAKEFLAGS} in your environment.  You can also set @code{MAKEFLAGS} in
> a makefile, to specify additional flags that should also be in effect for
> that makefile.  (Note that you cannot use @code{MFLAGS} this way.  That
> variable is set only for compatibility; @code{make} does not interpret a
> value you set for it in any way.)

I dug into this a bit.
The regression is caused because the 'j' flag now causes arg_job_slots
to be set, rather than setting job_slots directly.
arg_job_slots is conditionally copied into job_slots after the call to

    decode_env_switches (STRING_SIZE_TUPLE ("MAKEFLAGS"));

at line 1453, but that is called again at line 1993 and the value set
then (which is when the value from "MAKEFLAGS += -j" in a Makefile takes
effect) is lost.

A possible fix is:

diff --git a/main.c b/main.c
index 226b26b96862..46e28379d687 100644
--- a/main.c
+++ b/main.c
@@ -1995,6 +1995,10 @@ main (int argc, char **argv, char **envp)
     decode_env_switches (STRING_SIZE_TUPLE ("MFLAGS"));
 #endif
 
+    if (!jobserver_auth)
+    /* MAKEFLAGS might have changed arg_job_slots */
+    job_slots = arg_job_slots;
+
     /* Reset in case the switches changed our mind.  */
     syncing = (output_sync == OUTPUT_SYNC_LINE
                || output_sync == OUTPUT_SYNC_TARGET);

but that feels like a bit of a hack.  I don't know the code well enough
to be sure.

Thanks,
NeilBrown

_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make

signature.asc (847 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

Paul Smith-20
In reply to this post by NeilBrown
On Thu, 2017-09-28 at 16:28 +1000, NeilBrown wrote:
> One of my Makefiles contains
>   MAKEFLAGS += -j
>
> Prior to the above commit, this would cause make to run as if I had
> run
>    make -j
>
> Since that commit, it appears to be ignored.

This is a bug clearly.  I've been thinking about it today and I wonder
what behavior people would expect to see if there are mismatches
between the various ways -j can be specified.

If it's a top-level make (not a sub-make in a jobserver grouping) then
I think the behavior is pretty straightforward based on normal make
priorities: if there's a command line -j setting use that; if not and
there's a MAKEFLAGS -j set in the makefile use that; if not and there's
a MAKEFLAGS -j set in the environment use that.

I don't think make should warn if the different sources have different
-j values.  You might set MAKEFLAGS=-j5 in your environment or makefile
but have "make -j1" to explicitly turn it off--you wouldn't want a
message about that IMO.

More interesting: what about a sub-make which is part of an existing
jobserver group?  Consider a sub-make invoked with -jN on the command
line; you have a makefile like this:

  $ cat cmd.mk
  subdir: ; $(MAKE) -C $@ -j2

If someone runs a simple "make -f cmd.mk", then you'd want the sub-make
to be run with -j2 of course.  But what if you do this:

  $ make -f cmd.mk -j4

What happens today is that a _new_ jobserver instance with -j2 is
started for the sub-make.  This could mean we have 5 total jobs
running, even though we specified -j4 on the command line.

That seems defensible, although I don't know that it's correct.

Now... what about if a sub-make has a MAKEFLAGS variable assignment?
Does that override an existing jobserver and cause the current make
instance to be the master of a new jobserver group, as if it were set
on the command line?

For example:

  $ cat sub.mk
  MAKEFLAGS += -j3
  all: ...

  $ cat cmd.mk
  MAKEFLAGS += -j2
  subdir: ; $(MAKE) -f sub.mk

  $ make -f cmd.mk -j4

Here we have a top-level make running the "cmd.mk" file, and a sub-make
running the "sub.mk" file.

The top make runs with -j4, because the command line overrides the
MAKEFLAGS += -j2 in cmd.mk.

What about the MAKEFLAGS setting in the sub.mk sub-make?  Does it force
a new top-level jobserver with jobs set to 3, as if we ran $(MAKE) -j3
in the subdir rule?  Or is it ignored (with a warning) if the sub-make
is already part of a jobserver group?

_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

Tim Murphy-4
For my money -j is always a top-level setting.   To have it per makefile and then let makefiles depend on that behaviour would open the door to lots of scary bugs.

Regards,

Tim

On 29 October 2017 at 18:57, Paul Smith <[hidden email]> wrote:
On Thu, 2017-09-28 at 16:28 +1000, NeilBrown wrote:
> One of my Makefiles contains
>   MAKEFLAGS += -j
>
> Prior to the above commit, this would cause make to run as if I had
> run
>    make -j
>
> Since that commit, it appears to be ignored.

This is a bug clearly.  I've been thinking about it today and I wonder
what behavior people would expect to see if there are mismatches
between the various ways -j can be specified.

If it's a top-level make (not a sub-make in a jobserver grouping) then
I think the behavior is pretty straightforward based on normal make
priorities: if there's a command line -j setting use that; if not and
there's a MAKEFLAGS -j set in the makefile use that; if not and there's
a MAKEFLAGS -j set in the environment use that.

I don't think make should warn if the different sources have different
-j values.  You might set MAKEFLAGS=-j5 in your environment or makefile
but have "make -j1" to explicitly turn it off--you wouldn't want a
message about that IMO.

More interesting: what about a sub-make which is part of an existing
jobserver group?  Consider a sub-make invoked with -jN on the command
line; you have a makefile like this:

  $ cat cmd.mk
  subdir: ; $(MAKE) -C $@ -j2

If someone runs a simple "make -f cmd.mk", then you'd want the sub-make
to be run with -j2 of course.  But what if you do this:

  $ make -f cmd.mk -j4

What happens today is that a _new_ jobserver instance with -j2 is
started for the sub-make.  This could mean we have 5 total jobs
running, even though we specified -j4 on the command line.

That seems defensible, although I don't know that it's correct.

Now... what about if a sub-make has a MAKEFLAGS variable assignment?
Does that override an existing jobserver and cause the current make
instance to be the master of a new jobserver group, as if it were set
on the command line?

For example:

  $ cat sub.mk
  MAKEFLAGS += -j3
  all: ...

  $ cat cmd.mk
  MAKEFLAGS += -j2
  subdir: ; $(MAKE) -f sub.mk

  $ make -f cmd.mk -j4

Here we have a top-level make running the "cmd.mk" file, and a sub-make
running the "sub.mk" file.

The top make runs with -j4, because the command line overrides the
MAKEFLAGS += -j2 in cmd.mk.

What about the MAKEFLAGS setting in the sub.mk sub-make?  Does it force
a new top-level jobserver with jobs set to 3, as if we ran $(MAKE) -j3
in the subdir rule?  Or is it ignored (with a warning) if the sub-make
is already part of a jobserver group?

_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

Sven C. Dack


On 30/10/17 07:33, Tim Murphy wrote:
> For my money -j is always a top-level setting.   To have it per
> makefile and then let makefiles depend on that behaviour would open
> the door to lots of scary bugs.
>
> Regards,
>
> Tim

Agreed. Using -j in a Makefile is not portable, because it can cause a
system to run out of memory or just start an unwanted and excessive
amount of jobs.

However, if someone wanted to control the number of jobs from within a
Makefile then this shouldn't be discouraged either. Nor does make give
any kind of guarantees on how many jobs are started by other processes,
because this isn't something, which can be controlled easily from within
make, and I don't believe many expect such a form of control by make.

We do have flags such as -n, -k and -S, where users expect a strict
behaviour for -n, but not so for -k and -S (continue on errors, stop on
errors). The later two can both be set from within Makefiles, with -k
being potentially dangerous if set from within.

While I dislike the use of -j in Makefiles is it not as bad as if having
a (hypothetical) option to disable -n for example. As such do I see it
being in the same boat with -k. Any problems caused by using -j from
within Makefiles are the responsibility of the person who put it there.
It shouldn't be made a responsibility of the devs of make unless we have
very good reasons to do so.

Cheers,
Sven


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

NeilBrown
In reply to this post by Tim Murphy-4
On Mon, Oct 30 2017, Tim Murphy wrote:

> For my money -j is always a top-level setting.   To have it per makefile
> and then let makefiles depend on that behaviour would open the door to lots
> of scary bugs.

I'm beginning to agree with this perspective.
The "-j" setting is not really a function of the code, it is a function
of the hardware that make is being run on.
So I should set MAKEFLAGS=-j8 in my environment instead of if the
Makefile.

Maybe we just need to change the documentation...

Thanks,
NeilBrown

_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make

signature.asc (847 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

Paul Smith-20
On Tue, 2017-10-31 at 07:39 +1100, NeilBrown wrote:
> On Mon, Oct 30 2017, Tim Murphy wrote:
>
> > For my money -j is always a top-level setting.   To have it per makefile
> > and then let makefiles depend on that behaviour would open the door to lots
> > of scary bugs.

I don't see what scary bugs we might have; I've seen issues where
makefiles not intended to be used with -j will not work properly with
-j, but I'm not sure I've ever seen an issue with a makefile intended
to work with -j that fails when run without it, or with a different
value.

> I'm beginning to agree with this perspective.
> The "-j" setting is not really a function of the code, it is a
> function of the hardware that make is being run on.
> So I should set MAKEFLAGS=-j8 in my environment instead of if the
> Makefile.

I definitely agree that's best-practice.  You could also use the
(newer) GNUMAKEFLAGS environment variable which will ensure only GNU
make sees the value (in case you run other versions of make).

> Maybe we just need to change the documentation...

I definitely agree that setting -j inside a makefile is not a good
idea, just like adding -k or -d or whatever.

However people do it and it used to work.

The question is what should make do when it happens?  Silently ignoring
it as now doesn't seem like a winning alternative.

If we see -j in a makefile setting of MAKEFLAGS, we could:

   1. Always silently ignore it (today's behavior).
   2. Always print a message then ignore it.
   3. Treat it the same way as a recipe with $(MAKE) -j: that is, start a
      new jobserver group for this sub-make (and print a message if we're
      already part of a different jobserver group).

Personally I lean towards #3, even if it does allow people to do
bizarre things.

_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

Sven C. Dack

On 30/10/17 21:22, Paul Smith wrote:

> If we see -j in a makefile setting of MAKEFLAGS, we could:
>
>     1. Always silently ignore it (today's behavior).
>     2. Always print a message then ignore it.
>     3. Treat it the same way as a recipe with $(MAKE) -j: that is, start a
>        new jobserver group for this sub-make (and print a message if we're
>        already part of a different jobserver group).
>
> Personally I lean towards #3, even if it does allow people to do
> bizarre things.

I like 3, too. It gives more options to Makefile authors. We will only
see more parallelism in the future, with more cores per CPU and people
trying to utilise the hardware anyway they can. Who knows how helpful it
might become.

Also, the -l option (load limiting) when set should get passed along to
make it more meaningful - unless it, too, receives a new value for
whatever reason.


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

Paul Smith-20
In reply to this post by NeilBrown
On Thu, 2017-09-28 at 20:41 +1000, NeilBrown wrote:
> I dug into this a bit.
> The regression is caused because the 'j' flag now causes
> arg_job_slots to be set, rather than setting job_slots directly.

I pushed a fix for this.

_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

RE: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

sgardell
In reply to this post by NeilBrown
Our experience is that it is really only sensible as a top-level control. That said, it is convenient to have as an argument to make - which of course opens one up to seeing it specified on explicit sub-make invocations. I would also note that we have found that the performance behavior (this is real measured total build time) is rather tide to the output logging control so these two things need to be considered in tandem.

-----Original Message-----
From: Bug-make [mailto:bug-make-bounces+sgardell=[hidden email]] On Behalf Of NeilBrown
Sent: Monday, October 30, 2017 4:40 PM
To: Tim Murphy <[hidden email]>; Paul D. Smith <[hidden email]>
Cc: [hidden email]
Subject: Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

On Mon, Oct 30 2017, Tim Murphy wrote:

> For my money -j is always a top-level setting.   To have it per makefile
> and then let makefiles depend on that behaviour would open the door to
> lots of scary bugs.

I'm beginning to agree with this perspective.
The "-j" setting is not really a function of the code, it is a function of the hardware that make is being run on.
So I should set MAKEFLAGS=-j8 in my environment instead of if the Makefile.

Maybe we just need to change the documentation...

Thanks,
NeilBrown


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

Eric Melski
In reply to this post by Paul Smith-20
On 10/30/2017 02:22 PM, Paul Smith wrote:

> On Tue, 2017-10-31 at 07:39 +1100, NeilBrown wrote:
>> On Mon, Oct 30 2017, Tim Murphy wrote:
>>
>>> For my money -j is always a top-level setting.   To have it per makefile
>>> and then let makefiles depend on that behaviour would open the door to lots
>>> of scary bugs.
>
> I don't see what scary bugs we might have; I've seen issues where
> makefiles not intended to be used with -j will not work properly with
> -j, but I'm not sure I've ever seen an issue with a makefile intended
> to work with -j that fails when run without it, or with a different
> value.

Here's a trivial example that shows how a build could fail if -j is not
specified, but work if -j is specified (with any value, in this case),
assuming "output" does not exist to start:


        all: reader writer
       
        reader:
                sleep 2
                cat output
       
        writer:
                echo PASS > output

Of course "work" is used somewhat loosely here, as the build might
appear to work but in fact pick up stale data if "output" happens to
exist prior to the start of the run (eg, in an incremental build).

It's not a common scenario, to be sure, but I have encountered this "in
the wild".  Just last week I worked with a large commercial build which
exhibited this behavior, due to a missing dependency between the reader
and writer jobs.


Regards,

Eric Melski
Chief Architect
Electric Cloud, Inc.


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

NeilBrown
In reply to this post by Paul Smith-20
On Tue, Oct 31 2017, Paul Smith wrote:

> On Thu, 2017-09-28 at 20:41 +1000, NeilBrown wrote:
>> I dug into this a bit.
>> The regression is caused because the 'j' flag now causes
>> arg_job_slots to be set, rather than setting job_slots directly.
>
> I pushed a fix for this.

Thanks.  Your fix appear to work as expected for my use-case.

NeilBrown

_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make

signature.asc (847 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

NeilBrown
On Wed, Nov 01 2017, NeilBrown wrote:

> On Tue, Oct 31 2017, Paul Smith wrote:
>
>> On Thu, 2017-09-28 at 20:41 +1000, NeilBrown wrote:
>>> I dug into this a bit.
>>> The regression is caused because the 'j' flag now causes
>>> arg_job_slots to be set, rather than setting job_slots directly.
>>
>> I pushed a fix for this.
>
> Thanks.  Your fix appear to work as expected for my use-case.
>
> NeilBrown
Actually...
A few times when building the linux kernel I've received the message

make[1]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.

This isn't consistent so it is presumably only when some particular
target needs building. If I make a clean start, I always get the
message.
It seems to throw the whole build into -j1 mode.
In this case I pass '-j8' on the command line.

Any idea what could be causing this?

Thanks,
NeilBrown

_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make

signature.asc (847 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

Paul Smith-20
On Fri, 2017-11-03 at 12:27 +1100, NeilBrown wrote:
> Actually...
> A few times when building the linux kernel I've received the message
>
> make[1]: warning: jobserver unavailable: using -j1.  Add '+' to
> parent make rule.
>
> This isn't consistent so it is presumably only when some particular
> target needs building. If I make a clean start, I always get the
> message.

This message means that a sub-make is started with jobserver
authentication option available in the environment (so that the parent
make was part of a jobserver group).  However, when make attempted to
access the jobserver pipe, it was discovered to be closed and so the
sub-make couldn't access it.

Unfortunately some programs do NOT like to have extra file descriptors
open when they are started, for whatever reason.  Also if random
programs start reading/writing to the jobserver pipe Badness Happens.
So make tries to ensure the jobserver pipe is only available to sub-
make commands, and not available to non-sub-make commands.

If the command is a sub-make, then make will not close the jobserver
pipe when invoking that command so that the sub-make can access it.  If
make decides the command is NOT a sub-make, then make WILL close the
jobserver pipe before invoking that command.

If make decides incorrectly and something that it thought was not a
sub-make really is one, then you get the above error.

Make uses the normal decision process for this: any command that
contains the string $(MAKE) or ${MAKE} (before expansion) is considered
a sub-make invocation, or any command that starts with the "+" option.

It's strange that you're just seeing this now, though, and makes me
concerned that I messed up the logic around the jobserver start.
You're saying you didn't used to get this error before?

It would be good if you could track down the rule which is invoking the
sub-make that gives that error.

Is this the current HEAD Linux kernel or some other version?

_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make