[PATCH 0/2] x86,vdso,kbuild: Fix vdso_install

[PATCH 0/2] x86,vdso,kbuild: Fix vdso_install

[Andy Lutomirski]

This changes kbuild slightly because I didn't want to fight with using
static pattern rules and extra obfuscated fake targets just to write a
loop.

Better ideas / cleanups welcome.

Andy Lutomirski (2):
  kbuild: Add recipe-cmd, an @-less cmd variant
  x86,vdso: Fix vdso_install

 arch/x86/vdso/Makefile | 20 +++++++-------------
 scripts/Kbuild.include |  1 +
 2 files changed, 8 insertions(+), 13 deletions(-)

-- 
1.9.3

[PATCH 1/2] kbuild: Add recipe-cmd, an @-less cmd variant

[Andy Lutomirski]

It's awkward to put $(call cmd,xyz) in a loop in a recipe because
the pesky at sign gets in the way.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 scripts/Kbuild.include | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 122f95c..3e60923 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -194,6 +194,7 @@ echo-cmd = $(if $($(quiet)cmd_$(1)),\
 
 # printing commands
 cmd = @$(echo-cmd) $(cmd_$(1))
+recipe-cmd = $(echo-cmd) $(cmd_$(1))
 
 # Add $(obj)/ for paths that are not absolute
 objectify = $(foreach o,$(1),$(if $(filter /%,$(o)),$(o),$(obj)/$(o)))
-- 
1.9.3

[PATCH 2/2] x86,vdso: Fix vdso_install

[Andy Lutomirski]

Rather than monkeying with barely-comprehensible static pattern
rules, just use an explicit loop.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/vdso/Makefile | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index 9769df0..b1c70cc 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -9,11 +9,6 @@ VDSOX32-$(CONFIG_X86_X32_ABI)	:= y
 VDSO32-$(CONFIG_X86_32)		:= y
 VDSO32-$(CONFIG_COMPAT)		:= y
 
-vdso-install-$(VDSO64-y)	+= vdso.so
-vdso-install-$(VDSOX32-y)	+= vdsox32.so
-vdso-install-$(VDSO32-y)	+= $(vdso32-images)
-
-
 # files to link into the vdso
 vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
 
@@ -176,15 +171,14 @@ VDSO_LDFLAGS = -fPIC -shared $(call cc-ldoption, -Wl$(comma)--hash-style=sysv) \
 GCOV_PROFILE := n
 
 #
-# Install the unstripped copy of vdso*.so listed in $(vdso-install-y).
+# Install the unstripped copies of vdso*.so listed in $(vdso-install-y).
 #
-quiet_cmd_vdso_install = INSTALL $@
-      cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
-$(vdso-install-y): %.so: $(obj)/%.so.dbg FORCE
-	@mkdir -p $(MODLIB)/vdso
-	$(call cmd,vdso_install)
+quiet_cmd_vdso_install = INSTALL $(sofile)
+      cmd_vdso_install = cp $(obj)/$(sofile).dbg $(MODLIB)/vdso/$(sofile)
 
-PHONY += vdso_install $(vdso-install-y)
-vdso_install: $(vdso-install-y)
+PHONY += vdso_install
+vdso_install: $(vdso_img_sodbg:%=$(obj)/%) FORCE
+	@mkdir -p $(MODLIB)/vdso
+	@$(foreach sofile,$(vdso_img_sodbg:%.dbg=%),$(call recipe-cmd,vdso_install);)
 
 clean-files := vdso32-syscall* vdso32-sysenter* vdso32-int80*
-- 
1.9.3

Re: [PATCH 0/2] x86,vdso,kbuild: Fix vdso_install

[Josh Boyer]

On Wed, Jun 11, 2014 at 12:20 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> This changes kbuild slightly because I didn't want to fight with using
> static pattern rules and extra obfuscated fake targets just to write a
> loop.
>
> Better ideas / cleanups welcome.
>
> Andy Lutomirski (2):
>   kbuild: Add recipe-cmd, an @-less cmd variant
>   x86,vdso: Fix vdso_install

You won the race :).  I'll give these a spin shortly.

josh

Re: [PATCH 0/2] x86,vdso,kbuild: Fix vdso_install

