2019-09-26 09:01:14

by Kees Cook

[permalink] [raw]
Subject: [PATCH v3] docs: Use make invocation's -j argument for parallelism

While sphinx 1.7 and later supports "-jauto" for parallelism, this
effectively ignores the "-j" flag used in the "make" invocation, which
may cause confusion for build systems. Instead, extract the available
parallelism from "make"'s job server (since it is not exposed in any
special variables) and use that for the "sphinx-build" run. Now things
work correctly for builds where -j is specified at the top-level:

make -j16 htmldocs

If -j is not specified, continue to fallback to "-jauto" if available.

Signed-off-by: Kees Cook <[email protected]>
---
v3: python2, specific exceptions, correct SPDX, blocking writer
v2: retain "-jauto" default behavior with top-level -j is missing.
---
Documentation/Makefile | 3 ++-
scripts/jobserver-count | 58 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+), 1 deletion(-)
create mode 100755 scripts/jobserver-count

diff --git a/Documentation/Makefile b/Documentation/Makefile
index e145e4db508b..c6e564656a5b 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -33,7 +33,7 @@ ifeq ($(HAVE_SPHINX),0)

else # HAVE_SPHINX

-export SPHINXOPTS = $(shell perl -e 'open IN,"sphinx-build --version 2>&1 |"; while (<IN>) { if (m/([\d\.]+)/) { print "-jauto" if ($$1 >= "1.7") } ;} close IN')
+export SPHINX_PARALLEL = $(shell perl -e 'open IN,"sphinx-build --version 2>&1 |"; while (<IN>) { if (m/([\d\.]+)/) { print "auto" if ($$1 >= "1.7") } ;} close IN')

# User-friendly check for pdflatex and latexmk
HAVE_PDFLATEX := $(shell if which $(PDFLATEX) >/dev/null 2>&1; then echo 1; else echo 0; fi)
@@ -68,6 +68,7 @@ quiet_cmd_sphinx = SPHINX $@ --> file://$(abspath $(BUILDDIR)/$3/$4)
PYTHONDONTWRITEBYTECODE=1 \
BUILDDIR=$(abspath $(BUILDDIR)) SPHINX_CONF=$(abspath $(srctree)/$(src)/$5/$(SPHINX_CONF)) \
$(SPHINXBUILD) \
+ -j $(shell python $(srctree)/scripts/jobserver-count $(SPHINX_PARALLEL)) \
-b $2 \
-c $(abspath $(srctree)/$(src)) \
-d $(abspath $(BUILDDIR)/.doctrees/$3) \
diff --git a/scripts/jobserver-count b/scripts/jobserver-count
new file mode 100755
index 000000000000..0b482d6884d2
--- /dev/null
+++ b/scripts/jobserver-count
@@ -0,0 +1,58 @@
+#!/usr/bin/env python
+# SPDX-License-Identifier: GPL-2.0+
+#
+# This determines how many parallel tasks "make" is expecting, as it is
+# not exposed via an special variables.
+# https://www.gnu.org/software/make/manual/html_node/POSIX-Jobserver.html#POSIX-Jobserver
+from __future__ import print_function
+import os, sys, fcntl, errno
+
+# Default parallelism is "1" unless overridden on the command-line.
+default="1"
+if len(sys.argv) > 1:
+ default=sys.argv[1]
+
+# Set non-blocking for a given file descriptor.
+def nonblock(fd):
+ flags = fcntl.fcntl(fd, fcntl.F_GETFL)
+ fcntl.fcntl(fd, fcntl.F_SETFL, flags | os.O_NONBLOCK)
+ return fd
+
+# Extract and prepare jobserver file descriptors from envirnoment.
+try:
+ # Fetch the make environment options.
+ flags = os.environ['MAKEFLAGS']
+
+ # Look for "--jobserver=R,W"
+ opts = [x for x in flags.split(" ") if x.startswith("--jobserver")]
+
+ # Parse out R,W file descriptor numbers and set them nonblocking.
+ fds = opts[0].split("=", 1)[1]
+ reader, writer = [int(x) for x in fds.split(",", 1)]
+ reader = nonblock(reader)
+except (KeyError, IndexError, ValueError, IOError):
+ # Any missing environment strings or bad fds should result in just
+ # using the default specified parallelism.
+ print(default)
+ sys.exit(0)
+
+# Read out as many jobserver slots as possible.
+jobs = b""
+while True:
+ try:
+ slot = os.read(reader, 1)
+ jobs += slot
+ except (OSError, IOError) as e:
+ if e.errno == errno.EWOULDBLOCK:
+ # Stop when reach the end of the jobserver queue.
+ break
+ raise e
+# Return all the reserved slots.
+os.write(writer, jobs)
+
+# If the jobserver was (impossibly) full or communication failed, use default.
+if len(jobs) < 1:
+ print(default)
+
+# Report available slots (with a bump for our caller's reserveration).
+print(len(jobs) + 1)
--
2.17.1


