[PATCH] Enable haltpoll for arm64

[PATCH] Enable haltpoll for arm64

[Mihai Carabas]

This patchset enables the usage of haltpoll governer on arm64. This is
specifically interesting for KVM guests.

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.410 [sec]

       8.410882 usecs/op
         118893 ops/sec

b) with haltpoll:
perf bench sched pipe
# Running 'sched/pipe' benchmark:
# Executed 1000000 pipe operations between two processes
     Total time: 5.183 [sec]
       5.183491 usecs/op
         192920 ops/sec

Joao Martins (7):
  cpuidle-haltpoll: Make boot_option_idle_override check X86 specific
  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

 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                   | 6 ++++++
 drivers/cpuidle/Kconfig                 | 4 ++--
 drivers/cpuidle/cpuidle-haltpoll.c      | 6 ++++--
 drivers/cpuidle/governors/haltpoll.c    | 5 +----
 include/linux/cpuidle_haltpoll.h        | 5 +++++
 10 files changed, 30 insertions(+), 8 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

[PATCH 1/7] cpuidle-haltpoll: Make boot_option_idle_override check X86 specific

[Mihai Carabas]

From: Joao Martins <joao.m.martins@oracle.com>

In the pursuit of letting it build on ARM let's not include what is x86
specific.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 drivers/cpuidle/cpuidle-haltpoll.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/cpuidle/cpuidle-haltpoll.c b/drivers/cpuidle/cpuidle-haltpoll.c
index e66df22f9695..0ca3c8422eb6 100644
--- a/drivers/cpuidle/cpuidle-haltpoll.c
+++ b/drivers/cpuidle/cpuidle-haltpoll.c
@@ -104,9 +104,11 @@ static int __init haltpoll_init(void)
 	int ret;
 	struct cpuidle_driver *drv = &haltpoll_driver;
 
+#ifdef CONFIG_X86
 	/* Do not load haltpoll if idle= is passed */
 	if (boot_option_idle_override != IDLE_NO_OVERRIDE)
 		return -ENODEV;
+#endif
 
 	if (!kvm_para_available() || !haltpoll_want())
 		return -ENODEV;
-- 
1.8.3.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/7] cpuidle-haltpoll: Make boot_option_idle_override check X86 specific

[Rafael J. Wysocki]

On Wed, Aug 9, 2023 at 2:52 PM Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> From: Joao Martins <joao.m.martins@oracle.com>
>
> In the pursuit of letting it build on ARM let's not include what is x86
> specific.
>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
> ---
>  drivers/cpuidle/cpuidle-haltpoll.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/cpuidle/cpuidle-haltpoll.c b/drivers/cpuidle/cpuidle-haltpoll.c
> index e66df22f9695..0ca3c8422eb6 100644
> --- a/drivers/cpuidle/cpuidle-haltpoll.c
> +++ b/drivers/cpuidle/cpuidle-haltpoll.c
> @@ -104,9 +104,11 @@ static int __init haltpoll_init(void)
>         int ret;
>         struct cpuidle_driver *drv = &haltpoll_driver;
>
> +#ifdef CONFIG_X86
>         /* Do not load haltpoll if idle= is passed */
>         if (boot_option_idle_override != IDLE_NO_OVERRIDE)
>                 return -ENODEV;
> +#endif

I'm sure that adding this #ifdef to the function body is avoidable.

>         if (!kvm_para_available() || !haltpoll_want())
>                 return -ENODEV;
> --

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/7] x86: Move ARCH_HAS_CPU_RELAX to arch

[Mihai Carabas]

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 aff2746c8af2..31bd138f406d 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 7422db409770..2eab3be6abbb 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
-- 
1.8.3.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/7] x86/kvm: Move haltpoll_want() to be arch defined

[Mihai Carabas]

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. 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                   | 6 ++++++
 drivers/cpuidle/cpuidle-haltpoll.c      | 4 ++--
 include/linux/cpuidle_haltpoll.h        | 5 +++++
 4 files changed, 14 insertions(+), 2 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..817594cbda11 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -1151,4 +1151,10 @@ 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)
+{
+	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 0ca3c8422eb6..e2d4d78744ae 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)
@@ -110,7 +110,7 @@ static int __init haltpoll_init(void)
 		return -ENODEV;
 #endif
 
-	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

[PATCH 4/7] governors/haltpoll: Drop kvm_para_available() check

[Mihai Carabas]

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

