linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] powerpc/32: nohz full support
@ 2022-10-04  6:33 Nicholas Piggin
  2022-10-04  6:33 ` [RFC PATCH 1/3] powerpc/32: Implement HAVE_CONTEXT_TRACKING_USER support Nicholas Piggin
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Nicholas Piggin @ 2022-10-04  6:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

Doesn't seem to be much more involved in adding context tracking and
generic virt cpu accounting support for 32-bit, which is all that's
left to support NO_HZ_FULL.

I tested this with e5500 SMP kernel with isolated and nohz CPU, and
it seems to be doing the right thing -- periodic tick is stopped on
the nohz CPUs when they are running in userspace.

Context tracking warnings should catch quite quickly if we got
something wrong there (with the force context tracking option). I
don't have a 32-bit KVM environment to test so that might have some
issues but it should be quite easy to fix if it can be tested.

I assume the virt cpu accounting gen option removal is okay, but not
exactly sure what to look for in terms of possible problems, so we'll
see what comments that gets back.

Thanks,
Nick

Nicholas Piggin (3):
  powerpc/32: Implement HAVE_CONTEXT_TRACKING_USER support
  powerpc: remove the last remnants of cputime_t
  Remove HAVE_VIRT_CPU_ACCOUNTING_GEN option

 arch/Kconfig                         | 11 -----------
 arch/arm/Kconfig                     |  1 -
 arch/csky/Kconfig                    |  1 -
 arch/loongarch/Kconfig               |  1 -
 arch/mips/Kconfig                    |  1 -
 arch/powerpc/Kconfig                 |  2 +-
 arch/powerpc/include/asm/cputime.h   | 17 +----------------
 arch/powerpc/include/asm/interrupt.h | 21 ++++++---------------
 arch/powerpc/kernel/time.c           | 23 ++---------------------
 arch/xtensa/Kconfig                  |  1 -
 init/Kconfig                         |  1 -
 kernel/time/Kconfig                  |  2 --
 12 files changed, 10 insertions(+), 72 deletions(-)

-- 
2.37.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH 1/3] powerpc/32: Implement HAVE_CONTEXT_TRACKING_USER support
  2022-10-04  6:33 [RFC PATCH 0/3] powerpc/32: nohz full support Nicholas Piggin
@ 2022-10-04  6:33 ` Nicholas Piggin
  2022-10-04 11:32   ` Christophe Leroy
  2022-10-04  6:33 ` [RFC PATCH 2/3] powerpc: remove the last remnants of cputime_t Nicholas Piggin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Nicholas Piggin @ 2022-10-04  6:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

Context tracking involves tracking user, kernel, guest switches. This
enables existing context tracking code for interrupt entry on 32-bit.
KVM and interrupt exit already has context tracking calls.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/Kconfig                 |  2 +-
 arch/powerpc/include/asm/interrupt.h | 21 ++++++---------------
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 81c9f895d690..f667279ec74c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -204,7 +204,7 @@ config PPC
 	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_ASM_MODVERSIONS
-	select HAVE_CONTEXT_TRACKING_USER		if PPC64
+	select HAVE_CONTEXT_TRACKING_USER
 	select HAVE_C_RECORDMCOUNT
 	select HAVE_DEBUG_KMEMLEAK
 	select HAVE_DEBUG_STACKOVERFLOW
diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index 4745bb9998bd..8860a246d51a 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -85,6 +85,8 @@ do {									\
 	    (user_mode(regs) || (TRAP(regs) != INTERRUPT_PROGRAM)))	\
 		BUG_ON(cond);						\
 } while (0)
+#else
+#define INT_SOFT_MASK_BUG_ON(regs, cond)
 #endif
 
 #ifdef CONFIG_PPC_BOOK3S_64
