[PATCH] ix86: fix vDSO build

[PATCH] ix86: fix vDSO build

[Jan Beulich]

Relying on static functions used just once to get inlined (and
subsequently have dead code paths eliminated) is wrong: Compilers are
free to decide whether they do this, regardless of optimization level.
With this not happening for vdso_addr() (observed with gcc 4.1.x), an
unresolved reference to align_vdso_addr() causes the build to fail.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/vdso/vma.c    |    4 ++++
 1 file changed, 4 insertions(+)

--- 3.16-rc3/arch/x86/vdso/vma.c
+++ 3.16-rc3-x86-vdso-build/arch/x86/vdso/vma.c
@@ -62,6 +62,9 @@ struct linux_binprm;
    Only used for the 64-bit and x32 vdsos. */
 static unsigned long vdso_addr(unsigned long start, unsigned len)
 {
+#ifdef CONFIG_X86_32
+	return 0;
+#else
 	unsigned long addr, end;
 	unsigned offset;
 	end = (start + PMD_SIZE - 1) & PMD_MASK;
@@ -83,6 +86,7 @@ static unsigned long vdso_addr(unsigned
 	addr = align_vdso_addr(addr);
 
 	return addr;
+#endif
 }
 
 static int map_vdso(const struct vdso_image *image, bool calculate_addr)

Re: [PATCH] ix86: fix vDSO build

[Andy Lutomirski]

On Thu, Jul 3, 2014 at 7:35 AM, Jan Beulich <JBeulich@suse.com> wrote:
> Relying on static functions used just once to get inlined (and
> subsequently have dead code paths eliminated) is wrong: Compilers are
> free to decide whether they do this, regardless of optimization level.
> With this not happening for vdso_addr() (observed with gcc 4.1.x), an
> unresolved reference to align_vdso_addr() causes the build to fail.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Andy Lutomirski <luto@amacapital.net>

Acked-by: Andy Lutomirski <luto@amacapital.net>

Any chance you could send a dump of the symbol and relocation tables
of a .so.dbg with this problem?  I'm curious why checkundef.sh never
caught it.

--Andy

Re: [PATCH] ix86: fix vDSO build

[Jan Beulich]

>>> On 03.07.14 at 17:34, <luto@amacapital.net> wrote:
> On Thu, Jul 3, 2014 at 7:35 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> Relying on static functions used just once to get inlined (and
>> subsequently have dead code paths eliminated) is wrong: Compilers are
>> free to decide whether they do this, regardless of optimization level.
>> With this not happening for vdso_addr() (observed with gcc 4.1.x), an
>> unresolved reference to align_vdso_addr() causes the build to fail.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Cc: Andy Lutomirski <luto@amacapital.net>
> 
> Acked-by: Andy Lutomirski <luto@amacapital.net>

Thanks (also for the other one).

> Any chance you could send a dump of the symbol and relocation tables
> of a .so.dbg with this problem?  I'm curious why checkundef.sh never
> caught it.

vma.o is part of the kernel, not the .so.

Jan

Re: [PATCH] ix86: fix vDSO build

[Andy Lutomirski]

On Thu, Jul 3, 2014 at 8:44 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 03.07.14 at 17:34, <luto@amacapital.net> wrote:
>> On Thu, Jul 3, 2014 at 7:35 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>> Relying on static functions used just once to get inlined (and
>>> subsequently have dead code paths eliminated) is wrong: Compilers are
>>> free to decide whether they do this, regardless of optimization level.
>>> With this not happening for vdso_addr() (observed with gcc 4.1.x), an
>>> unresolved reference to align_vdso_addr() causes the build to fail.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> Cc: Andy Lutomirski <luto@amacapital.net>
>>
>> Acked-by: Andy Lutomirski <luto@amacapital.net>
>
> Thanks (also for the other one).
>
>> Any chance you could send a dump of the symbol and relocation tables
>> of a .so.dbg with this problem?  I'm curious why checkundef.sh never
>> caught it.
>
> vma.o is part of the kernel, not the .so.

Duh :)

--Andy

Re: [PATCH] ix86: fix vDSO build

[Boris Ostrovsky]