--
Kees Cook


2019-10-01 12:43:07

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH v3] docs: Use make invocation's -j argument for parallelism

On Tue, 24 Sep 2019 16:29:58 -0700
Kees Cook <[email protected]> wrote:

> While sphinx 1.7 and later supports "-jauto" for parallelism, this
> effectively ignores the "-j" flag used in the "make" invocation, which
> may cause confusion for build systems. Instead, extract the available
> parallelism from "make"'s job server (since it is not exposed in any
> special variables) and use that for the "sphinx-build" run. Now things
> work correctly for builds where -j is specified at the top-level:
>
> make -j16 htmldocs
>
> If -j is not specified, continue to fallback to "-jauto" if available.
>
> Signed-off-by: Kees Cook <[email protected]>

I finally messed with this a bit; it seems to do exactly what's written on
the box.

It seems to me that The Real Solution™ here is to send a patch to the
Sphinx folks adding a "-jgmake" (or some such) option. It also seems to
me that none of us is likely to get around to that in the near future. So
I just applied this, thanks for dealing with all my picky comments...

jon

2019-10-04 08:27:47

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v3] docs: Use make invocation's -j argument for parallelism


On 25.09.19 01:29, Kees Cook wrote:
> While sphinx 1.7 and later supports "-jauto" for parallelism, this
> effectively ignores the "-j" flag used in the "make" invocation, which
> may cause confusion for build systems. Instead, extract the available
> parallelism from "make"'s job server (since it is not exposed in any
> special variables) and use that for the "sphinx-build" run. Now things
> work correctly for builds where -j is specified at the top-level:
>
> make -j16 htmldocs
>
> If -j is not specified, continue to fallback to "-jauto" if available.
>
> Signed-off-by: Kees Cook <[email protected]>
> ---
> v3: python2, specific exceptions, correct SPDX, blocking writer
> v2: retain "-jauto" default behavior with top-level -j is missing.
[...]
> diff --git a/scripts/jobserver-count b/scripts/jobserver-count
> new file mode 100755
> index 000000000000..0b482d6884d2
> --- /dev/null
> +++ b/scripts/jobserver-count
> @@ -0,0 +1,58 @@
> +#!/usr/bin/env python


This breaks our daily linux-next build for an fedora 30 rpm on s390x:

+ /usr/lib/rpm/redhat/brp-mangle-shebangs
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/profile2linkerlist.pl from /usr/bin/env perl to #!/usr/bin/perl
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/headerdep.pl from /usr/bin/env perl to #!/usr/bin/perl
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/package/buildtar from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/package/builddeb from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/package/mkspec from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/package/mkdebian from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/checksyscalls.sh from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/gen_ksymdeps.sh from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/makelst from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/checkversion.pl from /usr/bin/env perl to #!/usr/bin/perl
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/gcc-plugin.sh from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/gfp-translate from /bin/bash to #!/usr/bin/bash
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/tags.sh from /bin/bash to #!/usr/bin/bash
*** ERROR: ambiguous python shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/jobserver-count: #!/usr/bin/env python. Change it to python3 (or python2) explicitly.
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/adjust_autoksyms.sh from /bin/sh to #!/usr/bin/sh
mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/kernel-doc from /usr/bin/env perl to #!/usr/bin/perl
[...]


