linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] docs: Use make invocation's -j argument for parallelism
@ 2019-09-19 21:44 Kees Cook
  2019-09-22 20:03 ` Jonathan Corbet
  0 siblings, 1 reply; 7+ messages in thread
From: Kees Cook @ 2019-09-19 21:44 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: Mauro Carvalho Chehab, linux-doc, linux-kernel

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 <keescook@chromium.org>
---
v2: retain "-jauto" default behavior with top-level -j is missing.
---
 Documentation/Makefile  |  3 ++-
 scripts/jobserver-count | 53 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100755 scripts/jobserver-count

diff --git a/Documentation/Makefile b/Documentation/Makefile
index e145e4db508b..8bfd38a865ff 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 python3 $(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..ff6ebe6b0194
--- /dev/null
+++ b/scripts/jobserver-count
@@ -0,0 +1,53 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# 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
+import os, sys, fcntl
+
+# 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 = [nonblock(int(x)) for x in fds.split(",", 1)]
+except:
+	# Any failures here 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:
+		break
+# 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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] docs: Use make invocation's -j argument for parallelism
  2019-09-19 21:44 [PATCH v2] docs: Use make invocation's -j argument for parallelism Kees Cook
@ 2019-09-22 20:03 ` Jonathan Corbet
  2019-09-23 22:40   ` Kees Cook
  0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Corbet @ 2019-09-22 20:03 UTC (permalink / raw)
  To: Kees Cook; +Cc: Mauro Carvalho Chehab, linux-doc, linux-kernel

On Thu, 19 Sep 2019 14:44:37 -0700
Kees Cook <keescook@chromium.org> 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

What sort of confusion might we expect?  Or, to channel akpm, "what are the
user-visible effects of this bug"?

> 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.

So this seems like a good thing to do.  I do have a couple of small issues,
though... 

[...]

> +	-j $(shell python3 $(srctree)/scripts/jobserver-count $(SPHINX_PARALLEL)) \

This (and the shebang line in the script itself) will cause the docs build
to fail on systems lacking Python 3.  While we have talked about requiring
Python 3 for the docs build, we have not actually taken that step yet.  We
probably shouldn't sneak it in here.  I don't see anything in the script
that should require a specific Python version, so I think it should be
tweaked to be version-independent and just invoke "python".

>  	-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..ff6ebe6b0194
> --- /dev/null
> +++ b/scripts/jobserver-count
> @@ -0,0 +1,53 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0-or-later

By license-rules.rst, this should be 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
> +import os, sys, fcntl
> +
> +# 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 = [nonblock(int(x)) for x in fds.split(",", 1)]
> +except:

So I have come to really dislike bare "except" clauses; I've seen them hide
too many bugs.  In this case, perhaps it's justified, but still ... it bugs
me ...

> +	# Any failures here 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:

This one, I think, should be explicit; anything other than EWOULDBLOCK
indicates a real problem, right?

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

You made writer nonblocking, so it seems plausible that we could leak some
slots here, no?  Does writer really need to be nonblocking?

> +# 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)

The last question I have is...why is it that we have to do this complex
dance rather than just passing the "-j" option through directly to sphinx?
That comes down to the "confusion" mentioned at the top, I assume.  It
would be good to understand that?

Thanks,

jon

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] docs: Use make invocation's -j argument for parallelism
  2019-09-22 20:03 ` Jonathan Corbet
@ 2019-09-23 22:40   ` Kees Cook
  2019-09-24  7:12     ` Jani Nikula
  2019-09-24 16:43     ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 7+ messages in thread
From: Kees Cook @ 2019-09-23 22:40 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: Mauro Carvalho Chehab, linux-doc, linux-kernel

On Sun, Sep 22, 2019 at 02:03:31PM -0600, Jonathan Corbet wrote:
> On Thu, 19 Sep 2019 14:44:37 -0700
> Kees Cook <keescook@chromium.org> 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
> 
> What sort of confusion might we expect?  Or, to channel akpm, "what are the
> user-visible effects of this bug"?