@@ -152,19 +154,8 @@ static inline void booke_restore_dbcr0(void)
 static inline void interrupt_enter_prepare(struct pt_regs *regs)
 {
 #ifdef CONFIG_PPC32
-	if (!arch_irq_disabled_regs(regs))
-		trace_hardirqs_off();
-
-	if (user_mode(regs))
-		kuap_lock();
-	else
-		kuap_save_and_lock(regs);
-
-	if (user_mode(regs))
-		account_cpu_user_entry();
-#endif
-
-#ifdef CONFIG_PPC64
+	bool trace_enable = !arch_irq_disabled_regs(regs);
+#else
 	bool trace_enable = false;
 
 	if (IS_ENABLED(CONFIG_TRACE_IRQFLAGS)) {
@@ -188,8 +179,9 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs)
 	} else {
 		__hard_RI_enable();
 	}
+	/* Only call trace_hardirqs_off when RI=1, it can cause SLB faults */
+#endif
 
-	/* Do this when RI=1 because it can cause SLB faults */
 	if (trace_enable)
 		trace_hardirqs_off();
 
@@ -215,7 +207,6 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs)
 	}
 	INT_SOFT_MASK_BUG_ON(regs, !arch_irq_disabled_regs(regs) &&
 				   !(regs->msr & MSR_EE));