Re: [PATCH 4/7] governors/haltpoll: Drop kvm_para_available() check

[Rafael J. Wysocki]

On Wed, Aug 9, 2023 at 2:54 PM Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> 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.

Fair enough.

> 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

[PATCH 5/7] arm64: Select ARCH_HAS_CPU_RELAX

[Mihai Carabas]

From: Joao Martins <joao.m.martins@oracle.com>

cpu_relax() is necessary to allow cpuidle poll-state to be used,
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

Re: [PATCH 5/7] arm64: Select ARCH_HAS_CPU_RELAX

[Peter Zijlstra]

On Wed, Aug 09, 2023 at 02:39:39PM +0300, Mihai Carabas wrote:
> From: Joao Martins <joao.m.martins@oracle.com>
> 
> cpu_relax() is necessary to allow cpuidle poll-state to be used,
> 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

Uh what ?! cpu_relax() is assumed present on all archs, no?



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/7] arm64: Select ARCH_HAS_CPU_RELAX

[Russell King (Oracle)]

On Wed, Aug 09, 2023 at 03:49:41PM +0200, Peter Zijlstra wrote:
> On Wed, Aug 09, 2023 at 02:39:39PM +0300, Mihai Carabas wrote:
> > From: Joao Martins <joao.m.martins@oracle.com>
> > 
> > cpu_relax() is necessary to allow cpuidle poll-state to be used,
> > 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
> 
> Uh what ?! cpu_relax() is assumed present on all archs, no?

I think you have x86 to blame for that!

That symbol is used in drivers/acpi/processor_idle.c to setup stuff
for the cpuidle polling, and also by cpuidle's Makefile to build
poll_state.o

It isn't to do with the presence of cpu_relax() or not.

It probably ought to be renamed to CPUIDLE_CPU_RELAX which would
better describe its modern purpose.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/7] arm64: Define TIF_POLLING_NRFLAG

[Mihai Carabas]

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 553d1bc559c6..d3010d0b2988 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
@@ -90,6 +93,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

[PATCH 7/7] cpuidle-haltpoll: ARM64 support

[Mihai Carabas]

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

Re: [PATCH] Enable haltpoll for arm64

[Peter Zijlstra]

On Wed, Aug 09, 2023 at 02:39:34PM +0300, Mihai Carabas wrote:

> Joao Martins (7):
>   cpuidle-haltpoll: Make boot_option_idle_override check X86 specific
>   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

You have far too many SOB's on some or all of these patches.

Using poll_state as is on arm64 seems sub-optimal, would not something
like the below make sense?

---
diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
index 9b6d90a72601..9ab40198b042 100644
--- a/drivers/cpuidle/poll_state.c
+++ b/drivers/cpuidle/poll_state.c
@@ -27,7 +27,11 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
 		limit = cpuidle_poll_time(drv, dev);
 
 		while (!need_resched()) {
-			cpu_relax();
+
+			smp_cond_load_relaxed(current_thread_info()->flags,
+					      (VAL & TIF_NEED_RESCHED) ||
+					      (loop_count++ >= POLL_IDLE_RELAX_COUNT));
+
 			if (loop_count++ < POLL_IDLE_RELAX_COUNT)
 				continue;
 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Enable haltpoll for arm64

[Mihai Carabas]

> Using poll_state as is on arm64 seems sub-optimal, would not something
> like the below make sense?
>
> ---
> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
> index 9b6d90a72601..9ab40198b042 100644
> --- a/drivers/cpuidle/poll_state.c
> +++ b/drivers/cpuidle/poll_state.c
> @@ -27,7 +27,11 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
>   		limit = cpuidle_poll_time(drv, dev);
>   
>   		while (!need_resched()) {
> -			cpu_relax();
> +
> +			smp_cond_load_relaxed(current_thread_info()->flags,
> +					      (VAL & TIF_NEED_RESCHED) ||
> +					      (loop_count++ >= POLL_IDLE_RELAX_COUNT));
> +
>   			if (loop_count++ < POLL_IDLE_RELAX_COUNT)
>   				continue;
>   

Thank you for the suggestion. I have tried it and also different 
variations like [1] to respect the initial logic
but I obtain poor performance compared to the initial one:

perf bench sched pipe
# Running 'sched/pipe' benchmark:
# Executed 1000000 pipe operations between two processes

      Total time: 136.215 [sec]

      136.215229 usecs/op
            7341 ops/sec


[1]

--- 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(&current_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;


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel