Skip to contentSkip to conversation

Remove HAVE_VIRT_CPU_ACCOUNTING_GEN option

PATCH In review Nicholas Piggin 3 participants 4 messages Apr 29, 2023 – May 3, 2023

Subsystems (12)
Arm (32-bit) PowerPC MIPS RISC-V +8 more subsystems
lore
Share & tools
lore

Nicholas Piggin

author reviewer
#0 lore raw

PATCH 1/1 Remove HAVE_VIRT_CPU_ACCOUNTING_GEN option

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 Vineet Gupta <[email protected]>
Cc Brian Cain <[email protected]>
Cc Huacai Chen <[email protected]>
Cc Geert Uytterhoeven <[email protected]>
Cc Michal Simek <[email protected]>
Cc Thomas Bogendoerfer <[email protected]>
Cc Dinh Nguyen <[email protected]>
Cc Jonas Bonn <[email protected]>
Cc Stefan Kristiansson <[email protected]>
Cc Stafford Horne <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>
Cc Helge Deller <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Albert Ou <[email protected]>
Cc Yoshinori Sato <[email protected]>
Cc Rich Felker <[email protected]>
Cc John Paul Adrian Glaubitz <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Richard Weinberger <[email protected]>
Cc Anton Ivanov <[email protected]>
Cc Johannes Berg <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Frederic Weisbecker <[email protected]>

---
Hi,

Could we tidy this? I don't know what tree it can go in, timers, sched, asm-generic, probably doesn't matter.

The only thing this actually does is gate VIRT_CPU_ACCOUNTING_GEN and NO_HZ_FULL so if your arch has some other issue that requires this then the documentation needs to change. Any concerns from the archs? I.e., 32-bit that does *not* define HAVE_VIRT_CPU_ACCOUNTING_GEN which looks to be:

arc
hexagon
loongarch 32-bit with SMP
m68k
microblaze
mips 32-bit with SMP
nios2
openrisc
parisc 32-bit
riscv 32-bit
sh
sparc 32-bit
um 32-bit
x86 32-bit

Thanks,
Nick
--

 arch/Kconfig           | 11 -----------
 arch/arm/Kconfig       |  1 -
 arch/csky/Kconfig      |  1 -
 arch/loongarch/Kconfig |  1 -
 arch/mips/Kconfig      |  1 -
 arch/powerpc/Kconfig   |  1 -
 arch/xtensa/Kconfig    |  1 -
 init/Kconfig           |  1 -
 kernel/time/Kconfig    |  2 --
 9 files changed, 20 deletions(-)
arch/Kconfig +0 -11
index 205fd23e0cad..b77b41d25e40 100644
@@ -869,17 +869,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
arch/arm/Kconfig +0 -1
index 0fb4b218f665..9c05f25db4e4 100644
@@ -123,7 +123,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
arch/csky/Kconfig +0 -1
index 00379a843c37..dd1decc2a22d 100644
@@ -73,7 +73,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
arch/loongarch/Kconfig +0 -1
index f44f6ea54e46..7dc71c89bfae 100644
@@ -122,7 +122,6 @@ config LOONGARCH
 	select HAVE_STACKPROTECTOR
 	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
arch/mips/Kconfig +0 -1
index c2f5498d207f..d84cb8bbee53 100644
@@ -87,7 +87,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
arch/powerpc/Kconfig +0 -1
index acffffbd5d77..185195f349d9 100644
@@ -271,7 +271,6 @@ config PPC
 	select HAVE_STATIC_CALL			if PPC32
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_VIRT_CPU_ACCOUNTING
-	select HAVE_VIRT_CPU_ACCOUNTING_GEN
 	select HUGETLB_PAGE_SIZE_VARIABLE	if PPC_BOOK3S_64 && HUGETLB_PAGE
 	select IOMMU_HELPER			if PPC64
 	select IRQ_DOMAIN
