PowerPC build fail

PowerPC build fail

[Tobin C. Harding]

The current (2bfe01e) torvalds git tree fails to build on powerpc64. Build machine
is virtualized.

- Build error
arch/powerpc/kernel/time.c: In function ‘running_clock’:
arch/powerpc/kernel/time.c:712:25: error: implicit declaration of function ‘cputime_to_nsecs’
   return local_clock() - cputime_to_nsecs(kcpustat_this_cpu->cpustat[CPUTIME_STEAL]);
                          ^
                          
$ cat .config | grep CONFIG_VIRT_CPU
# CONFIG_VIRT_CPU_ACCOUNTING_NATIVE is not set
# CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set

$ cat .config | grep CONFIG_PPC_PSERIES
CONFIG_PPC_PSERIES=y

Looking into it I found there are compile time guards on CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
preventing cputime_to_nsecs being defined.

Removing the compile time guard allows the build to proceed, surely
this is not the correct solution though.

diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
index 99b5418..15482cb 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -16,7 +16,7 @@
 #ifndef __POWERPC_CPUTIME_H
 #define __POWERPC_CPUTIME_H

-#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
+

 #include <linux/types.h>
 #include <linux/time.h>
@@ -53,5 +53,5 @@ void arch_vtime_task_switch(struct task_struct *tsk);
 #endif

 #endif /* __KERNEL__ */
-#endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
 #endif /* __POWERPC_CPUTIME_H */
diff --git a/include/linux/cputime.h b/include/linux/cputime.h
index a691dc4..f730c14 100644
--- a/include/linux/cputime.h
+++ b/include/linux/cputime.h
@@ -1,13 +1,13 @@
 #ifndef __LINUX_CPUTIME_H
 #define __LINUX_CPUTIME_H

-#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
+
 #include <asm/cputime.h>

 #ifndef cputime_to_nsecs
-# define cputime_to_nsecs(__ct)        \
+#define cputime_to_nsecs(__ct) \
        (cputime_to_usecs(__ct) * NSEC_PER_USEC)
 #endif

-#endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
+
 #endif /* __LINUX_CPUTIME_H */


If you think this can be fixed with the *little* kernel dev knowledge
I have please point me at a starting place and I will dig into it.

thanks,
Tobin.

Re: PowerPC build fail

[Andrew Donnellan]

On 22/02/17 12:22, Tobin C. Harding wrote:
> The current (2bfe01e) torvalds git tree fails to build on powerpc64. Build machine
> is virtualized.
>
> - Build error
> arch/powerpc/kernel/time.c: In function ‘running_clock’:
> arch/powerpc/kernel/time.c:712:25: error: implicit declaration of function ‘cputime_to_nsecs’
>    return local_clock() - cputime_to_nsecs(kcpustat_this_cpu->cpustat[CPUTIME_STEAL]);

Already reported, see 
https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-February/154433.html

See patch at http://patchwork.ozlabs.org/patch/730616/


Andrew

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

Re: PowerPC build fail

[Michael Ellerman]

Hi Tobin,

"Tobin C. Harding" <me@tobin.cc> writes:
> The current (2bfe01e) torvalds git tree fails to build on powerpc64. Buil=
d machine
> is virtualized.
>
> - Build error
> arch/powerpc/kernel/time.c: In function =E2=80=98running_clock=E2=80=99:
> arch/powerpc/kernel/time.c:712:25: error: implicit declaration of functio=
n =E2=80=98cputime_to_nsecs=E2=80=99
>    return local_clock() - cputime_to_nsecs(kcpustat_this_cpu->cpustat[CPU=
TIME_STEAL]);
>                           ^
...
>
> If you think this can be fixed with the *little* kernel dev knowledge
> I have please point me at a starting place and I will dig into it.

Thanks for trying to fix it.

You got unlucky and pulled Linus' tree just after a cleanup series was
merged, but before all the bugs had been shaken out.

cheers

Re: PowerPC build fail

[Tobin C. Harding]