-#endif
 
 	booke_restore_dbcr0();
 }
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC PATCH 2/3] powerpc: remove the last remnants of cputime_t
  2022-10-04  6:33 [RFC PATCH 0/3] powerpc/32: nohz full support Nicholas Piggin
  2022-10-04  6:33 ` [RFC PATCH 1/3] powerpc/32: Implement HAVE_CONTEXT_TRACKING_USER support Nicholas Piggin
@ 2022-10-04  6:33 ` Nicholas Piggin
  2022-10-06 10:06   ` Michael Ellerman
  2022-10-04  6:33 ` [RFC PATCH 3/3] Remove HAVE_VIRT_CPU_ACCOUNTING_GEN option Nicholas Piggin
  2022-10-04 11:58 ` [RFC PATCH 0/3] powerpc/32: nohz full support Christophe Leroy
  3 siblings, 1 reply; 9+ messages in thread
From: Nicholas Piggin @ 2022-10-04  6:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

cputime_t is no longer, converted to u64.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/cputime.h | 17 +----------------
 arch/powerpc/kernel/time.c         | 23 ++---------------------
 2 files changed, 3 insertions(+), 37 deletions(-)

diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
index 431ae2343022..4961fb38e438 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -21,23 +21,8 @@
 #include <asm/param.h>
 #include <asm/firmware.h>
 
-typedef u64 __nocast cputime_t;
-typedef u64 __nocast cputime64_t;
-
-#define cmpxchg_cputime(ptr, old, new) cmpxchg(ptr, old, new)
-
 #ifdef __KERNEL__
-/*
- * Convert cputime <-> microseconds
- */
-extern u64 __cputime_usec_factor;
-
-static inline unsigned long cputime_to_usecs(const cputime_t ct)
-{
-	return mulhdu((__force u64) ct, __cputime_usec_factor);
-}
-
-#define cputime_to_nsecs(cputime) tb_to_ns((__force u64)cputime)
+#define cputime_to_nsecs(cputime) tb_to_ns(cputime)
 
 /*
  * PPC64 uses PACA which is task independent for storing accounting data while
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index a2ab397065c6..d68de3618741 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -130,7 +130,7 @@ unsigned long tb_ticks_per_jiffy;
 unsigned long tb_ticks_per_usec = 100; /* sane default */
 EXPORT_SYMBOL(tb_ticks_per_usec);
 unsigned long tb_ticks_per_sec;
-EXPORT_SYMBOL(tb_ticks_per_sec);	/* for cputime_t conversions */
+EXPORT_SYMBOL(tb_ticks_per_sec);	/* for cputime conversions */
 
 DEFINE_SPINLOCK(rtc_lock);
 EXPORT_SYMBOL_GPL(rtc_lock);
@@ -150,21 +150,6 @@ EXPORT_SYMBOL_GPL(ppc_tb_freq);
 bool tb_invalid;
 
 #ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
-/*
- * Factor for converting from cputime_t (timebase ticks) to
- * microseconds. This is stored as 0.64 fixed-point binary fraction.
- */
-u64 __cputime_usec_factor;
-EXPORT_SYMBOL(__cputime_usec_factor);
-
-static void calc_cputime_factors(void)
-{
-	struct div_result res;
-
-	div128_by_32(1000000, 0, tb_ticks_per_sec, &res);
-	__cputime_usec_factor = res.result_low;
-}
-
 /*
  * Read the SPURR on systems that have it, otherwise the PURR,
  * or if that doesn't exist return the timebase value passed in.
@@ -369,10 +354,7 @@ void vtime_flush(struct task_struct *tsk)
 	acct->hardirq_time = 0;
 	acct->softirq_time = 0;
 }
-
-#else /* ! CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
-#define calc_cputime_factors()
-#endif
+#endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
 
 void __delay(unsigned long loops)
 {
@@ -914,7 +896,6 @@ void __init time_init(void)
 	tb_ticks_per_jiffy = ppc_tb_freq / HZ;
 	tb_ticks_per_sec = ppc_tb_freq;
 	tb_ticks_per_usec = ppc_tb_freq / 1000000;
-	calc_cputime_factors();
 
 	/*
 	 * Compute scale factor for sched_clock.
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC PATCH 3/3] Remove HAVE_VIRT_CPU_ACCOUNTING_GEN option
  2022-10-04  6:33 [RFC PATCH 0/3] powerpc/32: nohz full support Nicholas Piggin
  2022-10-04  6:33 ` [RFC PATCH 1/3] powerpc/32: Implement HAVE_CONTEXT_TRACKING_USER support Nicholas Piggin
  2022-10-04  6:33 ` [RFC PATCH 2/3] powerpc: remove the last remnants of cputime_t Nicholas Piggin
@ 2022-10-04  6:33 ` Nicholas Piggin
  2022-10-04 11:58 ` [RFC PATCH 0/3] powerpc/32: nohz full support Christophe Leroy
  3 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2022-10-04  6:33 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: linux-arch, Kevin Hilman, Frederic Weisbecker, Nicholas Piggin

This option was created in commit 554b0004d0ec4 ("vtime: Add
HAVE_VIRT_CPU_ACCOUNTING_GEN Kconfig") for architectures to indicate
they support the 64-bit cputime_t required for VIRT_CPU_ACCOUNTING_GEN.

The cputime_t type has since been removed, so this doesn't have any
meaning. Remove it.

Cc: linux-arch@vger.kernel.org
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/Kconfig           | 11 -----------
 arch/arm/Kconfig       |  1 -
 arch/csky/Kconfig      |  1 -
 arch/loongarch/Kconfig |  1 -
 arch/mips/Kconfig      |  1 -
 arch/xtensa/Kconfig    |  1 -
 init/Kconfig           |  1 -
 kernel/time/Kconfig    |  2 --
 8 files changed, 19 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 5dbf11a5ba4e..54c73e22c526 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -829,17 +829,6 @@ config HAVE_VIRT_CPU_ACCOUNTING_IDLE
 config ARCH_HAS_SCALED_CPUTIME
 	bool
 
-config HAVE_VIRT_CPU_ACCOUNTING_GEN
-	bool
-	default y if 64BIT
-	help
-	  With VIRT_CPU_ACCOUNTING_GEN, cputime_t becomes 64-bit.
-	  Before enabling this option, arch code must be audited
-	  to ensure there are no races in concurrent read/write of
-	  cputime_t. For example, reading/writing 64-bit cputime_t on
-	  some 32-bit arches may require multiple accesses, so proper
-	  locking is needed to protect against concurrent accesses.
-
 config HAVE_IRQ_TIME_ACCOUNTING
 	bool
 	help
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 87badeae3181..47f3a23564e8 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -119,7 +119,6 @@ config ARM
 	select HAVE_STACKPROTECTOR
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_UID16
-	select HAVE_VIRT_CPU_ACCOUNTING_GEN
 	select IRQ_FORCED_THREADING
 	select MODULES_USE_ELF_REL
 	select NEED_DMA_MAP_STATE
diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
index 3cbc2dc62baf..8102d0d3f3b3 100644
--- a/arch/csky/Kconfig
+++ b/arch/csky/Kconfig
@@ -72,7 +72,6 @@ config CSKY
 	select HAVE_ARCH_MMAP_RND_BITS
 	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_CONTEXT_TRACKING_USER
-	select HAVE_VIRT_CPU_ACCOUNTING_GEN
 	select HAVE_DEBUG_BUGVERBOSE
 	select HAVE_DEBUG_KMEMLEAK
 	select HAVE_DYNAMIC_FTRACE
diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 26aeb1408e56..201b5d4e5c25 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -97,7 +97,6 @@ config LOONGARCH
 	select HAVE_SETUP_PER_CPU_AREA if NUMA
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_TIF_NOHZ
-	select HAVE_VIRT_CPU_ACCOUNTING_GEN if !SMP
 	select IRQ_FORCED_THREADING
 	select IRQ_LOONGARCH_CPU
 	select MMU_GATHER_MERGE_VMAS if MMU
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index ec21f8999249..f67291d8e09c 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -90,7 +90,6 @@ config MIPS
 	select HAVE_SPARSE_SYSCALL_NR
 	select HAVE_STACKPROTECTOR
 	select HAVE_SYSCALL_TRACEPOINTS
-	select HAVE_VIRT_CPU_ACCOUNTING_GEN if 64BIT || !SMP
 	select IRQ_FORCED_THREADING
 	select ISA if EISA
 	select MODULES_USE_ELF_REL if MODULES
diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
index 12ac277282ba..18053fe9ec38 100644
--- a/arch/xtensa/Kconfig
+++ b/arch/xtensa/Kconfig
@@ -47,7 +47,6 @@ config XTENSA
 	select HAVE_PERF_EVENTS
 	select HAVE_STACKPROTECTOR
 	select HAVE_SYSCALL_TRACEPOINTS
-	select HAVE_VIRT_CPU_ACCOUNTING_GEN
 	select IRQ_DOMAIN
 	select MODULES_USE_ELF_RELA
 	select PERF_USE_VMALLOC
diff --git a/init/Kconfig b/init/Kconfig
index 94ce5a46a802..bb6d7f0d80fe 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -490,7 +490,6 @@ config VIRT_CPU_ACCOUNTING_NATIVE
 config VIRT_CPU_ACCOUNTING_GEN
 	bool "Full dynticks CPU time accounting"
 	depends on HAVE_CONTEXT_TRACKING_USER
-	depends on HAVE_VIRT_CPU_ACCOUNTING_GEN
 	depends on GENERIC_CLOCKEVENTS
 	select VIRT_CPU_ACCOUNTING
 	select CONTEXT_TRACKING_USER
diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index a41753be1a2b..ed480ba6cf35 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -121,8 +121,6 @@ config NO_HZ_FULL
 	# We need at least one periodic CPU for timekeeping
 	depends on SMP
 	depends on HAVE_CONTEXT_TRACKING_USER
-	# VIRT_CPU_ACCOUNTING_GEN dependency
-	depends on HAVE_VIRT_CPU_ACCOUNTING_GEN
 	select NO_HZ_COMMON
 	select RCU_NOCB_CPU
 	select VIRT_CPU_ACCOUNTING_GEN
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 1/3] powerpc/32: Implement HAVE_CONTEXT_TRACKING_USER support
  2022-10-04  6:33 ` [RFC PATCH 1/3] powerpc/32: Implement HAVE_CONTEXT_TRACKING_USER support Nicholas Piggin
