* [PATCH v2] Enable haltpoll for arm64
@ 2023-11-20 14:01 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
This patchset enables the usage of haltpoll governer on arm64. This is
specifically interesting for KVM guests by reducing the IPC latencies.
Here are some benchmarks without/with haltpoll for a KVM guest:
a) without haltpoll:
perf bench sched pipe
# Running 'sched/pipe' benchmark:
# Executed 1000000 pipe operations between two processes
Total time: 8.138 [sec]
8.138094 usecs/op
122878 ops/sec
b) with haltpoll:
perf bench sched pipe
# Running 'sched/pipe' benchmark:
# Executed 1000000 pipe operations between two processes
Total time: 5.003 [sec]
5.003085 usecs/op
199876 ops/sec
v2 changes from v1:
- added patch 7 where we change cpu_relax with smp_cond_load_relaxed per PeterZ
(this improves by 50% at least the CPU cycles consumed in the tests above:
10,716,881,137 now vs 14,503,014,257 before)
- removed the ifdef from patch 1 per RafaelW
Joao Martins (6):
x86: Move ARCH_HAS_CPU_RELAX to arch
x86/kvm: Move haltpoll_want() to be arch defined
governors/haltpoll: Drop kvm_para_available() check
arm64: Select ARCH_HAS_CPU_RELAX
arm64: Define TIF_POLLING_NRFLAG
cpuidle-haltpoll: ARM64 support
Mihai Carabas (1):
cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
arch/Kconfig | 3 +++
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/thread_info.h | 6 ++++++
arch/x86/Kconfig | 1 +
arch/x86/include/asm/cpuidle_haltpoll.h | 1 +
arch/x86/kernel/kvm.c | 10 ++++++++++
drivers/cpuidle/Kconfig | 4 ++--
drivers/cpuidle/cpuidle-haltpoll.c | 8 ++------
drivers/cpuidle/governors/haltpoll.c | 5 +----
drivers/cpuidle/poll_state.c | 14 +++++++++-----
include/linux/cpuidle_haltpoll.h | 5 +++++
11 files changed, 41 insertions(+), 17 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2] Enable haltpoll for arm64
@ 2023-11-20 14:01 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
This patchset enables the usage of haltpoll governer on arm64. This is
specifically interesting for KVM guests by reducing the IPC latencies.
Here are some benchmarks without/with haltpoll for a KVM guest:
a) without haltpoll:
perf bench sched pipe
# Running 'sched/pipe' benchmark:
# Executed 1000000 pipe operations between two processes
Total time: 8.138 [sec]
8.138094 usecs/op
122878 ops/sec
b) with haltpoll:
perf bench sched pipe
# Running 'sched/pipe' benchmark:
# Executed 1000000 pipe operations between two processes
Total time: 5.003 [sec]
5.003085 usecs/op
199876 ops/sec
v2 changes from v1:
- added patch 7 where we change cpu_relax with smp_cond_load_relaxed per PeterZ
(this improves by 50% at least the CPU cycles consumed in the tests above:
10,716,881,137 now vs 14,503,014,257 before)
- removed the ifdef from patch 1 per RafaelW
Joao Martins (6):
x86: Move ARCH_HAS_CPU_RELAX to arch
x86/kvm: Move haltpoll_want() to be arch defined
governors/haltpoll: Drop kvm_para_available() check
arm64: Select ARCH_HAS_CPU_RELAX
arm64: Define TIF_POLLING_NRFLAG
cpuidle-haltpoll: ARM64 support
Mihai Carabas (1):
cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
arch/Kconfig | 3 +++
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/thread_info.h | 6 ++++++
arch/x86/Kconfig | 1 +
arch/x86/include/asm/cpuidle_haltpoll.h | 1 +
arch/x86/kernel/kvm.c | 10 ++++++++++
drivers/cpuidle/Kconfig | 4 ++--
drivers/cpuidle/cpuidle-haltpoll.c | 8 ++------
drivers/cpuidle/governors/haltpoll.c | 5 +----
drivers/cpuidle/poll_state.c | 14 +++++++++-----
include/linux/cpuidle_haltpoll.h | 5 +++++
11 files changed, 41 insertions(+), 17 deletions(-)
--
1.8.3.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH 1/7] x86: Move ARCH_HAS_CPU_RELAX to arch
2023-11-20 14:01 ` Mihai Carabas
@ 2023-11-20 14:01 ` Mihai Carabas
-1 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
From: Joao Martins <joao.m.martins@oracle.com>
ARM64 is going to use it for haltpoll support (for poll-state)
so move the definition to be arch-agnostic and allow architectures
to override it.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
arch/Kconfig | 3 +++
arch/x86/Kconfig | 1 +
2 files changed, 4 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index 4a85a10b12fd..92af0e9bc35e 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1371,6 +1371,9 @@ config RELR
config ARCH_HAS_MEM_ENCRYPT
bool
+config ARCH_HAS_CPU_RELAX
+ bool
+
config ARCH_HAS_CC_PLATFORM
bool
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d1c362f479d9..0c77670d020e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -73,6 +73,7 @@ config X86
select ARCH_HAS_CACHE_LINE_SIZE
select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
select ARCH_HAS_CPU_FINALIZE_INIT
+ select ARCH_HAS_CPU_RELAX
select ARCH_HAS_CURRENT_STACK_POINTER
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEBUG_VM_PGTABLE if !X86_PAE
--
1.8.3.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 1/7] x86: Move ARCH_HAS_CPU_RELAX to arch
@ 2023-11-20 14:01 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
From: Joao Martins <joao.m.martins@oracle.com>
ARM64 is going to use it for haltpoll support (for poll-state)
so move the definition to be arch-agnostic and allow architectures
to override it.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
arch/Kconfig | 3 +++
arch/x86/Kconfig | 1 +
2 files changed, 4 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index 4a85a10b12fd..92af0e9bc35e 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1371,6 +1371,9 @@ config RELR
config ARCH_HAS_MEM_ENCRYPT
bool
+config ARCH_HAS_CPU_RELAX
+ bool
+
config ARCH_HAS_CC_PLATFORM
bool
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d1c362f479d9..0c77670d020e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -73,6 +73,7 @@ config X86
select ARCH_HAS_CACHE_LINE_SIZE
select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
select ARCH_HAS_CPU_FINALIZE_INIT
+ select ARCH_HAS_CPU_RELAX
select ARCH_HAS_CURRENT_STACK_POINTER
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEBUG_VM_PGTABLE if !X86_PAE
--
1.8.3.1
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 2/7] x86/kvm: Move haltpoll_want() to be arch defined
2023-11-20 14:01 ` Mihai Carabas
@ 2023-11-20 14:01 ` Mihai Carabas
-1 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
From: Joao Martins <joao.m.martins@oracle.com>
Right now, kvm_para_has_hint(KVM_HINTS_REALTIME) is x86 only, and so in the
pursuit of making cpuidle-haltpoll arch independent, move the check for
haltpoll enablement to be defined per architecture. Same thing for
boot_option_idle_override. To that end, add a arch_haltpoll_want() and move the
check there.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
arch/x86/include/asm/cpuidle_haltpoll.h | 1 +
arch/x86/kernel/kvm.c | 10 ++++++++++
drivers/cpuidle/cpuidle-haltpoll.c | 8 ++------
include/linux/cpuidle_haltpoll.h | 5 +++++
4 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/cpuidle_haltpoll.h b/arch/x86/include/asm/cpuidle_haltpoll.h
index c8b39c6716ff..2c5a53ce266f 100644
--- a/arch/x86/include/asm/cpuidle_haltpoll.h
+++ b/arch/x86/include/asm/cpuidle_haltpoll.h
@@ -4,5 +4,6 @@
void arch_haltpoll_enable(unsigned int cpu);
void arch_haltpoll_disable(unsigned int cpu);
+bool arch_haltpoll_want(void);
#endif
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 1cceac5984da..75a24f107b2a 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -1151,4 +1151,14 @@ void arch_haltpoll_disable(unsigned int cpu)
smp_call_function_single(cpu, kvm_enable_host_haltpoll, NULL, 1);
}
EXPORT_SYMBOL_GPL(arch_haltpoll_disable);
+
+bool arch_haltpoll_want(void)
+{
+ /* Do not load haltpoll if idle= is passed */
+ if (boot_option_idle_override != IDLE_NO_OVERRIDE)
+ return false;
+
+ return kvm_para_has_hint(KVM_HINTS_REALTIME);
+}
+EXPORT_SYMBOL_GPL(arch_haltpoll_want);
#endif
diff --git a/drivers/cpuidle/cpuidle-haltpoll.c b/drivers/cpuidle/cpuidle-haltpoll.c
index e66df22f9695..72f9c84990c5 100644
--- a/drivers/cpuidle/cpuidle-haltpoll.c
+++ b/drivers/cpuidle/cpuidle-haltpoll.c
@@ -96,7 +96,7 @@ static void haltpoll_uninit(void)
static bool haltpoll_want(void)
{
- return kvm_para_has_hint(KVM_HINTS_REALTIME) || force;
+ return (kvm_para_available() && arch_haltpoll_want()) || force;
}
static int __init haltpoll_init(void)
@@ -104,11 +104,7 @@ static int __init haltpoll_init(void)
int ret;
struct cpuidle_driver *drv = &haltpoll_driver;
- /* Do not load haltpoll if idle= is passed */
- if (boot_option_idle_override != IDLE_NO_OVERRIDE)
- return -ENODEV;
-
- if (!kvm_para_available() || !haltpoll_want())
+ if (!haltpoll_want())
return -ENODEV;
cpuidle_poll_state_init(drv);
diff --git a/include/linux/cpuidle_haltpoll.h b/include/linux/cpuidle_haltpoll.h
index d50c1e0411a2..bae68a6603e3 100644
--- a/include/linux/cpuidle_haltpoll.h
+++ b/include/linux/cpuidle_haltpoll.h
@@ -12,5 +12,10 @@ static inline void arch_haltpoll_enable(unsigned int cpu)
static inline void arch_haltpoll_disable(unsigned int cpu)
{
}
+
+static inline bool arch_haltpoll_want(void)
+{
+ return false;
+}
#endif
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 2/7] x86/kvm: Move haltpoll_want() to be arch defined
@ 2023-11-20 14:01 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
From: Joao Martins <joao.m.martins@oracle.com>
Right now, kvm_para_has_hint(KVM_HINTS_REALTIME) is x86 only, and so in the
pursuit of making cpuidle-haltpoll arch independent, move the check for
haltpoll enablement to be defined per architecture. Same thing for
boot_option_idle_override. To that end, add a arch_haltpoll_want() and move the
check there.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
arch/x86/include/asm/cpuidle_haltpoll.h | 1 +
arch/x86/kernel/kvm.c | 10 ++++++++++
drivers/cpuidle/cpuidle-haltpoll.c | 8 ++------
include/linux/cpuidle_haltpoll.h | 5 +++++
4 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/cpuidle_haltpoll.h b/arch/x86/include/asm/cpuidle_haltpoll.h
index c8b39c6716ff..2c5a53ce266f 100644
--- a/arch/x86/include/asm/cpuidle_haltpoll.h
+++ b/arch/x86/include/asm/cpuidle_haltpoll.h
@@ -4,5 +4,6 @@
void arch_haltpoll_enable(unsigned int cpu);
void arch_haltpoll_disable(unsigned int cpu);
+bool arch_haltpoll_want(void);
#endif
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 1cceac5984da..75a24f107b2a 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -1151,4 +1151,14 @@ void arch_haltpoll_disable(unsigned int cpu)
smp_call_function_single(cpu, kvm_enable_host_haltpoll, NULL, 1);
}
EXPORT_SYMBOL_GPL(arch_haltpoll_disable);
+
+bool arch_haltpoll_want(void)
+{
+ /* Do not load haltpoll if idle= is passed */
+ if (boot_option_idle_override != IDLE_NO_OVERRIDE)
+ return false;
+
+ return kvm_para_has_hint(KVM_HINTS_REALTIME);
+}
+EXPORT_SYMBOL_GPL(arch_haltpoll_want);
#endif
diff --git a/drivers/cpuidle/cpuidle-haltpoll.c b/drivers/cpuidle/cpuidle-haltpoll.c
index e66df22f9695..72f9c84990c5 100644
--- a/drivers/cpuidle/cpuidle-haltpoll.c
+++ b/drivers/cpuidle/cpuidle-haltpoll.c
@@ -96,7 +96,7 @@ static void haltpoll_uninit(void)
static bool haltpoll_want(void)
{
- return kvm_para_has_hint(KVM_HINTS_REALTIME) || force;
+ return (kvm_para_available() && arch_haltpoll_want()) || force;
}
static int __init haltpoll_init(void)
@@ -104,11 +104,7 @@ static int __init haltpoll_init(void)
int ret;
struct cpuidle_driver *drv = &haltpoll_driver;
- /* Do not load haltpoll if idle= is passed */
- if (boot_option_idle_override != IDLE_NO_OVERRIDE)
- return -ENODEV;
-
- if (!kvm_para_available() || !haltpoll_want())
+ if (!haltpoll_want())
return -ENODEV;
cpuidle_poll_state_init(drv);
diff --git a/include/linux/cpuidle_haltpoll.h b/include/linux/cpuidle_haltpoll.h
index d50c1e0411a2..bae68a6603e3 100644
--- a/include/linux/cpuidle_haltpoll.h
+++ b/include/linux/cpuidle_haltpoll.h
@@ -12,5 +12,10 @@ static inline void arch_haltpoll_enable(unsigned int cpu)
static inline void arch_haltpoll_disable(unsigned int cpu)
{
}
+
+static inline bool arch_haltpoll_want(void)
+{
+ return false;
+}
#endif
#endif
--
1.8.3.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 3/7] governors/haltpoll: Drop kvm_para_available() check
2023-11-20 14:01 ` Mihai Carabas
@ 2023-11-20 14:01 ` Mihai Carabas
-1 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
From: Joao Martins <joao.m.martins@oracle.com>
This is duplicated already in the haltpoll idle driver,
and there's no need to re-check KVM guest availability in
the governor.
Either guests uses the module which explicitly selects this
governor, and given that it has the lowest rating of all governors
(menu=20,teo=19,ladder=10/25,haltpoll=9) means that unless it's
the only one compiled in, it won't be selected.
Dropping such check also allows to test haltpoll in baremetal.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
Acked-by: Rafael J. Wysocki <rafael@kernel.org>
---
drivers/cpuidle/governors/haltpoll.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/cpuidle/governors/haltpoll.c b/drivers/cpuidle/governors/haltpoll.c
index 1dff3a52917d..c9b69651d377 100644
--- a/drivers/cpuidle/governors/haltpoll.c
+++ b/drivers/cpuidle/governors/haltpoll.c
@@ -143,10 +143,7 @@ static int haltpoll_enable_device(struct cpuidle_driver *drv,
static int __init init_haltpoll(void)
{
- if (kvm_para_available())
- return cpuidle_register_governor(&haltpoll_governor);
-
- return 0;
+ return cpuidle_register_governor(&haltpoll_governor);
}
postcore_initcall(init_haltpoll);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 3/7] governors/haltpoll: Drop kvm_para_available() check
@ 2023-11-20 14:01 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
From: Joao Martins <joao.m.martins@oracle.com>
This is duplicated already in the haltpoll idle driver,
and there's no need to re-check KVM guest availability in
the governor.
Either guests uses the module which explicitly selects this
governor, and given that it has the lowest rating of all governors
(menu=20,teo=19,ladder=10/25,haltpoll=9) means that unless it's
the only one compiled in, it won't be selected.
Dropping such check also allows to test haltpoll in baremetal.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
Acked-by: Rafael J. Wysocki <rafael@kernel.org>
---
drivers/cpuidle/governors/haltpoll.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/cpuidle/governors/haltpoll.c b/drivers/cpuidle/governors/haltpoll.c
index 1dff3a52917d..c9b69651d377 100644
--- a/drivers/cpuidle/governors/haltpoll.c
+++ b/drivers/cpuidle/governors/haltpoll.c
@@ -143,10 +143,7 @@ static int haltpoll_enable_device(struct cpuidle_driver *drv,
static int __init init_haltpoll(void)
{
- if (kvm_para_available())
- return cpuidle_register_governor(&haltpoll_governor);
-
- return 0;
+ return cpuidle_register_governor(&haltpoll_governor);
}
postcore_initcall(init_haltpoll);
--
1.8.3.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 4/7] arm64: Select ARCH_HAS_CPU_RELAX
2023-11-20 14:01 ` Mihai Carabas
@ 2023-11-20 14:01 ` Mihai Carabas
-1 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
From: Joao Martins <joao.m.martins@oracle.com>
ARCH_HAS_CPU_RELAX controls the build of poll-state, so select it from ARM64
kconfig.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
arch/arm64/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 87ade6549790..7c47617b5722 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -105,6 +105,7 @@ config ARM64
select ARCH_WANT_LD_ORPHAN_WARN
select ARCH_WANTS_NO_INSTR
select ARCH_WANTS_THP_SWAP if ARM64_4K_PAGES
+ select ARCH_HAS_CPU_RELAX
select ARCH_HAS_UBSAN_SANITIZE_ALL
select ARM_AMBA
select ARM_ARCH_TIMER
--
1.8.3.1
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 4/7] arm64: Select ARCH_HAS_CPU_RELAX
@ 2023-11-20 14:01 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
From: Joao Martins <joao.m.martins@oracle.com>
ARCH_HAS_CPU_RELAX controls the build of poll-state, so select it from ARM64
kconfig.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
arch/arm64/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 87ade6549790..7c47617b5722 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -105,6 +105,7 @@ config ARM64
select ARCH_WANT_LD_ORPHAN_WARN
select ARCH_WANTS_NO_INSTR
select ARCH_WANTS_THP_SWAP if ARM64_4K_PAGES
+ select ARCH_HAS_CPU_RELAX
select ARCH_HAS_UBSAN_SANITIZE_ALL
select ARM_AMBA
select ARM_ARCH_TIMER
--
1.8.3.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 5/7] arm64: Define TIF_POLLING_NRFLAG
2023-11-20 14:01 ` Mihai Carabas
@ 2023-11-20 14:01 ` Mihai Carabas
-1 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
From: Joao Martins <joao.m.martins@oracle.com>
The default idle method for arm64 is WFI and it therefore
unconditionally requires the reschedule interrupt when idle.
Commit 842514849a61 ("arm64: Remove TIF_POLLING_NRFLAG") had
reverted it because WFI was the only idle method. ARM64 support
for haltpoll means that poll_idle() polls for TIF_POLLING_NRFLAG,
so define on arm64 *only if* haltpoll is built, using the same bit.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
arch/arm64/include/asm/thread_info.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 2c62ab3aeb84..c5fd3107c8ce 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -69,6 +69,9 @@ struct thread_info {
#define TIF_SYSCALL_TRACEPOINT 10 /* syscall tracepoint for ftrace */
#define TIF_SECCOMP 11 /* syscall secure computing */
#define TIF_SYSCALL_EMU 12 /* syscall emulation active */
+#if IS_ENABLED(CONFIG_HALTPOLL_CPUIDLE) || IS_ENABLED(CONFIG_HALTPOLL_CPUIDLE_MODULE)
+#define TIF_POLLING_NRFLAG 16 /* poll_idle() polls TIF_NEED_RESCHED */
+#endif
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
#define TIF_FREEZE 19
#define TIF_RESTORE_SIGMASK 20
@@ -94,6 +97,9 @@ struct thread_info {
#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT)
#define _TIF_SECCOMP (1 << TIF_SECCOMP)
#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU)
+#if IS_ENABLED(CONFIG_HALTPOLL_CPUIDLE) || IS_ENABLED(CONFIG_HALTPOLL_CPUIDLE_MODULE)
+#define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG)
+#endif
#define _TIF_UPROBE (1 << TIF_UPROBE)
#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP)
#define _TIF_32BIT (1 << TIF_32BIT)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 5/7] arm64: Define TIF_POLLING_NRFLAG
@ 2023-11-20 14:01 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
From: Joao Martins <joao.m.martins@oracle.com>
The default idle method for arm64 is WFI and it therefore
unconditionally requires the reschedule interrupt when idle.
Commit 842514849a61 ("arm64: Remove TIF_POLLING_NRFLAG") had
reverted it because WFI was the only idle method. ARM64 support
for haltpoll means that poll_idle() polls for TIF_POLLING_NRFLAG,
so define on arm64 *only if* haltpoll is built, using the same bit.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
arch/arm64/include/asm/thread_info.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 2c62ab3aeb84..c5fd3107c8ce 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -69,6 +69,9 @@ struct thread_info {
#define TIF_SYSCALL_TRACEPOINT 10 /* syscall tracepoint for ftrace */
#define TIF_SECCOMP 11 /* syscall secure computing */
#define TIF_SYSCALL_EMU 12 /* syscall emulation active */
+#if IS_ENABLED(CONFIG_HALTPOLL_CPUIDLE) || IS_ENABLED(CONFIG_HALTPOLL_CPUIDLE_MODULE)
+#define TIF_POLLING_NRFLAG 16 /* poll_idle() polls TIF_NEED_RESCHED */
+#endif
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
#define TIF_FREEZE 19
#define TIF_RESTORE_SIGMASK 20
@@ -94,6 +97,9 @@ struct thread_info {
#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT)
#define _TIF_SECCOMP (1 << TIF_SECCOMP)
#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU)
+#if IS_ENABLED(CONFIG_HALTPOLL_CPUIDLE) || IS_ENABLED(CONFIG_HALTPOLL_CPUIDLE_MODULE)
+#define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG)
+#endif
#define _TIF_UPROBE (1 << TIF_UPROBE)
#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP)
#define _TIF_32BIT (1 << TIF_32BIT)
--
1.8.3.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 6/7] cpuidle-haltpoll: ARM64 support
2023-11-20 14:01 ` Mihai Carabas
@ 2023-11-20 14:01 ` Mihai Carabas
-1 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
From: Joao Martins <joao.m.martins@oracle.com>
To test whether it's a guest or not for the default cases, the haltpoll
driver uses the kvm_para* helpers to find out if it's a guest or not.
ARM64 doesn't have or defined any of these, so it remains disabled on
the default. Although it allows to be force-loaded.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
drivers/cpuidle/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
index cac5997dca50..067927eda466 100644
--- a/drivers/cpuidle/Kconfig
+++ b/drivers/cpuidle/Kconfig
@@ -35,7 +35,7 @@ config CPU_IDLE_GOV_TEO
config CPU_IDLE_GOV_HALTPOLL
bool "Haltpoll governor (for virtualized systems)"
- depends on KVM_GUEST
+ depends on (X86 && KVM_GUEST) || ARM64
help
This governor implements haltpoll idle state selection, to be
used in conjunction with the haltpoll cpuidle driver, allowing
@@ -73,7 +73,7 @@ endmenu
config HALTPOLL_CPUIDLE
tristate "Halt poll cpuidle driver"
- depends on X86 && KVM_GUEST
+ depends on (X86 && KVM_GUEST) || ARM64
select CPU_IDLE_GOV_HALTPOLL
default y
help
--
1.8.3.1
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 6/7] cpuidle-haltpoll: ARM64 support
@ 2023-11-20 14:01 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
From: Joao Martins <joao.m.martins@oracle.com>
To test whether it's a guest or not for the default cases, the haltpoll
driver uses the kvm_para* helpers to find out if it's a guest or not.
ARM64 doesn't have or defined any of these, so it remains disabled on
the default. Although it allows to be force-loaded.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
drivers/cpuidle/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
index cac5997dca50..067927eda466 100644
--- a/drivers/cpuidle/Kconfig
+++ b/drivers/cpuidle/Kconfig
@@ -35,7 +35,7 @@ config CPU_IDLE_GOV_TEO
config CPU_IDLE_GOV_HALTPOLL
bool "Haltpoll governor (for virtualized systems)"
- depends on KVM_GUEST
+ depends on (X86 && KVM_GUEST) || ARM64
help
This governor implements haltpoll idle state selection, to be
used in conjunction with the haltpoll cpuidle driver, allowing
@@ -73,7 +73,7 @@ endmenu
config HALTPOLL_CPUIDLE
tristate "Halt poll cpuidle driver"
- depends on X86 && KVM_GUEST
+ depends on (X86 && KVM_GUEST) || ARM64
select CPU_IDLE_GOV_HALTPOLL
default y
help
--
1.8.3.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
2023-11-20 14:01 ` Mihai Carabas
@ 2023-11-20 14:01 ` Mihai Carabas
-1 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
cpu_relax on ARM64 does a simple "yield". Thus we replace it with
smp_cond_load_relaxed which basically does a "wfe".
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
drivers/cpuidle/poll_state.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
index 9b6d90a72601..440cd713e39a 100644
--- a/drivers/cpuidle/poll_state.c
+++ b/drivers/cpuidle/poll_state.c
@@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
limit = cpuidle_poll_time(drv, dev);
- while (!need_resched()) {
- cpu_relax();
- if (loop_count++ < POLL_IDLE_RELAX_COUNT)
- continue;
-
+ for (;;) {
loop_count = 0;
+
+ smp_cond_load_relaxed(¤t_thread_info()->flags,
+ (VAL & _TIF_NEED_RESCHED) ||
+ (loop_count++ >= POLL_IDLE_RELAX_COUNT));
+
+ if (loop_count < POLL_IDLE_RELAX_COUNT)
+ break;
+
if (local_clock_noinstr() - time_start > limit) {
dev->poll_time_limit = true;
break;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
@ 2023-11-20 14:01 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic,
mihai.carabas, arnd, ankur.a.arora
cpu_relax on ARM64 does a simple "yield". Thus we replace it with
smp_cond_load_relaxed which basically does a "wfe".
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
drivers/cpuidle/poll_state.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
index 9b6d90a72601..440cd713e39a 100644
--- a/drivers/cpuidle/poll_state.c
+++ b/drivers/cpuidle/poll_state.c
@@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
limit = cpuidle_poll_time(drv, dev);
- while (!need_resched()) {
- cpu_relax();
- if (loop_count++ < POLL_IDLE_RELAX_COUNT)
- continue;
-
+ for (;;) {
loop_count = 0;
+
+ smp_cond_load_relaxed(¤t_thread_info()->flags,
+ (VAL & _TIF_NEED_RESCHED) ||
+ (loop_count++ >= POLL_IDLE_RELAX_COUNT));
+
+ if (loop_count < POLL_IDLE_RELAX_COUNT)
+ break;
+
if (local_clock_noinstr() - time_start > limit) {
dev->poll_time_limit = true;
break;
--
1.8.3.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
2023-11-20 14:01 ` Mihai Carabas
@ 2023-11-22 20:51 ` Christoph Lameter
-1 siblings, 0 replies; 46+ messages in thread
From: Christoph Lameter @ 2023-11-22 20:51 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
On Mon, 20 Nov 2023, Mihai Carabas wrote:
> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
> smp_cond_load_relaxed which basically does a "wfe".
Well it clears events first (which requires the first WFE) and then does a
WFE waiting for any events if no events were pending.
WFE does not cause a VMEXIT? Or does the inner loop of
smp_cond_load_relaxed now do 2x VMEXITS?
KVM ARM64 code seems to indicate that WFE causes a VMEXIT. See
kvm_handle_wfx().
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
@ 2023-11-22 20:51 ` Christoph Lameter
0 siblings, 0 replies; 46+ messages in thread
From: Christoph Lameter @ 2023-11-22 20:51 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
On Mon, 20 Nov 2023, Mihai Carabas wrote:
> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
> smp_cond_load_relaxed which basically does a "wfe".
Well it clears events first (which requires the first WFE) and then does a
WFE waiting for any events if no events were pending.
WFE does not cause a VMEXIT? Or does the inner loop of
smp_cond_load_relaxed now do 2x VMEXITS?
KVM ARM64 code seems to indicate that WFE causes a VMEXIT. See
kvm_handle_wfx().
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
2023-11-22 20:51 ` Christoph Lameter
@ 2023-11-22 21:33 ` Mihai Carabas
-1 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-22 21:33 UTC (permalink / raw)
To: Christoph Lameter
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
La 22.11.2023 22:51, Christoph Lameter a scris:
>
> On Mon, 20 Nov 2023, Mihai Carabas wrote:
>
>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>> smp_cond_load_relaxed which basically does a "wfe".
>
> Well it clears events first (which requires the first WFE) and then
> does a WFE waiting for any events if no events were pending.
>
> WFE does not cause a VMEXIT? Or does the inner loop of
> smp_cond_load_relaxed now do 2x VMEXITS?
>
> KVM ARM64 code seems to indicate that WFE causes a VMEXIT. See
> kvm_handle_wfx().
In KVM ARM64 the WFE traping is dynamic: it is enabled only if there are
more tasks waiting on the same core (e.g. on an oversubscribed system).
In arch/arm64/kvm/arm.c:
457 >-------if (single_task_running())
458 >------->-------vcpu_clear_wfx_traps(vcpu);
459 >-------else
460 >------->-------vcpu_set_wfx_traps(vcpu);
This of course can be improved by having a knob where you can completly
disable wfx traping by your needs, but I left this as another subject to
tackle.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
@ 2023-11-22 21:33 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-22 21:33 UTC (permalink / raw)
To: Christoph Lameter
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
La 22.11.2023 22:51, Christoph Lameter a scris:
>
> On Mon, 20 Nov 2023, Mihai Carabas wrote:
>
>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>> smp_cond_load_relaxed which basically does a "wfe".
>
> Well it clears events first (which requires the first WFE) and then
> does a WFE waiting for any events if no events were pending.
>
> WFE does not cause a VMEXIT? Or does the inner loop of
> smp_cond_load_relaxed now do 2x VMEXITS?
>
> KVM ARM64 code seems to indicate that WFE causes a VMEXIT. See
> kvm_handle_wfx().
In KVM ARM64 the WFE traping is dynamic: it is enabled only if there are
more tasks waiting on the same core (e.g. on an oversubscribed system).
In arch/arm64/kvm/arm.c:
457 >-------if (single_task_running())
458 >------->-------vcpu_clear_wfx_traps(vcpu);
459 >-------else
460 >------->-------vcpu_set_wfx_traps(vcpu);
This of course can be improved by having a knob where you can completly
disable wfx traping by your needs, but I left this as another subject to
tackle.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/7] x86: Move ARCH_HAS_CPU_RELAX to arch
2023-11-20 14:01 ` Mihai Carabas
@ 2023-11-27 14:44 ` Petr Mladek
-1 siblings, 0 replies; 46+ messages in thread
From: Petr Mladek @ 2023-11-27 14:44 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
On Mon 2023-11-20 16:01:32, Mihai Carabas wrote:
> From: Joao Martins <joao.m.martins@oracle.com>
>
> ARM64 is going to use it for haltpoll support (for poll-state)
> so move the definition to be arch-agnostic and allow architectures
> to override it.
This says that the definition is moved.
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 4a85a10b12fd..92af0e9bc35e 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -1371,6 +1371,9 @@ config RELR
> config ARCH_HAS_MEM_ENCRYPT
> bool
>
> +config ARCH_HAS_CPU_RELAX
> + bool
> +
> config ARCH_HAS_CC_PLATFORM
> bool
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index d1c362f479d9..0c77670d020e 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -73,6 +73,7 @@ config X86
> select ARCH_HAS_CACHE_LINE_SIZE
> select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
> select ARCH_HAS_CPU_FINALIZE_INIT
> + select ARCH_HAS_CPU_RELAX
> select ARCH_HAS_CURRENT_STACK_POINTER
> select ARCH_HAS_DEBUG_VIRTUAL
> select ARCH_HAS_DEBUG_VM_PGTABLE if !X86_PAE
But the definion is only added here.
I would expect that the patch also removes the original definion.
Best Regards,
Petr
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/7] x86: Move ARCH_HAS_CPU_RELAX to arch
@ 2023-11-27 14:44 ` Petr Mladek
0 siblings, 0 replies; 46+ messages in thread
From: Petr Mladek @ 2023-11-27 14:44 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
On Mon 2023-11-20 16:01:32, Mihai Carabas wrote:
> From: Joao Martins <joao.m.martins@oracle.com>
>
> ARM64 is going to use it for haltpoll support (for poll-state)
> so move the definition to be arch-agnostic and allow architectures
> to override it.
This says that the definition is moved.
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 4a85a10b12fd..92af0e9bc35e 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -1371,6 +1371,9 @@ config RELR
> config ARCH_HAS_MEM_ENCRYPT
> bool
>
> +config ARCH_HAS_CPU_RELAX
> + bool
> +
> config ARCH_HAS_CC_PLATFORM
> bool
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index d1c362f479d9..0c77670d020e 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -73,6 +73,7 @@ config X86
> select ARCH_HAS_CACHE_LINE_SIZE
> select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
> select ARCH_HAS_CPU_FINALIZE_INIT
> + select ARCH_HAS_CPU_RELAX
> select ARCH_HAS_CURRENT_STACK_POINTER
> select ARCH_HAS_DEBUG_VIRTUAL
> select ARCH_HAS_DEBUG_VM_PGTABLE if !X86_PAE
But the definion is only added here.
I would expect that the patch also removes the original definion.
Best Regards,
Petr
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
2023-11-22 21:33 ` Mihai Carabas
@ 2023-11-27 20:17 ` Christoph Lameter (Ampere)
-1 siblings, 0 replies; 46+ messages in thread
From: Christoph Lameter (Ampere) @ 2023-11-27 20:17 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]
On Wed, 22 Nov 2023, Mihai Carabas wrote:
> La 22.11.2023 22:51, Christoph Lameter a scris:
>>
>> On Mon, 20 Nov 2023, Mihai Carabas wrote:
>>
>>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>>> smp_cond_load_relaxed which basically does a "wfe".
>>
>> Well it clears events first (which requires the first WFE) and then does a
>> WFE waiting for any events if no events were pending.
>>
>> WFE does not cause a VMEXIT? Or does the inner loop of
>> smp_cond_load_relaxed now do 2x VMEXITS?
>>
>> KVM ARM64 code seems to indicate that WFE causes a VMEXIT. See
>> kvm_handle_wfx().
>
> In KVM ARM64 the WFE traping is dynamic: it is enabled only if there are more
> tasks waiting on the same core (e.g. on an oversubscribed system).
>
> In arch/arm64/kvm/arm.c:
>
> 457 >-------if (single_task_running())
> 458 >------->-------vcpu_clear_wfx_traps(vcpu);
> 459 >-------else
> 460 >------->-------vcpu_set_wfx_traps(vcpu);
Ahh. Cool did not know about that. But still: Lots of VMEXITs once the
load has to be shared.
> This of course can be improved by having a knob where you can completly
> disable wfx traping by your needs, but I left this as another subject to
> tackle.
kvm_arch_vcpu_load() looks strange. On the one hand we pass a cpu
number into it and then we use functions that only work if we are running
on that cpu?
It would be better to use smp_processor_id() in the function
and not pass the cpu number to it.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
@ 2023-11-27 20:17 ` Christoph Lameter (Ampere)
0 siblings, 0 replies; 46+ messages in thread
From: Christoph Lameter (Ampere) @ 2023-11-27 20:17 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]
On Wed, 22 Nov 2023, Mihai Carabas wrote:
> La 22.11.2023 22:51, Christoph Lameter a scris:
>>
>> On Mon, 20 Nov 2023, Mihai Carabas wrote:
>>
>>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>>> smp_cond_load_relaxed which basically does a "wfe".
>>
>> Well it clears events first (which requires the first WFE) and then does a
>> WFE waiting for any events if no events were pending.
>>
>> WFE does not cause a VMEXIT? Or does the inner loop of
>> smp_cond_load_relaxed now do 2x VMEXITS?
>>
>> KVM ARM64 code seems to indicate that WFE causes a VMEXIT. See
>> kvm_handle_wfx().
>
> In KVM ARM64 the WFE traping is dynamic: it is enabled only if there are more
> tasks waiting on the same core (e.g. on an oversubscribed system).
>
> In arch/arm64/kvm/arm.c:
>
> 457 >-------if (single_task_running())
> 458 >------->-------vcpu_clear_wfx_traps(vcpu);
> 459 >-------else
> 460 >------->-------vcpu_set_wfx_traps(vcpu);
Ahh. Cool did not know about that. But still: Lots of VMEXITs once the
load has to be shared.
> This of course can be improved by having a knob where you can completly
> disable wfx traping by your needs, but I left this as another subject to
> tackle.
kvm_arch_vcpu_load() looks strange. On the one hand we pass a cpu
number into it and then we use functions that only work if we are running
on that cpu?
It would be better to use smp_processor_id() in the function
and not pass the cpu number to it.
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/7] x86: Move ARCH_HAS_CPU_RELAX to arch
2023-11-27 14:44 ` Petr Mladek
@ 2023-11-28 14:09 ` Mihai Carabas
-1 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-28 14:09 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
>> From: Joao Martins <joao.m.martins@oracle.com>
>>
>> ARM64 is going to use it for haltpoll support (for poll-state)
>> so move the definition to be arch-agnostic and allow architectures
>> to override it.
> This says that the definition is moved.
>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index 4a85a10b12fd..92af0e9bc35e 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -1371,6 +1371,9 @@ config RELR
>> config ARCH_HAS_MEM_ENCRYPT
>> bool
>>
>> +config ARCH_HAS_CPU_RELAX
>> + bool
>> +
>> config ARCH_HAS_CC_PLATFORM
>> bool
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index d1c362f479d9..0c77670d020e 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -73,6 +73,7 @@ config X86
>> select ARCH_HAS_CACHE_LINE_SIZE
>> select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
>> select ARCH_HAS_CPU_FINALIZE_INIT
>> + select ARCH_HAS_CPU_RELAX
>> select ARCH_HAS_CURRENT_STACK_POINTER
>> select ARCH_HAS_DEBUG_VIRTUAL
>> select ARCH_HAS_DEBUG_VM_PGTABLE if !X86_PAE
> But the definion is only added here.
>
> I would expect that the patch also removes the original definion.
Thanks for catching this. I updated the patch:
diff --git a/arch/Kconfig b/arch/Kconfig
index 12d51495caec..626ddd9ba7e0 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1371,6 +1371,9 @@ config RELR
config ARCH_HAS_MEM_ENCRYPT
bool
+config ARCH_HAS_CPU_RELAX
+ bool
+
config ARCH_HAS_CC_PLATFORM
bool
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 66bfabae8814..aaca90ba791a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -72,6 +72,7 @@ config X86
select ARCH_HAS_CACHE_LINE_SIZE
select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
select ARCH_HAS_CPU_FINALIZE_INIT
+ select ARCH_HAS_CPU_RELAX
select ARCH_HAS_CURRENT_STACK_POINTER
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEBUG_VM_PGTABLE if !X86_PAE
@@ -363,9 +364,6 @@ config ARCH_MAY_HAVE_PC_FDC
config GENERIC_CALIBRATE_DELAY
def_bool y
-config ARCH_HAS_CPU_RELAX
- def_bool y
-
config ARCH_HIBERNATION_POSSIBLE
def_bool y
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCH 1/7] x86: Move ARCH_HAS_CPU_RELAX to arch
@ 2023-11-28 14:09 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2023-11-28 14:09 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
>> From: Joao Martins <joao.m.martins@oracle.com>
>>
>> ARM64 is going to use it for haltpoll support (for poll-state)
>> so move the definition to be arch-agnostic and allow architectures
>> to override it.
> This says that the definition is moved.
>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index 4a85a10b12fd..92af0e9bc35e 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -1371,6 +1371,9 @@ config RELR
>> config ARCH_HAS_MEM_ENCRYPT
>> bool
>>
>> +config ARCH_HAS_CPU_RELAX
>> + bool
>> +
>> config ARCH_HAS_CC_PLATFORM
>> bool
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index d1c362f479d9..0c77670d020e 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -73,6 +73,7 @@ config X86
>> select ARCH_HAS_CACHE_LINE_SIZE
>> select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
>> select ARCH_HAS_CPU_FINALIZE_INIT
>> + select ARCH_HAS_CPU_RELAX
>> select ARCH_HAS_CURRENT_STACK_POINTER
>> select ARCH_HAS_DEBUG_VIRTUAL
>> select ARCH_HAS_DEBUG_VM_PGTABLE if !X86_PAE
> But the definion is only added here.
>
> I would expect that the patch also removes the original definion.
Thanks for catching this. I updated the patch:
diff --git a/arch/Kconfig b/arch/Kconfig
index 12d51495caec..626ddd9ba7e0 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1371,6 +1371,9 @@ config RELR
config ARCH_HAS_MEM_ENCRYPT
bool
+config ARCH_HAS_CPU_RELAX
+ bool
+
config ARCH_HAS_CC_PLATFORM
bool
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 66bfabae8814..aaca90ba791a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -72,6 +72,7 @@ config X86
select ARCH_HAS_CACHE_LINE_SIZE
select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
select ARCH_HAS_CPU_FINALIZE_INIT
+ select ARCH_HAS_CPU_RELAX
select ARCH_HAS_CURRENT_STACK_POINTER
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEBUG_VM_PGTABLE if !X86_PAE
@@ -363,9 +364,6 @@ config ARCH_MAY_HAVE_PC_FDC
config GENERIC_CALIBRATE_DELAY
def_bool y
-config ARCH_HAS_CPU_RELAX
- def_bool y
-
config ARCH_HIBERNATION_POSSIBLE
def_bool y
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCH 2/7] x86/kvm: Move haltpoll_want() to be arch defined
2023-11-20 14:01 ` Mihai Carabas
@ 2023-11-29 20:55 ` Rafael J. Wysocki
-1 siblings, 0 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2023-11-29 20:55 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
On Mon, Nov 20, 2023 at 4:15 PM Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> From: Joao Martins <joao.m.martins@oracle.com>
>
> Right now, kvm_para_has_hint(KVM_HINTS_REALTIME) is x86 only, and so in the
> pursuit of making cpuidle-haltpoll arch independent, move the check for
> haltpoll enablement to be defined per architecture. Same thing for
> boot_option_idle_override. To that end, add a arch_haltpoll_want() and move the
> check there.
>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
From the cpuidle side:
Acked-by: Rafael J. Wysocki <rafael@kernel.org>
> ---
> arch/x86/include/asm/cpuidle_haltpoll.h | 1 +
> arch/x86/kernel/kvm.c | 10 ++++++++++
> drivers/cpuidle/cpuidle-haltpoll.c | 8 ++------
> include/linux/cpuidle_haltpoll.h | 5 +++++
> 4 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/cpuidle_haltpoll.h b/arch/x86/include/asm/cpuidle_haltpoll.h
> index c8b39c6716ff..2c5a53ce266f 100644
> --- a/arch/x86/include/asm/cpuidle_haltpoll.h
> +++ b/arch/x86/include/asm/cpuidle_haltpoll.h
> @@ -4,5 +4,6 @@
>
> void arch_haltpoll_enable(unsigned int cpu);
> void arch_haltpoll_disable(unsigned int cpu);
> +bool arch_haltpoll_want(void);
>
> #endif
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 1cceac5984da..75a24f107b2a 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -1151,4 +1151,14 @@ void arch_haltpoll_disable(unsigned int cpu)
> smp_call_function_single(cpu, kvm_enable_host_haltpoll, NULL, 1);
> }
> EXPORT_SYMBOL_GPL(arch_haltpoll_disable);
> +
> +bool arch_haltpoll_want(void)
> +{
> + /* Do not load haltpoll if idle= is passed */
> + if (boot_option_idle_override != IDLE_NO_OVERRIDE)
> + return false;
> +
> + return kvm_para_has_hint(KVM_HINTS_REALTIME);
> +}
> +EXPORT_SYMBOL_GPL(arch_haltpoll_want);
> #endif
> diff --git a/drivers/cpuidle/cpuidle-haltpoll.c b/drivers/cpuidle/cpuidle-haltpoll.c
> index e66df22f9695..72f9c84990c5 100644
> --- a/drivers/cpuidle/cpuidle-haltpoll.c
> +++ b/drivers/cpuidle/cpuidle-haltpoll.c
> @@ -96,7 +96,7 @@ static void haltpoll_uninit(void)
>
> static bool haltpoll_want(void)
> {
> - return kvm_para_has_hint(KVM_HINTS_REALTIME) || force;
> + return (kvm_para_available() && arch_haltpoll_want()) || force;
> }
>
> static int __init haltpoll_init(void)
> @@ -104,11 +104,7 @@ static int __init haltpoll_init(void)
> int ret;
> struct cpuidle_driver *drv = &haltpoll_driver;
>
> - /* Do not load haltpoll if idle= is passed */
> - if (boot_option_idle_override != IDLE_NO_OVERRIDE)
> - return -ENODEV;
> -
> - if (!kvm_para_available() || !haltpoll_want())
> + if (!haltpoll_want())
> return -ENODEV;
>
> cpuidle_poll_state_init(drv);
> diff --git a/include/linux/cpuidle_haltpoll.h b/include/linux/cpuidle_haltpoll.h
> index d50c1e0411a2..bae68a6603e3 100644
> --- a/include/linux/cpuidle_haltpoll.h
> +++ b/include/linux/cpuidle_haltpoll.h
> @@ -12,5 +12,10 @@ static inline void arch_haltpoll_enable(unsigned int cpu)
> static inline void arch_haltpoll_disable(unsigned int cpu)
> {
> }
> +
> +static inline bool arch_haltpoll_want(void)
> +{
> + return false;
> +}
> #endif
> #endif
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 2/7] x86/kvm: Move haltpoll_want() to be arch defined
@ 2023-11-29 20:55 ` Rafael J. Wysocki
0 siblings, 0 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2023-11-29 20:55 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
On Mon, Nov 20, 2023 at 4:15 PM Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> From: Joao Martins <joao.m.martins@oracle.com>
>
> Right now, kvm_para_has_hint(KVM_HINTS_REALTIME) is x86 only, and so in the
> pursuit of making cpuidle-haltpoll arch independent, move the check for
> haltpoll enablement to be defined per architecture. Same thing for
> boot_option_idle_override. To that end, add a arch_haltpoll_want() and move the
> check there.
>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
From the cpuidle side:
Acked-by: Rafael J. Wysocki <rafael@kernel.org>
> ---
> arch/x86/include/asm/cpuidle_haltpoll.h | 1 +
> arch/x86/kernel/kvm.c | 10 ++++++++++
> drivers/cpuidle/cpuidle-haltpoll.c | 8 ++------
> include/linux/cpuidle_haltpoll.h | 5 +++++
> 4 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/cpuidle_haltpoll.h b/arch/x86/include/asm/cpuidle_haltpoll.h
> index c8b39c6716ff..2c5a53ce266f 100644
> --- a/arch/x86/include/asm/cpuidle_haltpoll.h
> +++ b/arch/x86/include/asm/cpuidle_haltpoll.h
> @@ -4,5 +4,6 @@
>
> void arch_haltpoll_enable(unsigned int cpu);
> void arch_haltpoll_disable(unsigned int cpu);
> +bool arch_haltpoll_want(void);
>
> #endif
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 1cceac5984da..75a24f107b2a 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -1151,4 +1151,14 @@ void arch_haltpoll_disable(unsigned int cpu)
> smp_call_function_single(cpu, kvm_enable_host_haltpoll, NULL, 1);
> }
> EXPORT_SYMBOL_GPL(arch_haltpoll_disable);
> +
> +bool arch_haltpoll_want(void)
> +{
> + /* Do not load haltpoll if idle= is passed */
> + if (boot_option_idle_override != IDLE_NO_OVERRIDE)
> + return false;
> +
> + return kvm_para_has_hint(KVM_HINTS_REALTIME);
> +}
> +EXPORT_SYMBOL_GPL(arch_haltpoll_want);
> #endif
> diff --git a/drivers/cpuidle/cpuidle-haltpoll.c b/drivers/cpuidle/cpuidle-haltpoll.c
> index e66df22f9695..72f9c84990c5 100644
> --- a/drivers/cpuidle/cpuidle-haltpoll.c
> +++ b/drivers/cpuidle/cpuidle-haltpoll.c
> @@ -96,7 +96,7 @@ static void haltpoll_uninit(void)
>
> static bool haltpoll_want(void)
> {
> - return kvm_para_has_hint(KVM_HINTS_REALTIME) || force;
> + return (kvm_para_available() && arch_haltpoll_want()) || force;
> }
>
> static int __init haltpoll_init(void)
> @@ -104,11 +104,7 @@ static int __init haltpoll_init(void)
> int ret;
> struct cpuidle_driver *drv = &haltpoll_driver;
>
> - /* Do not load haltpoll if idle= is passed */
> - if (boot_option_idle_override != IDLE_NO_OVERRIDE)
> - return -ENODEV;
> -
> - if (!kvm_para_available() || !haltpoll_want())
> + if (!haltpoll_want())
> return -ENODEV;
>
> cpuidle_poll_state_init(drv);
> diff --git a/include/linux/cpuidle_haltpoll.h b/include/linux/cpuidle_haltpoll.h
> index d50c1e0411a2..bae68a6603e3 100644
> --- a/include/linux/cpuidle_haltpoll.h
> +++ b/include/linux/cpuidle_haltpoll.h
> @@ -12,5 +12,10 @@ static inline void arch_haltpoll_enable(unsigned int cpu)
> static inline void arch_haltpoll_disable(unsigned int cpu)
> {
> }
> +
> +static inline bool arch_haltpoll_want(void)
> +{
> + return false;
> +}
> #endif
> #endif
> --
> 1.8.3.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
2023-11-27 20:17 ` Christoph Lameter (Ampere)
@ 2023-12-01 6:59 ` Ankur Arora
-1 siblings, 0 replies; 46+ messages in thread
From: Ankur Arora @ 2023-12-01 6:59 UTC (permalink / raw)
To: Christoph Lameter (Ampere)
Cc: Mihai Carabas, linux-arm-kernel, kvm, linux-pm, linux-kernel,
catalin.marinas, will, tglx, mingo, bp, x86, hpa, pbonzini,
wanpengli, vkuznets, rafael, daniel.lezcano, akpm, pmladek,
peterz, dianders, npiggin, rick.p.edgecombe, joao.m.martins,
juerg.haefliger, mic, arnd, ankur.a.arora
Christoph Lameter (Ampere) <cl@linux.com> writes:
> On Wed, 22 Nov 2023, Mihai Carabas wrote:
>
>> La 22.11.2023 22:51, Christoph Lameter a scris:
>>> On Mon, 20 Nov 2023, Mihai Carabas wrote:
>>>
>>>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>>>> smp_cond_load_relaxed which basically does a "wfe".
>>> Well it clears events first (which requires the first WFE) and then does a
>>> WFE waiting for any events if no events were pending.
>>> WFE does not cause a VMEXIT? Or does the inner loop of
>>> smp_cond_load_relaxed now do 2x VMEXITS?
>>> KVM ARM64 code seems to indicate that WFE causes a VMEXIT. See
>>> kvm_handle_wfx().
>>
>> In KVM ARM64 the WFE traping is dynamic: it is enabled only if there are more
>> tasks waiting on the same core (e.g. on an oversubscribed system).
>>
>> In arch/arm64/kvm/arm.c:
>>
>> 457 >-------if (single_task_running())
>> 458 >------->-------vcpu_clear_wfx_traps(vcpu);
>> 459 >-------else
>> 460 >------->-------vcpu_set_wfx_traps(vcpu);
>
> Ahh. Cool did not know about that. But still: Lots of VMEXITs once the load has
> to be shared.
Yeah, anytime there's more than one runnable process. Another, more
critical place where we will vmexit is the qspinlock slowpath which
uses smp_cond_load.
>> This of course can be improved by having a knob where you can completly
>> disable wfx traping by your needs, but I left this as another subject to
>> tackle.
Probably needs to be adaptive since we use WFE in error paths as well
(for instance to park the CPU.)
Ankur
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
@ 2023-12-01 6:59 ` Ankur Arora
0 siblings, 0 replies; 46+ messages in thread
From: Ankur Arora @ 2023-12-01 6:59 UTC (permalink / raw)
To: Christoph Lameter (Ampere)
Cc: Mihai Carabas, linux-arm-kernel, kvm, linux-pm, linux-kernel,
catalin.marinas, will, tglx, mingo, bp, x86, hpa, pbonzini,
wanpengli, vkuznets, rafael, daniel.lezcano, akpm, pmladek,
peterz, dianders, npiggin, rick.p.edgecombe, joao.m.martins,
juerg.haefliger, mic, arnd, ankur.a.arora
Christoph Lameter (Ampere) <cl@linux.com> writes:
> On Wed, 22 Nov 2023, Mihai Carabas wrote:
>
>> La 22.11.2023 22:51, Christoph Lameter a scris:
>>> On Mon, 20 Nov 2023, Mihai Carabas wrote:
>>>
>>>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>>>> smp_cond_load_relaxed which basically does a "wfe".
>>> Well it clears events first (which requires the first WFE) and then does a
>>> WFE waiting for any events if no events were pending.
>>> WFE does not cause a VMEXIT? Or does the inner loop of
>>> smp_cond_load_relaxed now do 2x VMEXITS?
>>> KVM ARM64 code seems to indicate that WFE causes a VMEXIT. See
>>> kvm_handle_wfx().
>>
>> In KVM ARM64 the WFE traping is dynamic: it is enabled only if there are more
>> tasks waiting on the same core (e.g. on an oversubscribed system).
>>
>> In arch/arm64/kvm/arm.c:
>>
>> 457 >-------if (single_task_running())
>> 458 >------->-------vcpu_clear_wfx_traps(vcpu);
>> 459 >-------else
>> 460 >------->-------vcpu_set_wfx_traps(vcpu);
>
> Ahh. Cool did not know about that. But still: Lots of VMEXITs once the load has
> to be shared.
Yeah, anytime there's more than one runnable process. Another, more
critical place where we will vmexit is the qspinlock slowpath which
uses smp_cond_load.
>> This of course can be improved by having a knob where you can completly
>> disable wfx traping by your needs, but I left this as another subject to
>> tackle.
Probably needs to be adaptive since we use WFE in error paths as well
(for instance to park the CPU.)
Ankur
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
2023-11-20 14:01 ` Mihai Carabas
@ 2023-12-11 11:46 ` Will Deacon
-1 siblings, 0 replies; 46+ messages in thread
From: Will Deacon @ 2023-12-11 11:46 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
On Mon, Nov 20, 2023 at 04:01:38PM +0200, Mihai Carabas wrote:
> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
> smp_cond_load_relaxed which basically does a "wfe".
>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
> ---
> drivers/cpuidle/poll_state.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
> index 9b6d90a72601..440cd713e39a 100644
> --- a/drivers/cpuidle/poll_state.c
> +++ b/drivers/cpuidle/poll_state.c
> @@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
>
> limit = cpuidle_poll_time(drv, dev);
>
> - while (!need_resched()) {
> - cpu_relax();
> - if (loop_count++ < POLL_IDLE_RELAX_COUNT)
> - continue;
> -
> + for (;;) {
> loop_count = 0;
> +
> + smp_cond_load_relaxed(¤t_thread_info()->flags,
> + (VAL & _TIF_NEED_RESCHED) ||
> + (loop_count++ >= POLL_IDLE_RELAX_COUNT));
> +
> + if (loop_count < POLL_IDLE_RELAX_COUNT)
> + break;
> +
> if (local_clock_noinstr() - time_start > limit) {
> dev->poll_time_limit = true;
> break;
Doesn't this make ARCH_HAS_CPU_RELAX a complete misnomer?
Will
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
@ 2023-12-11 11:46 ` Will Deacon
0 siblings, 0 replies; 46+ messages in thread
From: Will Deacon @ 2023-12-11 11:46 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
On Mon, Nov 20, 2023 at 04:01:38PM +0200, Mihai Carabas wrote:
> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
> smp_cond_load_relaxed which basically does a "wfe".
>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
> ---
> drivers/cpuidle/poll_state.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
> index 9b6d90a72601..440cd713e39a 100644
> --- a/drivers/cpuidle/poll_state.c
> +++ b/drivers/cpuidle/poll_state.c
> @@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
>
> limit = cpuidle_poll_time(drv, dev);
>
> - while (!need_resched()) {
> - cpu_relax();
> - if (loop_count++ < POLL_IDLE_RELAX_COUNT)
> - continue;
> -
> + for (;;) {
> loop_count = 0;
> +
> + smp_cond_load_relaxed(¤t_thread_info()->flags,
> + (VAL & _TIF_NEED_RESCHED) ||
> + (loop_count++ >= POLL_IDLE_RELAX_COUNT));
> +
> + if (loop_count < POLL_IDLE_RELAX_COUNT)
> + break;
> +
> if (local_clock_noinstr() - time_start > limit) {
> dev->poll_time_limit = true;
> break;
Doesn't this make ARCH_HAS_CPU_RELAX a complete misnomer?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2] Enable haltpoll for arm64
2023-11-20 14:01 ` Mihai Carabas
@ 2024-01-17 21:19 ` Christoph Lameter (Ampere)
-1 siblings, 0 replies; 46+ messages in thread
From: Christoph Lameter (Ampere) @ 2024-01-17 21:19 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
On Mon, 20 Nov 2023, Mihai Carabas wrote:
> This patchset enables the usage of haltpoll governer on arm64. This is
> specifically interesting for KVM guests by reducing the IPC latencies.
Any updates on this one? We see good improvements with this patchset using
a variety of loads on Ampere platforms.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2] Enable haltpoll for arm64
@ 2024-01-17 21:19 ` Christoph Lameter (Ampere)
0 siblings, 0 replies; 46+ messages in thread
From: Christoph Lameter (Ampere) @ 2024-01-17 21:19 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
will, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets,
rafael, daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
On Mon, 20 Nov 2023, Mihai Carabas wrote:
> This patchset enables the usage of haltpoll governer on arm64. This is
> specifically interesting for KVM guests by reducing the IPC latencies.
Any updates on this one? We see good improvements with this patchset using
a variety of loads on Ampere platforms.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2] Enable haltpoll for arm64
2023-11-20 14:01 ` Mihai Carabas
@ 2024-01-25 14:39 ` Mihai Carabas
-1 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2024-01-25 14:39 UTC (permalink / raw)
To: linux-arm-kernel, Peter Zijlstra
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
Hello,
How can we move this patchset forward?
Thank you,
Mihai
La 20.11.2023 16:01, Mihai Carabas a scris:
> This patchset enables the usage of haltpoll governer on arm64. This is
> specifically interesting for KVM guests by reducing the IPC latencies.
>
> Here are some benchmarks without/with haltpoll for a KVM guest:
>
> a) without haltpoll:
> perf bench sched pipe
> # Running 'sched/pipe' benchmark:
> # Executed 1000000 pipe operations between two processes
>
> Total time: 8.138 [sec]
>
> 8.138094 usecs/op
> 122878 ops/sec
>
> b) with haltpoll:
> perf bench sched pipe
> # Running 'sched/pipe' benchmark:
> # Executed 1000000 pipe operations between two processes
>
> Total time: 5.003 [sec]
>
> 5.003085 usecs/op
> 199876 ops/sec
>
> v2 changes from v1:
> - added patch 7 where we change cpu_relax with smp_cond_load_relaxed per PeterZ
> (this improves by 50% at least the CPU cycles consumed in the tests above:
> 10,716,881,137 now vs 14,503,014,257 before)
> - removed the ifdef from patch 1 per RafaelW
>
>
> Joao Martins (6):
> x86: Move ARCH_HAS_CPU_RELAX to arch
> x86/kvm: Move haltpoll_want() to be arch defined
> governors/haltpoll: Drop kvm_para_available() check
> arm64: Select ARCH_HAS_CPU_RELAX
> arm64: Define TIF_POLLING_NRFLAG
> cpuidle-haltpoll: ARM64 support
>
> Mihai Carabas (1):
> cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
>
> arch/Kconfig | 3 +++
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/thread_info.h | 6 ++++++
> arch/x86/Kconfig | 1 +
> arch/x86/include/asm/cpuidle_haltpoll.h | 1 +
> arch/x86/kernel/kvm.c | 10 ++++++++++
> drivers/cpuidle/Kconfig | 4 ++--
> drivers/cpuidle/cpuidle-haltpoll.c | 8 ++------
> drivers/cpuidle/governors/haltpoll.c | 5 +----
> drivers/cpuidle/poll_state.c | 14 +++++++++-----
> include/linux/cpuidle_haltpoll.h | 5 +++++
> 11 files changed, 41 insertions(+), 17 deletions(-)
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2] Enable haltpoll for arm64
@ 2024-01-25 14:39 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2024-01-25 14:39 UTC (permalink / raw)
To: linux-arm-kernel, Peter Zijlstra
Cc: kvm, linux-pm, linux-kernel, catalin.marinas, will, tglx, mingo,
bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
Hello,
How can we move this patchset forward?
Thank you,
Mihai
La 20.11.2023 16:01, Mihai Carabas a scris:
> This patchset enables the usage of haltpoll governer on arm64. This is
> specifically interesting for KVM guests by reducing the IPC latencies.
>
> Here are some benchmarks without/with haltpoll for a KVM guest:
>
> a) without haltpoll:
> perf bench sched pipe
> # Running 'sched/pipe' benchmark:
> # Executed 1000000 pipe operations between two processes
>
> Total time: 8.138 [sec]
>
> 8.138094 usecs/op
> 122878 ops/sec
>
> b) with haltpoll:
> perf bench sched pipe
> # Running 'sched/pipe' benchmark:
> # Executed 1000000 pipe operations between two processes
>
> Total time: 5.003 [sec]
>
> 5.003085 usecs/op
> 199876 ops/sec
>
> v2 changes from v1:
> - added patch 7 where we change cpu_relax with smp_cond_load_relaxed per PeterZ
> (this improves by 50% at least the CPU cycles consumed in the tests above:
> 10,716,881,137 now vs 14,503,014,257 before)
> - removed the ifdef from patch 1 per RafaelW
>
>
> Joao Martins (6):
> x86: Move ARCH_HAS_CPU_RELAX to arch
> x86/kvm: Move haltpoll_want() to be arch defined
> governors/haltpoll: Drop kvm_para_available() check
> arm64: Select ARCH_HAS_CPU_RELAX
> arm64: Define TIF_POLLING_NRFLAG
> cpuidle-haltpoll: ARM64 support
>
> Mihai Carabas (1):
> cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
>
> arch/Kconfig | 3 +++
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/thread_info.h | 6 ++++++
> arch/x86/Kconfig | 1 +
> arch/x86/include/asm/cpuidle_haltpoll.h | 1 +
> arch/x86/kernel/kvm.c | 10 ++++++++++
> drivers/cpuidle/Kconfig | 4 ++--
> drivers/cpuidle/cpuidle-haltpoll.c | 8 ++------
> drivers/cpuidle/governors/haltpoll.c | 5 +----
> drivers/cpuidle/poll_state.c | 14 +++++++++-----
> include/linux/cpuidle_haltpoll.h | 5 +++++
> 11 files changed, 41 insertions(+), 17 deletions(-)
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2] Enable haltpoll for arm64
2024-01-25 14:39 ` Mihai Carabas
@ 2024-01-25 15:16 ` Rafael J. Wysocki
-1 siblings, 0 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2024-01-25 15:16 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, Peter Zijlstra, kvm, linux-pm, linux-kernel,
catalin.marinas, will, tglx, mingo, bp, x86, hpa, pbonzini,
wanpengli, vkuznets, rafael, daniel.lezcano, akpm, pmladek,
dianders, npiggin, rick.p.edgecombe, joao.m.martins,
juerg.haefliger, mic, arnd, ankur.a.arora
Hi,
On Thu, Jan 25, 2024 at 3:40 PM Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> Hello,
>
> How can we move this patchset forward?
There have been some comments and a tag has been given.
Update the series (to address the comments and add the tag where
applicable) and resend.
Thanks!
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2] Enable haltpoll for arm64
@ 2024-01-25 15:16 ` Rafael J. Wysocki
0 siblings, 0 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2024-01-25 15:16 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, Peter Zijlstra, kvm, linux-pm, linux-kernel,
catalin.marinas, will, tglx, mingo, bp, x86, hpa, pbonzini,
wanpengli, vkuznets, rafael, daniel.lezcano, akpm, pmladek,
dianders, npiggin, rick.p.edgecombe, joao.m.martins,
juerg.haefliger, mic, arnd, ankur.a.arora
Hi,
On Thu, Jan 25, 2024 at 3:40 PM Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> Hello,
>
> How can we move this patchset forward?
There have been some comments and a tag has been given.
Update the series (to address the comments and add the tag where
applicable) and resend.
Thanks!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
2023-12-11 11:46 ` Will Deacon
@ 2024-01-28 21:22 ` Mihai Carabas
-1 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2024-01-28 21:22 UTC (permalink / raw)
To: Will Deacon
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
La 11.12.2023 13:46, Will Deacon a scris:
> On Mon, Nov 20, 2023 at 04:01:38PM +0200, Mihai Carabas wrote:
>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>> smp_cond_load_relaxed which basically does a "wfe".
>>
>> Suggested-by: Peter Zijlstra <peterz@infradead.org>
>> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
>> ---
>> drivers/cpuidle/poll_state.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
>> index 9b6d90a72601..440cd713e39a 100644
>> --- a/drivers/cpuidle/poll_state.c
>> +++ b/drivers/cpuidle/poll_state.c
>> @@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
>>
>> limit = cpuidle_poll_time(drv, dev);
>>
>> - while (!need_resched()) {
>> - cpu_relax();
>> - if (loop_count++ < POLL_IDLE_RELAX_COUNT)
>> - continue;
>> -
>> + for (;;) {
>> loop_count = 0;
>> +
>> + smp_cond_load_relaxed(¤t_thread_info()->flags,
>> + (VAL & _TIF_NEED_RESCHED) ||
>> + (loop_count++ >= POLL_IDLE_RELAX_COUNT));
>> +
>> + if (loop_count < POLL_IDLE_RELAX_COUNT)
>> + break;
>> +
>> if (local_clock_noinstr() - time_start > limit) {
>> dev->poll_time_limit = true;
>> break;
> Doesn't this make ARCH_HAS_CPU_RELAX a complete misnomer?
This controls the build of poll_state.c and the generic definition of
smp_cond_load_relaxed (used by x86) is using cpu_relax(). Do you propose
other approach here?
>
> Will
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
@ 2024-01-28 21:22 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2024-01-28 21:22 UTC (permalink / raw)
To: Will Deacon
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
La 11.12.2023 13:46, Will Deacon a scris:
> On Mon, Nov 20, 2023 at 04:01:38PM +0200, Mihai Carabas wrote:
>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>> smp_cond_load_relaxed which basically does a "wfe".
>>
>> Suggested-by: Peter Zijlstra <peterz@infradead.org>
>> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
>> ---
>> drivers/cpuidle/poll_state.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
>> index 9b6d90a72601..440cd713e39a 100644
>> --- a/drivers/cpuidle/poll_state.c
>> +++ b/drivers/cpuidle/poll_state.c
>> @@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
>>
>> limit = cpuidle_poll_time(drv, dev);
>>
>> - while (!need_resched()) {
>> - cpu_relax();
>> - if (loop_count++ < POLL_IDLE_RELAX_COUNT)
>> - continue;
>> -
>> + for (;;) {
>> loop_count = 0;
>> +
>> + smp_cond_load_relaxed(¤t_thread_info()->flags,
>> + (VAL & _TIF_NEED_RESCHED) ||
>> + (loop_count++ >= POLL_IDLE_RELAX_COUNT));
>> +
>> + if (loop_count < POLL_IDLE_RELAX_COUNT)
>> + break;
>> +
>> if (local_clock_noinstr() - time_start > limit) {
>> dev->poll_time_limit = true;
>> break;
> Doesn't this make ARCH_HAS_CPU_RELAX a complete misnomer?
This controls the build of poll_state.c and the generic definition of
smp_cond_load_relaxed (used by x86) is using cpu_relax(). Do you propose
other approach here?
>
> Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
2024-01-28 21:22 ` Mihai Carabas
@ 2024-01-29 18:15 ` Will Deacon
-1 siblings, 0 replies; 46+ messages in thread
From: Will Deacon @ 2024-01-29 18:15 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
On Sun, Jan 28, 2024 at 11:22:50PM +0200, Mihai Carabas wrote:
> La 11.12.2023 13:46, Will Deacon a scris:
> > On Mon, Nov 20, 2023 at 04:01:38PM +0200, Mihai Carabas wrote:
> > > cpu_relax on ARM64 does a simple "yield". Thus we replace it with
> > > smp_cond_load_relaxed which basically does a "wfe".
> > >
> > > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > > Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
> > > ---
> > > drivers/cpuidle/poll_state.c | 14 +++++++++-----
> > > 1 file changed, 9 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
> > > index 9b6d90a72601..440cd713e39a 100644
> > > --- a/drivers/cpuidle/poll_state.c
> > > +++ b/drivers/cpuidle/poll_state.c
> > > @@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
> > > limit = cpuidle_poll_time(drv, dev);
> > > - while (!need_resched()) {
> > > - cpu_relax();
> > > - if (loop_count++ < POLL_IDLE_RELAX_COUNT)
> > > - continue;
> > > -
> > > + for (;;) {
> > > loop_count = 0;
> > > +
> > > + smp_cond_load_relaxed(¤t_thread_info()->flags,
> > > + (VAL & _TIF_NEED_RESCHED) ||
> > > + (loop_count++ >= POLL_IDLE_RELAX_COUNT));
> > > +
> > > + if (loop_count < POLL_IDLE_RELAX_COUNT)
> > > + break;
> > > +
> > > if (local_clock_noinstr() - time_start > limit) {
> > > dev->poll_time_limit = true;
> > > break;
> > Doesn't this make ARCH_HAS_CPU_RELAX a complete misnomer?
>
> This controls the build of poll_state.c and the generic definition of
> smp_cond_load_relaxed (used by x86) is using cpu_relax(). Do you propose
> other approach here?
Give it a better name? Having ARCH_HAS_CPU_RELAX control a piece of code
that doesn't use cpu_relax() doesn't make sense to me.
Will
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
@ 2024-01-29 18:15 ` Will Deacon
0 siblings, 0 replies; 46+ messages in thread
From: Will Deacon @ 2024-01-29 18:15 UTC (permalink / raw)
To: Mihai Carabas
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
On Sun, Jan 28, 2024 at 11:22:50PM +0200, Mihai Carabas wrote:
> La 11.12.2023 13:46, Will Deacon a scris:
> > On Mon, Nov 20, 2023 at 04:01:38PM +0200, Mihai Carabas wrote:
> > > cpu_relax on ARM64 does a simple "yield". Thus we replace it with
> > > smp_cond_load_relaxed which basically does a "wfe".
> > >
> > > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > > Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
> > > ---
> > > drivers/cpuidle/poll_state.c | 14 +++++++++-----
> > > 1 file changed, 9 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
> > > index 9b6d90a72601..440cd713e39a 100644
> > > --- a/drivers/cpuidle/poll_state.c
> > > +++ b/drivers/cpuidle/poll_state.c
> > > @@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
> > > limit = cpuidle_poll_time(drv, dev);
> > > - while (!need_resched()) {
> > > - cpu_relax();
> > > - if (loop_count++ < POLL_IDLE_RELAX_COUNT)
> > > - continue;
> > > -
> > > + for (;;) {
> > > loop_count = 0;
> > > +
> > > + smp_cond_load_relaxed(¤t_thread_info()->flags,
> > > + (VAL & _TIF_NEED_RESCHED) ||
> > > + (loop_count++ >= POLL_IDLE_RELAX_COUNT));
> > > +
> > > + if (loop_count < POLL_IDLE_RELAX_COUNT)
> > > + break;
> > > +
> > > if (local_clock_noinstr() - time_start > limit) {
> > > dev->poll_time_limit = true;
> > > break;
> > Doesn't this make ARCH_HAS_CPU_RELAX a complete misnomer?
>
> This controls the build of poll_state.c and the generic definition of
> smp_cond_load_relaxed (used by x86) is using cpu_relax(). Do you propose
> other approach here?
Give it a better name? Having ARCH_HAS_CPU_RELAX control a piece of code
that doesn't use cpu_relax() doesn't make sense to me.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
2024-01-29 18:15 ` Will Deacon
@ 2024-02-05 12:28 ` Mihai Carabas
-1 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2024-02-05 12:28 UTC (permalink / raw)
To: Will Deacon
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
>>>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>>>> smp_cond_load_relaxed which basically does a "wfe".
>>>>
>>>> Suggested-by: Peter Zijlstra <peterz@infradead.org>
>>>> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
>>>> ---
>>>> drivers/cpuidle/poll_state.c | 14 +++++++++-----
>>>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
>>>> index 9b6d90a72601..440cd713e39a 100644
>>>> --- a/drivers/cpuidle/poll_state.c
>>>> +++ b/drivers/cpuidle/poll_state.c
>>>> @@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
>>>> limit = cpuidle_poll_time(drv, dev);
>>>> - while (!need_resched()) {
>>>> - cpu_relax();
>>>> - if (loop_count++ < POLL_IDLE_RELAX_COUNT)
>>>> - continue;
>>>> -
>>>> + for (;;) {
>>>> loop_count = 0;
>>>> +
>>>> + smp_cond_load_relaxed(¤t_thread_info()->flags,
>>>> + (VAL & _TIF_NEED_RESCHED) ||
>>>> + (loop_count++ >= POLL_IDLE_RELAX_COUNT));
>>>> +
>>>> + if (loop_count < POLL_IDLE_RELAX_COUNT)
>>>> + break;
>>>> +
>>>> if (local_clock_noinstr() - time_start > limit) {
>>>> dev->poll_time_limit = true;
>>>> break;
>>> Doesn't this make ARCH_HAS_CPU_RELAX a complete misnomer?
>> This controls the build of poll_state.c and the generic definition of
>> smp_cond_load_relaxed (used by x86) is using cpu_relax(). Do you propose
>> other approach here?
> Give it a better name? Having ARCH_HAS_CPU_RELAX control a piece of code
> that doesn't use cpu_relax() doesn't make sense to me.
The generic code for smp_cond_load_relaxed is using cpu_relax and this
one is used on x86 - so ARCH_HAS_CPU_RELAX is a prerequisite on x86 when
using haltpoll. Only on ARM64 this is overwritten. Moreover
ARCH_HAS_CPU_RELAX is controlling the function definition for
cpuidle_poll_state_init (this is how it was originally designed).
Thanks,
Mihai
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
@ 2024-02-05 12:28 ` Mihai Carabas
0 siblings, 0 replies; 46+ messages in thread
From: Mihai Carabas @ 2024-02-05 12:28 UTC (permalink / raw)
To: Will Deacon
Cc: linux-arm-kernel, kvm, linux-pm, linux-kernel, catalin.marinas,
tglx, mingo, bp, x86, hpa, pbonzini, wanpengli, vkuznets, rafael,
daniel.lezcano, akpm, pmladek, peterz, dianders, npiggin,
rick.p.edgecombe, joao.m.martins, juerg.haefliger, mic, arnd,
ankur.a.arora
>>>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>>>> smp_cond_load_relaxed which basically does a "wfe".
>>>>
>>>> Suggested-by: Peter Zijlstra <peterz@infradead.org>
>>>> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
>>>> ---
>>>> drivers/cpuidle/poll_state.c | 14 +++++++++-----
>>>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
>>>> index 9b6d90a72601..440cd713e39a 100644
>>>> --- a/drivers/cpuidle/poll_state.c
>>>> +++ b/drivers/cpuidle/poll_state.c
>>>> @@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
>>>> limit = cpuidle_poll_time(drv, dev);
>>>> - while (!need_resched()) {
>>>> - cpu_relax();
>>>> - if (loop_count++ < POLL_IDLE_RELAX_COUNT)
>>>> - continue;
>>>> -
>>>> + for (;;) {
>>>> loop_count = 0;
>>>> +
>>>> + smp_cond_load_relaxed(¤t_thread_info()->flags,
>>>> + (VAL & _TIF_NEED_RESCHED) ||
>>>> + (loop_count++ >= POLL_IDLE_RELAX_COUNT));
>>>> +
>>>> + if (loop_count < POLL_IDLE_RELAX_COUNT)
>>>> + break;
>>>> +
>>>> if (local_clock_noinstr() - time_start > limit) {
>>>> dev->poll_time_limit = true;
>>>> break;
>>> Doesn't this make ARCH_HAS_CPU_RELAX a complete misnomer?
>> This controls the build of poll_state.c and the generic definition of
>> smp_cond_load_relaxed (used by x86) is using cpu_relax(). Do you propose
>> other approach here?
> Give it a better name? Having ARCH_HAS_CPU_RELAX control a piece of code
> that doesn't use cpu_relax() doesn't make sense to me.
The generic code for smp_cond_load_relaxed is using cpu_relax and this
one is used on x86 - so ARCH_HAS_CPU_RELAX is a prerequisite on x86 when
using haltpoll. Only on ARM64 this is overwritten. Moreover
ARCH_HAS_CPU_RELAX is controlling the function definition for
cpuidle_poll_state_init (this is how it was originally designed).
Thanks,
Mihai
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
2024-02-05 12:28 ` Mihai Carabas
@ 2024-02-05 19:33 ` Ankur Arora
-1 siblings, 0 replies; 46+ messages in thread
From: Ankur Arora @ 2024-02-05 19:33 UTC (permalink / raw)
To: Mihai Carabas
Cc: Will Deacon, linux-arm-kernel, kvm, linux-pm, linux-kernel,
catalin.marinas, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli,
vkuznets, rafael, daniel.lezcano, akpm, pmladek, peterz,
dianders, npiggin, rick.p.edgecombe, joao.m.martins,
juerg.haefliger, mic, arnd, ankur.a.arora
Mihai Carabas <mihai.carabas@oracle.com> writes:
>>>>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>>>>> smp_cond_load_relaxed which basically does a "wfe".
>>>>>
>>>>> Suggested-by: Peter Zijlstra <peterz@infradead.org>
>>>>> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
>>>>> ---
>>>>> drivers/cpuidle/poll_state.c | 14 +++++++++-----
>>>>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
>>>>> index 9b6d90a72601..440cd713e39a 100644
>>>>> --- a/drivers/cpuidle/poll_state.c
>>>>> +++ b/drivers/cpuidle/poll_state.c
>>>>> @@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
>>>>> limit = cpuidle_poll_time(drv, dev);
>>>>> - while (!need_resched()) {
>>>>> - cpu_relax();
>>>>> - if (loop_count++ < POLL_IDLE_RELAX_COUNT)
>>>>> - continue;
>>>>> -
>>>>> + for (;;) {
>>>>> loop_count = 0;
>>>>> +
>>>>> + smp_cond_load_relaxed(¤t_thread_info()->flags,
>>>>> + (VAL & _TIF_NEED_RESCHED) ||
>>>>> + (loop_count++ >= POLL_IDLE_RELAX_COUNT));
>>>>> +
>>>>> + if (loop_count < POLL_IDLE_RELAX_COUNT)
>>>>> + break;
>>>>> +
>>>>> if (local_clock_noinstr() - time_start > limit) {
>>>>> dev->poll_time_limit = true;
>>>>> break;
>>>> Doesn't this make ARCH_HAS_CPU_RELAX a complete misnomer?
>>> This controls the build of poll_state.c and the generic definition of
>>> smp_cond_load_relaxed (used by x86) is using cpu_relax(). Do you propose
>>> other approach here?
>> Give it a better name? Having ARCH_HAS_CPU_RELAX control a piece of code
>> that doesn't use cpu_relax() doesn't make sense to me.
>
> The generic code for smp_cond_load_relaxed is using cpu_relax and this one is
> used on x86 - so ARCH_HAS_CPU_RELAX is a prerequisite on x86 when using
> haltpoll. Only on ARM64 this is overwritten. Moreover ARCH_HAS_CPU_RELAX is
> controlling the function definition for cpuidle_poll_state_init (this is how it
> was originally designed).
I suspect Will's point is that the term ARCH_HAS_CPU_RELAX doesn't make
a whole lot of sense when we are only indirectly using cpu_relax() in
the series.
Also, all archs define cpu_relax() (though some as just a barrier()) so
ARCH_HAS_CPU_RELAX .
Maybe an arch can instead just opt into polling in idle?
Perhaps something like this trivial patch:
--
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5edec175b9bf..d80c98c64fd4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -367,7 +367,7 @@ config ARCH_MAY_HAVE_PC_FDC
config GENERIC_CALIBRATE_DELAY
def_bool y
-config ARCH_HAS_CPU_RELAX
+config ARCH_WANTS_IDLE_POLL
def_bool y
config ARCH_HIBERNATION_POSSIBLE
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 55437f5e0c3a..6a0a1f16a5c3 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -36,7 +36,7 @@
#include <asm/cpu.h>
#endif
-#define ACPI_IDLE_STATE_START (IS_ENABLED(CONFIG_ARCH_HAS_CPU_RELAX) ? 1 : 0)
+#define ACPI_IDLE_STATE_START (IS_ENABLED(CONFIG_ARCH_WANTS_IDLE_POLL) ? 1 : 0)
static unsigned int max_cstate __read_mostly = ACPI_PROCESSOR_MAX_POWER;
module_param(max_cstate, uint, 0400);
@@ -787,7 +787,7 @@ static int acpi_processor_setup_cstates(struct acpi_processor *pr)
if (max_cstate == 0)
max_cstate = 1;
- if (IS_ENABLED(CONFIG_ARCH_HAS_CPU_RELAX)) {
+ if (IS_ENABLED(CONFIG_ARCH_WANTS_IDLE_POLL)) {
cpuidle_poll_state_init(drv);
count = 1;
} else {
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index d103342b7cfc..23f48d99f0f2 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -7,7 +7,7 @@ obj-y += cpuidle.o driver.o governor.o sysfs.o governors/
obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
obj-$(CONFIG_DT_IDLE_STATES) += dt_idle_states.o
obj-$(CONFIG_DT_IDLE_GENPD) += dt_idle_genpd.o
-obj-$(CONFIG_ARCH_HAS_CPU_RELAX) += poll_state.o
+obj-$(CONFIG_ARCH_WANTS_IDLE_POLL) += poll_state.o
obj-$(CONFIG_HALTPOLL_CPUIDLE) += cpuidle-haltpoll.o
##################################################################################
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 3183aeb7f5b4..53e55a91d55d 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -275,7 +275,7 @@ static inline void cpuidle_coupled_parallel_barrier(struct cpuidle_device *dev,
}
#endif
-#if defined(CONFIG_CPU_IDLE) && defined(CONFIG_ARCH_HAS_CPU_RELAX)
+#if defined(CONFIG_CPU_IDLE) && defined(CONFIG_ARCH_WANTS_IDLE_POLL)
void cpuidle_poll_state_init(struct cpuidle_driver *drv);
#else
static inline void cpuidle_poll_state_init(struct cpuidle_driver *drv) {}
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
@ 2024-02-05 19:33 ` Ankur Arora
0 siblings, 0 replies; 46+ messages in thread
From: Ankur Arora @ 2024-02-05 19:33 UTC (permalink / raw)
To: Mihai Carabas
Cc: Will Deacon, linux-arm-kernel, kvm, linux-pm, linux-kernel,
catalin.marinas, tglx, mingo, bp, x86, hpa, pbonzini, wanpengli,
vkuznets, rafael, daniel.lezcano, akpm, pmladek, peterz,
dianders, npiggin, rick.p.edgecombe, joao.m.martins,
juerg.haefliger, mic, arnd, ankur.a.arora
Mihai Carabas <mihai.carabas@oracle.com> writes:
>>>>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>>>>> smp_cond_load_relaxed which basically does a "wfe".
>>>>>
>>>>> Suggested-by: Peter Zijlstra <peterz@infradead.org>
>>>>> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
>>>>> ---
>>>>> drivers/cpuidle/poll_state.c | 14 +++++++++-----
>>>>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
>>>>> index 9b6d90a72601..440cd713e39a 100644
>>>>> --- a/drivers/cpuidle/poll_state.c
>>>>> +++ b/drivers/cpuidle/poll_state.c
>>>>> @@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
>>>>> limit = cpuidle_poll_time(drv, dev);
>>>>> - while (!need_resched()) {
>>>>> - cpu_relax();
>>>>> - if (loop_count++ < POLL_IDLE_RELAX_COUNT)
>>>>> - continue;
>>>>> -
>>>>> + for (;;) {
>>>>> loop_count = 0;
>>>>> +
>>>>> + smp_cond_load_relaxed(¤t_thread_info()->flags,
>>>>> + (VAL & _TIF_NEED_RESCHED) ||
>>>>> + (loop_count++ >= POLL_IDLE_RELAX_COUNT));
>>>>> +
>>>>> + if (loop_count < POLL_IDLE_RELAX_COUNT)
>>>>> + break;
>>>>> +
>>>>> if (local_clock_noinstr() - time_start > limit) {
>>>>> dev->poll_time_limit = true;
>>>>> break;
>>>> Doesn't this make ARCH_HAS_CPU_RELAX a complete misnomer?
>>> This controls the build of poll_state.c and the generic definition of
>>> smp_cond_load_relaxed (used by x86) is using cpu_relax(). Do you propose
>>> other approach here?
>> Give it a better name? Having ARCH_HAS_CPU_RELAX control a piece of code
>> that doesn't use cpu_relax() doesn't make sense to me.
>
> The generic code for smp_cond_load_relaxed is using cpu_relax and this one is
> used on x86 - so ARCH_HAS_CPU_RELAX is a prerequisite on x86 when using
> haltpoll. Only on ARM64 this is overwritten. Moreover ARCH_HAS_CPU_RELAX is
> controlling the function definition for cpuidle_poll_state_init (this is how it
> was originally designed).
I suspect Will's point is that the term ARCH_HAS_CPU_RELAX doesn't make
a whole lot of sense when we are only indirectly using cpu_relax() in
the series.
Also, all archs define cpu_relax() (though some as just a barrier()) so
ARCH_HAS_CPU_RELAX .
Maybe an arch can instead just opt into polling in idle?
Perhaps something like this trivial patch:
--
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5edec175b9bf..d80c98c64fd4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -367,7 +367,7 @@ config ARCH_MAY_HAVE_PC_FDC
config GENERIC_CALIBRATE_DELAY
def_bool y
-config ARCH_HAS_CPU_RELAX
+config ARCH_WANTS_IDLE_POLL
def_bool y
config ARCH_HIBERNATION_POSSIBLE
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 55437f5e0c3a..6a0a1f16a5c3 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -36,7 +36,7 @@
#include <asm/cpu.h>
#endif
-#define ACPI_IDLE_STATE_START (IS_ENABLED(CONFIG_ARCH_HAS_CPU_RELAX) ? 1 : 0)
+#define ACPI_IDLE_STATE_START (IS_ENABLED(CONFIG_ARCH_WANTS_IDLE_POLL) ? 1 : 0)
static unsigned int max_cstate __read_mostly = ACPI_PROCESSOR_MAX_POWER;
module_param(max_cstate, uint, 0400);
@@ -787,7 +787,7 @@ static int acpi_processor_setup_cstates(struct acpi_processor *pr)
if (max_cstate == 0)
max_cstate = 1;
- if (IS_ENABLED(CONFIG_ARCH_HAS_CPU_RELAX)) {
+ if (IS_ENABLED(CONFIG_ARCH_WANTS_IDLE_POLL)) {
cpuidle_poll_state_init(drv);
count = 1;
} else {
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index d103342b7cfc..23f48d99f0f2 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -7,7 +7,7 @@ obj-y += cpuidle.o driver.o governor.o sysfs.o governors/
obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
obj-$(CONFIG_DT_IDLE_STATES) += dt_idle_states.o
obj-$(CONFIG_DT_IDLE_GENPD) += dt_idle_genpd.o
-obj-$(CONFIG_ARCH_HAS_CPU_RELAX) += poll_state.o
+obj-$(CONFIG_ARCH_WANTS_IDLE_POLL) += poll_state.o
obj-$(CONFIG_HALTPOLL_CPUIDLE) += cpuidle-haltpoll.o
##################################################################################
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 3183aeb7f5b4..53e55a91d55d 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -275,7 +275,7 @@ static inline void cpuidle_coupled_parallel_barrier(struct cpuidle_device *dev,
}
#endif
-#if defined(CONFIG_CPU_IDLE) && defined(CONFIG_ARCH_HAS_CPU_RELAX)
+#if defined(CONFIG_CPU_IDLE) && defined(CONFIG_ARCH_WANTS_IDLE_POLL)
void cpuidle_poll_state_init(struct cpuidle_driver *drv);
#else
static inline void cpuidle_poll_state_init(struct cpuidle_driver *drv) {}
^ permalink raw reply related [flat|nested] 46+ messages in thread
end of thread, other threads:[~2024-02-05 19:34 UTC | newest]
Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-20 14:01 [PATCH v2] Enable haltpoll for arm64 Mihai Carabas
2023-11-20 14:01 ` Mihai Carabas
2023-11-20 14:01 ` [PATCH 1/7] x86: Move ARCH_HAS_CPU_RELAX to arch Mihai Carabas
2023-11-20 14:01 ` Mihai Carabas
2023-11-27 14:44 ` Petr Mladek
2023-11-27 14:44 ` Petr Mladek
2023-11-28 14:09 ` Mihai Carabas
2023-11-28 14:09 ` Mihai Carabas
2023-11-20 14:01 ` [PATCH 2/7] x86/kvm: Move haltpoll_want() to be arch defined Mihai Carabas
2023-11-20 14:01 ` Mihai Carabas
2023-11-29 20:55 ` Rafael J. Wysocki
2023-11-29 20:55 ` Rafael J. Wysocki
2023-11-20 14:01 ` [PATCH 3/7] governors/haltpoll: Drop kvm_para_available() check Mihai Carabas
2023-11-20 14:01 ` Mihai Carabas
2023-11-20 14:01 ` [PATCH 4/7] arm64: Select ARCH_HAS_CPU_RELAX Mihai Carabas
2023-11-20 14:01 ` Mihai Carabas
2023-11-20 14:01 ` [PATCH 5/7] arm64: Define TIF_POLLING_NRFLAG Mihai Carabas
2023-11-20 14:01 ` Mihai Carabas
2023-11-20 14:01 ` [PATCH 6/7] cpuidle-haltpoll: ARM64 support Mihai Carabas
2023-11-20 14:01 ` Mihai Carabas
2023-11-20 14:01 ` [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed Mihai Carabas
2023-11-20 14:01 ` Mihai Carabas
2023-11-22 20:51 ` Christoph Lameter
2023-11-22 20:51 ` Christoph Lameter
2023-11-22 21:33 ` Mihai Carabas
2023-11-22 21:33 ` Mihai Carabas
2023-11-27 20:17 ` Christoph Lameter (Ampere)
2023-11-27 20:17 ` Christoph Lameter (Ampere)
2023-12-01 6:59 ` Ankur Arora
2023-12-01 6:59 ` Ankur Arora
2023-12-11 11:46 ` Will Deacon
2023-12-11 11:46 ` Will Deacon
2024-01-28 21:22 ` Mihai Carabas
2024-01-28 21:22 ` Mihai Carabas
2024-01-29 18:15 ` Will Deacon
2024-01-29 18:15 ` Will Deacon
2024-02-05 12:28 ` Mihai Carabas
2024-02-05 12:28 ` Mihai Carabas
2024-02-05 19:33 ` Ankur Arora
2024-02-05 19:33 ` Ankur Arora
2024-01-17 21:19 ` [PATCH v2] Enable haltpoll for arm64 Christoph Lameter (Ampere)
2024-01-17 21:19 ` Christoph Lameter (Ampere)
2024-01-25 14:39 ` Mihai Carabas
2024-01-25 14:39 ` Mihai Carabas
2024-01-25 15:16 ` Rafael J. Wysocki
2024-01-25 15:16 ` Rafael J. Wysocki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.