2019-10-04 10:20:37

by Rasmus Villemoes

[permalink] [raw]
Subject: Re: [PATCH v3] docs: Use make invocation's -j argument for parallelism

On 25/09/2019 01.29, Kees Cook wrote:
>
> # User-friendly check for pdflatex and latexmk
> HAVE_PDFLATEX := $(shell if which $(PDFLATEX) >/dev/null 2>&1; then echo 1; else echo 0; fi)
> @@ -68,6 +68,7 @@ quiet_cmd_sphinx = SPHINX $@ --> file://$(abspath $(BUILDDIR)/$3/$4)
> PYTHONDONTWRITEBYTECODE=1 \
> BUILDDIR=$(abspath $(BUILDDIR)) SPHINX_CONF=$(abspath $(srctree)/$(src)/$5/$(SPHINX_CONF)) \
> $(SPHINXBUILD) \
> + -j $(shell python $(srctree)/scripts/jobserver-count $(SPHINX_PARALLEL)) \
> -b $2 \
> -c $(abspath $(srctree)/$(src)) \
> -d $(abspath $(BUILDDIR)/.doctrees/$3) \
> diff --git a/scripts/jobserver-count b/scripts/jobserver-count
> new file mode 100755
> index 000000000000..0b482d6884d2
> --- /dev/null
> +++ b/scripts/jobserver-count
> @@ -0,0 +1,58 @@
> +#!/usr/bin/env python
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# This determines how many parallel tasks "make" is expecting, as it is
> +# not exposed via an special variables.
> +# https://www.gnu.org/software/make/manual/html_node/POSIX-Jobserver.html#POSIX-Jobserver
> +from __future__ import print_function
> +import os, sys, fcntl, errno
> +
> +# Default parallelism is "1" unless overridden on the command-line.
> +default="1"
> +if len(sys.argv) > 1:
> + default=sys.argv[1]
> +
> +# Set non-blocking for a given file descriptor.
> +def nonblock(fd):
> + flags = fcntl.fcntl(fd, fcntl.F_GETFL)
> + fcntl.fcntl(fd, fcntl.F_SETFL, flags | os.O_NONBLOCK)
> + return fd
> +
> +# Extract and prepare jobserver file descriptors from envirnoment.
> +try:
> + # Fetch the make environment options.
> + flags = os.environ['MAKEFLAGS']
> +
> + # Look for "--jobserver=R,W"
> + opts = [x for x in flags.split(" ") if x.startswith("--jobserver")]

OK, this handles the fact that Make changed from --jobserver-fds to
--jobserver-auth at some point. Probably the comment could be more accurate.

> + # Parse out R,W file descriptor numbers and set them nonblocking.
> + fds = opts[0].split("=", 1)[1]
> + reader, writer = [int(x) for x in fds.split(",", 1)]
> + reader = nonblock(reader)
> +except (KeyError, IndexError, ValueError, IOError):
> + # Any missing environment strings or bad fds should result in just
> + # using the default specified parallelism.
> + print(default)
> + sys.exit(0)
> +
> +# Read out as many jobserver slots as possible.
> +jobs = b""
> +while True:
> + try:
> + slot = os.read(reader, 1)
> + jobs += slot
> + except (OSError, IOError) as e:
> + if e.errno == errno.EWOULDBLOCK:
> + # Stop when reach the end of the jobserver queue.
> + break
> + raise e

<strikethrough>Only very new Make (e.g. not make 4.1 shipped with Ubuntu
18.04) sets the rfd as O_NONBLOCK (and only when it detected
HAVE_PSELECT at configure time, but that can probably be assumed). So
won't the above just hang forever if run under such a make?</strikethrough>