[Sam Ravnborg]

On Wed, Jun 11, 2014 at 09:20:05AM -0700, Andy Lutomirski wrote:
> This changes kbuild slightly because I didn't want to fight with using
> static pattern rules and extra obfuscated fake targets just to write a
> loop.

What are you trying to fix with this?

On my box (32 bit Intel) "make MODLIB=~/mod vdso_install"
works as expected.
Two *.so files are installed as a copy of the so.dbg files.

	Sam

Re: [PATCH 0/2] x86,vdso,kbuild: Fix vdso_install

[Josh Boyer]

On Wed, Jun 11, 2014 at 12:36 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> On Wed, Jun 11, 2014 at 09:20:05AM -0700, Andy Lutomirski wrote:
>> This changes kbuild slightly because I didn't want to fight with using
>> static pattern rules and extra obfuscated fake targets just to write a
>> loop.
>
> What are you trying to fix with this?
>
> On my box (32 bit Intel) "make MODLIB=~/mod vdso_install"
> works as expected.
> Two *.so files are installed as a copy of the so.dbg files.

With what tree?  With Linus' latest, I reported an error because
commit 6f121e548f83674ab4920a4e60afb58d4f61b829 breaks the
vdso_install target.

josh

Re: [PATCH 2/2] x86,vdso: Fix vdso_install

[Josh Boyer]

On Wed, Jun 11, 2014 at 12:20 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> Rather than monkeying with barely-comprehensible static pattern
> rules, just use an explicit loop.
>
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> ---
>  arch/x86/vdso/Makefile | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
> index 9769df0..b1c70cc 100644
> --- a/arch/x86/vdso/Makefile
> +++ b/arch/x86/vdso/Makefile
> @@ -9,11 +9,6 @@ VDSOX32-$(CONFIG_X86_X32_ABI)  := y
>  VDSO32-$(CONFIG_X86_32)                := y
>  VDSO32-$(CONFIG_COMPAT)                := y
>
> -vdso-install-$(VDSO64-y)       += vdso.so
> -vdso-install-$(VDSOX32-y)      += vdsox32.so
> -vdso-install-$(VDSO32-y)       += $(vdso32-images)
> -
> -
>  # files to link into the vdso
>  vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
>
> @@ -176,15 +171,14 @@ VDSO_LDFLAGS = -fPIC -shared $(call cc-ldoption, -Wl$(comma)--hash-style=sysv) \
>  GCOV_PROFILE := n
>
>  #
> -# Install the unstripped copy of vdso*.so listed in $(vdso-install-y).
> +# Install the unstripped copies of vdso*.so listed in $(vdso-install-y).
>  #
> -quiet_cmd_vdso_install = INSTALL $@
> -      cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
> -$(vdso-install-y): %.so: $(obj)/%.so.dbg FORCE
> -       @mkdir -p $(MODLIB)/vdso
> -       $(call cmd,vdso_install)
> +quiet_cmd_vdso_install = INSTALL $(sofile)
> +      cmd_vdso_install = cp $(obj)/$(sofile).dbg $(MODLIB)/vdso/$(sofile)
>
> -PHONY += vdso_install $(vdso-install-y)
> -vdso_install: $(vdso-install-y)
> +PHONY += vdso_install
> +vdso_install: $(vdso_img_sodbg:%=$(obj)/%) FORCE
> +       @mkdir -p $(MODLIB)/vdso
> +       @$(foreach sofile,$(vdso_img_sodbg:%.dbg=%),$(call recipe-cmd,vdso_install);)

So this does fix the invocation of 'make vdso_install' and the
resulting files look to be accurate to me, with the glaring exception
that now we get e.g. vdso64.so on x86_64 as the installed file instead
of vdso.so.  How much that actually matters, I have no idea.
Plausibly fixed with a symlink if we really need to perhaps.

josh

Re: [PATCH 2/2] x86,vdso: Fix vdso_install

[H. Peter Anvin]

On 06/11/2014 10:23 AM, Josh Boyer wrote:
> 
> So this does fix the invocation of 'make vdso_install' and the
> resulting files look to be accurate to me, with the glaring exception
> that now we get e.g. vdso64.so on x86_64 as the installed file instead
> of vdso.so.  How much that actually matters, I have no idea.
> Plausibly fixed with a symlink if we really need to perhaps.
> 

