Re: [PATCH v1 4/9] powerpc/vdso: Remove unnecessary ifdefs in vdso_pagelist initialization

[Christophe Leroy]

On 08/26/2020 02:58 PM, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@xxxxxxxxxx> writes:
> > diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
> > index daef14a284a3..bbb69832fd46 100644
> > --- a/arch/powerpc/kernel/vdso.c
> > +++ b/arch/powerpc/kernel/vdso.c
> > @@ -718,16 +710,14 @@ static int __init vdso_init(void)
> ...
> >   
> > -
> > -#ifdef CONFIG_VDSO32
> >   	vdso32_kbase = &vdso32_start;
> >   
> >   	/*
> > @@ -735,8 +725,6 @@ static int __init vdso_init(void)
> >   	 */
> >   	vdso32_pages = (&vdso32_end - &vdso32_start) >> PAGE_SHIFT;
> >   	DBG("vdso32_kbase: %p, 0x%x pages\n", vdso32_kbase, vdso32_pages);
> > -#endif
>
> This didn't build for ppc64le:
>
>    /opt/cross/gcc-8.20_binutils-2.32/powerpc64-unknown-linux-gnu/bin/powerpc64-unknown-linux-gnu-ld: arch/powerpc/kernel/vdso.o:(.toc+0x0): undefined reference to `vdso32_end'
>    /opt/cross/gcc-8.20_binutils-2.32/powerpc64-unknown-linux-gnu/bin/powerpc64-unknown-linux-gnu-ld: arch/powerpc/kernel/vdso.o:(.toc+0x8): undefined reference to `vdso32_start'
>    make[1]: *** [/scratch/michael/build/maint/Makefile:1166: vmlinux] Error 1
>    make: *** [Makefile:185: __sub-make] Error 2
>
> So I just put that ifdef back.

The problem is because is_32bit() can still return true even when 
CONFIG_VDSO32 is not set.

The change below fixes the problem:

diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index bbb69832fd46..38abff60cbe2 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -132,11 +132,7 @@ int arch_setup_additional_pages(struct linux_binprm 
*bprm, int uses_interp)
 	if (!vdso_ready)
 		return 0;

-	if (is_32bit_task()) {
-		vdso_pagelist = vdso32_pagelist;
-		vdso_pages = vdso32_pages;
-		vdso_base = VDSO32_MBASE;
-	} else {
+	if (!is_32bit_task()) {
 		vdso_pagelist = vdso64_pagelist;
 		vdso_pages = vdso64_pages;
 		/*
@@ -145,6 +141,12 @@ int arch_setup_additional_pages(struct linux_binprm 
*bprm, int uses_interp)
 		 * and most likely share a SLB entry.
 		 */
 		vdso_base = 0;
+	} else if (IS_ENABLED(CONFIG_VDSO32)) {
+		vdso_pagelist = vdso32_pagelist;
+		vdso_pages = vdso32_pages;
+		vdso_base = VDSO32_MBASE;
+	} else {
+		vdso_pages = 0;
 	}

 	current->mm->context.vdso_base = 0;

With this change all vdso32 static objects (functions and vars) go away 
as expected.

We get a simple conflict with the following patch.

Do you prefer an updated series or a follow up patch, or you take the 
above change yourself ?

Christophe