* Re: [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING
2016-02-11 16:16 [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING Christophe Leroy
@ 2016-02-12 8:25 ` Denis Kirjanov
2016-02-14 20:40 ` Denis Kirjanov
2016-02-16 21:21 ` Scott Wood
2 siblings, 0 replies; 10+ messages in thread
From: Denis Kirjanov @ 2016-02-12 8:25 UTC (permalink / raw)
To: Christophe Leroy
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Scott Wood, linuxppc-dev, linux-kernel
On 2/11/16, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> This patch provides VIRT_CPU_ACCOUTING to PPC32 architecture.
> PPC32 doesn't have the PACA structure, so we use the task_info
> structure to store the accounting data.
>
> In order to reuse on PPC32 the PPC64 functions, all u64 data has
> been replaced by 'unsigned long' so that it is u32 on PPC32 and
> u64 on PPC64
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
I'll try it out on my ppc32 machines.
Thanks!
> ---
> Changes in v3: unlike previous version of the patch that was inspired
> from IA64 architecture, this new version tries to reuse as much as
> possible the PPC64 implementation.
>
> PPC32 doesn't have PACA and past discusion on v2 version has shown
> that it is not worth implementing a PACA in PPC32 architecture
> (see below benh opinion)
>
> benh: PACA is actually a data structure and you really really don't want it
> on ppc32 :-) Having a register point to current works, having a register
> point to per-cpu data instead works too (ie, change what we do today),
> but don't introduce a PACA *please* :-)
>
> Changes in v4: ACCOUNT_CPU_USER_ENTRY/EXIT() needed updates in other
> places than entry_32.S and entry_64.S (reported by kbuild-robot)
> Related defines in asm-offset.c need to be conditional to
> CONFIG_VIRT_CPU_ACCOUNTING_NATIVE (reported by kbuild-robot)
>
> Changes in v5: Using PPC_LL et PPC_STL instead of defining new macros
> AC_LD and AC_STD
>
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/cputime.h | 4 ++++
> arch/powerpc/include/asm/exception-64s.h | 2 +-
> arch/powerpc/include/asm/ppc_asm.h | 24 ++++++++++----------
> arch/powerpc/include/asm/reg.h | 1 +
> arch/powerpc/include/asm/thread_info.h | 11 +++++++++
> arch/powerpc/kernel/asm-offsets.c | 7 ++++++
> arch/powerpc/kernel/entry_32.S | 17 ++++++++++++++
> arch/powerpc/kernel/entry_64.S | 6 ++---
> arch/powerpc/kernel/exceptions-64e.S | 4 ++--
> arch/powerpc/kernel/time.c | 38
> ++++++++++++++++++++++++++------
> arch/powerpc/platforms/Kconfig.cputype | 1 -
> 12 files changed, 90 insertions(+), 26 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 3a557be..57ce4ff 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -159,6 +159,7 @@ config PPC
> select ARCH_HAS_DEVMEM_IS_ALLOWED
> select HAVE_ARCH_SECCOMP_FILTER
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> + select HAVE_VIRT_CPU_ACCOUNTING
>
> config GENERIC_CSUM
> def_bool CPU_LITTLE_ENDIAN
> diff --git a/arch/powerpc/include/asm/cputime.h
> b/arch/powerpc/include/asm/cputime.h
> index e245255..c4c33be 100644
> --- a/arch/powerpc/include/asm/cputime.h
> +++ b/arch/powerpc/include/asm/cputime.h
> @@ -230,7 +230,11 @@ static inline cputime_t clock_t_to_cputime(const
> unsigned long clk)
>
> #define cputime64_to_clock_t(ct) cputime_to_clock_t((cputime_t)(ct))
>
> +#ifdef CONFIG_PPC64
> static inline void arch_vtime_task_switch(struct task_struct *tsk) { }
> +#else
> +void arch_vtime_task_switch(struct task_struct *tsk);
> +#endif
>
> #endif /* __KERNEL__ */
> #endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
> diff --git a/arch/powerpc/include/asm/exception-64s.h
> b/arch/powerpc/include/asm/exception-64s.h
> index 93ae809..8bc38d1 100644
> --- a/arch/powerpc/include/asm/exception-64s.h
> +++ b/arch/powerpc/include/asm/exception-64s.h
> @@ -287,7 +287,7 @@ do_kvm_##n: \
> std r0,GPR0(r1); /* save r0 in stackframe */ \
> std r10,GPR1(r1); /* save r1 in stackframe */ \
> beq 4f; /* if from kernel mode */ \
> - ACCOUNT_CPU_USER_ENTRY(r9, r10); \
> + ACCOUNT_CPU_USER_ENTRY(r13, r9, r10); \
> SAVE_PPR(area, r9, r10); \
> 4: EXCEPTION_PROLOG_COMMON_2(area) \
> EXCEPTION_PROLOG_COMMON_3(n) \
> diff --git a/arch/powerpc/include/asm/ppc_asm.h
> b/arch/powerpc/include/asm/ppc_asm.h
> index 499d9f8..07d1bfc 100644
> --- a/arch/powerpc/include/asm/ppc_asm.h
> +++ b/arch/powerpc/include/asm/ppc_asm.h
> @@ -24,27 +24,27 @@
> */
>
> #ifndef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> -#define ACCOUNT_CPU_USER_ENTRY(ra, rb)
> -#define ACCOUNT_CPU_USER_EXIT(ra, rb)
> +#define ACCOUNT_CPU_USER_ENTRY(ptr, ra, rb)
> +#define ACCOUNT_CPU_USER_EXIT(ptr, ra, rb)
> #define ACCOUNT_STOLEN_TIME
> #else
> -#define ACCOUNT_CPU_USER_ENTRY(ra, rb) \
> +#define ACCOUNT_CPU_USER_ENTRY(ptr, ra, rb) \
> MFTB(ra); /* get timebase */ \
> - ld rb,PACA_STARTTIME_USER(r13); \
> - std ra,PACA_STARTTIME(r13); \
> + PPC_LL rb, PACA_STARTTIME_USER(ptr); \
> + PPC_STL ra, PACA_STARTTIME(ptr); \
> subf rb,rb,ra; /* subtract start value */ \
> - ld ra,PACA_USER_TIME(r13); \
> + PPC_LL ra, PACA_USER_TIME(ptr); \
> add ra,ra,rb; /* add on to user time */ \
> - std ra,PACA_USER_TIME(r13); \
> + PPC_STL ra, PACA_USER_TIME(ptr); \
>
> -#define ACCOUNT_CPU_USER_EXIT(ra, rb) \
> +#define ACCOUNT_CPU_USER_EXIT(ptr, ra, rb) \
> MFTB(ra); /* get timebase */ \
> - ld rb,PACA_STARTTIME(r13); \
> - std ra,PACA_STARTTIME_USER(r13); \
> + PPC_LL rb, PACA_STARTTIME(ptr); \
> + PPC_STL ra, PACA_STARTTIME_USER(ptr); \
> subf rb,rb,ra; /* subtract start value */ \
> - ld ra,PACA_SYSTEM_TIME(r13); \
> + PPC_LL ra, PACA_SYSTEM_TIME(ptr); \
> add ra,ra,rb; /* add on to system time */ \
> - std ra,PACA_SYSTEM_TIME(r13)
> + PPC_STL ra, PACA_SYSTEM_TIME(ptr)
>
> #ifdef CONFIG_PPC_SPLPAR
> #define ACCOUNT_STOLEN_TIME \
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index c4cb2ff..ff6b591 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -1275,6 +1275,7 @@ static inline unsigned long mfvtb (void)
> asm volatile("mfspr %0, %1" : "=r" (rval) : \
> "i" (SPRN_TBRU)); rval;})
> #endif
> +#define mftb() mftbl()
> #endif /* !__powerpc64__ */
>
> #define mttbl(v) asm volatile("mttbl %0":: "r"(v))
> diff --git a/arch/powerpc/include/asm/thread_info.h
> b/arch/powerpc/include/asm/thread_info.h
> index 7efee4a..4f19e96 100644
> --- a/arch/powerpc/include/asm/thread_info.h
> +++ b/arch/powerpc/include/asm/thread_info.h
> @@ -44,6 +44,17 @@ struct thread_info {
> <0 => BUG */
> unsigned long local_flags; /* private flags for thread */
>
> +#if defined(CONFIG_VIRT_CPU_ACCOUNTING_NATIVE) && defined(CONFIG_PPC32)
> + /* Stuff for accurate time accounting */
> + unsigned long user_time; /* accumulated usermode TB ticks */
> + unsigned long system_time; /* accumulated system TB ticks */
> + unsigned long user_time_scaled; /* accumulated usermode SPURR ticks */
> + unsigned long starttime; /* TB value snapshot */
> + unsigned long starttime_user; /* TB value on exit to usermode */
> + unsigned long startspurr; /* SPURR value snapshot */
> + unsigned long utime_sspurr; /* ->user_time when ->startspurr set */
> +#endif
> +
> /* low level flags - has atomic operations done on it */
> unsigned long flags ____cacheline_aligned_in_smp;
> };
> diff --git a/arch/powerpc/kernel/asm-offsets.c
> b/arch/powerpc/kernel/asm-offsets.c
> index 07cebc3..b04b957 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -256,6 +256,13 @@ int main(void)
> DEFINE(PACA_TRAP_SAVE, offsetof(struct paca_struct, trap_save));
> DEFINE(PACA_NAPSTATELOST, offsetof(struct paca_struct, nap_state_lost));
> DEFINE(PACA_SPRG_VDSO, offsetof(struct paca_struct, sprg_vdso));
> +#else /* CONFIG_PPC64 */
> +#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> + DEFINE(PACA_STARTTIME, offsetof(struct thread_info, starttime));
> + DEFINE(PACA_STARTTIME_USER, offsetof(struct thread_info, starttime_user));
> + DEFINE(PACA_USER_TIME, offsetof(struct thread_info, user_time));
> + DEFINE(PACA_SYSTEM_TIME, offsetof(struct thread_info, system_time));
> +#endif
> #endif /* CONFIG_PPC64 */
>
> /* RTAS */
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 2405631..9899032 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -175,6 +175,12 @@ transfer_to_handler:
> addi r12,r12,-1
> stw r12,4(r11)
> #endif
> +#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> + CURRENT_THREAD_INFO(r9, r1)
> + tophys(r9, r9)
> + ACCOUNT_CPU_USER_ENTRY(r9, r11, r12)
> +#endif
> +
> b 3f
>
> 2: /* if from kernel, check interrupted DOZE/NAP mode and
> @@ -398,6 +404,13 @@ BEGIN_FTR_SECTION
> lwarx r7,0,r1
> END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
> stwcx. r0,0,r1 /* to clear the reservation */
> +#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> + andi. r4,r8,MSR_PR
> + beq 3f
> + CURRENT_THREAD_INFO(r4, r1)
> + ACCOUNT_CPU_USER_EXIT(r4, r5, r7)
> +3:
> +#endif
> lwz r4,_LINK(r1)
> lwz r5,_CCR(r1)
> mtlr r4
> @@ -769,6 +782,10 @@ restore_user:
> andis. r10,r0,DBCR0_IDM@h
> bnel- load_dbcr0
> #endif
> +#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> + CURRENT_THREAD_INFO(r9, r1)
> + ACCOUNT_CPU_USER_EXIT(r9, r10, r11)
> +#endif
>
> b restore
>
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 0d525ce..d9bf82b 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -70,7 +70,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_TM)
> std r0,GPR0(r1)
> std r10,GPR1(r1)
> beq 2f /* if from kernel mode */
> - ACCOUNT_CPU_USER_ENTRY(r10, r11)
> + ACCOUNT_CPU_USER_ENTRY(r13, r10, r11)
> 2: std r2,GPR2(r1)
> std r3,GPR3(r1)
> mfcr r2
> @@ -222,7 +222,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
> ld r4,_LINK(r1)
>
> beq- 1f
> - ACCOUNT_CPU_USER_EXIT(r11, r12)
> + ACCOUNT_CPU_USER_EXIT(r13, r11, r12)
>
> BEGIN_FTR_SECTION
> HMT_MEDIUM_LOW
> @@ -822,7 +822,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> BEGIN_FTR_SECTION
> mtspr SPRN_PPR,r2 /* Restore PPR */
> END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> - ACCOUNT_CPU_USER_EXIT(r2, r4)
> + ACCOUNT_CPU_USER_EXIT(r13, r2, r4)
> REST_GPR(13, r1)
> 1:
> mtspr SPRN_SRR1,r3
> diff --git a/arch/powerpc/kernel/exceptions-64e.S
> b/arch/powerpc/kernel/exceptions-64e.S
> index 488e631..a9bc548 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -386,7 +386,7 @@ exc_##n##_common: \
> std r10,_NIP(r1); /* save SRR0 to stackframe */ \
> std r11,_MSR(r1); /* save SRR1 to stackframe */ \
> beq 2f; /* if from kernel mode */ \
> - ACCOUNT_CPU_USER_ENTRY(r10,r11);/* accounting (uses cr0+eq) */ \
> + ACCOUNT_CPU_USER_ENTRY(r13,r10,r11);/* accounting (uses cr0+eq) */ \
> 2: ld r3,excf+EX_R10(r13); /* get back r10 */ \
> ld r4,excf+EX_R11(r13); /* get back r11 */ \
> mfspr r5,scratch; /* get back r13 */ \
> @@ -1059,7 +1059,7 @@ fast_exception_return:
> andi. r6,r10,MSR_PR
> REST_2GPRS(6, r1)
> beq 1f
> - ACCOUNT_CPU_USER_EXIT(r10, r11)
> + ACCOUNT_CPU_USER_EXIT(r13, r10, r11)
> ld r0,GPR13(r1)
>
> 1: stdcx. r0,0,r1 /* to clear the reservation */
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index 81b0900..6307b09 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -165,7 +165,13 @@ DEFINE_PER_CPU(unsigned long,
> cputime_scaled_last_delta);
>
> cputime_t cputime_one_jiffy;
>
> +#ifdef CONFIG_PPC_SPLPAR
> void (*dtl_consumer)(struct dtl_entry *, u64);
> +#endif
> +
> +#ifdef CONFIG_PPC32
> +#define get_paca() task_thread_info(tsk)
> +#endif
>
> static void calc_cputime_factors(void)
> {
> @@ -185,7 +191,7 @@ static void calc_cputime_factors(void)
> * Read the SPURR on systems that have it, otherwise the PURR,
> * or if that doesn't exist return the timebase value passed in.
> */
> -static u64 read_spurr(u64 tb)
> +static unsigned long read_spurr(unsigned long tb)
> {
> if (cpu_has_feature(CPU_FTR_SPURR))
> return mfspr(SPRN_SPURR);
> @@ -294,11 +300,12 @@ static inline u64 calculate_stolen_time(u64 stop_tb)
> * Account time for a transition between system, hard irq
> * or soft irq state.
> */
> -static u64 vtime_delta(struct task_struct *tsk,
> - u64 *sys_scaled, u64 *stolen)
> +static unsigned long vtime_delta(struct task_struct *tsk,
> + unsigned long *sys_scaled,
> + unsigned long *stolen)
> {
> - u64 now, nowscaled, deltascaled;
> - u64 udelta, delta, user_scaled;
> + unsigned long now, nowscaled, deltascaled;
> + unsigned long udelta, delta, user_scaled;
>
> WARN_ON_ONCE(!irqs_disabled());
>
> @@ -343,7 +350,7 @@ static u64 vtime_delta(struct task_struct *tsk,
>
> void vtime_account_system(struct task_struct *tsk)
> {
> - u64 delta, sys_scaled, stolen;
> + unsigned long delta, sys_scaled, stolen;
>
> delta = vtime_delta(tsk, &sys_scaled, &stolen);
> account_system_time(tsk, 0, delta, sys_scaled);
> @@ -354,7 +361,7 @@ EXPORT_SYMBOL_GPL(vtime_account_system);
>
> void vtime_account_idle(struct task_struct *tsk)
> {
> - u64 delta, sys_scaled, stolen;
> + unsigned long delta, sys_scaled, stolen;
>
> delta = vtime_delta(tsk, &sys_scaled, &stolen);
> account_idle_time(delta + stolen);
> @@ -381,6 +388,23 @@ void vtime_account_user(struct task_struct *tsk)
> account_user_time(tsk, utime, utimescaled);
> }
>
> +#ifdef CONFIG_PPC32
> +/*
> + * Called from the context switch with interrupts disabled, to charge all
> + * accumulated times to the current process, and to prepare accounting on
> + * the next process.
> + */
> +void arch_vtime_task_switch(struct task_struct *prev)
> +{
> + struct thread_info *pi = task_thread_info(prev);
> + struct thread_info *ni = task_thread_info(current);
> +
> + ni->starttime = pi->starttime;
> + ni->system_time = 0;
> + ni->user_time = 0;
> +}
> +#endif /* CONFIG_PPC32 */
> +
> #else /* ! CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
> #define calc_cputime_factors()
> #endif
> diff --git a/arch/powerpc/platforms/Kconfig.cputype
> b/arch/powerpc/platforms/Kconfig.cputype
> index 142dff5..54b8043 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -1,7 +1,6 @@
> config PPC64
> bool "64-bit kernel"
> default n
> - select HAVE_VIRT_CPU_ACCOUNTING
> select ZLIB_DEFLATE
> help
> This option selects whether a 32-bit or a 64-bit kernel
> --
> 2.1.0
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING
2016-02-11 16:16 [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING Christophe Leroy
2016-02-12 8:25 ` Denis Kirjanov
@ 2016-02-14 20:40 ` Denis Kirjanov
2016-02-15 9:33 ` Christophe Leroy
2016-02-16 21:21 ` Scott Wood
2 siblings, 1 reply; 10+ messages in thread
From: Denis Kirjanov @ 2016-02-14 20:40 UTC (permalink / raw)
To: Christophe Leroy
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Scott Wood, linuxppc-dev, linux-kernel
On 2/11/16, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> This patch provides VIRT_CPU_ACCOUTING to PPC32 architecture.
> PPC32 doesn't have the PACA structure, so we use the task_info
> structure to store the accounting data.
>
> In order to reuse on PPC32 the PPC64 functions, all u64 data has
> been replaced by 'unsigned long' so that it is u32 on PPC32 and
> u64 on PPC64
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Doesn't build for me with the patch applied
To see full details build your kernel with:
'make CONFIG_DEBUG_SECTION_MISMATCH=y'
GEN .version
CHK include/generated/compile.h
UPD include/generated/compile.h
CC init/version.o
LD init/built-in.o
drivers/built-in.o: In function `get_cpu_idle_time':
(.text+0x1261c4): undefined reference to `__umoddi3'
drivers/built-in.o: In function `get_cpu_idle_time':
(.text+0x1261e0): undefined reference to `__udivdi3'
Makefile:936: recipe for target 'vmlinux' failed
make: *** [vmlinux] Error 1
> ---
> Changes in v3: unlike previous version of the patch that was inspired
> from IA64 architecture, this new version tries to reuse as much as
> possible the PPC64 implementation.
>
> PPC32 doesn't have PACA and past discusion on v2 version has shown
> that it is not worth implementing a PACA in PPC32 architecture
> (see below benh opinion)
>
> benh: PACA is actually a data structure and you really really don't want it
> on ppc32 :-) Having a register point to current works, having a register
> point to per-cpu data instead works too (ie, change what we do today),
> but don't introduce a PACA *please* :-)
>
> Changes in v4: ACCOUNT_CPU_USER_ENTRY/EXIT() needed updates in other
> places than entry_32.S and entry_64.S (reported by kbuild-robot)
> Related defines in asm-offset.c need to be conditional to
> CONFIG_VIRT_CPU_ACCOUNTING_NATIVE (reported by kbuild-robot)
>
> Changes in v5: Using PPC_LL et PPC_STL instead of defining new macros
> AC_LD and AC_STD
>
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/cputime.h | 4 ++++
> arch/powerpc/include/asm/exception-64s.h | 2 +-
> arch/powerpc/include/asm/ppc_asm.h | 24 ++++++++++----------
> arch/powerpc/include/asm/reg.h | 1 +
> arch/powerpc/include/asm/thread_info.h | 11 +++++++++
> arch/powerpc/kernel/asm-offsets.c | 7 ++++++
> arch/powerpc/kernel/entry_32.S | 17 ++++++++++++++
> arch/powerpc/kernel/entry_64.S | 6 ++---
> arch/powerpc/kernel/exceptions-64e.S | 4 ++--
> arch/powerpc/kernel/time.c | 38
> ++++++++++++++++++++++++++------
> arch/powerpc/platforms/Kconfig.cputype | 1 -
> 12 files changed, 90 insertions(+), 26 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 3a557be..57ce4ff 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -159,6 +159,7 @@ config PPC
> select ARCH_HAS_DEVMEM_IS_ALLOWED
> select HAVE_ARCH_SECCOMP_FILTER
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> + select HAVE_VIRT_CPU_ACCOUNTING
>
> config GENERIC_CSUM
> def_bool CPU_LITTLE_ENDIAN
> diff --git a/arch/powerpc/include/asm/cputime.h
> b/arch/powerpc/include/asm/cputime.h
> index e245255..c4c33be 100644
> --- a/arch/powerpc/include/asm/cputime.h
> +++ b/arch/powerpc/include/asm/cputime.h
> @@ -230,7 +230,11 @@ static inline cputime_t clock_t_to_cputime(const
> unsigned long clk)
>
> #define cputime64_to_clock_t(ct) cputime_to_clock_t((cputime_t)(ct))
>
> +#ifdef CONFIG_PPC64
> static inline void arch_vtime_task_switch(struct task_struct *tsk) { }
> +#else
> +void arch_vtime_task_switch(struct task_struct *tsk);
> +#endif
>
> #endif /* __KERNEL__ */
> #endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
> diff --git a/arch/powerpc/include/asm/exception-64s.h
> b/arch/powerpc/include/asm/exception-64s.h
> index 93ae809..8bc38d1 100644
> --- a/arch/powerpc/include/asm/exception-64s.h
> +++ b/arch/powerpc/include/asm/exception-64s.h
> @@ -287,7 +287,7 @@ do_kvm_##n: \
> std r0,GPR0(r1); /* save r0 in stackframe */ \
> std r10,GPR1(r1); /* save r1 in stackframe */ \
> beq 4f; /* if from kernel mode */ \
> - ACCOUNT_CPU_USER_ENTRY(r9, r10); \
> + ACCOUNT_CPU_USER_ENTRY(r13, r9, r10); \
> SAVE_PPR(area, r9, r10); \
> 4: EXCEPTION_PROLOG_COMMON_2(area) \
> EXCEPTION_PROLOG_COMMON_3(n) \
> diff --git a/arch/powerpc/include/asm/ppc_asm.h
> b/arch/powerpc/include/asm/ppc_asm.h
> index 499d9f8..07d1bfc 100644
> --- a/arch/powerpc/include/asm/ppc_asm.h
> +++ b/arch/powerpc/include/asm/ppc_asm.h
> @@ -24,27 +24,27 @@
> */
>
> #ifndef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> -#define ACCOUNT_CPU_USER_ENTRY(ra, rb)
> -#define ACCOUNT_CPU_USER_EXIT(ra, rb)
> +#define ACCOUNT_CPU_USER_ENTRY(ptr, ra, rb)
> +#define ACCOUNT_CPU_USER_EXIT(ptr, ra, rb)
> #define ACCOUNT_STOLEN_TIME
> #else
> -#define ACCOUNT_CPU_USER_ENTRY(ra, rb) \
> +#define ACCOUNT_CPU_USER_ENTRY(ptr, ra, rb) \
> MFTB(ra); /* get timebase */ \
> - ld rb,PACA_STARTTIME_USER(r13); \
> - std ra,PACA_STARTTIME(r13); \
> + PPC_LL rb, PACA_STARTTIME_USER(ptr); \
> + PPC_STL ra, PACA_STARTTIME(ptr); \
> subf rb,rb,ra; /* subtract start value */ \
> - ld ra,PACA_USER_TIME(r13); \
> + PPC_LL ra, PACA_USER_TIME(ptr); \
> add ra,ra,rb; /* add on to user time */ \
> - std ra,PACA_USER_TIME(r13); \
> + PPC_STL ra, PACA_USER_TIME(ptr); \
>
> -#define ACCOUNT_CPU_USER_EXIT(ra, rb) \
> +#define ACCOUNT_CPU_USER_EXIT(ptr, ra, rb) \
> MFTB(ra); /* get timebase */ \
> - ld rb,PACA_STARTTIME(r13); \
> - std ra,PACA_STARTTIME_USER(r13); \
> + PPC_LL rb, PACA_STARTTIME(ptr); \
> + PPC_STL ra, PACA_STARTTIME_USER(ptr); \
> subf rb,rb,ra; /* subtract start value */ \
> - ld ra,PACA_SYSTEM_TIME(r13); \
> + PPC_LL ra, PACA_SYSTEM_TIME(ptr); \
> add ra,ra,rb; /* add on to system time */ \
> - std ra,PACA_SYSTEM_TIME(r13)
> + PPC_STL ra, PACA_SYSTEM_TIME(ptr)
>
> #ifdef CONFIG_PPC_SPLPAR
> #define ACCOUNT_STOLEN_TIME \
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index c4cb2ff..ff6b591 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -1275,6 +1275,7 @@ static inline unsigned long mfvtb (void)
> asm volatile("mfspr %0, %1" : "=r" (rval) : \
> "i" (SPRN_TBRU)); rval;})
> #endif
> +#define mftb() mftbl()
> #endif /* !__powerpc64__ */
>
> #define mttbl(v) asm volatile("mttbl %0":: "r"(v))
> diff --git a/arch/powerpc/include/asm/thread_info.h
> b/arch/powerpc/include/asm/thread_info.h
> index 7efee4a..4f19e96 100644
> --- a/arch/powerpc/include/asm/thread_info.h
> +++ b/arch/powerpc/include/asm/thread_info.h
> @@ -44,6 +44,17 @@ struct thread_info {
> <0 => BUG */
> unsigned long local_flags; /* private flags for thread */
>
> +#if defined(CONFIG_VIRT_CPU_ACCOUNTING_NATIVE) && defined(CONFIG_PPC32)
> + /* Stuff for accurate time accounting */
> + unsigned long user_time; /* accumulated usermode TB ticks */
> + unsigned long system_time; /* accumulated system TB ticks */
> + unsigned long user_time_scaled; /* accumulated usermode SPURR ticks */
> + unsigned long starttime; /* TB value snapshot */
> + unsigned long starttime_user; /* TB value on exit to usermode */
> + unsigned long startspurr; /* SPURR value snapshot */
> + unsigned long utime_sspurr; /* ->user_time when ->startspurr set */
> +#endif
> +
> /* low level flags - has atomic operations done on it */
> unsigned long flags ____cacheline_aligned_in_smp;
> };
> diff --git a/arch/powerpc/kernel/asm-offsets.c
> b/arch/powerpc/kernel/asm-offsets.c
> index 07cebc3..b04b957 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -256,6 +256,13 @@ int main(void)
> DEFINE(PACA_TRAP_SAVE, offsetof(struct paca_struct, trap_save));
> DEFINE(PACA_NAPSTATELOST, offsetof(struct paca_struct, nap_state_lost));
> DEFINE(PACA_SPRG_VDSO, offsetof(struct paca_struct, sprg_vdso));
> +#else /* CONFIG_PPC64 */
> +#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> + DEFINE(PACA_STARTTIME, offsetof(struct thread_info, starttime));
> + DEFINE(PACA_STARTTIME_USER, offsetof(struct thread_info, starttime_user));
> + DEFINE(PACA_USER_TIME, offsetof(struct thread_info, user_time));
> + DEFINE(PACA_SYSTEM_TIME, offsetof(struct thread_info, system_time));
> +#endif
> #endif /* CONFIG_PPC64 */
>
> /* RTAS */
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 2405631..9899032 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -175,6 +175,12 @@ transfer_to_handler:
> addi r12,r12,-1
> stw r12,4(r11)
> #endif
> +#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> + CURRENT_THREAD_INFO(r9, r1)
> + tophys(r9, r9)
> + ACCOUNT_CPU_USER_ENTRY(r9, r11, r12)
> +#endif
> +
> b 3f
>
> 2: /* if from kernel, check interrupted DOZE/NAP mode and
> @@ -398,6 +404,13 @@ BEGIN_FTR_SECTION
> lwarx r7,0,r1
> END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
> stwcx. r0,0,r1 /* to clear the reservation */
> +#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> + andi. r4,r8,MSR_PR
> + beq 3f
> + CURRENT_THREAD_INFO(r4, r1)
> + ACCOUNT_CPU_USER_EXIT(r4, r5, r7)
> +3:
> +#endif
> lwz r4,_LINK(r1)
> lwz r5,_CCR(r1)
> mtlr r4
> @@ -769,6 +782,10 @@ restore_user:
> andis. r10,r0,DBCR0_IDM@h
> bnel- load_dbcr0
> #endif
> +#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> + CURRENT_THREAD_INFO(r9, r1)
> + ACCOUNT_CPU_USER_EXIT(r9, r10, r11)
> +#endif
>
> b restore
>
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 0d525ce..d9bf82b 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -70,7 +70,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_TM)
> std r0,GPR0(r1)
> std r10,GPR1(r1)
> beq 2f /* if from kernel mode */
> - ACCOUNT_CPU_USER_ENTRY(r10, r11)
> + ACCOUNT_CPU_USER_ENTRY(r13, r10, r11)
> 2: std r2,GPR2(r1)
> std r3,GPR3(r1)
> mfcr r2
> @@ -222,7 +222,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
> ld r4,_LINK(r1)
>
> beq- 1f
> - ACCOUNT_CPU_USER_EXIT(r11, r12)
> + ACCOUNT_CPU_USER_EXIT(r13, r11, r12)
>
> BEGIN_FTR_SECTION
> HMT_MEDIUM_LOW
> @@ -822,7 +822,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> BEGIN_FTR_SECTION
> mtspr SPRN_PPR,r2 /* Restore PPR */
> END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> - ACCOUNT_CPU_USER_EXIT(r2, r4)
> + ACCOUNT_CPU_USER_EXIT(r13, r2, r4)
> REST_GPR(13, r1)
> 1:
> mtspr SPRN_SRR1,r3
> diff --git a/arch/powerpc/kernel/exceptions-64e.S
> b/arch/powerpc/kernel/exceptions-64e.S
> index 488e631..a9bc548 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -386,7 +386,7 @@ exc_##n##_common: \
> std r10,_NIP(r1); /* save SRR0 to stackframe */ \
> std r11,_MSR(r1); /* save SRR1 to stackframe */ \
> beq 2f; /* if from kernel mode */ \
> - ACCOUNT_CPU_USER_ENTRY(r10,r11);/* accounting (uses cr0+eq) */ \
> + ACCOUNT_CPU_USER_ENTRY(r13,r10,r11);/* accounting (uses cr0+eq) */ \
> 2: ld r3,excf+EX_R10(r13); /* get back r10 */ \
> ld r4,excf+EX_R11(r13); /* get back r11 */ \
> mfspr r5,scratch; /* get back r13 */ \
> @@ -1059,7 +1059,7 @@ fast_exception_return:
> andi. r6,r10,MSR_PR
> REST_2GPRS(6, r1)
> beq 1f
> - ACCOUNT_CPU_USER_EXIT(r10, r11)
> + ACCOUNT_CPU_USER_EXIT(r13, r10, r11)
> ld r0,GPR13(r1)
>
> 1: stdcx. r0,0,r1 /* to clear the reservation */
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index 81b0900..6307b09 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -165,7 +165,13 @@ DEFINE_PER_CPU(unsigned long,
> cputime_scaled_last_delta);
>
> cputime_t cputime_one_jiffy;
>
> +#ifdef CONFIG_PPC_SPLPAR
> void (*dtl_consumer)(struct dtl_entry *, u64);
> +#endif
> +
> +#ifdef CONFIG_PPC32
> +#define get_paca() task_thread_info(tsk)
> +#endif
>
> static void calc_cputime_factors(void)
> {
> @@ -185,7 +191,7 @@ static void calc_cputime_factors(void)
> * Read the SPURR on systems that have it, otherwise the PURR,
> * or if that doesn't exist return the timebase value passed in.
> */
> -static u64 read_spurr(u64 tb)
> +static unsigned long read_spurr(unsigned long tb)
> {
> if (cpu_has_feature(CPU_FTR_SPURR))
> return mfspr(SPRN_SPURR);
> @@ -294,11 +300,12 @@ static inline u64 calculate_stolen_time(u64 stop_tb)
> * Account time for a transition between system, hard irq
> * or soft irq state.
> */
> -static u64 vtime_delta(struct task_struct *tsk,
> - u64 *sys_scaled, u64 *stolen)
> +static unsigned long vtime_delta(struct task_struct *tsk,
> + unsigned long *sys_scaled,
> + unsigned long *stolen)
> {
> - u64 now, nowscaled, deltascaled;
> - u64 udelta, delta, user_scaled;
> + unsigned long now, nowscaled, deltascaled;
> + unsigned long udelta, delta, user_scaled;
>
> WARN_ON_ONCE(!irqs_disabled());
>
> @@ -343,7 +350,7 @@ static u64 vtime_delta(struct task_struct *tsk,
>
> void vtime_account_system(struct task_struct *tsk)
> {
> - u64 delta, sys_scaled, stolen;
> + unsigned long delta, sys_scaled, stolen;
>
> delta = vtime_delta(tsk, &sys_scaled, &stolen);
> account_system_time(tsk, 0, delta, sys_scaled);
> @@ -354,7 +361,7 @@ EXPORT_SYMBOL_GPL(vtime_account_system);
>
> void vtime_account_idle(struct task_struct *tsk)
> {
> - u64 delta, sys_scaled, stolen;
> + unsigned long delta, sys_scaled, stolen;
>
> delta = vtime_delta(tsk, &sys_scaled, &stolen);
> account_idle_time(delta + stolen);
> @@ -381,6 +388,23 @@ void vtime_account_user(struct task_struct *tsk)
> account_user_time(tsk, utime, utimescaled);
> }
>
> +#ifdef CONFIG_PPC32
> +/*
> + * Called from the context switch with interrupts disabled, to charge all
> + * accumulated times to the current process, and to prepare accounting on
> + * the next process.
> + */
> +void arch_vtime_task_switch(struct task_struct *prev)
> +{
> + struct thread_info *pi = task_thread_info(prev);
> + struct thread_info *ni = task_thread_info(current);
> +
> + ni->starttime = pi->starttime;
> + ni->system_time = 0;
> + ni->user_time = 0;
> +}
> +#endif /* CONFIG_PPC32 */
> +
> #else /* ! CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
> #define calc_cputime_factors()
> #endif
> diff --git a/arch/powerpc/platforms/Kconfig.cputype
> b/arch/powerpc/platforms/Kconfig.cputype
> index 142dff5..54b8043 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -1,7 +1,6 @@
> config PPC64
> bool "64-bit kernel"
> default n
> - select HAVE_VIRT_CPU_ACCOUNTING
> select ZLIB_DEFLATE
> help
> This option selects whether a 32-bit or a 64-bit kernel
> --
> 2.1.0
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING
2016-02-14 20:40 ` Denis Kirjanov
@ 2016-02-15 9:33 ` Christophe Leroy
0 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2016-02-15 9:33 UTC (permalink / raw)
To: Denis Kirjanov
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Scott Wood, linuxppc-dev, linux-kernel, David Woodhouse
Le 14/02/2016 21:40, Denis Kirjanov a écrit :
> On 2/11/16, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>> This patch provides VIRT_CPU_ACCOUTING to PPC32 architecture.
>> PPC32 doesn't have the PACA structure, so we use the task_info
>> structure to store the accounting data.
>>
>> In order to reuse on PPC32 the PPC64 functions, all u64 data has
>> been replaced by 'unsigned long' so that it is u32 on PPC32 and
>> u64 on PPC64
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Doesn't build for me with the patch applied
>
> To see full details build your kernel with:
> 'make CONFIG_DEBUG_SECTION_MISMATCH=y'
> GEN .version
> CHK include/generated/compile.h
> UPD include/generated/compile.h
> CC init/version.o
> LD init/built-in.o
> drivers/built-in.o: In function `get_cpu_idle_time':
> (.text+0x1261c4): undefined reference to `__umoddi3'
> drivers/built-in.o: In function `get_cpu_idle_time':
> (.text+0x1261e0): undefined reference to `__udivdi3'
> Makefile:936: recipe for target 'vmlinux' failed
> make: *** [vmlinux] Error 1
>
>
Looks like you have CONFIG_CPU_FREQ, which I don't have.
The issue comes from the jiffies64_to_cputime64() defined in
arch/powerpc/include/asm/cputime.h :
static inline cputime64_t jiffies64_to_cputime64(const u64 jif)
{
u64 ct;
u64 sec;
/* have to be a little careful about overflow */
ct = jif % HZ;
sec = jif / HZ;
On 32 bits, 64 bits % and / require __udivdi3() and __umoddi3(), which
are not implemented in the kernel.
As HZ fits in 32 bits, the solution is to use do_div(). I should not
change anything on PPC64 and would solve your issue.
I will submit an update of the patch within an hour.
Christophe
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING
2016-02-11 16:16 [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING Christophe Leroy
2016-02-12 8:25 ` Denis Kirjanov
2016-02-14 20:40 ` Denis Kirjanov
@ 2016-02-16 21:21 ` Scott Wood
2016-02-17 16:29 ` Christophe Leroy
2016-02-23 2:04 ` Michael Ellerman
2 siblings, 2 replies; 10+ messages in thread
From: Scott Wood @ 2016-02-16 21:21 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linux-kernel, linuxppc-dev
On Thu, 2016-02-11 at 17:16 +0100, Christophe Leroy wrote:
> This patch provides VIRT_CPU_ACCOUTING to PPC32 architecture.
> PPC32 doesn't have the PACA structure, so we use the task_info
> structure to store the accounting data.
>
> In order to reuse on PPC32 the PPC64 functions, all u64 data has
> been replaced by 'unsigned long' so that it is u32 on PPC32 and
> u64 on PPC64
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> Changes in v3: unlike previous version of the patch that was inspired
> from IA64 architecture, this new version tries to reuse as much as
> possible the PPC64 implementation.
>
> PPC32 doesn't have PACA and past discusion on v2 version has shown
> that it is not worth implementing a PACA in PPC32 architecture
> (see below benh opinion)
>
> benh: PACA is actually a data structure and you really really don't want it
> on ppc32 :-) Having a register point to current works, having a register
> point to per-cpu data instead works too (ie, change what we do today),
> but don't introduce a PACA *please* :-)
And Ben never replied to my reply at the time:
"What is special about 64-bit that warrants doing things differently from 32
-bit? What is the difference between PACA and "per-cpu data", other than the
obscure name?"
I can understand wanting to avoid churn, but other than that, doing things
differently on 64-bit versus 32-bit sucks.
> b/arch/powerpc/include/asm/cputime.h
> index e245255..c4c33be 100644
> --- a/arch/powerpc/include/asm/cputime.h
> +++ b/arch/powerpc/include/asm/cputime.h
> @@ -230,7 +230,11 @@ static inline cputime_t clock_t_to_cputime(const
> unsigned long clk)
>
> #define cputime64_to_clock_t(ct) cputime_to_clock_t((cputime_t)(ct))
>
> +#ifdef CONFIG_PPC64
> static inline void arch_vtime_task_switch(struct task_struct *tsk) { }
> +#else
> +void arch_vtime_task_switch(struct task_struct *tsk);
> +#endif
Add a comment explaining why this is empty on 64-bit but non-empty on 32-bit.
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm
> -offsets.c
> index 07cebc3..b04b957 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -256,6 +256,13 @@ int main(void)
> DEFINE(PACA_TRAP_SAVE, offsetof(struct paca_struct, trap_save));
> DEFINE(PACA_NAPSTATELOST, offsetof(struct paca_struct,
> nap_state_lost));
> DEFINE(PACA_SPRG_VDSO, offsetof(struct paca_struct, sprg_vdso));
> +#else /* CONFIG_PPC64 */
> +#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> + DEFINE(PACA_STARTTIME, offsetof(struct thread_info, starttime));
> + DEFINE(PACA_STARTTIME_USER, offsetof(struct thread_info,
> starttime_user));
> + DEFINE(PACA_USER_TIME, offsetof(struct thread_info, user_time));
> + DEFINE(PACA_SYSTEM_TIME, offsetof(struct thread_info,
> system_time));
> +#endif
> #endif /* CONFIG_PPC64 */
Can you change the name if it's not always going to be relative to a PACA?
> +#ifdef CONFIG_PPC32
> +#define get_paca() task_thread_info(tsk)
> +#endif
Likewise, this is just going to cause confusion.
Can you bundle up the time accounting fields into a struct, that you share
between the paca and the 32-bit thread_info, and then have a macro to grab a
pointer to that?
-Scott
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING
2016-02-16 21:21 ` Scott Wood
@ 2016-02-17 16:29 ` Christophe Leroy
2016-02-23 1:22 ` Scott Wood
2016-02-23 2:04 ` Michael Ellerman
1 sibling, 1 reply; 10+ messages in thread
From: Christophe Leroy @ 2016-02-17 16:29 UTC (permalink / raw)
To: Scott Wood
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linux-kernel, linuxppc-dev
Le 16/02/2016 22:21, Scott Wood a écrit :
> On Thu, 2016-02-11 at 17:16 +0100, Christophe Leroy wrote:
>> This patch provides VIRT_CPU_ACCOUTING to PPC32 architecture.
>> PPC32 doesn't have the PACA structure, so we use the task_info
>> structure to store the accounting data.
>>
>> In order to reuse on PPC32 the PPC64 functions, all u64 data has
>> been replaced by 'unsigned long' so that it is u32 on PPC32 and
>> u64 on PPC64
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> Changes in v3: unlike previous version of the patch that was inspired
>> from IA64 architecture, this new version tries to reuse as much as
>> possible the PPC64 implementation.
>>
>> PPC32 doesn't have PACA and past discusion on v2 version has shown
>> that it is not worth implementing a PACA in PPC32 architecture
>> (see below benh opinion)
>>
>> benh: PACA is actually a data structure and you really really don't want it
>> on ppc32 :-) Having a register point to current works, having a register
>> point to per-cpu data instead works too (ie, change what we do today),
>> but don't introduce a PACA *please* :-)
> And Ben never replied to my reply at the time:
>
> "What is special about 64-bit that warrants doing things differently from 32
> -bit? What is the difference between PACA and "per-cpu data", other than the
> obscure name?"
>
> I can understand wanting to avoid churn, but other than that, doing things
> differently on 64-bit versus 32-bit sucks.
>
What I can see is that PACA is always available via register r13. Do we
have anything equivalent on PPC32 ?
If we define a per-cpu data for accounting, what will be the quick way
to get access to it in entry_32.S ?
Something like a table of accounting data for each CPU, that we index
with thread_info->cpu ?
This would allow a quite quick access, is it the good way to proceed in
order to have something closer to PPC64 ?
Christophe
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING
2016-02-17 16:29 ` Christophe Leroy
@ 2016-02-23 1:22 ` Scott Wood
0 siblings, 0 replies; 10+ messages in thread
From: Scott Wood @ 2016-02-23 1:22 UTC (permalink / raw)
To: Christophe Leroy
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linux-kernel, linuxppc-dev
On Wed, 2016-02-17 at 17:29 +0100, Christophe Leroy wrote:
>
> Le 16/02/2016 22:21, Scott Wood a écrit :
> > On Thu, 2016-02-11 at 17:16 +0100, Christophe Leroy wrote:
> > > This patch provides VIRT_CPU_ACCOUTING to PPC32 architecture.
> > > PPC32 doesn't have the PACA structure, so we use the task_info
> > > structure to store the accounting data.
> > >
> > > In order to reuse on PPC32 the PPC64 functions, all u64 data has
> > > been replaced by 'unsigned long' so that it is u32 on PPC32 and
> > > u64 on PPC64
> > >
> > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > > ---
> > > Changes in v3: unlike previous version of the patch that was inspired
> > > from IA64 architecture, this new version tries to reuse as much as
> > > possible the PPC64 implementation.
> > >
> > > PPC32 doesn't have PACA and past discusion on v2 version has shown
> > > that it is not worth implementing a PACA in PPC32 architecture
> > > (see below benh opinion)
> > >
> > > benh: PACA is actually a data structure and you really really don't want
> > > it
> > > on ppc32 :-) Having a register point to current works, having a register
> > > point to per-cpu data instead works too (ie, change what we do today),
> > > but don't introduce a PACA *please* :-)
> > And Ben never replied to my reply at the time:
> >
> > "What is special about 64-bit that warrants doing things differently from
> > 32
> > -bit? What is the difference between PACA and "per-cpu data", other than
> > the
> > obscure name?"
> >
> > I can understand wanting to avoid churn, but other than that, doing things
> > differently on 64-bit versus 32-bit sucks.
> >
>
> What I can see is that PACA is always available via register r13. Do we
> have anything equivalent on PPC32 ?
Just current in r2, which is the task_struct, not a task-independent per-cpu
area.
> If we define a per-cpu data for accounting, what will be the quick way
> to get access to it in entry_32.S ?
> Something like a table of accounting data for each CPU, that we index
> with thread_info->cpu ?
> This would allow a quite quick access, is it the good way to proceed in
> order to have something closer to PPC64 ?
Possibly.
-Scott
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING
2016-02-16 21:21 ` Scott Wood
2016-02-17 16:29 ` Christophe Leroy
@ 2016-02-23 2:04 ` Michael Ellerman
2016-02-23 2:15 ` Scott Wood
1 sibling, 1 reply; 10+ messages in thread
From: Michael Ellerman @ 2016-02-23 2:04 UTC (permalink / raw)
To: Scott Wood, Christophe Leroy, Benjamin Herrenschmidt,
Paul Mackerras
Cc: linux-kernel, linuxppc-dev
On Tue, 2016-02-16 at 15:21 -0600, Scott Wood wrote:
> On Thu, 2016-02-11 at 17:16 +0100, Christophe Leroy wrote:
> > This patch provides VIRT_CPU_ACCOUTING to PPC32 architecture.
> > PPC32 doesn't have the PACA structure, so we use the task_info
> > structure to store the accounting data.
> >
> > In order to reuse on PPC32 the PPC64 functions, all u64 data has
> > been replaced by 'unsigned long' so that it is u32 on PPC32 and
> > u64 on PPC64
> >
> > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > ---
> > Changes in v3: unlike previous version of the patch that was inspired
> > from IA64 architecture, this new version tries to reuse as much as
> > possible the PPC64 implementation.
> >
> > PPC32 doesn't have PACA and past discusion on v2 version has shown
> > that it is not worth implementing a PACA in PPC32 architecture
> > (see below benh opinion)
> >
> > benh: PACA is actually a data structure and you really really don't want it
> > on ppc32 :-) Having a register point to current works, having a register
> > point to per-cpu data instead works too (ie, change what we do today),
> > but don't introduce a PACA *please* :-)
>
> And Ben never replied to my reply at the time:
>
> "What is special about 64-bit that warrants doing things differently from 32
> -bit?
Nothing. It's just historical cruft. But we're not realistically going to get
rid of it anytime soon on 64-bit.
> What is the difference between PACA and "per-cpu data", other than the
> obscure name?"
Not much. The pacas are allocated differently to per-cpu data, they're
available earlier in boot etc. What we'd like is to have r13 point to the
per-cpu data area, and then the contents of the paca could just be regular
per-cpu data. But like I said above that's a big change.
cheers
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING
2016-02-23 2:04 ` Michael Ellerman
@ 2016-02-23 2:15 ` Scott Wood
2016-02-23 3:25 ` Michael Ellerman
0 siblings, 1 reply; 10+ messages in thread
From: Scott Wood @ 2016-02-23 2:15 UTC (permalink / raw)
To: Michael Ellerman, Christophe Leroy, Benjamin Herrenschmidt,
Paul Mackerras
Cc: linux-kernel, linuxppc-dev
On Tue, 2016-02-23 at 13:04 +1100, Michael Ellerman wrote:
> On Tue, 2016-02-16 at 15:21 -0600, Scott Wood wrote:
>
> > On Thu, 2016-02-11 at 17:16 +0100, Christophe Leroy wrote:
>
> > > This patch provides VIRT_CPU_ACCOUTING to PPC32 architecture.
> > > PPC32 doesn't have the PACA structure, so we use the task_info
> > > structure to store the accounting data.
> > >
> > > In order to reuse on PPC32 the PPC64 functions, all u64 data has
> > > been replaced by 'unsigned long' so that it is u32 on PPC32 and
> > > u64 on PPC64
> > >
> > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > > ---
> > > Changes in v3: unlike previous version of the patch that was inspired
> > > from IA64 architecture, this new version tries to reuse as much as
> > > possible the PPC64 implementation.
> > >
> > > PPC32 doesn't have PACA and past discusion on v2 version has shown
> > > that it is not worth implementing a PACA in PPC32 architecture
> > > (see below benh opinion)
> > >
> > > benh: PACA is actually a data structure and you really really don't want
> > > it
> > > on ppc32 :-) Having a register point to current works, having a register
> > > point to per-cpu data instead works too (ie, change what we do today),
> > > but don't introduce a PACA *please* :-)
> >
> > And Ben never replied to my reply at the time:
> >
> > "What is special about 64-bit that warrants doing things differently from
> > 32
> > -bit?
>
> Nothing. It's just historical cruft. But we're not realistically going to
> get
> rid of it anytime soon on 64-bit.
I wasn't suggesting getting rid of it on 64-bit, but rather adding it on 32
-bit, to hold things that are used by both. I was confused by the vehemence
of Ben's objection.
> > What is the difference between PACA and "per-cpu data", other than the
> > obscure name?"
>
> Not much. The pacas are allocated differently to per-cpu data, they're
> available earlier in boot etc.
Ah, I was thinking of the general concept of per-cpu data, not the specific
mechanism that Linux implements in percpu.h etc.
> What we'd like is to have r13 point to the
> per-cpu data area, and then the contents of the paca could just be regular
> per-cpu data. But like I said above that's a big change.
That change seems orthogonal to the question of making the mechanism available
on 32-bit to ease unification of code which uses it.
-Scott
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING
2016-02-23 2:15 ` Scott Wood
@ 2016-02-23 3:25 ` Michael Ellerman
0 siblings, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2016-02-23 3:25 UTC (permalink / raw)
To: Scott Wood, Christophe Leroy, Benjamin Herrenschmidt,
Paul Mackerras
Cc: linux-kernel, linuxppc-dev
On Mon, 2016-02-22 at 20:15 -0600, Scott Wood wrote:
> On Tue, 2016-02-23 at 13:04 +1100, Michael Ellerman wrote:
> > On Tue, 2016-02-16 at 15:21 -0600, Scott Wood wrote:
> > > On Thu, 2016-02-11 at 17:16 +0100, Christophe Leroy wrote:
> > > > This patch provides VIRT_CPU_ACCOUTING to PPC32 architecture.
> > > > PPC32 doesn't have the PACA structure, so we use the task_info
> > > > structure to store the accounting data.
> > > >
> > > > In order to reuse on PPC32 the PPC64 functions, all u64 data has
> > > > been replaced by 'unsigned long' so that it is u32 on PPC32 and
> > > > u64 on PPC64
> > > >
> > > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > > > ---
> > > > Changes in v3: unlike previous version of the patch that was inspired
> > > > from IA64 architecture, this new version tries to reuse as much as
> > > > possible the PPC64 implementation.
> > > >
> > > > PPC32 doesn't have PACA and past discusion on v2 version has shown
> > > > that it is not worth implementing a PACA in PPC32 architecture
> > > > (see below benh opinion)
> > > >
> > > > benh: PACA is actually a data structure and you really really don't want
> > > > it
> > > > on ppc32 :-) Having a register point to current works, having a register
> > > > point to per-cpu data instead works too (ie, change what we do today),
> > > > but don't introduce a PACA *please* :-)
> > >
> > > And Ben never replied to my reply at the time:
> > >
> > > "What is special about 64-bit that warrants doing things differently from
> > > 32
> > > -bit?
> >
> > Nothing. It's just historical cruft. But we're not realistically going to
> > get
> > rid of it anytime soon on 64-bit.
>
> I wasn't suggesting getting rid of it on 64-bit, but rather adding it on 32
> -bit, to hold things that are used by both. I was confused by the vehemence
> of Ben's objection.
OK right. I think he's just saying we'd like to (eventually) get rid of it on
64-bit, so adding it on 32-bit would be a step backward.
> > > What is the difference between PACA and "per-cpu data", other than the
> > > obscure name?"
> >
> > Not much. The pacas are allocated differently to per-cpu data, they're
> > available earlier in boot etc.
>
> Ah, I was thinking of the general concept of per-cpu data, not the specific
> mechanism that Linux implements in percpu.h etc.
Oh ok, in that case no it's not special at all.
> > What we'd like is to have r13 point to the
> > per-cpu data area, and then the contents of the paca could just be regular
> > per-cpu data. But like I said above that's a big change.
>
> That change seems orthogonal to the question of making the mechanism available
> on 32-bit to ease unification of code which uses it.
That's true.
Though in this case I think you actually do want to store those values in the thread_info.
If you look at eg. vtime_delta() where we use those values, it's passed a task
struct.
So your suggestion to define a common struct that is shared between the 32-bit
thread_info and the 64-bit paca would be a good solution I think.
cheers
^ permalink raw reply [flat|nested] 10+ messages in thread