You have that problem anyway, no?  After all, there are three different
vdso images for 32 bits, and you can run 32-bit apps on 64-bit systems, too.

Is there realistically any way for the debugger to pick up the correct one?

	-hpa

Re: [PATCH 2/2] x86,vdso: Fix vdso_install

[Josh Boyer]

On Wed, Jun 11, 2014 at 1:27 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 06/11/2014 10:23 AM, Josh Boyer wrote:
>>
>> So this does fix the invocation of 'make vdso_install' and the
>> resulting files look to be accurate to me, with the glaring exception
>> that now we get e.g. vdso64.so on x86_64 as the installed file instead
>> of vdso.so.  How much that actually matters, I have no idea.
>> Plausibly fixed with a symlink if we really need to perhaps.
>>
>
> You have that problem anyway, no?  After all, there are three different
> vdso images for 32 bits, and you can run 32-bit apps on 64-bit systems, too.

Yeah, true.

> Is there realistically any way for the debugger to pick up the correct one?

Probably not.  I'm planning on pushing out our first 3.16 build with
these two patches with no symlink.  I very much doubt anyone is going
to complain.  It was just something I noticed.

josh

Re: [PATCH 2/2] x86,vdso: Fix vdso_install

[Andy Lutomirski]

On Wed, Jun 11, 2014 at 10:33 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> On Wed, Jun 11, 2014 at 1:27 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 06/11/2014 10:23 AM, Josh Boyer wrote:
>>>
>>> So this does fix the invocation of 'make vdso_install' and the
>>> resulting files look to be accurate to me, with the glaring exception
>>> that now we get e.g. vdso64.so on x86_64 as the installed file instead
>>> of vdso.so.  How much that actually matters, I have no idea.
>>> Plausibly fixed with a symlink if we really need to perhaps.
>>>
>>
>> You have that problem anyway, no?  After all, there are three different
>> vdso images for 32 bits, and you can run 32-bit apps on 64-bit systems, too.
>
> Yeah, true.
>
>> Is there realistically any way for the debugger to pick up the correct one?
>
> Probably not.

Sure there is: build ids.  See /usr/lib/debug/.build-id.

It would be great if we could teach the various debugging tools
(libdw?  gdb?  I don't know what's responsible for the search path) to
search both /usr/lib/debug/.build-id and /lib/modules/`uname
-r`/build-id or something like that.

>
> I'm planning on pushing out our first 3.16 build with
> these two patches with no symlink.  I very much doubt anyone is going
> to complain.  It was just something I noticed.

Does the Fedora RPM magic debuginfo script notice these files and
symlink them into the .build-id directory?

Of course, we don't seem to be generating build ids right now.  I
thought we were.  I'll see if I can fix it.

--Andy

>
> josh



-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH 2/2] x86,vdso: Fix vdso_install

[Josh Boyer]

On Wed, Jun 11, 2014 at 1:42 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, Jun 11, 2014 at 10:33 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
>> On Wed, Jun 11, 2014 at 1:27 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>> On 06/11/2014 10:23 AM, Josh Boyer wrote:
>>>>
>>>> So this does fix the invocation of 'make vdso_install' and the
>>>> resulting files look to be accurate to me, with the glaring exception
>>>> that now we get e.g. vdso64.so on x86_64 as the installed file instead
>>>> of vdso.so.  How much that actually matters, I have no idea.
>>>> Plausibly fixed with a symlink if we really need to perhaps.
>>>>
>>>
>>> You have that problem anyway, no?  After all, there are three different
>>> vdso images for 32 bits, and you can run 32-bit apps on 64-bit systems, too.
>>
>> Yeah, true.
>>
>>> Is there realistically any way for the debugger to pick up the correct one?
>>
>> Probably not.
>
> Sure there is: build ids.  See /usr/lib/debug/.build-id.

Oh, duh.