@ 2022-10-04 11:32   ` Christophe Leroy
  2022-10-06  8:22     ` Nicholas Piggin
  0 siblings, 1 reply; 9+ messages in thread
From: Christophe Leroy @ 2022-10-04 11:32 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev



Le 04/10/2022 à 08:33, Nicholas Piggin a écrit :
> Context tracking involves tracking user, kernel, guest switches. This
> enables existing context tracking code for interrupt entry on 32-bit.
> KVM and interrupt exit already has context tracking calls.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   arch/powerpc/Kconfig                 |  2 +-
>   arch/powerpc/include/asm/interrupt.h | 21 ++++++---------------
>   2 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 81c9f895d690..f667279ec74c 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -204,7 +204,7 @@ config PPC
>   	select HAVE_ARCH_SECCOMP_FILTER
>   	select HAVE_ARCH_TRACEHOOK
>   	select HAVE_ASM_MODVERSIONS
> -	select HAVE_CONTEXT_TRACKING_USER		if PPC64
> +	select HAVE_CONTEXT_TRACKING_USER
>   	select HAVE_C_RECORDMCOUNT
>   	select HAVE_DEBUG_KMEMLEAK
>   	select HAVE_DEBUG_STACKOVERFLOW
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index 4745bb9998bd..8860a246d51a 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -85,6 +85,8 @@ do {									\
>   	    (user_mode(regs) || (TRAP(regs) != INTERRUPT_PROGRAM)))	\
>   		BUG_ON(cond);						\
>   } while (0)
> +#else
> +#define INT_SOFT_MASK_BUG_ON(regs, cond)