arch/xtensa/Kconfig +0 -1
index 3c6e5471f025..04f1399d9ce8 100644
@@ -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
init/Kconfig +0 -1
index 32c24950c4ce..835fb5e78a8f 100644
@@ -504,7 +504,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
kernel/time/Kconfig +0 -2
index bae8f11070be..d4325dae1f03 100644
@@ -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
Signature · 1 line
2.40.1
maintainer Acked-by on v1 1/1
#1 lore raw
Quoted 63 lines · On Fri, 28 Apr 2023 23:33:48 PDT (-0700), [email protected] wrote:

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 Vineet Gupta <[email protected]>
Cc Brian Cain <[email protected]>
Cc Huacai Chen <[email protected]>
Cc Geert Uytterhoeven <[email protected]>
Cc Michal Simek <[email protected]>
Cc Thomas Bogendoerfer <[email protected]>
Cc Dinh Nguyen <[email protected]>
Cc Jonas Bonn <[email protected]>
Cc Stefan Kristiansson <[email protected]>
Cc Stafford Horne <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>
Cc Helge Deller <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Albert Ou <[email protected]>
Cc Yoshinori Sato <[email protected]>
Cc Rich Felker <[email protected]>
Cc John Paul Adrian Glaubitz <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Richard Weinberger <[email protected]>
Cc Anton Ivanov <[email protected]>
Cc Johannes Berg <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Frederic Weisbecker <[email protected]>

---
Hi,

Could we tidy this? I don't know what tree it can go in, timers, sched, asm-generic, probably doesn't matter.

The only thing this actually does is gate VIRT_CPU_ACCOUNTING_GEN and NO_HZ_FULL so if your arch has some other issue that requires this then the documentation needs to change. Any concerns from the archs? I.e., 32-bit that does *not* define HAVE_VIRT_CPU_ACCOUNTING_GEN which looks to be:

Quoted 10 lines

arc
hexagon
loongarch 32-bit with SMP
m68k
microblaze
mips 32-bit with SMP
nios2
openrisc
parisc 32-bit
riscv 32-bit

Nothing's jumping out, though I haven't tested this yet so I'm not 100%. I assume this isn't aimed for this merge window, given the timing? Probably best to give this sort of thing time to bake in linux-next, but I doubt anyone is even paying attention to rv32/NO_HZ_FULL so no big deal either way on my end.

Quoted 7 lines

sh
sparc 32-bit
um 32-bit
x86 32-bit

Thanks,
Nick

author reviewer
#2 lore raw
Quoted 75 lines · Palmer Dabbelt
Quoted 63 lines · On Fri, 28 Apr 2023 23:33:48 PDT (-0700), [email protected] wrote:

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 Vineet Gupta <[email protected]>
Cc Brian Cain <[email protected]>
Cc Huacai Chen <[email protected]>
Cc Geert Uytterhoeven <[email protected]>
Cc Michal Simek <[email protected]>
Cc Thomas Bogendoerfer <[email protected]>
Cc Dinh Nguyen <[email protected]>
Cc Jonas Bonn <[email protected]>
Cc Stefan Kristiansson <[email protected]>
Cc Stafford Horne <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>
Cc Helge Deller <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Albert Ou <[email protected]>
Cc Yoshinori Sato <[email protected]>
Cc Rich Felker <[email protected]>
Cc John Paul Adrian Glaubitz <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Richard Weinberger <[email protected]>
Cc Anton Ivanov <[email protected]>
Cc Johannes Berg <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Frederic Weisbecker <[email protected]>

---
Hi,

Could we tidy this? I don't know what tree it can go in, timers, sched, asm-generic, probably doesn't matter.

The only thing this actually does is gate VIRT_CPU_ACCOUNTING_GEN and NO_HZ_FULL so if your arch has some other issue that requires this then the documentation needs to change. Any concerns from the archs? I.e., 32-bit that does *not* define HAVE_VIRT_CPU_ACCOUNTING_GEN which looks to be:

Quoted 10 lines

arc
hexagon
loongarch 32-bit with SMP
m68k
microblaze
mips 32-bit with SMP
nios2
openrisc
parisc 32-bit
riscv 32-bit

Quoted 2 lines

Nothing's jumping out, though I haven't tested this yet so I'm not 100%. I assume this isn't aimed for this merge window, given the timing?

No, maybe the next one though.

Quoted 5 lines

Probably best to give this sort of thing time to bake in linux-next, but I doubt anyone is even paying attention to rv32/NO_HZ_FULL so no big deal either way on my end.

Thanks,
Nick

#3 lore raw
In reply to Nicholas Piggin
Quoted 6 lines · 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.

Well, cputime_t has disappeared but not the u64 type used for task/cpu time accounting.

But now we have the vtime seqcount. Though we already had it when we introduced that Kconfig switch so I can't remember why this was necessary :-(

It _looks_ OK but I might be missing something...

Thanks.