Re: Lazy preemption on arm64

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

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:
> > On 2024-12-17 07:31:51 [+0100], Petr Tesarik wrote:
> > > V Mon, 16 Dec 2024 19:04:43 +0000
> > > Mark Rutland <mark.rutland@arm.com> napsáno:
> > > 
> > > > On Mon, Dec 16, 2024 at 07:04:51PM +0100, Petr Tesarik wrote:
> > > > > Hi all,
> > > > > 
> > > > > what is the plan for implementing PREEMPT_LAZY on arm64?
> > > > > 
> > > > > There used to be RT patch series which enabled lazy preemption on
> > > > > arm64, but this architecture was "sacrificed" in v6.6-rc6-rt10, as
> > > > > collateral damage of switching to PREEMPT_AUTO.
> > > > > 
> > > > > IIUC lazy preemption is currently implemented only for architectures
> > > > > with CONFIG_GENERIC_ENTRY, but there is no inherent dependency on it.
> > > > > So, is the plan to convert arm64 to GENERIC_ENTRY (and then get
> > > > > PREEMPT_LAZY for free), or is somebody working on CONFIG_PREEMPT_LAZY
> > > > > for arm64 without that conversion?  
> > > > 
> > > > I don't think there's an agreed upon plan either way.
> > > > 
> > > > Jinjie Ruan has been looking to move arm64 over to GENERIC_ENTRY:
> > > > 
> > > >   https://lore.kernel.org/all/20241206101744.4161990-1-ruanjinjie@huawei.com/
> > > > 
> > > > AFAICT, the only bits that we get "for free" from GENERIC_ENTRY would be
> > > > the logic in raw_irqentry_exit_cond_resched() and
> > > > exit_to_user_mode_loop(), and all we'd need to enable this on arm64
> > > > as-is would be as below.
> > > 
> > > @bigeasy: Would it be OK for you to add the below patch to the next
> > > 6.13 RT patches?
> > 
> > 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.

> > Other than that I would be happy to take it then hoping arm64 does the
> > same.
> 
> If PREEMPT_LAZY is something that people need urgently then I can go
> turn the hack into a proepr patch and see if we can queue that ahead of
> the larger rework for GENERIC_ENTRY.

I would appreciate it. However if there is reason to delay it I could
hold to it for some time…

> > > Mark tagged it with "HACK", but to me it actually looks just as good as
> > > the good old (pre-PREEMPT_AUTO) arm64 patch. ;-)
> > 
> > The old lazy-preempt had also tweaks in should_resched() and
> > __preempt_count_dec_and_test(). So it is slightly different.
> 
> Hmm... what needed to change there?
> 
> Currently we're relying on the union trick to check both
> thread_info::preempt::{count,need_resched}, where the latter should have
> TIF_NEED_RESCHED folded in (but not TIF_NEED_RESCHED_LAZY), which IIUC
> is sufficient?

The old lazy-preempt dates back to around v3.0-RT+. The logic back then
was slightly different and had also a counter (similar to the counter
used by preempt_disable()) so we had to ensure preempt_enable() does not
schedule if the lazy-counter > 0 and the caller was not a RT task.
With the improvements over time and the current design a lot of the old
cruft simply removed. So nothing to worry :)

> Mark.

Sebastian