Here you can just drop the ifdef CONFIG_PPC64 I guess instead of adding 
an additional empty macro.

>   #endif
>   
>   #ifdef CONFIG_PPC_BOOK3S_64
> @@ -152,19 +154,8 @@ static inline void booke_restore_dbcr0(void)
>   static inline void interrupt_enter_prepare(struct pt_regs *regs)
>   {
>   #ifdef CONFIG_PPC32
> -	if (!arch_irq_disabled_regs(regs))
> -		trace_hardirqs_off();
> -
> -	if (user_mode(regs))
> -		kuap_lock();
> -	else
> -		kuap_save_and_lock(regs);
> -
> -	if (user_mode(regs))
> -		account_cpu_user_entry();
> -#endif
> -
> -#ifdef CONFIG_PPC64
> +	bool trace_enable = !arch_irq_disabled_regs(regs);

nit: You could be put this as an #else to the existing #ifdef CONFIG_PPC64

> +#else
>   	bool trace_enable = false;
>   
>   	if (IS_ENABLED(CONFIG_TRACE_IRQFLAGS)) {
> @@ -188,8 +179,9 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs)
>   	} else {
>   		__hard_RI_enable();
>   	}
> +	/* Only call trace_hardirqs_off when RI=1, it can cause SLB faults */
> +#endif
>   
> -	/* Do this when RI=1 because it can cause SLB faults */
>   	if (trace_enable)
>   		trace_hardirqs_off();
>   
> @@ -215,7 +207,6 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs)
>   	}
>   	INT_SOFT_MASK_BUG_ON(regs, !arch_irq_disabled_regs(regs) &&
>   				   !(regs->msr & MSR_EE));
> -#endif
>   
>   	booke_restore_dbcr0();
>   }

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 0/3] powerpc/32: nohz full support
  2022-10-04  6:33 [RFC PATCH 0/3] powerpc/32: nohz full support Nicholas Piggin
                   ` (2 preceding siblings ...)
  2022-10-04  6:33 ` [RFC PATCH 3/3] Remove HAVE_VIRT_CPU_ACCOUNTING_GEN option Nicholas Piggin
@ 2022-10-04 11:58 ` Christophe Leroy
  2022-10-06  8:18   ` Nicholas Piggin
  3 siblings, 1 reply; 9+ messages in thread
From: Christophe Leroy @ 2022-10-04 11:58 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev



Le 04/10/2022 à 08:33, Nicholas Piggin a écrit :
> Doesn't seem to be much more involved in adding context tracking and
> generic virt cpu accounting support for 32-bit, which is all that's
> left to support NO_HZ_FULL.
> 
> I tested this with e5500 SMP kernel with isolated and nohz CPU, and
> it seems to be doing the right thing -- periodic tick is stopped on
> the nohz CPUs when they are running in userspace.
> 
> Context tracking warnings should catch quite quickly if we got
> something wrong there (with the force context tracking option). I
> don't have a 32-bit KVM environment to test so that might have some
> issues but it should be quite easy to fix if it can be tested.
> 
> I assume the virt cpu accounting gen option removal is okay, but not
> exactly sure what to look for in terms of possible problems, so we'll
> see what comments that gets back.

I'm having hard time finding the link between patch 1 and patch 2/3.

Christophe

> 
> Thanks,
> Nick
> 
> Nicholas Piggin (3):
>    powerpc/32: Implement HAVE_CONTEXT_TRACKING_USER support
>    powerpc: remove the last remnants of cputime_t
>    Remove HAVE_VIRT_CPU_ACCOUNTING_GEN option
> 
>   arch/Kconfig                         | 11 -----------
>   arch/arm/Kconfig                     |  1 -
>   arch/csky/Kconfig                    |  1 -
>   arch/loongarch/Kconfig               |  1 -
>   arch/mips/Kconfig                    |  1 -
>   arch/powerpc/Kconfig                 |  2 +-
>   arch/powerpc/include/asm/cputime.h   | 17 +----------------
>   arch/powerpc/include/asm/interrupt.h | 21 ++++++---------------
>   arch/powerpc/kernel/time.c           | 23 ++---------------------
>   arch/xtensa/Kconfig                  |  1 -
>   init/Kconfig                         |  1 -
>   kernel/time/Kconfig                  |  2 --
>   12 files changed, 10 insertions(+), 72 deletions(-)
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 0/3] powerpc/32: nohz full support
  2022-10-04 11:58 ` [RFC PATCH 0/3] powerpc/32: nohz full support Christophe Leroy
@ 2022-10-06  8:18   ` Nicholas Piggin
  0 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2022-10-06  8:18 UTC (permalink / raw)
  To: Christophe Leroy, linuxppc-dev

On Tue Oct 4, 2022 at 9:58 PM AEST, Christophe Leroy wrote:
>
>
> Le 04/10/2022 à 08:33, Nicholas Piggin a écrit :
> > Doesn't seem to be much more involved in adding context tracking and
> > generic virt cpu accounting support for 32-bit, which is all that's
> > left to support NO_HZ_FULL.
> > 
> > I tested this with e5500 SMP kernel with isolated and nohz CPU, and
> > it seems to be doing the right thing -- periodic tick is stopped on
> > the nohz CPUs when they are running in userspace.
> > 
> > Context tracking warnings should catch quite quickly if we got
> > something wrong there (with the force context tracking option). I
> > don't have a 32-bit KVM environment to test so that might have some
> > issues but it should be quite easy to fix if it can be tested.
> > 
> > I assume the virt cpu accounting gen option removal is okay, but not
> > exactly sure what to look for in terms of possible problems, so we'll
> > see what comments that gets back.
>
> I'm having hard time finding the link between patch 1 and patch 2/3.

Oh, it's just that NO_HZ_FULL needs both context tracking and
VIRT_CPU_ACCOUNTING_GEN. The patches aren't logically related
at all (and enabling the latter requires no more than removing
the 'if PPC64' clause in our arch support so it doesn't depend
on making these arch changes either -- I'll submit that as the
patch if 2 and 3 have not been picked up before then.

Thanks,
Nick

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 1/3] powerpc/32: Implement HAVE_CONTEXT_TRACKING_USER support
  2022-10-04 11:32   ` Christophe Leroy
@ 2022-10-06  8:22     ` Nicholas Piggin
  0 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2022-10-06  8:22 UTC (permalink / raw)
  To: Christophe Leroy, linuxppc-dev