> It would be great if we could teach the various debugging tools
> (libdw?  gdb?  I don't know what's responsible for the search path) to
> search both /usr/lib/debug/.build-id and /lib/modules/`uname
> -r`/build-id or something like that.
>
>>
>> I'm planning on pushing out our first 3.16 build with
>> these two patches with no symlink.  I very much doubt anyone is going
>> to complain.  It was just something I noticed.
>
> Does the Fedora RPM magic debuginfo script notice these files and
> symlink them into the .build-id directory?
>
> Of course, we don't seem to be generating build ids right now.  I
> thought we were.  I'll see if I can fix it.

My builds have it:

[jwboyer@sb ~]$ file /lib/modules/3.16.0-0.rc0.git1.1.fc21.x86_64/vdso/vdso64.so
/lib/modules/3.16.0-0.rc0.git1.1.fc21.x86_64/vdso/vdso64.so: ELF
64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically
linked, BuildID[sha1]=0x85ab0014891b5be97871e5e1183b761b9d2e8d9d,
stripped
[jwboyer@sb ~]$

I forget if we're doing something special to get that or not.

josh

Re: [PATCH 0/2] x86,vdso,kbuild: Fix vdso_install

[Sam Ravnborg]

On Wed, Jun 11, 2014 at 12:41:56PM -0400, Josh Boyer wrote:
> On Wed, Jun 11, 2014 at 12:36 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> > On Wed, Jun 11, 2014 at 09:20:05AM -0700, Andy Lutomirski wrote:
> >> This changes kbuild slightly because I didn't want to fight with using
> >> static pattern rules and extra obfuscated fake targets just to write a
> >> loop.
> >
> > What are you trying to fix with this?
> >
> > On my box (32 bit Intel) "make MODLIB=~/mod vdso_install"
> > works as expected.
> > Two *.so files are installed as a copy of the so.dbg files.
> 
> With what tree?
Linus latest - yes.

>  With Linus' latest, I reported an error because
> commit 6f121e548f83674ab4920a4e60afb58d4f61b829 breaks the
> vdso_install target.
This is your bug-report: http://marc.info/?l=linux-kernel&m=140249607823283&w=2

+ make -s ARCH=x86_64
INSTALL_MOD_PATH=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.16.0-0.rc0.git1.1.fc21.x86_64
vdso_install KERNELRELEASE=3.16.0-0.rc0.git1.1.fc21.x86_64
make[1]: *** No rule to make target `arch/x86/vdso/vdso.so.dbg',
needed by `vdso.so'.  Stop.
make: *** [vdso_install] Error 2

I am building for i386 which is why I do not see the bug.

	Sam

Re: [PATCH 2/2] x86,vdso: Fix vdso_install

[Andy Lutomirski]

On Wed, Jun 11, 2014 at 10:45 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> On Wed, Jun 11, 2014 at 1:42 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Wed, Jun 11, 2014 at 10:33 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
>>> On Wed, Jun 11, 2014 at 1:27 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>>> On 06/11/2014 10:23 AM, Josh Boyer wrote:
>>>>>
>>>>> So this does fix the invocation of 'make vdso_install' and the
>>>>> resulting files look to be accurate to me, with the glaring exception
>>>>> that now we get e.g. vdso64.so on x86_64 as the installed file instead
>>>>> of vdso.so.  How much that actually matters, I have no idea.
>>>>> Plausibly fixed with a symlink if we really need to perhaps.
>>>>>
>>>>
>>>> You have that problem anyway, no?  After all, there are three different
>>>> vdso images for 32 bits, and you can run 32-bit apps on 64-bit systems, too.
>>>
>>> Yeah, true.
>>>
>>>> Is there realistically any way for the debugger to pick up the correct one?
>>>
>>> Probably not.
>>
>> Sure there is: build ids.  See /usr/lib/debug/.build-id.
>
> Oh, duh.
>
>> It would be great if we could teach the various debugging tools
>> (libdw?  gdb?  I don't know what's responsible for the search path) to
>> search both /usr/lib/debug/.build-id and /lib/modules/`uname
>> -r`/build-id or something like that.
>>
>>>
>>> I'm planning on pushing out our first 3.16 build with
>>> these two patches with no symlink.  I very much doubt anyone is going
>>> to complain.  It was just something I noticed.
>>
>> Does the Fedora RPM magic debuginfo script notice these files and
>> symlink them into the .build-id directory?
>>
>> Of course, we don't seem to be generating build ids right now.  I
>> thought we were.  I'll see if I can fix it.
>
> My builds have it:
>
> [jwboyer@sb ~]$ file /lib/modules/3.16.0-0.rc0.git1.1.fc21.x86_64/vdso/vdso64.so
> /lib/modules/3.16.0-0.rc0.git1.1.fc21.x86_64/vdso/vdso64.so: ELF
> 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically
> linked, BuildID[sha1]=0x85ab0014891b5be97871e5e1183b761b9d2e8d9d,
> stripped
> [jwboyer@sb ~]$
>
> I forget if we're doing something special to get that or not.