Ah, reading more carefully you set O_NONBLOCK explicitly. Well, older
Makes are going to be very unhappy about that (remember that it's a
property of the file description and not file descriptor). They don't
expect EAGAIN when fetching a token, so fail hard. Probably not when
htmldocs is the only target, because in that case the toplevel Make just
reads back the exact number of tokens it put in as a sanity check, but
if it builds other targets/spawns other submakes, I think this breaks.

Yeah, it's a mess, and the Make documentation should be much more
explicit about how one is supposed to interact with the job server and
the file descriptors. Some of the pain would vanish if it just used a
named pipe and had each client open its own fds to that so they could
each choose O_NONBLOCK or not.

> +# Return all the reserved slots.
> +os.write(writer, jobs)

Well, that probably works ok for the isolated case of a toplevel "make
-j12 htmldocs", but if you're building other targets ("make -j12
htmldocs vmlinux") this will effectively inject however many tokens the
above loop grabbed (which might not be all if the top-level make has
started things related to the vmlinux target), so for the duration of
the docs build, there will be more processes running than asked for.

> +# If the jobserver was (impossibly) full or communication failed, use default.
> +if len(jobs) < 1:
> + print(default)
> +
> +# Report available slots (with a bump for our caller's reserveration).
> +print(len(jobs) + 1)

There's a missing exit() or else: here; if len(jobs) < 1 you print both
default (probably "1") and 0+1 aka "1".

Rasmus

2019-10-04 11:00:47

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v3] docs: Use make invocation's -j argument for parallelism



On 04.10.19 10:04, Christian Borntraeger wrote:
>
> On 25.09.19 01:29, Kees Cook wrote:
>> While sphinx 1.7 and later supports "-jauto" for parallelism, this
>> effectively ignores the "-j" flag used in the "make" invocation, which
>> may cause confusion for build systems. Instead, extract the available
>> parallelism from "make"'s job server (since it is not exposed in any
>> special variables) and use that for the "sphinx-build" run. Now things
>> work correctly for builds where -j is specified at the top-level:
>>
>> make -j16 htmldocs
>>
>> If -j is not specified, continue to fallback to "-jauto" if available.
>>
>> Signed-off-by: Kees Cook <[email protected]>
>> ---
>> v3: python2, specific exceptions, correct SPDX, blocking writer
>> v2: retain "-jauto" default behavior with top-level -j is missing.
> [...]
>> diff --git a/scripts/jobserver-count b/scripts/jobserver-count
>> new file mode 100755
>> index 000000000000..0b482d6884d2
>> --- /dev/null
>> +++ b/scripts/jobserver-count
>> @@ -0,0 +1,58 @@
>> +#!/usr/bin/env python
>
>
> This breaks our daily linux-next build for an fedora 30 rpm on s390x:
>
> + /usr/lib/rpm/redhat/brp-mangle-shebangs
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/profile2linkerlist.pl from /usr/bin/env perl to #!/usr/bin/perl
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/headerdep.pl from /usr/bin/env perl to #!/usr/bin/perl
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/package/buildtar from /bin/sh to #!/usr/bin/sh
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/package/builddeb from /bin/sh to #!/usr/bin/sh
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/package/mkspec from /bin/sh to #!/usr/bin/sh
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/package/mkdebian from /bin/sh to #!/usr/bin/sh
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/checksyscalls.sh from /bin/sh to #!/usr/bin/sh
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/gen_ksymdeps.sh from /bin/sh to #!/usr/bin/sh
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/makelst from /bin/sh to #!/usr/bin/sh
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/checkversion.pl from /usr/bin/env perl to #!/usr/bin/perl
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/gcc-plugin.sh from /bin/sh to #!/usr/bin/sh
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/gfp-translate from /bin/bash to #!/usr/bin/bash
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/tags.sh from /bin/bash to #!/usr/bin/bash
> *** ERROR: ambiguous python shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/jobserver-count: #!/usr/bin/env python. Change it to python3 (or python2) explicitly.
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/adjust_autoksyms.sh from /bin/sh to #!/usr/bin/sh
> mangling shebang in /usr/src/kernels/5.4.0-20191004.rc1.git155.311ef88adfa3.301.fc30.s390x+next/scripts/kernel-doc from /usr/bin/env perl to #!/usr/bin/perl
> [...]


