Re: Lazy preemption on arm64

[Mike Galbraith <efault@gmx.de>]

Greetings,

On Tue, 2024-12-17 at 12:23 +0000, Mark Rutland wrote:
> On Tue, Dec 17, 2024 at 12:59:31PM +0100, Sebastian Andrzej Siewior wrote:
> > On 2024-12-17 11:34:43 [+0000], Mark Rutland wrote:
> > > On Tue, Dec 17, 2024 at 09:50:31AM +0100, Sebastian Andrzej Siewior wrote:
> > > > This bits below are actually the same ones I made last week. I stopped
> > > > there because it was late and I didn't find GENERIC_ENTRY nor a
> > > > TIF_NEED_RESCHED check in arm64 so I paused. Where is this?
> > >
> > > Currently arm64 doesn't use GENERIC_ENTRY; people are working on that
> > > (see the link above), but it's likely to take a short while. IIUC
> > > there's no strict dependency on GENERIC_ENTRY here, unless I'm missing
> > > something?
> >
> > No, not really, that is perfect.
> >
> > > For TIF_NEED_RESCHED, arm64 relies upon the core code to call
> > > set_preempt_need_resched() (e.g. via preempt_fold_need_resched()) to
> > > fold that into thread_info::preempt::need_resched. That's checked by
> > > arm64_preempt_schedule_irq(), which reads thread_info::preempt_count,
> > > which is unioned with thread_info::preempt::{count,need_resched} such
> > > that the two fields can be checked together.
> >
> > All sounds fine. Now, if that bit is set, we need schedule() before
> > returning to userland. I didn't it initially but now I did:
> >
> > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> > index b260ddc4d3e9a..2e2f13ce076da 100644
> > --- a/arch/arm64/kernel/entry-common.c
> > +++ b/arch/arm64/kernel/entry-common.c
> > @@ -132,7 +132,7 @@ static void do_notify_resume(struct pt_regs *regs, unsigned long thread_flags)
> >         do {
> >                 local_irq_enable();
> >  
> > -               if (thread_flags & _TIF_NEED_RESCHED)
> > +               if (thread_flags & _TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY)
> >                         schedule();
> >  
> >                 if (thread_flags & _TIF_UPROBE)
> >
> > With that piece we should be fine.
>
> Yep, I had that in my HACK patch:
>
>   https://lore.kernel.org/linux-rt-users/20241217115931.wjw_HO2V@linutronix.de/T/#m12eece66786a3a207e4e952bdf58570ab75c6a89

Posting this for mainline inclusion was suggested, but I don't see
waiting for GENERIC_ENTRY as having any meaningful impact for the
general case, everyone's used to whatever preemption model they've been
using.  OTOH, not having PREEMPT_LAZY for RT users has been having at
least some impact, which your patch can alleviate, so I'll post the
fixed up and lightly tested version here... including something
resembling a changelog in case anyone disagrees about submission, but
is too lazy to write the way better one they most definitely should :)

From 16ae3c84d8e1691fe670dbfb4c643cab4fa2065f Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Mon, 16 Dec 2024 18:32:44 +0000
Subject: [PATCH] HACK: arm64: enable PREEMPT_LAZY

GENERIC_ENTRY is not yet implemented for arm64, but is in the pipe.
Meanwhile, the below can serve to bridge the gap, particularly for
those building/running PREEMPT_RT kernels, where lazy preemption
markedly improves SCHED_OTHER load component throughput.

Mike: testdrive, _TIF_WORK_MASK fixlet and changelog.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Mike Galbraith <efault@gmx.de>
---
 arch/arm64/Kconfig                   |    1 +
 arch/arm64/include/asm/thread_info.h |   16 +++++++++-------
 arch/arm64/kernel/entry-common.c     |    2 +-
 3 files changed, 11 insertions(+), 8 deletions(-)

--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -39,6 +39,7 @@ config ARM64
 	select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
 	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
 	select ARCH_HAS_NONLEAF_PMD_YOUNG if ARM64_HAFT
+	select ARCH_HAS_PREEMPT_LAZY
 	select ARCH_HAS_PTE_DEVMAP
 	select ARCH_HAS_PTE_SPECIAL
 	select ARCH_HAS_HW_PTE_YOUNG
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -59,11 +59,12 @@ void arch_setup_new_exec(void);

 #define TIF_SIGPENDING		0	/* signal pending */
 #define TIF_NEED_RESCHED	1	/* rescheduling necessary */
-#define TIF_NOTIFY_RESUME	2	/* callback before returning to user */
-#define TIF_FOREIGN_FPSTATE	3	/* CPU's FP state is not current's */
-#define TIF_UPROBE		4	/* uprobe breakpoint or singlestep */
-#define TIF_MTE_ASYNC_FAULT	5	/* MTE Asynchronous Tag Check Fault */
-#define TIF_NOTIFY_SIGNAL	6	/* signal notifications exist */
+#define TIF_NEED_RESCHED_LAZY	2	/* Lazy rescheduling needed */
+#define TIF_NOTIFY_RESUME	3	/* callback before returning to user */
+#define TIF_FOREIGN_FPSTATE	4	/* CPU's FP state is not current's */
+#define TIF_UPROBE		5	/* uprobe breakpoint or singlestep */
+#define TIF_MTE_ASYNC_FAULT	6	/* MTE Asynchronous Tag Check Fault */
+#define TIF_NOTIFY_SIGNAL	7	/* signal notifications exist */
 #define TIF_SYSCALL_TRACE	8	/* syscall trace active */
 #define TIF_SYSCALL_AUDIT	9	/* syscall auditing */
 #define TIF_SYSCALL_TRACEPOINT	10	/* syscall tracepoint for ftrace */
@@ -85,6 +86,7 @@ void arch_setup_new_exec(void);

 #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
 #define _TIF_NEED_RESCHED	(1 << TIF_NEED_RESCHED)
+#define _TIF_NEED_RESCHED_LAZY	(1 << TIF_NEED_RESCHED_LAZY)
 #define _TIF_NOTIFY_RESUME	(1 << TIF_NOTIFY_RESUME)
 #define _TIF_FOREIGN_FPSTATE	(1 << TIF_FOREIGN_FPSTATE)
 #define _TIF_SYSCALL_TRACE	(1 << TIF_SYSCALL_TRACE)
@@ -100,10 +102,10 @@ void arch_setup_new_exec(void);
 #define _TIF_NOTIFY_SIGNAL	(1 << TIF_NOTIFY_SIGNAL)
 #define _TIF_TSC_SIGSEGV	(1 << TIF_TSC_SIGSEGV)

-#define _TIF_WORK_MASK		(_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
+#define _TIF_WORK_MASK		(_TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY | \
 				 _TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE | \
 				 _TIF_UPROBE | _TIF_MTE_ASYNC_FAULT | \
-				 _TIF_NOTIFY_SIGNAL)
+				 _TIF_NOTIFY_SIGNAL | _TIF_SIGPENDING)

 #define _TIF_SYSCALL_WORK	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
 				 _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP | \
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -132,7 +132,7 @@ static void do_notify_resume(struct pt_r
 	do {
 		local_irq_enable();

-		if (thread_flags & _TIF_NEED_RESCHED)
+		if (thread_flags & (_TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY))
 			schedule();

 		if (thread_flags & _TIF_UPROBE)