Apparently not.  file can see it, but apparently objdump can't.  go figure.

>
> josh



-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH 2/2] x86,vdso: Fix vdso_install

[Sam Ravnborg]

On Wed, Jun 11, 2014 at 01:23:59PM -0400, Josh Boyer wrote:
> On Wed, Jun 11, 2014 at 12:20 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> > Rather than monkeying with barely-comprehensible static pattern
> > rules, just use an explicit loop.
> >
> > Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> > ---
> >  arch/x86/vdso/Makefile | 20 +++++++-------------
> >  1 file changed, 7 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
> > index 9769df0..b1c70cc 100644
> > --- a/arch/x86/vdso/Makefile
> > +++ b/arch/x86/vdso/Makefile
> > @@ -9,11 +9,6 @@ VDSOX32-$(CONFIG_X86_X32_ABI)  := y
> >  VDSO32-$(CONFIG_X86_32)                := y
> >  VDSO32-$(CONFIG_COMPAT)                := y
> >
> > -vdso-install-$(VDSO64-y)       += vdso.so
> > -vdso-install-$(VDSOX32-y)      += vdsox32.so
> > -vdso-install-$(VDSO32-y)       += $(vdso32-images)
> > -
> > -
> >  # files to link into the vdso
> >  vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
> >
> > @@ -176,15 +171,14 @@ VDSO_LDFLAGS = -fPIC -shared $(call cc-ldoption, -Wl$(comma)--hash-style=sysv) \
> >  GCOV_PROFILE := n
> >
> >  #
> > -# Install the unstripped copy of vdso*.so listed in $(vdso-install-y).
> > +# Install the unstripped copies of vdso*.so listed in $(vdso-install-y).
> >  #
> > -quiet_cmd_vdso_install = INSTALL $@
> > -      cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
> > -$(vdso-install-y): %.so: $(obj)/%.so.dbg FORCE
> > -       @mkdir -p $(MODLIB)/vdso
> > -       $(call cmd,vdso_install)
> > +quiet_cmd_vdso_install = INSTALL $(sofile)
> > +      cmd_vdso_install = cp $(obj)/$(sofile).dbg $(MODLIB)/vdso/$(sofile)
> >
> > -PHONY += vdso_install $(vdso-install-y)
> > -vdso_install: $(vdso-install-y)
> > +PHONY += vdso_install
> > +vdso_install: $(vdso_img_sodbg:%=$(obj)/%) FORCE
> > +       @mkdir -p $(MODLIB)/vdso
> > +       @$(foreach sofile,$(vdso_img_sodbg:%.dbg=%),$(call recipe-cmd,vdso_install);)

Can we please fix this in a way where we do not need to add stuff to core kbuild.
If the original approach was used then make took care of the looping
and the foreach part was not needed.


	Sam

Re: [PATCH 0/2] x86,vdso,kbuild: Fix vdso_install

[Andy Lutomirski]