When I run "make htmldocs -j16" with a pre-1.7 sphinx, it is not
parallelized. When I run "make htmldocs -j8" with 1.7+ sphinx, it uses
all my CPUs instead of 8. :)

> > +	-j $(shell python3 $(srctree)/scripts/jobserver-count $(SPHINX_PARALLEL)) \
> 
> This (and the shebang line in the script itself) will cause the docs build
> to fail on systems lacking Python 3.  While we have talked about requiring
> Python 3 for the docs build, we have not actually taken that step yet.  We
> probably shouldn't sneak it in here.  I don't see anything in the script
> that should require a specific Python version, so I think it should be
> tweaked to be version-independent and just invoke "python".

Ah, no problem. I can fix this. In a quick scan it looked like sphinx
was python3, but I see now that's just my install. :)

> >  	-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..ff6ebe6b0194
> > --- /dev/null
> > +++ b/scripts/jobserver-count
> > @@ -0,0 +1,53 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> 
> By license-rules.rst, this should be GPL-2.0+

Whoops, thanks.

> > +# 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 = [nonblock(int(x)) for x in fds.split(",", 1)]
> > +except:
> 
> So I have come to really dislike bare "except" clauses; I've seen them hide
> too many bugs.  In this case, perhaps it's justified, but still ... it bugs
> me ...

Fair enough. I will adjust this (and the later instance).

> 
> > +	# Any failures here 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:
> 
> This one, I think, should be explicit; anything other than EWOULDBLOCK
> indicates a real problem, right?
> 
> > +		break
> > +# Return all the reserved slots.
> > +os.write(writer, jobs)
> 
> You made writer nonblocking, so it seems plausible that we could leak some
> slots here, no?  Does writer really need to be nonblocking?

Good point. I will fix this too.

> 
> > +# 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)
> 
> The last question I have is...why is it that we have to do this complex
> dance rather than just passing the "-j" option through directly to sphinx?
> That comes down to the "confusion" mentioned at the top, I assume.  It
> would be good to understand that?