Ok, adding something like

+pathfix.py -pni "%{__python3} %{py3_shbang_opts}" scripts/jobserver-count

to the spec file fixed the problem.

Question is, if we want to make the python version more specific or not.

2019-10-04 16:12:23

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v3] docs: Use make invocation's -j argument for parallelism

On Fri, Oct 04, 2019 at 11:15:46AM +0200, Rasmus Villemoes wrote:
> On 25/09/2019 01.29, Kees Cook wrote:
> > +# Extract and prepare jobserver file descriptors from envirnoment.
> > +try:
> > + # Fetch the make environment options.
> > + flags = os.environ['MAKEFLAGS']
> > +
> > + # Look for "--jobserver=R,W"
> > + opts = [x for x in flags.split(" ") if x.startswith("--jobserver")]
>
> OK, this handles the fact that Make changed from --jobserver-fds to
> --jobserver-auth at some point. Probably the comment could be more accurate.

I can update that, sure.

> > +# Read out as many jobserver slots as possible.
> > +jobs = b""
> > +while True:
> > + try:
> > + slot = os.read(reader, 1)
> > + jobs += slot
> > + except (OSError, IOError) as e:
> > + if e.errno == errno.EWOULDBLOCK:
> > + # Stop when reach the end of the jobserver queue.
> > + break
> > + raise e
>
> Ah, reading more carefully you set O_NONBLOCK explicitly. Well, older
> Makes are going to be very unhappy about that (remember that it's a
> property of the file description and not file descriptor). They don't
> expect EAGAIN when fetching a token, so fail hard. Probably not when
> htmldocs is the only target, because in that case the toplevel Make just
> reads back the exact number of tokens it put in as a sanity check, but
> if it builds other targets/spawns other submakes, I think this breaks.

Do you mean the processes sharing the file will suddenly gain
O_NONBLOCK? I didn't think that was true, but I can test. If it is true,
we could easily just restore the state before exit.

> > +# Return all the reserved slots.
> > +os.write(writer, jobs)
>
> Well, that probably works ok for the isolated case of a toplevel "make
> -j12 htmldocs", but if you're building other targets ("make -j12
> htmldocs vmlinux") this will effectively inject however many tokens the
> above loop grabbed (which might not be all if the top-level make has
> started things related to the vmlinux target), so for the duration of
> the docs build, there will be more processes running than asked for.

That is true, yes, though I still think it's an improvement over the
existing case of sphinx-build getting run with -jauto which lands us in
the same (or worse) position.

The best solution would be to teach sphinx-build about the Make
jobserver, though I expect that would be weird. Another idea would be to
hold the reservation until sphinx-build finishes and THEN return the
slots? That would likely need to change from a utility to a sphinx-build
wrapper...

> > +# If the jobserver was (impossibly) full or communication failed, use default.
> > +if len(jobs) < 1:
> > + print(default)
> > +
> > +# Report available slots (with a bump for our caller's reserveration).
> > +print(len(jobs) + 1)
>
> There's a missing exit() or else: here; if len(jobs) < 1 you print both
> default (probably "1") and 0+1 aka "1".

Whoops! Yes, that needs fixing too. I'll send a patch...

--
Kees Cook

2019-10-06 19:33:49

by Rasmus Villemoes

[permalink] [raw]
Subject: Re: [PATCH v3] docs: Use make invocation's -j argument for parallelism