On Wed, Jun 11, 2014 at 11:44 AM, Sam Ravnborg <sam@ravnborg.org> wrote:
> On Wed, Jun 11, 2014 at 12:41:56PM -0400, Josh Boyer wrote:
>> On Wed, Jun 11, 2014 at 12:36 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
>> > On Wed, Jun 11, 2014 at 09:20:05AM -0700, Andy Lutomirski wrote:
>> >> This changes kbuild slightly because I didn't want to fight with using
>> >> static pattern rules and extra obfuscated fake targets just to write a
>> >> loop.
>> >
>> > What are you trying to fix with this?
>> >
>> > On my box (32 bit Intel) "make MODLIB=~/mod vdso_install"
>> > works as expected.
>> > Two *.so files are installed as a copy of the so.dbg files.
>>
>> With what tree?
> Linus latest - yes.
>
>>  With Linus' latest, I reported an error because
>> commit 6f121e548f83674ab4920a4e60afb58d4f61b829 breaks the
>> vdso_install target.
> This is your bug-report: http://marc.info/?l=linux-kernel&m=140249607823283&w=2
>
> + make -s ARCH=x86_64
> INSTALL_MOD_PATH=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.16.0-0.rc0.git1.1.fc21.x86_64
> vdso_install KERNELRELEASE=3.16.0-0.rc0.git1.1.fc21.x86_64
> make[1]: *** No rule to make target `arch/x86/vdso/vdso.so.dbg',
> needed by `vdso.so'.  Stop.
> make: *** [vdso_install] Error 2
>
> I am building for i386 which is why I do not see the bug.

I didn't actually check whether all of the code I removed in the patch
was wrong or just some.

If you have a better suggestion for how to structure the makefile
rule, I'd be interested.  I don't speak makefilese very well.

--Andy

Re: [PATCH 2/2] x86,vdso: Fix vdso_install

[Andy Lutomirski]

On Wed, Jun 11, 2014 at 11:51 AM, Sam Ravnborg <sam@ravnborg.org> wrote:
> On Wed, Jun 11, 2014 at 01:23:59PM -0400, Josh Boyer wrote:
>> On Wed, Jun 11, 2014 at 12:20 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> > Rather than monkeying with barely-comprehensible static pattern
>> > rules, just use an explicit loop.
>> >
>> > Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>> > ---
>> >  arch/x86/vdso/Makefile | 20 +++++++-------------
>> >  1 file changed, 7 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
>> > index 9769df0..b1c70cc 100644
>> > --- a/arch/x86/vdso/Makefile
>> > +++ b/arch/x86/vdso/Makefile
>> > @@ -9,11 +9,6 @@ VDSOX32-$(CONFIG_X86_X32_ABI)  := y
>> >  VDSO32-$(CONFIG_X86_32)                := y
>> >  VDSO32-$(CONFIG_COMPAT)                := y
>> >
>> > -vdso-install-$(VDSO64-y)       += vdso.so
>> > -vdso-install-$(VDSOX32-y)      += vdsox32.so
>> > -vdso-install-$(VDSO32-y)       += $(vdso32-images)
>> > -
>> > -
>> >  # files to link into the vdso
>> >  vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
>> >
>> > @@ -176,15 +171,14 @@ VDSO_LDFLAGS = -fPIC -shared $(call cc-ldoption, -Wl$(comma)--hash-style=sysv) \
>> >  GCOV_PROFILE := n
>> >
>> >  #
>> > -# Install the unstripped copy of vdso*.so listed in $(vdso-install-y).
>> > +# Install the unstripped copies of vdso*.so listed in $(vdso-install-y).
>> >  #
>> > -quiet_cmd_vdso_install = INSTALL $@
>> > -      cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
>> > -$(vdso-install-y): %.so: $(obj)/%.so.dbg FORCE
>> > -       @mkdir -p $(MODLIB)/vdso
>> > -       $(call cmd,vdso_install)
>> > +quiet_cmd_vdso_install = INSTALL $(sofile)
>> > +      cmd_vdso_install = cp $(obj)/$(sofile).dbg $(MODLIB)/vdso/$(sofile)
>> >
>> > -PHONY += vdso_install $(vdso-install-y)
>> > -vdso_install: $(vdso-install-y)
>> > +PHONY += vdso_install
>> > +vdso_install: $(vdso_img_sodbg:%=$(obj)/%) FORCE
>> > +       @mkdir -p $(MODLIB)/vdso
>> > +       @$(foreach sofile,$(vdso_img_sodbg:%.dbg=%),$(call recipe-cmd,vdso_install);)
>
> Can we please fix this in a way where we do not need to add stuff to core kbuild.
> If the original approach was used then make took care of the looping
> and the foreach part was not needed.

Would expanding out the cmd macro here be okay?  Or is that too
dependent on kbuild internals?

I can fiddle with static patterns again (ugh).

--Andy

Re: [PATCH 2/2] x86,vdso: Fix vdso_install

