[PATCH v2 05/10] um: Determine sleep based on need_resched()

Tiwei Bie posted 10 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v2 05/10] um: Determine sleep based on need_resched()
Posted by Tiwei Bie 1 month, 3 weeks ago
From: Tiwei Bie <tiwei.btw@antgroup.com>

With SMP and NO_HZ enabled, the CPU may still need to sleep even
if the timer is disarmed. Switch to deciding whether to sleep based
on pending resched. This is a preparation for adding SMP support.

Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
---
 arch/um/include/shared/kern_util.h |  1 +
 arch/um/kernel/process.c           | 12 ++++++++++--
 arch/um/os-Linux/time.c            | 11 +++++------
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/um/include/shared/kern_util.h b/arch/um/include/shared/kern_util.h
index 00ca3e12fd9a..3daaa5c4b35d 100644
--- a/arch/um/include/shared/kern_util.h
+++ b/arch/um/include/shared/kern_util.h
@@ -55,6 +55,7 @@ extern int __uml_cant_sleep(void);
 extern int get_current_pid(void);
 extern int copy_from_user_proc(void *to, void *from, int size);
 extern char *uml_strdup(const char *string);
+int uml_need_resched(void);
 
 extern unsigned long to_irq_stack(unsigned long *mask_out);
 extern unsigned long from_irq_stack(int nested);
diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c
index 1be644de9e41..01b935c00454 100644
--- a/arch/um/kernel/process.c
+++ b/arch/um/kernel/process.c
@@ -209,10 +209,13 @@ int arch_dup_task_struct(struct task_struct *dst,
 
 void um_idle_sleep(void)
 {
-	if (time_travel_mode != TT_MODE_OFF)
+	if (time_travel_mode != TT_MODE_OFF) {
 		time_travel_sleep();
-	else
+	} else {
+		raw_local_irq_enable();
 		os_idle_sleep();
+		raw_local_irq_disable();
+	}
 }
 
 void arch_cpu_idle(void)
@@ -225,6 +228,11 @@ int __uml_cant_sleep(void) {
 	/* Is in_interrupt() really needed? */
 }
 
+int uml_need_resched(void)
+{
+	return need_resched();
+}
+
 extern exitcall_t __uml_exitcall_begin, __uml_exitcall_end;
 
 void do_uml_exitcalls(void)
diff --git a/arch/um/os-Linux/time.c b/arch/um/os-Linux/time.c
index 4d5591d96d8c..f25a4196bab7 100644
--- a/arch/um/os-Linux/time.c
+++ b/arch/um/os-Linux/time.c
@@ -98,18 +98,17 @@ long long os_nsecs(void)
  */
 void os_idle_sleep(void)
 {
-	struct itimerspec its;
 	sigset_t set, old;
 
-	/* block SIGALRM while we analyze the timer state */
+	/* Block SIGALRM while performing the need_resched check. */
 	sigemptyset(&set);
 	sigaddset(&set, SIGALRM);
 	sigprocmask(SIG_BLOCK, &set, &old);
 
-	/* check the timer, and if it'll fire then wait for it */
-	timer_gettime(event_high_res_timer, &its);
-	if (its.it_value.tv_sec || its.it_value.tv_nsec)
+	/* Sleep if no resched is pending. */
+	if (!uml_need_resched())
 		sigsuspend(&old);
-	/* either way, restore the signal mask */
+
+	/* Restore the signal mask. */
 	sigprocmask(SIG_UNBLOCK, &set, NULL);
 }
-- 
2.34.1
Re: [PATCH v2 05/10] um: Determine sleep based on need_resched()
Posted by Johannes Berg 3 weeks, 2 days ago
On Sun, 2025-08-10 at 13:51 +0800, Tiwei Bie wrote:
> 
>  void um_idle_sleep(void)
>  {
> -	if (time_travel_mode != TT_MODE_OFF)
> +	if (time_travel_mode != TT_MODE_OFF) {
>  		time_travel_sleep();
> -	else
> +	} else {
> +		raw_local_irq_enable();
>  		os_idle_sleep();
> +		raw_local_irq_disable();
> +	}

This seems wrong, with it, lockdep gets really unhappy, and if I remove
this change it seems to work OK?

I'll note that arch_cpu_idle() for x86 also doesn't change anything with
interrupts.

Was there something else that required this change that I'm missing?

johannes
Re: [PATCH v2 05/10] um: Determine sleep based on need_resched()
Posted by Tiwei Bie 3 weeks, 2 days ago
On Thu, 11 Sep 2025 11:27:04 +0200, Johannes Berg wrote:
> On Sun, 2025-08-10 at 13:51 +0800, Tiwei Bie wrote:
> > 
> >  void um_idle_sleep(void)
> >  {
> > -	if (time_travel_mode != TT_MODE_OFF)
> > +	if (time_travel_mode != TT_MODE_OFF) {
> >  		time_travel_sleep();
> > -	else
> > +	} else {
> > +		raw_local_irq_enable();
> >  		os_idle_sleep();
> > +		raw_local_irq_disable();
> > +	}
> 
> This seems wrong, with it, lockdep gets really unhappy, and if I remove
> this change it seems to work OK?
> 
> I'll note that arch_cpu_idle() for x86 also doesn't change anything with
> interrupts.

Thanks for catching that! I missed it. I'll take a closer look.

Regards,
Tiwei
Re: [PATCH v2 05/10] um: Determine sleep based on need_resched()
Posted by Johannes Berg 3 weeks, 3 days ago
On Sun, 2025-08-10 at 13:51 +0800, Tiwei Bie wrote:
> From: Tiwei Bie <tiwei.btw@antgroup.com>
> 
> With SMP and NO_HZ enabled, the CPU may still need to sleep even
> if the timer is disarmed. Switch to deciding whether to sleep based
> on pending resched. This is a preparation for adding SMP support.

What's the rationale for need_resched()? Is that somehow defined for
this? Is it what other architectures use? I guess I'm just not entirely
sure what it means.

johannes
Re: [PATCH v2 05/10] um: Determine sleep based on need_resched()
Posted by Tiwei Bie 3 weeks, 2 days ago
On Wed, 10 Sep 2025 14:10:37 +0200, Johannes Berg wrote:
> On Sun, 2025-08-10 at 13:51 +0800, Tiwei Bie wrote:
> > From: Tiwei Bie <tiwei.btw@antgroup.com>
> > 
> > With SMP and NO_HZ enabled, the CPU may still need to sleep even
> > if the timer is disarmed. Switch to deciding whether to sleep based
> > on pending resched. This is a preparation for adding SMP support.
> 
> What's the rationale for need_resched()? Is that somehow defined for
> this? Is it what other architectures use? I guess I'm just not entirely
> sure what it means.

Here is a relevant document:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/scheduler/sched-arch.rst?h=v6.17-rc5#n37

There is a similar check on x86:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/process.c?h=v6.17-rc5#n916

Regards,
Tiwei
Re: [PATCH v2 05/10] um: Determine sleep based on need_resched()
Posted by Johannes Berg 3 weeks, 2 days ago
On Thu, 2025-09-11 at 12:39 +0800, Tiwei Bie wrote:
> On Wed, 10 Sep 2025 14:10:37 +0200, Johannes Berg wrote:
> > On Sun, 2025-08-10 at 13:51 +0800, Tiwei Bie wrote:
> > > From: Tiwei Bie <tiwei.btw@antgroup.com>
> > > 
> > > With SMP and NO_HZ enabled, the CPU may still need to sleep even
> > > if the timer is disarmed. Switch to deciding whether to sleep based
> > > on pending resched. This is a preparation for adding SMP support.
> > 
> > What's the rationale for need_resched()? Is that somehow defined for
> > this? Is it what other architectures use? I guess I'm just not entirely
> > sure what it means.
> 
> Here is a relevant document:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/scheduler/sched-arch.rst?h=v6.17-rc5#n37
> 
> There is a similar check on x86:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/process.c?h=v6.17-rc5#n916

Ah cool, thanks for the pointers. Though "1." part there is a bit
confusing, I guess I'll send a patch to reword it :)

FWIW I already applied a few patches, but I'll take another look,
hopefully today.

johannes
Re: [PATCH v2 05/10] um: Determine sleep based on need_resched()
Posted by Tiwei Bie 3 weeks, 2 days ago
On Thu, 11 Sep 2025 08:59:57 +0200, Johannes Berg wrote:
> On Thu, 2025-09-11 at 12:39 +0800, Tiwei Bie wrote:
> > On Wed, 10 Sep 2025 14:10:37 +0200, Johannes Berg wrote:
> > > On Sun, 2025-08-10 at 13:51 +0800, Tiwei Bie wrote:
> > > > From: Tiwei Bie <tiwei.btw@antgroup.com>
> > > > 
> > > > With SMP and NO_HZ enabled, the CPU may still need to sleep even
> > > > if the timer is disarmed. Switch to deciding whether to sleep based
> > > > on pending resched. This is a preparation for adding SMP support.
> > > 
> > > What's the rationale for need_resched()? Is that somehow defined for
> > > this? Is it what other architectures use? I guess I'm just not entirely
> > > sure what it means.
> > 
> > Here is a relevant document:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/scheduler/sched-arch.rst?h=v6.17-rc5#n37
> > 
> > There is a similar check on x86:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/process.c?h=v6.17-rc5#n916
> 
> Ah cool, thanks for the pointers. Though "1." part there is a bit
> confusing, I guess I'll send a patch to reword it :)
> 
> FWIW I already applied a few patches, but I'll take another look,
> hopefully today.

Thanks for the review! :)

Regards,
Tiwei