On 07/03/2014 10:35 AM, Jan Beulich wrote:
> Relying on static functions used just once to get inlined (and
> subsequently have dead code paths eliminated) is wrong: Compilers are
> free to decide whether they do this, regardless of optimization level.
> With this not happening for vdso_addr() (observed with gcc 4.1.x), an
> unresolved reference to align_vdso_addr() causes the build to fail.

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> ---
>   arch/x86/vdso/vma.c    |    4 ++++
>   1 file changed, 4 insertions(+)
>
> --- 3.16-rc3/arch/x86/vdso/vma.c
> +++ 3.16-rc3-x86-vdso-build/arch/x86/vdso/vma.c
> @@ -62,6 +62,9 @@ struct linux_binprm;
>      Only used for the 64-bit and x32 vdsos. */
>   static unsigned long vdso_addr(unsigned long start, unsigned len)
>   {
> +#ifdef CONFIG_X86_32
> +	return 0;
> +#else
>   	unsigned long addr, end;
>   	unsigned offset;
>   	end = (start + PMD_SIZE - 1) & PMD_MASK;
> @@ -83,6 +86,7 @@ static unsigned long vdso_addr(unsigned
>   	addr = align_vdso_addr(addr);
>   
>   	return addr;
> +#endif
>   }
>   
>   static int map_vdso(const struct vdso_image *image, bool calculate_addr)
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ix86: fix vDSO build

[Andrew Morton]

On Thu, 03 Jul 2014 15:35:07 +0100 "Jan Beulich" <JBeulich@suse.com> wrote:

> Relying on static functions used just once to get inlined (and
> subsequently have dead code paths eliminated) is wrong: Compilers are
> free to decide whether they do this, regardless of optimization level.
> With this not happening for vdso_addr() (observed with gcc 4.1.x), an
> unresolved reference to align_vdso_addr() causes the build to fail.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> ---
>  arch/x86/vdso/vma.c    |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> --- 3.16-rc3/arch/x86/vdso/vma.c
> +++ 3.16-rc3-x86-vdso-build/arch/x86/vdso/vma.c
> @@ -62,6 +62,9 @@ struct linux_binprm;
>     Only used for the 64-bit and x32 vdsos. */
>  static unsigned long vdso_addr(unsigned long start, unsigned len)
>  {
> +#ifdef CONFIG_X86_32
> +	return 0;
> +#else
>  	unsigned long addr, end;
>  	unsigned offset;
>  	end = (start + PMD_SIZE - 1) & PMD_MASK;
> @@ -83,6 +86,7 @@ static unsigned long vdso_addr(unsigned
>  	addr = align_vdso_addr(addr);
>  
>  	return addr;
> +#endif
>  }
>  
>  static int map_vdso(const struct vdso_image *image, bool calculate_addr)

Tested-by: Andrew Morton <akpm@linux-foundation.org>

Re: [PATCH] ix86: fix vDSO build

[H. Peter Anvin]

On 07/03/2014 07:35 AM, Jan Beulich wrote:
> Relying on static functions used just once to get inlined (and
> subsequently have dead code paths eliminated) is wrong: Compilers are
> free to decide whether they do this, regardless of optimization level.
> With this not happening for vdso_addr() (observed with gcc 4.1.x), an
> unresolved reference to align_vdso_addr() causes the build to fail.

The fix seems odd... more of the flavor of "happens to work" unless I'm
misunderstanding something.  If this needs to be inlined, wouldn't
__always_inline make more sense?

What am I missing?

	-hpa

[tip:x86/urgent] x86-32, vdso: Fix vDSO build error due to missing align_vdso_addr()

[tip-bot for Jan Beulich]

Commit-ID:  d093601be5e97d2729614419d0d256ed3b6a56b0
Gitweb:     http://git.kernel.org/tip/d093601be5e97d2729614419d0d256ed3b6a56b0
Author:     Jan Beulich <JBeulich@suse.com>
AuthorDate: Thu, 3 Jul 2014 15:35:07 +0100
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Thu, 10 Jul 2014 16:06:04 -0700

x86-32, vdso: Fix vDSO build error due to missing align_vdso_addr()

Relying on static functions used just once to get inlined (and
subsequently have dead code paths eliminated) is wrong: Compilers are
free to decide whether they do this, regardless of optimization level.
With this not happening for vdso_addr() (observed with gcc 4.1.x), an
unresolved reference to align_vdso_addr() causes the build to fail.

[ hpa: vdso_addr() is never actually used on x86-32, as calculate_addr
  in map_vdso() is always false.  It ought to be possible to clean
  this up further, but this fixes the immediate problem. ]

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Link: http://lkml.kernel.org/r/53B5863B02000078000204D5@mail.emea.novell.com
Acked-by: Andy Lutomirski <luto@amacapital.net>
Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Tested-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/vdso/vma.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/vdso/vma.c b/arch/x86/vdso/vma.c
index e1513c4..5a5176d 100644
--- a/arch/x86/vdso/vma.c
+++ b/arch/x86/vdso/vma.c
@@ -62,6 +62,9 @@ struct linux_binprm;
    Only used for the 64-bit and x32 vdsos. */
 static unsigned long vdso_addr(unsigned long start, unsigned len)
 {
+#ifdef CONFIG_X86_32
+	return 0;
+#else
 	unsigned long addr, end;
 	unsigned offset;
 	end = (start + PMD_SIZE - 1) & PMD_MASK;
@@ -83,6 +86,7 @@ static unsigned long vdso_addr(unsigned long start, unsigned len)
 	addr = align_vdso_addr(addr);
 
 	return addr;
+#endif
 }
 
 static int map_vdso(const struct vdso_image *image, bool calculate_addr)

Re: [PATCH] ix86: fix vDSO build

[Jan Beulich]

>>> On 11.07.14 at 00:58, <hpa@zytor.com> wrote:
> On 07/03/2014 07:35 AM, Jan Beulich wrote:
>> Relying on static functions used just once to get inlined (and
>> subsequently have dead code paths eliminated) is wrong: Compilers are
>> free to decide whether they do this, regardless of optimization level.
>> With this not happening for vdso_addr() (observed with gcc 4.1.x), an
>> unresolved reference to align_vdso_addr() causes the build to fail.
> 
> The fix seems odd... more of the flavor of "happens to work" unless I'm
> misunderstanding something.  If this needs to be inlined, wouldn't
> __always_inline make more sense?
> 
> What am I missing?

I think this is a matter of taste: Personally I think __always_inline should
be used rather rarely, as it takes away decisions from the compiler that
it would (generally) be in the better position to make.

Sorry for the late reply (was on vacation),
Jan