On 04/10/2019 18.08, Kees Cook wrote:
> On Fri, Oct 04, 2019 at 11:15:46AM +0200, Rasmus Villemoes wrote:
>> On 25/09/2019 01.29, Kees Cook wrote:
>>> +# Extract and prepare jobserver file descriptors from envirnoment.
>>
>> Ah, reading more carefully you set O_NONBLOCK explicitly. Well, older
>> Makes are going to be very unhappy about that (remember that it's a
>> property of the file description and not file descriptor). They don't
>> expect EAGAIN when fetching a token, so fail hard. Probably not when
>> htmldocs is the only target, because in that case the toplevel Make just
>> reads back the exact number of tokens it put in as a sanity check, but
>> if it builds other targets/spawns other submakes, I think this breaks.
>
> Do you mean the processes sharing the file will suddenly gain
> O_NONBLOCK? I didn't think that was true,

It is. Quoting man fcntl

File status flags
Each open file description has certain associated status
flags, initialized by open(2) and possibly modified by
fcntl(). Duplicated file descriptors (made with dup(2),
fcntl(F_DUPFD), fork(2), etc.) refer to the same open
file description, and thus share the same file status flags.

... On Linux, this command
can change only the O_APPEND, O_ASYNC, O_DIRECT,
O_NOATIME, and O_NONBLOCK flags.

> we could easily just restore the state before exit.

That doesn't really help - and I'm a bit surprised you'd even suggest
that. I don't know if open(/proc/self/fd/...) would give you a new
struct file.

>>> +# Return all the reserved slots.
>>> +os.write(writer, jobs)
>>
>> Well, that probably works ok for the isolated case of a toplevel "make
>> -j12 htmldocs", but if you're building other targets ("make -j12
>> htmldocs vmlinux") this will effectively inject however many tokens the
>> above loop grabbed (which might not be all if the top-level make has
>> started things related to the vmlinux target), so for the duration of
>> the docs build, there will be more processes running than asked for.
>
> That is true, yes, though I still think it's an improvement over the
> existing case of sphinx-build getting run with -jauto which lands us in
> the same (or worse) position.

Yes, I agree that that's not ideal either. And probably it's not a big
problem in practice (I don't think a lot of people build the docs, let
alone do it while also building the kernel), but it might be rather
surprising and somewhat hard to "debug" to suddenly have a load twice
what one expected.

> The best solution would be to teach sphinx-build about the Make
> jobserver, though I expect that would be weird. Another idea would be to
> hold the reservation until sphinx-build finishes and THEN return the
> slots? That would likely need to change from a utility to a sphinx-build
> wrapper...

Yes, a more general solution would be some kind of generic wrapper that
would hog however many tokens it could get hold of and run a given
command with a commandline slightly modified to hand over those tokens -
then wait for that process to exit and give back the tokens. That would
work for any command that knows about parallelism but doesn't support
the make jobserver model. (I'd probably implement that by creating a
pipe, fork(), then exec into the real command, while the child simply
blocks in a read on the pipe waiting for EOF and then writes back the
tokens, to simplify the "we have to report exit/killed-by-signal status
to the parent).

Rasmus

2019-10-16 03:55:03

by Rasmus Villemoes

[permalink] [raw]
Subject: Re: [PATCH v3] docs: Use make invocation's -j argument for parallelism

On 06/10/2019 21.33, Rasmus Villemoes wrote:
> On 04/10/2019 18.08, Kees Cook wrote:

>> The best solution would be to teach sphinx-build about the Make
>> jobserver, though I expect that would be weird. Another idea would be to
>> hold the reservation until sphinx-build finishes and THEN return the
>> slots? That would likely need to change from a utility to a sphinx-build
>> wrapper...
>
> Yes, a more general solution would be some kind of generic wrapper that
> would hog however many tokens it could get hold of and run a given
> command with a commandline slightly modified to hand over those tokens -
> then wait for that process to exit and give back the tokens. That would
> work for any command that knows about parallelism but doesn't support
> the make jobserver model.

On the off-chance that anybody cares I tried implementing that, because
I've wanted something like that to make "ninja" play nice when invoked
from Make for a long time. Rough sketch at
https://github.com/Villemoes/jobhog .

Rasmus