[Sam Ravnborg]

> >
> > Can we please fix this in a way where we do not need to add stuff to core kbuild.
> > If the original approach was used then make took care of the looping
> > and the foreach part was not needed.
Following is my first quick approach.
It reuses the old - a little complex logic to do the looping and copying.
Thsi is only tested on my i386 box / build.

	Sam


diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index 9769df0..39fb152 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -9,11 +9,6 @@ VDSOX32-$(CONFIG_X86_X32_ABI)	:= y
 VDSO32-$(CONFIG_X86_32)		:= y
 VDSO32-$(CONFIG_COMPAT)		:= y
 
-vdso-install-$(VDSO64-y)	+= vdso.so
-vdso-install-$(VDSOX32-y)	+= vdsox32.so
-vdso-install-$(VDSO32-y)	+= $(vdso32-images)
-
-
 # files to link into the vdso
 vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
 
@@ -179,12 +174,12 @@ GCOV_PROFILE := n
 # Install the unstripped copy of vdso*.so listed in $(vdso-install-y).
 #
 quiet_cmd_vdso_install = INSTALL $@
-      cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
-$(vdso-install-y): %.so: $(obj)/%.so.dbg FORCE
+      cmd_vdso_install = cp $(obj)/$@ $(MODLIB)/vdso/$(@:.dbg=)
+$(vdso_img_sodbg): %.so.dbg: $(obj)/%.so.dbg FORCE
 	@mkdir -p $(MODLIB)/vdso
 	$(call cmd,vdso_install)
 
-PHONY += vdso_install $(vdso-install-y)
-vdso_install: $(vdso-install-y)
+PHONY += vdso_install $(vdso_img_sodbg)
+vdso_install: $(vdso_img_sodbg)
 
 clean-files := vdso32-syscall* vdso32-sysenter* vdso32-int80*

Re: [PATCH 2/2] x86,vdso: Fix vdso_install

[Andy Lutomirski]

On Wed, Jun 11, 2014 at 12:16 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
>> >
>> > Can we please fix this in a way where we do not need to add stuff to core kbuild.
>> > If the original approach was used then make took care of the looping
>> > and the foreach part was not needed.
> Following is my first quick approach.
> It reuses the old - a little complex logic to do the looping and copying.
> Thsi is only tested on my i386 box / build.
>
>         Sam
>
>
> diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
> index 9769df0..39fb152 100644
> --- a/arch/x86/vdso/Makefile
> +++ b/arch/x86/vdso/Makefile
> @@ -9,11 +9,6 @@ VDSOX32-$(CONFIG_X86_X32_ABI)  := y
>  VDSO32-$(CONFIG_X86_32)                := y
>  VDSO32-$(CONFIG_COMPAT)                := y
>
> -vdso-install-$(VDSO64-y)       += vdso.so
> -vdso-install-$(VDSOX32-y)      += vdsox32.so
> -vdso-install-$(VDSO32-y)       += $(vdso32-images)
> -
> -
>  # files to link into the vdso
>  vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
>
> @@ -179,12 +174,12 @@ GCOV_PROFILE := n
>  # Install the unstripped copy of vdso*.so listed in $(vdso-install-y).
>  #
>  quiet_cmd_vdso_install = INSTALL $@
> -      cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
> -$(vdso-install-y): %.so: $(obj)/%.so.dbg FORCE
> +      cmd_vdso_install = cp $(obj)/$@ $(MODLIB)/vdso/$(@:.dbg=)


> +$(vdso_img_sodbg): %.so.dbg: $(obj)/%.so.dbg FORCE
>         @mkdir -p $(MODLIB)/vdso
>         $(call cmd,vdso_install)

The part that scares me is that this rule defines targets with names
like vdso64.so.dbg, which are the names of actual files (admittedly,
they're missing the path, but still).  This seems fragile.

>
> -PHONY += vdso_install $(vdso-install-y)
> -vdso_install: $(vdso-install-y)
> +PHONY += vdso_install $(vdso_img_sodbg)
> +vdso_install: $(vdso_img_sodbg)
>
>  clean-files := vdso32-syscall* vdso32-sysenter* vdso32-int80*
>



-- 
Andy Lutomirski
AMA Capital Management, LLC