* [dpdk-dev] [PATCH] mk: fix parallel build of test resources
@ 2016-06-23 22:36 Thomas Monjalon
2016-06-24 11:22 ` [dpdk-dev] [PATCH v2] " Thomas Monjalon
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Monjalon @ 2016-06-23 22:36 UTC (permalink / raw)
To: Jan Viktorin; +Cc: dev
The build was failing sometimes when building with multiple
parallel jobs:
# rm build/build/app/test/*res*
# make -j6
objcopy: 'resource.tmp': No such file
The reason is that each resource was built from the same temporary file.
The failure is seen because of a race condition when removing the
temporary file after each resource creation.
It also means that some resources may be created from the wrong source.
The fix is to have a different temporary file for each resource.
We do not use the original source file because it has a long path
which is used by objcopy to name the symbols after some transformations.
The trick is to create a symbolic link of the source file if it is not
already in the current build directory. Then we just have to replace
the dot by an underscore to predict the symbol names to redefine.
Fixes: 1e9e0a6270 ("app/test: fix resource creation with objcopy on FreeBSD")
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
app/test/Makefile | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/app/test/Makefile b/app/test/Makefile
index 9fa03fb..46f6005 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -43,14 +43,14 @@ define linked_resource
SRCS-y += $(1).res.o
$(1).res.o: $(2)
@ echo ' MKRES $$@'
- $Q ln -fs $$< resource.tmp
+ $Q [ "$$(<D)" = . ] || ln -fs $$<
$Q $(OBJCOPY) -I binary -B $(RTE_OBJCOPY_ARCH) -O $(RTE_OBJCOPY_TARGET) \
--rename-section \
.data=.rodata,alloc,load,data,contents,readonly \
- --redefine-sym _binary_resource_tmp_start=beg_$(1) \
- --redefine-sym _binary_resource_tmp_end=end_$(1) \
- --redefine-sym _binary_resource_tmp_size=siz_$(1) \
- resource.tmp $$@ && rm -f resource.tmp
+ --redefine-sym _binary_$$(subst .,_,$$(<F))_start=beg_$(1) \
+ --redefine-sym _binary_$$(subst .,_,$$(<F))_end=end_$(1) \
+ --redefine-sym _binary_$$(subst .,_,$$(<F))_size=siz_$(1) \
+ $$(<F) $$@
endef
ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
--
2.7.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [dpdk-dev] [PATCH v2] mk: fix parallel build of test resources
2016-06-23 22:36 [dpdk-dev] [PATCH] mk: fix parallel build of test resources Thomas Monjalon
@ 2016-06-24 11:22 ` Thomas Monjalon
2016-06-24 14:06 ` Olivier Matz
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Monjalon @ 2016-06-24 11:22 UTC (permalink / raw)
To: dev
The build was failing sometimes when building with multiple
parallel jobs:
# rm build/build/app/test/*res*
# make -j6
objcopy: 'resource.tmp': No such file
The reason is that each resource was built from the same temporary file.
The failure is seen because of a race condition when removing the
temporary file after each resource creation.
It also means that some resources may be created from the wrong source.
The fix is to have a different input file for each resource.
The source file is not directly used because it may have a long path
which is used by objcopy to name the symbols after some transformations.
When linking a tar resource, the input file is already in the current
directory. The hard case is for simply linked resources.
The trick is to create a symbolic link of the source file if it is not
already in the current build directory.
Then there is a replacement of dot by an underscore to predict the
symbol names computed by objcopy which must be redefined.
There is an additional change for the test_resource_c which is both
a real source file and a test resource. An intermediate file
test_resource.res is created to avoid compiling resource.c from the
wrong directory through a symbolic link.
Fixes: 1e9e0a6270 ("app/test: fix resource creation with objcopy on FreeBSD")
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
v2: fix rebuild error due to link test_resource.c in build directory
---
app/test/Makefile | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/app/test/Makefile b/app/test/Makefile
index 9fa03fb..1f24dd6 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -43,14 +43,14 @@ define linked_resource
SRCS-y += $(1).res.o
$(1).res.o: $(2)
@ echo ' MKRES $$@'
- $Q ln -fs $$< resource.tmp
+ $Q [ "$$(<D)" = . ] || ln -fs $$<
$Q $(OBJCOPY) -I binary -B $(RTE_OBJCOPY_ARCH) -O $(RTE_OBJCOPY_TARGET) \
--rename-section \
.data=.rodata,alloc,load,data,contents,readonly \
- --redefine-sym _binary_resource_tmp_start=beg_$(1) \
- --redefine-sym _binary_resource_tmp_end=end_$(1) \
- --redefine-sym _binary_resource_tmp_size=siz_$(1) \
- resource.tmp $$@ && rm -f resource.tmp
+ --redefine-sym _binary_$$(subst .,_,$$(<F))_start=beg_$(1) \
+ --redefine-sym _binary_$$(subst .,_,$$(<F))_end=end_$(1) \
+ --redefine-sym _binary_$$(subst .,_,$$(<F))_size=siz_$(1) \
+ $$(<F) $$@
endef
ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
@@ -76,7 +76,9 @@ SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) := commands.c
SRCS-y += test.c
SRCS-y += resource.c
SRCS-y += test_resource.c
-$(eval $(call linked_resource,test_resource_c,resource.c))
+test_resource.res: test_resource.c
+ @ cp $< $@
+$(eval $(call linked_resource,test_resource_c,test_resource.res))
$(eval $(call linked_tar_resource,test_resource_tar,test_resource.c))
SRCS-$(CONFIG_RTE_APP_TEST_RESOURCE_TAR) += test_pci.c
$(eval $(call linked_tar_resource,test_pci_sysfs,test_pci_sysfs))
--
2.7.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH v2] mk: fix parallel build of test resources
2016-06-24 11:22 ` [dpdk-dev] [PATCH v2] " Thomas Monjalon
@ 2016-06-24 14:06 ` Olivier Matz
2016-06-24 14:19 ` Thomas Monjalon
0 siblings, 1 reply; 6+ messages in thread
From: Olivier Matz @ 2016-06-24 14:06 UTC (permalink / raw)
To: Thomas Monjalon, dev
Hi Thomas,
On 06/24/2016 01:22 PM, Thomas Monjalon wrote:
> --- a/app/test/Makefile
> +++ b/app/test/Makefile
> @@ -43,14 +43,14 @@ define linked_resource
> SRCS-y += $(1).res.o
> $(1).res.o: $(2)
> @ echo ' MKRES $$@'
> - $Q ln -fs $$< resource.tmp
> + $Q [ "$$(<D)" = . ] || ln -fs $$<
Maybe the symbolic link file in the build directory could be prefixed
with "lnk_"... (see following below)
> $Q $(OBJCOPY) -I binary -B $(RTE_OBJCOPY_ARCH) -O $(RTE_OBJCOPY_TARGET) \
> --rename-section \
> .data=.rodata,alloc,load,data,contents,readonly \
> - --redefine-sym _binary_resource_tmp_start=beg_$(1) \
> - --redefine-sym _binary_resource_tmp_end=end_$(1) \
> - --redefine-sym _binary_resource_tmp_size=siz_$(1) \
> - resource.tmp $$@ && rm -f resource.tmp
> + --redefine-sym _binary_$$(subst .,_,$$(<F))_start=beg_$(1) \
> + --redefine-sym _binary_$$(subst .,_,$$(<F))_end=end_$(1) \
> + --redefine-sym _binary_$$(subst .,_,$$(<F))_size=siz_$(1) \
> + $$(<F) $$@
> endef
>
> ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
> @@ -76,7 +76,9 @@ SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) := commands.c
> SRCS-y += test.c
> SRCS-y += resource.c
> SRCS-y += test_resource.c
> -$(eval $(call linked_resource,test_resource_c,resource.c))
> +test_resource.res: test_resource.c
> + @ cp $< $@
> +$(eval $(call linked_resource,test_resource_c,test_resource.res))
> $(eval $(call linked_tar_resource,test_resource_tar,test_resource.c))
> SRCS-$(CONFIG_RTE_APP_TEST_RESOURCE_TAR) += test_pci.c
> $(eval $(call linked_tar_resource,test_pci_sysfs,test_pci_sysfs))
...this would avoid to rename the resource file and make the patch
easier to understand.
Olivier
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mk: fix parallel build of test resources
2016-06-24 14:06 ` Olivier Matz
@ 2016-06-24 14:19 ` Thomas Monjalon
2016-06-24 14:24 ` Olivier Matz
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Monjalon @ 2016-06-24 14:19 UTC (permalink / raw)
To: Olivier Matz; +Cc: dev
2016-06-24 16:06, Olivier Matz:
> Hi Thomas,
>
> On 06/24/2016 01:22 PM, Thomas Monjalon wrote:
> > --- a/app/test/Makefile
> > +++ b/app/test/Makefile
> > @@ -43,14 +43,14 @@ define linked_resource
> > SRCS-y += $(1).res.o
> > $(1).res.o: $(2)
> > @ echo ' MKRES $$@'
> > - $Q ln -fs $$< resource.tmp
> > + $Q [ "$$(<D)" = . ] || ln -fs $$<
>
> Maybe the symbolic link file in the build directory could be prefixed
> with "lnk_"... (see following below)
yes...
> > $Q $(OBJCOPY) -I binary -B $(RTE_OBJCOPY_ARCH) -O $(RTE_OBJCOPY_TARGET) \
> > --rename-section \
> > .data=.rodata,alloc,load,data,contents,readonly \
> > - --redefine-sym _binary_resource_tmp_start=beg_$(1) \
> > - --redefine-sym _binary_resource_tmp_end=end_$(1) \
> > - --redefine-sym _binary_resource_tmp_size=siz_$(1) \
> > - resource.tmp $$@ && rm -f resource.tmp
> > + --redefine-sym _binary_$$(subst .,_,$$(<F))_start=beg_$(1) \
> > + --redefine-sym _binary_$$(subst .,_,$$(<F))_end=end_$(1) \
> > + --redefine-sym _binary_$$(subst .,_,$$(<F))_size=siz_$(1) \
> > + $$(<F) $$@
> > endef
> >
> > ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
> > @@ -76,7 +76,9 @@ SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) := commands.c
> > SRCS-y += test.c
> > SRCS-y += resource.c
> > SRCS-y += test_resource.c
> > -$(eval $(call linked_resource,test_resource_c,resource.c))
> > +test_resource.res: test_resource.c
> > + @ cp $< $@
> > +$(eval $(call linked_resource,test_resource_c,test_resource.res))
> > $(eval $(call linked_tar_resource,test_resource_tar,test_resource.c))
> > SRCS-$(CONFIG_RTE_APP_TEST_RESOURCE_TAR) += test_pci.c
> > $(eval $(call linked_tar_resource,test_pci_sysfs,test_pci_sysfs))
>
> ...this would avoid to rename the resource file and make the patch
> easier to understand.
... but it would be harder to understand how are named the resources in
the build directory ;)
Ideally we should not use a source file (referenced in SRCS-y) as a
test resource.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mk: fix parallel build of test resources
2016-06-24 14:19 ` Thomas Monjalon
@ 2016-06-24 14:24 ` Olivier Matz
2016-06-24 14:59 ` Thomas Monjalon
0 siblings, 1 reply; 6+ messages in thread
From: Olivier Matz @ 2016-06-24 14:24 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
On 06/24/2016 04:19 PM, Thomas Monjalon wrote:
> 2016-06-24 16:06, Olivier Matz:
>> Hi Thomas,
>>
>> On 06/24/2016 01:22 PM, Thomas Monjalon wrote:
>>> --- a/app/test/Makefile
>>> +++ b/app/test/Makefile
>>> @@ -43,14 +43,14 @@ define linked_resource
>>> SRCS-y += $(1).res.o
>>> $(1).res.o: $(2)
>>> @ echo ' MKRES $$@'
>>> - $Q ln -fs $$< resource.tmp
>>> + $Q [ "$$(<D)" = . ] || ln -fs $$<
>>
>> Maybe the symbolic link file in the build directory could be prefixed
>> with "lnk_"... (see following below)
>
> yes...
>
>>> $Q $(OBJCOPY) -I binary -B $(RTE_OBJCOPY_ARCH) -O $(RTE_OBJCOPY_TARGET) \
>>> --rename-section \
>>> .data=.rodata,alloc,load,data,contents,readonly \
>>> - --redefine-sym _binary_resource_tmp_start=beg_$(1) \
>>> - --redefine-sym _binary_resource_tmp_end=end_$(1) \
>>> - --redefine-sym _binary_resource_tmp_size=siz_$(1) \
>>> - resource.tmp $$@ && rm -f resource.tmp
>>> + --redefine-sym _binary_$$(subst .,_,$$(<F))_start=beg_$(1) \
>>> + --redefine-sym _binary_$$(subst .,_,$$(<F))_end=end_$(1) \
>>> + --redefine-sym _binary_$$(subst .,_,$$(<F))_size=siz_$(1) \
>>> + $$(<F) $$@
>>> endef
>>>
>>> ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
>>> @@ -76,7 +76,9 @@ SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) := commands.c
>>> SRCS-y += test.c
>>> SRCS-y += resource.c
>>> SRCS-y += test_resource.c
>>> -$(eval $(call linked_resource,test_resource_c,resource.c))
>>> +test_resource.res: test_resource.c
>>> + @ cp $< $@
>>> +$(eval $(call linked_resource,test_resource_c,test_resource.res))
>>> $(eval $(call linked_tar_resource,test_resource_tar,test_resource.c))
>>> SRCS-$(CONFIG_RTE_APP_TEST_RESOURCE_TAR) += test_pci.c
>>> $(eval $(call linked_tar_resource,test_pci_sysfs,test_pci_sysfs))
>>
>> ...this would avoid to rename the resource file and make the patch
>> easier to understand.
>
> ... but it would be harder to understand how are named the resources in
> the build directory ;)
> Ideally we should not use a source file (referenced in SRCS-y) as a
> test resource.
>
OK alright :)
Acked-by: Olivier Matz <olivier.matz@6wind.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-24 14:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-23 22:36 [dpdk-dev] [PATCH] mk: fix parallel build of test resources Thomas Monjalon
2016-06-24 11:22 ` [dpdk-dev] [PATCH v2] " Thomas Monjalon
2016-06-24 14:06 ` Olivier Matz
2016-06-24 14:19 ` Thomas Monjalon
2016-06-24 14:24 ` Olivier Matz
2016-06-24 14:59 ` Thomas Monjalon
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).