On Tue Oct 4, 2022 at 9:32 PM AEST, Christophe Leroy wrote:
>
>
> Le 04/10/2022 à 08:33, Nicholas Piggin a écrit :
> > Context tracking involves tracking user, kernel, guest switches. This
> > enables existing context tracking code for interrupt entry on 32-bit.
> > KVM and interrupt exit already has context tracking calls.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   arch/powerpc/Kconfig                 |  2 +-
> >   arch/powerpc/include/asm/interrupt.h | 21 ++++++---------------
> >   2 files changed, 7 insertions(+), 16 deletions(-)
> > 
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 81c9f895d690..f667279ec74c 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -204,7 +204,7 @@ config PPC
> >   	select HAVE_ARCH_SECCOMP_FILTER
> >   	select HAVE_ARCH_TRACEHOOK
> >   	select HAVE_ASM_MODVERSIONS
> > -	select HAVE_CONTEXT_TRACKING_USER		if PPC64
> > +	select HAVE_CONTEXT_TRACKING_USER
> >   	select HAVE_C_RECORDMCOUNT
> >   	select HAVE_DEBUG_KMEMLEAK
> >   	select HAVE_DEBUG_STACKOVERFLOW
> > diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> > index 4745bb9998bd..8860a246d51a 100644
> > --- a/arch/powerpc/include/asm/interrupt.h
> > +++ b/arch/powerpc/include/asm/interrupt.h
> > @@ -85,6 +85,8 @@ do {									\
> >   	    (user_mode(regs) || (TRAP(regs) != INTERRUPT_PROGRAM)))	\
> >   		BUG_ON(cond);						\
> >   } while (0)
> > +#else
> > +#define INT_SOFT_MASK_BUG_ON(regs, cond)
>
> Here you can just drop the ifdef CONFIG_PPC64 I guess instead of adding 
> an additional empty macro.

I couldn't because some of the statements use some PPC64-only variables.
It ends up looking a bit better this way.

> >   #endif
> >   
> >   #ifdef CONFIG_PPC_BOOK3S_64
> > @@ -152,19 +154,8 @@ static inline void booke_restore_dbcr0(void)
> >   static inline void interrupt_enter_prepare(struct pt_regs *regs)
> >   {
> >   #ifdef CONFIG_PPC32
> > -	if (!arch_irq_disabled_regs(regs))
> > -		trace_hardirqs_off();
> > -
> > -	if (user_mode(regs))
> > -		kuap_lock();
> > -	else
> > -		kuap_save_and_lock(regs);
> > -
> > -	if (user_mode(regs))
> > -		account_cpu_user_entry();
> > -#endif
> > -
> > -#ifdef CONFIG_PPC64
> > +	bool trace_enable = !arch_irq_disabled_regs(regs);
>
> nit: You could be put this as an #else to the existing #ifdef CONFIG_PPC64

Yep, I was able to clean it up even a bit better actually.

Thanks,
Nick

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 2/3] powerpc: remove the last remnants of cputime_t
  2022-10-04  6:33 ` [RFC PATCH 2/3] powerpc: remove the last remnants of cputime_t Nicholas Piggin
@ 2022-10-06 10:06   ` Michael Ellerman
  0 siblings, 0 replies; 9+ messages in thread
From: Michael Ellerman @ 2022-10-06 10:06 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin

Nicholas Piggin <npiggin@gmail.com> writes:
> cputime_t is no longer, converted to u64.

Would be good to have some explanation of why we had it and why we don't
need it anymore.

cheers

> diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
> index 431ae2343022..4961fb38e438 100644
> --- a/arch/powerpc/include/asm/cputime.h
> +++ b/arch/powerpc/include/asm/cputime.h
> @@ -21,23 +21,8 @@
>  #include <asm/param.h>
>  #include <asm/firmware.h>
>  
> -typedef u64 __nocast cputime_t;
> -typedef u64 __nocast cputime64_t;
> -
> -#define cmpxchg_cputime(ptr, old, new) cmpxchg(ptr, old, new)
> -
>  #ifdef __KERNEL__
> -/*
> - * Convert cputime <-> microseconds
> - */
> -extern u64 __cputime_usec_factor;
> -
> -static inline unsigned long cputime_to_usecs(const cputime_t ct)
> -{
> -	return mulhdu((__force u64) ct, __cputime_usec_factor);
> -}
> -
> -#define cputime_to_nsecs(cputime) tb_to_ns((__force u64)cputime)
> +#define cputime_to_nsecs(cputime) tb_to_ns(cputime)
>  
>  /*
>   * PPC64 uses PACA which is task independent for storing accounting data while
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index a2ab397065c6..d68de3618741 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -130,7 +130,7 @@ unsigned long tb_ticks_per_jiffy;
>  unsigned long tb_ticks_per_usec = 100; /* sane default */
>  EXPORT_SYMBOL(tb_ticks_per_usec);
>  unsigned long tb_ticks_per_sec;
> -EXPORT_SYMBOL(tb_ticks_per_sec);	/* for cputime_t conversions */
> +EXPORT_SYMBOL(tb_ticks_per_sec);	/* for cputime conversions */
>  
>  DEFINE_SPINLOCK(rtc_lock);
>  EXPORT_SYMBOL_GPL(rtc_lock);
> @@ -150,21 +150,6 @@ EXPORT_SYMBOL_GPL(ppc_tb_freq);
>  bool tb_invalid;
>  
>  #ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> -/*
> - * Factor for converting from cputime_t (timebase ticks) to
> - * microseconds. This is stored as 0.64 fixed-point binary fraction.
> - */
> -u64 __cputime_usec_factor;
> -EXPORT_SYMBOL(__cputime_usec_factor);
> -
> -static void calc_cputime_factors(void)
> -{
> -	struct div_result res;
> -
> -	div128_by_32(1000000, 0, tb_ticks_per_sec, &res);
> -	__cputime_usec_factor = res.result_low;
> -}
> -
>  /*
>   * Read the SPURR on systems that have it, otherwise the PURR,
>   * or if that doesn't exist return the timebase value passed in.
> @@ -369,10 +354,7 @@ void vtime_flush(struct task_struct *tsk)
>  	acct->hardirq_time = 0;
>  	acct->softirq_time = 0;
>  }
> -
> -#else /* ! CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
> -#define calc_cputime_factors()
> -#endif
> +#endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
>  
>  void __delay(unsigned long loops)
>  {
> @@ -914,7 +896,6 @@ void __init time_init(void)
>  	tb_ticks_per_jiffy = ppc_tb_freq / HZ;
>  	tb_ticks_per_sec = ppc_tb_freq;
>  	tb_ticks_per_usec = ppc_tb_freq / 1000000;
> -	calc_cputime_factors();
>  
>  	/*
>  	 * Compute scale factor for sched_clock.
> -- 
> 2.37.2

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-10-06 10:07 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-04  6:33 [RFC PATCH 0/3] powerpc/32: nohz full support Nicholas Piggin
2022-10-04  6:33 ` [RFC PATCH 1/3] powerpc/32: Implement HAVE_CONTEXT_TRACKING_USER support Nicholas Piggin
2022-10-04 11:32   ` Christophe Leroy
2022-10-06  8:22     ` Nicholas Piggin
2022-10-04  6:33 ` [RFC PATCH 2/3] powerpc: remove the last remnants of cputime_t Nicholas Piggin
2022-10-06 10:06   ` Michael Ellerman
2022-10-04  6:33 ` [RFC PATCH 3/3] Remove HAVE_VIRT_CPU_ACCOUNTING_GEN option Nicholas Piggin
2022-10-04 11:58 ` [RFC PATCH 0/3] powerpc/32: nohz full support Christophe Leroy
2022-10-06  8:18   ` Nicholas Piggin

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).