There is no method I have found to discover the -j option's contents
(intentionally so, it seems) from within make. :(

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] docs: Use make invocation's -j argument for parallelism
  2019-09-23 22:40   ` Kees Cook
@ 2019-09-24  7:12     ` Jani Nikula
  2019-09-24 16:11       ` Kees Cook
  2019-09-24 16:43     ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 7+ messages in thread
From: Jani Nikula @ 2019-09-24  7:12 UTC (permalink / raw)
  To: Kees Cook, Jonathan Corbet; +Cc: Mauro Carvalho Chehab, linux-doc, linux-kernel

On Mon, 23 Sep 2019, Kees Cook <keescook@chromium.org> wrote:
> On Sun, Sep 22, 2019 at 02:03:31PM -0600, Jonathan Corbet wrote:
>> On Thu, 19 Sep 2019 14:44:37 -0700
>> Kees Cook <keescook@chromium.org> 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
>> 
>> What sort of confusion might we expect?  Or, to channel akpm, "what are the
>> user-visible effects of this bug"?
>
> When I run "make htmldocs -j16" with a pre-1.7 sphinx, it is not
> parallelized. When I run "make htmldocs -j8" with 1.7+ sphinx, it uses
> all my CPUs instead of 8. :)

To be honest, part of the solution should be to require Sphinx 1.8 or
later. Even Debian stable has it. If your distro doesn't have it
(really?), using the latest Sphinx in a virtual environment should be a
matter of:

$ python3 -m venv .venv
$ . .venv/bin/activate
(.venv) $ pip install sphinx sphinx_rtd_theme
(.venv) $ make htmldocs

BR,
Jani.


>
>> > +	-j $(shell python3 $(srctree)/scripts/jobserver-count $(SPHINX_PARALLEL)) \
>> 
>> This (and the shebang line in the script itself) will cause the docs build
>> to fail on systems lacking Python 3.  While we have talked about requiring
>> Python 3 for the docs build, we have not actually taken that step yet.  We
>> probably shouldn't sneak it in here.  I don't see anything in the script
>> that should require a specific Python version, so I think it should be
>> tweaked to be version-independent and just invoke "python".
>
> Ah, no problem. I can fix this. In a quick scan it looked like sphinx
> was python3, but I see now that's just my install. :)
>
>> >  	-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..ff6ebe6b0194
>> > --- /dev/null
>> > +++ b/scripts/jobserver-count
>> > @@ -0,0 +1,53 @@
>> > +#!/usr/bin/env python3
>> > +# SPDX-License-Identifier: GPL-2.0-or-later
>> 
>> By license-rules.rst, this should be GPL-2.0+
>
> Whoops, thanks.
>
>> > +# 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 = [nonblock(int(x)) for x in fds.split(",", 1)]
>> > +except:
>> 
>> So I have come to really dislike bare "except" clauses; I've seen them hide
>> too many bugs.  In this case, perhaps it's justified, but still ... it bugs
>> me ...
>
> Fair enough. I will adjust this (and the later instance).
>
>> 
>> > +	# Any failures here 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:
>> 
>> This one, I think, should be explicit; anything other than EWOULDBLOCK
>> indicates a real problem, right?
>> 
>> > +		break
>> > +# Return all the reserved slots.
>> > +os.write(writer, jobs)
>> 
>> You made writer nonblocking, so it seems plausible that we could leak some
>> slots here, no?  Does writer really need to be nonblocking?
>
> Good point. I will fix this too.
>
>> 
>> > +# 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)
>> 
>> The last question I have is...why is it that we have to do this complex
>> dance rather than just passing the "-j" option through directly to sphinx?
>> That comes down to the "confusion" mentioned at the top, I assume.  It
>> would be good to understand that?
>
> There is no method I have found to discover the -j option's contents
> (intentionally so, it seems) from within make. :(

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] docs: Use make invocation's -j argument for parallelism
  2019-09-24  7:12     ` Jani Nikula
@ 2019-09-24 16:11       ` Kees Cook
  2019-09-25  8:35         ` Jani Nikula
  0 siblings, 1 reply; 7+ messages in thread
From: Kees Cook @ 2019-09-24 16:11 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Jonathan Corbet, Mauro Carvalho Chehab, linux-doc, linux-kernel

On Tue, Sep 24, 2019 at 10:12:22AM +0300, Jani Nikula wrote:
> On Mon, 23 Sep 2019, Kees Cook <keescook@chromium.org> wrote:
> > On Sun, Sep 22, 2019 at 02:03:31PM -0600, Jonathan Corbet wrote:
> >> On Thu, 19 Sep 2019 14:44:37 -0700
> >> Kees Cook <keescook@chromium.org> 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
> >> 
> >> What sort of confusion might we expect?  Or, to channel akpm, "what are the
> >> user-visible effects of this bug"?
> >
> > When I run "make htmldocs -j16" with a pre-1.7 sphinx, it is not
> > parallelized. When I run "make htmldocs -j8" with 1.7+ sphinx, it uses
> > all my CPUs instead of 8. :)
> 
> To be honest, part of the solution should be to require Sphinx 1.8 or
> later. Even Debian stable has it. If your distro doesn't have it
> (really?), using the latest Sphinx in a virtual environment should be a
> matter of:
> 
> $ python3 -m venv .venv
> $ . .venv/bin/activate
> (.venv) $ pip install sphinx sphinx_rtd_theme
> (.venv) $ make htmldocs

I don't mind having sphinx 1.8 (I did, in fact, already update it), but
that still doesn't solve the whole problem: my -j argument is being
ignored...

