Re: [Xen-devel] [PATCH] x86emul: fix test harness and fuzzer build dependencies

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.12.18 at 17:05, <JBeulich@xxxxxxxx> wrote:
>>>> On 20.12.18 at 16:23, <ian.jackson@xxxxxxxxxx> wrote:
>> Jan Beulich writes ("Re: [PATCH] x86emul: fix test harness and fuzzer build 
>> dependencies"):
>>> On 20.12.18 at 15:46, <ian.jackson@xxxxxxxxxx> wrote:
>>> > I think this introduces a `make -j' hazard ?  The problem is that this
>>> > branch of the Makefiles tree might enter tools/include while
>>> > another branch is also doing so, resulting in two simultaneous
>>> > executions in the same directory.
>>> 
>>> What is "another branch" here? So far I was under the impression
>>> that the ability of building x86 emulator fuzzer and test harness
>>> independently is an exception, and that all other parts of the
>>> tools/ subtree are supposed to be built by going through the top
>>> level. Otherwise further dependency issues might arise, due to
>>> top level Makefile's %-tools-public-headers rule.
>>> 
>>> Hence whether there's a "make -j" hazard here depends on what
>>> that top level rule's purpose is.
>> 
>> I don't follow.
>> 
>> The top-level %-tools-public-headers rule is there to be something
>> that you can write in the dependencies of other subdirs, to arrange
>> that $(MAKE) -C tools/include is run before that other subdir.
>> 
>> Ie, it is there to satisfy the requirement I mention above, that the
>> dependency directory is built first.
> 
> Which effectively means anything underneath tools/ (and whatever
> else subdir which depend on one of said rules) is liable to fail to
> build without having come through this (top level) rule.
> 
> But this is not a property this patch introduces or changes. It just
> re-arranges how things get done. That is, I'd like to ask for the
> change to be acked (or a concrete proposal be made for what
> needs to change) _without_ fixing breakage that might be there
> and the introduction of which you may have missed, or else I'm
> sure you would have commented on what is now eddf9559c9.
> 
>> I wrote:
>> 
>>    (It is possible to violate this rule without creating races but it
>>   is tricky and inadvisable.)
>> 
>> If we are determined that it must be possible to run make in the x86
>> emulator fuzzer directory *without having previously built the rest of
>> the tree normally*, then perhaps it is necessary to do this
>> $(MAKE) -C thing.
>> 
>> But in that case we need to make sure that either:
>> 
>>  A. 1. The top-level Makefiles ensure that *a* build of
>>        tools/include completes *before* starting to enter
>>        tools/fuzz/x86_instruction_emulator.  (Which I
>>        think is the case.)
> 
> Yes, by said dependency on %-tools-public-headers.
> 
>>     AND
>> 
>>     2. make -C tools/include is definitely completely read-only if the
>>        thing has already been built.  (This is somewhat hard to check
>>        and maintain, and would need a comment in that Makefile to
>>        ensure that this property is preserved.)
> 
> Isn't that a property that's supposed to hold for almost all (sub)trees,
> i.e. it's rather the exception that an already built tree will see further
> changes when re-built incrementally without the sources having
> changed? That said, we have to consider such a case here, due to
> the use of move-if-changed in xen/include/xen/lib/x86/Makefile. But
> this still doesn't violate the fully-read-only requirement, as the rule's
> commands won't be executed again when the dependencies are
> older than the target (as is going to be the case after the invocation
> of the build from the top level Makefile).

So the conclusion was wrong here. If the dependencies changed
(as in are newer than the target) but the target file is the same as
before, move-if-changed will hide the effect from consumers of the
produced file, but the temporary file created gets in the way of -j
here.

There being just one invocation of the build in tools/include/ prior
to the patch here (and hence no race afaict), would you consider
it reasonable to make the two new invocations dependent upon
$(MAKELEVEL), thus protecting things in the recursive case?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel