All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: "Peter Maydell" <peter.maydell@linaro.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Igor Mammedov <imammedo@redhat.com>,
	Shannon Zhao <shannon.zhaosl@gmail.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	qemu devel list <qemu-devel@nongnu.org>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 4/5] tests/uefi-test-tools: add build scripts
Date: Tue, 22 Jan 2019 13:02:35 +0100	[thread overview]
Message-ID: <5cc9c1e9-4fd7-27c4-46bc-2388148945c5@redhat.com> (raw)
In-Reply-To: <CAFEAcA8uVAXMuwVk2ZLAP34Hk_tp9dvWjy06gauyohesOvCuNg@mail.gmail.com>

On 01/21/19 20:30, Peter Maydell wrote:
> On Mon, 21 Jan 2019 at 19:09, Laszlo Ersek <lersek@redhat.com> wrote:
>> It wasn't clear to me whether and how multi-threaded builds were
>> supposed to be used by maintainers, whenever they'd update
>> "tests/data/uefi-boot-images/*".
>>
>> I saw that "make" was invoked everywhere as $(MAKE), but that didn't
>> clarify any intent around "-j". So I didn't test "-j" at all, and in
>> fact I wouldn't have expected it to work:
> 
> The usual assumption with make is that "-jN" should work
> and at least (if the thing being built can't actually
> be parallelized usefully) be no worse than if you'd not
> specified a -j option. We have occasionally had problems
> with -jN in 'make check' (usually because several test cases
> were trying to use the same temp filename or similar) but
> we've treated them as bugs and squashed them.
> 
>> The "build" base tool in edk2 implements part of the job with generated
>> makefiles and invoking "make" itself, thus, despite .NOTPARALLEL, it
>> likely inherits the outermost -j<N> setting -- and it doesn't expect such.
>>
>> So the best I can offer here is to check $MAKEFLAGS in "build.sh", and
>> exit with an early, explicit error if $MAKEFLAGS contains "-j", "-l", or
>> their long variants (--jobs, --load-average).
> 
> Could you sanitize MAKEFLAGS in build.sh instead to remove the
> parallelization options?

I've looked into MAKEFLAGS in a bit more depth now; both the
documentation and some debug prints. Manipulating MAKEFLAGS looks
somewhat brittle.

So, I was about to suggest that I use the .NOTPARALLEL special target
recommended by Phil, for the .efi binaries (which would ensure that no
two instances of the "build" base tool run at the same time), *plus*
that I submit a patch to edk2 so that the makefiles generated by the
"build" tool also contain .NOTPARALLEL.

(Because, to quote the make docs again, "If .NOTPARALLEL is mentioned as
a target, then this invocation of make will be run serially, even if the
‘-j’ option is given. Any recursively invoked make command will still
run recipes in parallel (unless its makefile also contains this target).")

However: when I wanted to see the actual error from using .NOTPARALLEL
*only* in "tests/uefi-test-tools/Makefile", and not in the
build-generated makefiles, I failed to get any error. All the output
images were built just fine.

Phil: when you wrote that "The following patch didn't help" -- referring
to .NOTPARALLEL, added only to "tests/uefi-test-tools/Makefile"-- , did
you clean your tree first (with "make clean" or "git clean -ffdx")?

Because now I'm thinking that the *individual* makefiles generated by
edk2's "build" base tool might actually compatible with "-j", and your
testing of .NOTPARALLEL failed only because your build tree (for example
the Conf/ subdir -- list it with "-A") was in a messy state from your
previous -j4 attempt (where you didn't use .NOTPARALLEL at all).

Thanks
Laszlo

  reply	other threads:[~2019-01-22 19:09 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-18 22:33 [Qemu-devel] [PATCH 0/5] add the BiosTablesTest UEFI app, build it with the new roms/edk2 submodule Laszlo Ersek
2019-01-18 22:33 ` [Qemu-devel] [PATCH 1/5] roms: add the edk2 project as a git submodule Laszlo Ersek
2019-01-21  7:35   ` Gerd Hoffmann
2019-01-21 11:25   ` Philippe Mathieu-Daudé
2019-01-21 18:41     ` Laszlo Ersek
2019-01-21 19:45       ` Philippe Mathieu-Daudé
2019-01-22 10:51         ` Laszlo Ersek
2019-01-18 22:33 ` [Qemu-devel] [PATCH 2/5] roms: build the EfiRom utility from the roms/edk2 submodule Laszlo Ersek
2019-01-21  7:35   ` Gerd Hoffmann
2019-01-21 11:27   ` Philippe Mathieu-Daudé
2019-01-21 11:37     ` Philippe Mathieu-Daudé
2019-01-21 18:34       ` Laszlo Ersek
2019-01-21 18:33     ` Laszlo Ersek
2019-01-18 22:33 ` [Qemu-devel] [PATCH 3/5] tests: introduce "uefi-test-tools" with the BiosTablesTest UEFI app Laszlo Ersek
2019-01-18 22:33 ` [Qemu-devel] [PATCH 4/5] tests/uefi-test-tools: add build scripts Laszlo Ersek
2019-01-21 12:17   ` Philippe Mathieu-Daudé
2019-01-21 19:05     ` Laszlo Ersek
2019-01-21 19:30       ` Peter Maydell
2019-01-22 12:02         ` Laszlo Ersek [this message]
2019-01-23 16:13           ` Laszlo Ersek
2019-01-24 17:05             ` Laszlo Ersek
2019-01-18 22:34 ` [Qemu-devel] [PATCH 5/5] tests/data: introduce "uefi-boot-images" with the "bios-tables-test" ISOs Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5cc9c1e9-4fd7-27c4-46bc-2388148945c5@redhat.com \
    --to=lersek@redhat.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=imammedo@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=mst@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=shannon.zhaosl@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.