-Kees

> 
> BR,
> Jani.
> 
> 
> >
> >> > +	-j $(shell python3 $(srctree)/scripts/jobserver-count $(SPHINX_PARALLEL)) \
> >> 
> >> This (and the shebang line in the script itself) will cause the docs build
> >> to fail on systems lacking Python 3.  While we have talked about requiring
> >> Python 3 for the docs build, we have not actually taken that step yet.  We
> >> probably shouldn't sneak it in here.  I don't see anything in the script
> >> that should require a specific Python version, so I think it should be
> >> tweaked to be version-independent and just invoke "python".
> >
> > Ah, no problem. I can fix this. In a quick scan it looked like sphinx
> > was python3, but I see now that's just my install. :)
> >
> >> >  	-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..ff6ebe6b0194
> >> > --- /dev/null
> >> > +++ b/scripts/jobserver-count
> >> > @@ -0,0 +1,53 @@
> >> > +#!/usr/bin/env python3
> >> > +# SPDX-License-Identifier: GPL-2.0-or-later
> >> 
> >> By license-rules.rst, this should be GPL-2.0+
> >
> > Whoops, thanks.
> >
> >> > +# 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 = [nonblock(int(x)) for x in fds.split(",", 1)]
> >> > +except:
> >> 
> >> So I have come to really dislike bare "except" clauses; I've seen them hide
> >> too many bugs.  In this case, perhaps it's justified, but still ... it bugs
> >> me ...
> >
> > Fair enough. I will adjust this (and the later instance).
> >
> >> 
> >> > +	# Any failures here 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:
> >> 
> >> This one, I think, should be explicit; anything other than EWOULDBLOCK
> >> indicates a real problem, right?
> >> 
> >> > +		break
> >> > +# Return all the reserved slots.
> >> > +os.write(writer, jobs)
> >> 
> >> You made writer nonblocking, so it seems plausible that we could leak some
> >> slots here, no?  Does writer really need to be nonblocking?
> >
> > Good point. I will fix this too.
> >
> >> 
> >> > +# 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)
> >> 
> >> The last question I have is...why is it that we have to do this complex
> >> dance rather than just passing the "-j" option through directly to sphinx?
> >> That comes down to the "confusion" mentioned at the top, I assume.  It
> >> would be good to understand that?
> >
> > There is no method I have found to discover the -j option's contents
> > (intentionally so, it seems) from within make. :(
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] docs: Use make invocation's -j argument for parallelism
  2019-09-23 22:40   ` Kees Cook
  2019-09-24  7:12     ` Jani Nikula
@ 2019-09-24 16:43     ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2019-09-24 16:43 UTC (permalink / raw)
  To: Kees Cook; +Cc: Jonathan Corbet, linux-doc, linux-kernel

Em Mon, 23 Sep 2019 15:40:41 -0700
Kees Cook <keescook@chromium.org> escreveu:

> On Sun, Sep 22, 2019 at 02:03:31PM -0600, Jonathan Corbet wrote:
> > On Thu, 19 Sep 2019 14:44:37 -0700
> > Kees Cook <keescook@chromium.org> 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  
> > 
> > What sort of confusion might we expect?  Or, to channel akpm, "what are the
> > user-visible effects of this bug"?  
> 
> When I run "make htmldocs -j16" with a pre-1.7 sphinx, it is not
> parallelized.

Sphinx supports parallel builds for a while. With pre-1.7, you could do
something like:

	make SPHINXOPTS="-j16" htmldocs

Yet, on my experiences on big machines (tested here with Xeon with 40 and 64
CPU threads), parallel build doesn't actually benefit with values higher than
-j5 to -j8, with pre-1.7.

Sphinx 1.7 and higher seem to have improved a lot with "-jauto"
(although I didn't time it comparing with -j5 or -j8 on a big server).

> When I run "make htmldocs -j8" with 1.7+ sphinx, it uses
> all my CPUs instead of 8. :)