On Mon, Feb 27, 2017 at 09:38:13PM +1100, Michael Ellerman wrote:
> Hi Tobin,
> 
> "Tobin C. Harding" <me@tobin.cc> writes:
> > The current (2bfe01e) torvalds git tree fails to build on powerpc64. Build machine
> > is virtualized.
> >
> > - Build error
> > arch/powerpc/kernel/time.c: In function ‘running_clock’:
> > arch/powerpc/kernel/time.c:712:25: error: implicit declaration of function ‘cputime_to_nsecs’
> >    return local_clock() - cputime_to_nsecs(kcpustat_this_cpu->cpustat[CPUTIME_STEAL]);
> >                           ^
> ...
> >
> > If you think this can be fixed with the *little* kernel dev knowledge
> > I have please point me at a starting place and I will dig into it.
> 
> Thanks for trying to fix it.
> 
> You got unlucky and pulled Linus' tree just after a cleanup series was
> merged, but before all the bugs had been shaken out.

Au contraire, I felt lucky to have bumped into a bug. I would like to
find more :)

Tobin

Re: PowerPC build fail

[Michael Ellerman]

"Tobin C. Harding" <me@tobin.cc> writes:

> On Mon, Feb 27, 2017 at 09:38:13PM +1100, Michael Ellerman wrote:
>> Hi Tobin,
>>=20
>> "Tobin C. Harding" <me@tobin.cc> writes:
>> > The current (2bfe01e) torvalds git tree fails to build on powerpc64. B=
uild machine
>> > is virtualized.
>> >
>> > - Build error
>> > arch/powerpc/kernel/time.c: In function =E2=80=98running_clock=E2=80=
=99:
>> > arch/powerpc/kernel/time.c:712:25: error: implicit declaration of func=
tion =E2=80=98cputime_to_nsecs=E2=80=99
>> >    return local_clock() - cputime_to_nsecs(kcpustat_this_cpu->cpustat[=
CPUTIME_STEAL]);
>> >                           ^
>> ...
>> >
>> > If you think this can be fixed with the *little* kernel dev knowledge
>> > I have please point me at a starting place and I will dig into it.
>>=20
>> Thanks for trying to fix it.
>>=20
>> You got unlucky and pulled Linus' tree just after a cleanup series was
>> merged, but before all the bugs had been shaken out.
>
> Au contraire, I felt lucky to have bumped into a bug. I would like to
> find more :)

Hah, well, we have *plenty* :)

I keep a rough list of TODO/FIXME type things on github:

  https://github.com/linuxppc/linux/issues/


If there's anything there that you want to have a go at please feel
free. If you have questions you can either comment on the issues or mail
me and/or this list.

cheers

powerpc build fail

[Tobin C. Harding]

The current (2bfe01e) torvalds git tree fails to build with the following error

arch/powerpc/kernel/time.c: In function ?running_clock?:
arch/powerpc/kernel/time.c:712:25: error: implicit declaration of function ?cputime_to_nsecs? [-Werror=implicit-function-declaration]
  return local_clock() - cputime_to_nsecs(kcpustat_this_cpu->cpustat[CPUTIME_STEAL]);
                         ^
I would like to do more than just file a bug report. Further
investigation (cputime.h) shows that cputime_to_nsecs has a
preprocessor guard on the config option (currently enabled)
CONFIG_VIRT_CPU_ACCOUNTING_NATIVE. 

CONFIG_PPC_PSERIES is also enabled.

The offending code in arch/powerpc/kernel/time.c is

#ifdef CONFIG_PPC_PSERIES

/*                                                                                    
 * Running clock - attempts to give a view of time passing for a virtualised          
 * kernels.                                                                           
 * Uses the VTB register if available otherwise a next best guess.                    
 */
unsigned long long running_clock(void)
{
 	/*                                                                            
         * Don't read the VTB as a host since KVM does not switch in host             
         * timebase into the VTB when it takes a guest off the CPU, reading the       
         * VTB would result in reading 'last switched out' guest VTB.                 
         *                                                                            
         * Host kernels are often compiled with CONFIG_PPC_PSERIES checked, it        
         * would be unsafe to rely only on the #ifdef above.                          
         */
	if (firmware_has_feature(FW_FEATURE_LPAR) &&
            cpu_has_feature(CPU_FTR_ARCH_207S))
		return mulhdu(get_vtb() - boot_tb, tb_to_ns_scale) << tb_to_ns_shift;

	/*                                                                            
         * This is a next best approximation without a VTB.                           
         * On a host which is running bare metal there should never be any stolen     
         * time and on a host which doesn't do any virtualisation TB *should* equal   
         * VTB so it makes no difference anyway.                                      
         */
	return local_clock() - cputime_to_nsecs(kcpustat_this_cpu->cpustat[CPUTIME_STEAL]);
}
#endif

