linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] powerpc32: provide VIRT_CPU_ACCOUNTING
@ 2016-02-11 16:16 Christophe Leroy
  2016-02-12  8:25 ` Denis Kirjanov
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Christophe Leroy @ 2016-02-11 16:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Scott Wood
  Cc: linux-kernel, linuxppc-dev

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* :-)

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

^ permalink raw reply related	[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
  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

end of thread, other threads:[~2016-02-23  3:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2016-02-17 16:29   ` Christophe Leroy
2016-02-23  1:22     ` Scott Wood
2016-02-23  2:04   ` Michael Ellerman
2016-02-23  2:15     ` Scott Wood
2016-02-23  3:25       ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).