diff mbox series

[XEN,v4,16/18] build,xsm: Fix multiple call

Message ID 20200331103102.1105674-17-anthony.perard@citrix.com (mailing list archive)
State Superseded
Headers show
Series xen: Build system improvements | expand

Commit Message

Anthony PERARD March 31, 2020, 10:31 a.m. UTC
Both script mkflask.sh and mkaccess_vector.sh generates multiple
files. Exploits the 'multi-target pattern rule' trick to call each
scripts only once.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    v4:
    - new patch

 xen/xsm/flask/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jan Beulich April 8, 2020, 1:28 p.m. UTC | #1
On 31.03.2020 12:31, Anthony PERARD wrote:
> Both script mkflask.sh and mkaccess_vector.sh generates multiple
> files. Exploits the 'multi-target pattern rule' trick to call each
> scripts only once.

Isn't this a general fix, which may even want backporting? If so,
this would better be at or near the beginning of the series.

> --- a/xen/xsm/flask/Makefile
> +++ b/xen/xsm/flask/Makefile
> @@ -26,14 +26,14 @@ mkflask := policy/mkflask.sh
>  quiet_cmd_mkflask = MKFLASK $@
>  cmd_mkflask = $(CONFIG_SHELL) $(mkflask) $(AWK) include $(FLASK_H_DEPEND)
>  
> -$(FLASK_H_FILES): $(FLASK_H_DEPEND) $(mkflask) FORCE
> +$(patsubst include/%,\%/%,$(FLASK_H_FILES)): $(FLASK_H_DEPEND) $(mkflask) FORCE

Since what $(FLASK_H_FILES) contains is well under our control,
how about the simpler

$(subst include/,%/,$(FLASK_H_FILES)): ...

? Preferably with this and preferably with it moved ahead
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
Anthony PERARD April 16, 2020, 1:02 p.m. UTC | #2
On Wed, Apr 08, 2020 at 03:28:06PM +0200, Jan Beulich wrote:
> On 31.03.2020 12:31, Anthony PERARD wrote:
> > Both script mkflask.sh and mkaccess_vector.sh generates multiple
> > files. Exploits the 'multi-target pattern rule' trick to call each
> > scripts only once.
> 
> Isn't this a general fix, which may even want backporting? If so,
> this would better be at or near the beginning of the series.

It is mostly a performance improvement, avoiding doing the same thing
several time. I don't think anything bad happens from concurrent calls,
or we would already have bug report I think. But I can try to move the
patch up.

> > --- a/xen/xsm/flask/Makefile
> > +++ b/xen/xsm/flask/Makefile
> > @@ -26,14 +26,14 @@ mkflask := policy/mkflask.sh
> >  quiet_cmd_mkflask = MKFLASK $@
> >  cmd_mkflask = $(CONFIG_SHELL) $(mkflask) $(AWK) include $(FLASK_H_DEPEND)
> >  
> > -$(FLASK_H_FILES): $(FLASK_H_DEPEND) $(mkflask) FORCE
> > +$(patsubst include/%,\%/%,$(FLASK_H_FILES)): $(FLASK_H_DEPEND) $(mkflask) FORCE
> 
> Since what $(FLASK_H_FILES) contains is well under our control,
> how about the simpler
> 
> $(subst include/,%/,$(FLASK_H_FILES)): ...
> 
> ? Preferably with this and preferably with it moved ahead
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

I'll do that, thanks,
Jan Beulich April 16, 2020, 2:30 p.m. UTC | #3
On 16.04.2020 15:02, Anthony PERARD wrote:
> On Wed, Apr 08, 2020 at 03:28:06PM +0200, Jan Beulich wrote:
>> On 31.03.2020 12:31, Anthony PERARD wrote:
>>> Both script mkflask.sh and mkaccess_vector.sh generates multiple
>>> files. Exploits the 'multi-target pattern rule' trick to call each
>>> scripts only once.
>>
>> Isn't this a general fix, which may even want backporting? If so,
>> this would better be at or near the beginning of the series.
> 
> It is mostly a performance improvement, avoiding doing the same thing
> several time. I don't think anything bad happens from concurrent calls,
> or we would already have bug report I think. But I can try to move the
> patch up.

Up to three processes in parallel writing to the same file(s) is
almost certainly a recipe for eventual / random breakage.

Jan
diff mbox series

Patch

diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 7d0831e2b865..48577cbe3f04 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -26,14 +26,14 @@  mkflask := policy/mkflask.sh
 quiet_cmd_mkflask = MKFLASK $@
 cmd_mkflask = $(CONFIG_SHELL) $(mkflask) $(AWK) include $(FLASK_H_DEPEND)
 
-$(FLASK_H_FILES): $(FLASK_H_DEPEND) $(mkflask) FORCE
+$(patsubst include/%,\%/%,$(FLASK_H_FILES)): $(FLASK_H_DEPEND) $(mkflask) FORCE
 	$(call if_changed,mkflask)
 
 mkaccess := policy/mkaccess_vector.sh
 quiet_cmd_mkaccess = MKACCESS VECTOR $@
 cmd_mkaccess = $(CONFIG_SHELL) $(mkaccess) $(AWK) $(AV_H_DEPEND)
 
-$(AV_H_FILES): $(AV_H_DEPEND) $(mkaccess) FORCE
+$(patsubst include/%,\%/%,$(AV_H_FILES)): $(AV_H_DEPEND) $(mkaccess) FORCE
 	$(call if_changed,mkaccess)
 
 obj-bin-$(CONFIG_XSM_FLASK_POLICY) += flask-policy.o