Can anyone give me a hint on this one? Where to start reading?

thanks,
Tobin.

powerpc build fail

[Tobin C. Harding]

On Tue, Feb 21, 2017 at 09:53:01PM +1100, Tobin C. Harding wrote:
> The current (2bfe01e) torvalds git tree fails to build with the following error
> 
> arch/powerpc/kernel/time.c: In function ?running_clock?:
> arch/powerpc/kernel/time.c:712:25: error: implicit declaration of function ?cputime_to_nsecs? [-Werror=implicit-function-declaration]
>   return local_clock() - cputime_to_nsecs(kcpustat_this_cpu->cpustat[CPUTIME_STEAL]);
>                          ^
> I would like to do more than just file a bug report. Further
> investigation (cputime.h) shows that cputime_to_nsecs has a
> preprocessor guard on the config option (currently enabled)
> CONFIG_VIRT_CPU_ACCOUNTING_NATIVE. 

Mistake made, CONFIG_VIRT_CPU_ACCOUNTING_NATIVE is *not* enabled.

$ cat .config | grep CONFIG_VIRT_CPU
# CONFIG_VIRT_CPU_ACCOUNTING_NATIVE is not set
# CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set

$ cat .config | grep CONFIG_PPC_PSERIES
CONFIG_PPC_PSERIES=y

> 
> CONFIG_PPC_PSERIES is also enabled.
> 
> The offending code in arch/powerpc/kernel/time.c is
> 
> #ifdef CONFIG_PPC_PSERIES
> 
> /*                                                                                    
>  * Running clock - attempts to give a view of time passing for a virtualised          
>  * kernels.                                                                           
>  * Uses the VTB register if available otherwise a next best guess.                    
>  */
> unsigned long long running_clock(void)
> {
>  	/*                                                                            
>          * Don't read the VTB as a host since KVM does not switch in host             
>          * timebase into the VTB when it takes a guest off the CPU, reading the       
>          * VTB would result in reading 'last switched out' guest VTB.                 
>          *                                                                            
>          * Host kernels are often compiled with CONFIG_PPC_PSERIES checked, it        
>          * would be unsafe to rely only on the #ifdef above.                          
>          */
> 	if (firmware_has_feature(FW_FEATURE_LPAR) &&
>             cpu_has_feature(CPU_FTR_ARCH_207S))
> 		return mulhdu(get_vtb() - boot_tb, tb_to_ns_scale) << tb_to_ns_shift;
> 
> 	/*                                                                            
>          * This is a next best approximation without a VTB.                           
>          * On a host which is running bare metal there should never be any stolen     
>          * time and on a host which doesn't do any virtualisation TB *should* equal   
>          * VTB so it makes no difference anyway.                                      
>          */
> 	return local_clock() - cputime_to_nsecs(kcpustat_this_cpu->cpustat[CPUTIME_STEAL]);
> }
> #endif
> 
> Can anyone give me a hint on this one? Where to start reading?

Removing the compile time guard allows the build to proceed, surely
this is not the correct solution though.

diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
index 99b5418..15482cb 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -16,7 +16,7 @@
 #ifndef __POWERPC_CPUTIME_H
 #define __POWERPC_CPUTIME_H
 
-#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
+
 
 #include <linux/types.h>
 #include <linux/time.h>
@@ -53,5 +53,5 @@ void arch_vtime_task_switch(struct task_struct *tsk);
 #endif
 
 #endif /* __KERNEL__ */
-#endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
+
 #endif /* __POWERPC_CPUTIME_H */
diff --git a/include/linux/cputime.h b/include/linux/cputime.h
index a691dc4..f730c14 100644
--- a/include/linux/cputime.h
+++ b/include/linux/cputime.h
@@ -1,13 +1,13 @@
 #ifndef __LINUX_CPUTIME_H
 #define __LINUX_CPUTIME_H
 
-#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
+
 #include <asm/cputime.h>
 
 #ifndef cputime_to_nsecs
-# define cputime_to_nsecs(__ct)	\
+#define cputime_to_nsecs(__ct)	\
 	(cputime_to_usecs(__ct) * NSEC_PER_USEC)
 #endif
 
-#endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
+
 #endif /* __LINUX_CPUTIME_H */

thanks,
Tobin.