This should do the trick:

	make SPHINXOPTS="-j8" htmldocs

But yeah, IMHO, the best is if it could honor the Makefile flag:

	make -j8 htmldocs

If SPHINXOPTS doesn't contain "-j".

> > > +	-j $(shell python3 $(srctree)/scripts/jobserver-count $(SPHINX_PARALLEL)) \  
> > 
> > This (and the shebang line in the script itself) will cause the docs build
> > to fail on systems lacking Python 3.  While we have talked about requiring
> > Python 3 for the docs build, we have not actually taken that step yet.  We
> > probably shouldn't sneak it in here.  I don't see anything in the script
> > that should require a specific Python version, so I think it should be
> > tweaked to be version-independent and just invoke "python".  
> 
> Ah, no problem. I can fix this. In a quick scan it looked like sphinx
> was python3, but I see now that's just my install. :)

On Fedora 30, both python2 and python3 versions are available:

	python2-sphinx.noarch : Python documentation generator
	python3-sphinx.noarch : Python documentation generator

However, if one tries to install it without specifying "2" or "3", it
defaults to python2 version:

	$ sudo dnf install python-sphinx
	...
	Installing:
	 python2-sphinx                  noarch 1:1.8.4-1.fc30            fedora  1.8 M

AFAIKT, this also applies when distro upgrades takes place: upgrading
a python2 sphinx from Fedora 30 to Rawhide will very likely keep
the python2 version.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] docs: Use make invocation's -j argument for parallelism
  2019-09-24 16:11       ` Kees Cook
@ 2019-09-25  8:35         ` Jani Nikula
  0 siblings, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2019-09-25  8:35 UTC (permalink / raw)
  To: Kees Cook; +Cc: Jonathan Corbet, Mauro Carvalho Chehab, linux-doc, linux-kernel

On Tue, 24 Sep 2019, Kees Cook <keescook@chromium.org> wrote:
> On Tue, Sep 24, 2019 at 10:12:22AM +0300, Jani Nikula wrote:
>> On Mon, 23 Sep 2019, Kees Cook <keescook@chromium.org> wrote:
>> > On Sun, Sep 22, 2019 at 02:03:31PM -0600, Jonathan Corbet wrote:
>> >> On Thu, 19 Sep 2019 14:44:37 -0700
>> >> Kees Cook <keescook@chromium.org> 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
>> >> 
>> >> What sort of confusion might we expect?  Or, to channel akpm, "what are the
>> >> user-visible effects of this bug"?
>> >
>> > When I run "make htmldocs -j16" with a pre-1.7 sphinx, it is not
>> > parallelized. When I run "make htmldocs -j8" with 1.7+ sphinx, it uses
>> > all my CPUs instead of 8. :)
>> 
>> To be honest, part of the solution should be to require Sphinx 1.8 or
>> later. Even Debian stable has it. If your distro doesn't have it
>> (really?), using the latest Sphinx in a virtual environment should be a
>> matter of:
>> 
>> $ python3 -m venv .venv
>> $ . .venv/bin/activate
>> (.venv) $ pip install sphinx sphinx_rtd_theme
>> (.venv) $ make htmldocs
>
> I don't mind having sphinx 1.8 (I did, in fact, already update it), but
> that still doesn't solve the whole problem: my -j argument is being
> ignored...

I meant, *part* of the solution should be to not have to deal with
ancient Sphinx.

BR,
Jani.



-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-09-25  8:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-19 21:44 [PATCH v2] docs: Use make invocation's -j argument for parallelism Kees Cook
2019-09-22 20:03 ` Jonathan Corbet
2019-09-23 22:40   ` Kees Cook
2019-09-24  7:12     ` Jani Nikula
2019-09-24 16:11       ` Kees Cook
2019-09-25  8:35         ` Jani Nikula
2019-09-24 16:43     ` Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).