From 8a6aa5f5bfe064caad96a41ace1bcd02e7e8dbab Mon Sep 17 00:00:00 2001
From: hujun5 <hujun5@xiaomi.com>
Date: Tue, 16 Apr 2024 09:36:10 +0800
Subject: [PATCH] sched/sched: There is no need to use sched_[un]lock
purpose:
1 sched_lock is very time-consuming, and reducing its invocations can improve performance.
2 sched_lock is prone to misuse, and narrowing its scope of use is to prevent people from referencing incorrect code and using it
test:
We can use qemu for testing.
compiling
make distclean -j20; ./tools/configure.sh -l qemu-armv8a:nsh_smp ;make -j20
running
qemu-system-aarch64 -cpu cortex-a53 -smp 4 -nographic -machine virt,virtualization=on,gic-version=3 -net none -chardev stdio,id=con,mux=on -serial chardev:con -mon chardev=con,mode=readline -kernel ./nuttx
We have also tested this patch on other ARM hardware platforms.
Signed-off-by: hujun5 <hujun5@xiaomi.com>
---
sched/sched/sched_getaffinity.c | 6 ++++--
sched/sched/sched_getparam.c | 5 +++--
sched/sched/sched_waitid.c | 22 ++--------------------
sched/sched/sched_waitpid.c | 30 ++----------------------------
4 files changed, 11 insertions(+), 52 deletions(-)
diff --git a/sched/sched/sched_getaffinity.c b/sched/sched/sched_getaffinity.c
index b94f997eb2..5c350758e9 100644
--- a/sched/sched/sched_getaffinity.c
+++ b/sched/sched/sched_getaffinity.c
@@ -69,13 +69,15 @@
int nxsched_get_affinity(pid_t pid, size_t cpusetsize, FAR cpu_set_t *mask)
{
FAR struct tcb_s *tcb;
+ irqstate_t flags;
int ret;
DEBUGASSERT(cpusetsize == sizeof(cpu_set_t) && mask != NULL);
/* Verify that the PID corresponds to a real task */
- sched_lock();
+ flags = enter_critical_section();
+
if (pid == 0)
{
tcb = this_task();
@@ -97,7 +99,7 @@ int nxsched_get_affinity(pid_t pid, size_t cpusetsize, FAR cpu_set_t *mask)
ret = OK;
}
- sched_unlock();
+ leave_critical_section(flags);
return ret;
}
diff --git a/sched/sched/sched_getparam.c b/sched/sched/sched_getparam.c
index fd196be53b..89320cd886 100644
--- a/sched/sched/sched_getparam.c
+++ b/sched/sched/sched_getparam.c
@@ -70,6 +70,7 @@ int nxsched_get_param(pid_t pid, FAR struct sched_param *param)
{
FAR struct tcb_s *rtcb;
FAR struct tcb_s *tcb;
+ irqstate_t flags;
int ret = OK;
if (param == NULL)
@@ -93,7 +94,7 @@ int nxsched_get_param(pid_t pid, FAR struct sched_param *param)
{
/* Get the TCB associated with this PID */
- sched_lock();
+ flags = enter_critical_section();
tcb = nxsched_get_tcb(pid);
if (!tcb)
{
@@ -137,7 +138,7 @@ int nxsched_get_param(pid_t pid, FAR struct sched_param *param)
#endif
}
- sched_unlock();
+ leave_critical_section(flags);
}
return ret;
diff --git a/sched/sched/sched_waitid.c b/sched/sched/sched_waitid.c
index 0dcbb087fe..22188f5755 100644
--- a/sched/sched/sched_waitid.c
+++ b/sched/sched/sched_waitid.c
@@ -152,6 +152,7 @@ int waitid(idtype_t idtype, id_t id, FAR siginfo_t *info, int options)
FAR struct child_status_s *child;
bool retains;
#endif
+ irqstate_t flags;
sigset_t set;
int errcode;
int ret;
@@ -198,18 +199,7 @@ int waitid(idtype_t idtype, id_t id, FAR siginfo_t *info, int options)
sigemptyset(&set);
nxsig_addset(&set, SIGCHLD);
-
- /* NOTE: sched_lock() is not enough for SMP
- * because the child task is running on another CPU
- */
-
-#ifdef CONFIG_SMP
- irqstate_t flags = enter_critical_section();
-#else
- /* Disable pre-emption so that nothing changes while the loop executes */
-
- sched_lock();
-#endif
+ flags = enter_critical_section();
/* Verify that this task actually has children and that the requested
* TCB is actually a child of this task.
@@ -464,20 +454,12 @@ int waitid(idtype_t idtype, id_t id, FAR siginfo_t *info, int options)
}
}
-#ifdef CONFIG_SMP
leave_critical_section(flags);
-#else
- sched_unlock();
-#endif
leave_cancellation_point();
return OK;
errout:
-#ifdef CONFIG_SMP
leave_critical_section(flags);
-#else
- sched_unlock();
-#endif
leave_cancellation_point();
set_errno(errcode);
return ERROR;
diff --git a/sched/sched/sched_waitpid.c b/sched/sched/sched_waitpid.c
index 078a35a7ba..ff77a83e44 100644
--- a/sched/sched/sched_waitpid.c
+++ b/sched/sched/sched_waitpid.c
@@ -101,13 +101,7 @@ pid_t nxsched_waitpid(pid_t pid, int *stat_loc, int options)
* because the child task is running on another CPU
*/
-#ifdef CONFIG_SMP
irqstate_t flags = enter_critical_section();
-#else
- /* Disable pre-emption so that nothing changes in the following tests */
-
- sched_lock();
-#endif
/* Get the TCB corresponding to this PID */
@@ -198,12 +192,7 @@ pid_t nxsched_waitpid(pid_t pid, int *stat_loc, int options)
ret = pid;
errout:
-#ifdef CONFIG_SMP
leave_critical_section(flags);
-#else
- sched_unlock();
-#endif
-
return ret;
}
@@ -230,6 +219,7 @@ pid_t nxsched_waitpid(pid_t pid, int *stat_loc, int options)
bool retains;
#endif
FAR struct siginfo info;
+ irqstate_t flags;
sigset_t set;
int ret;
@@ -237,18 +227,7 @@ pid_t nxsched_waitpid(pid_t pid, int *stat_loc, int options)
sigemptyset(&set);
nxsig_addset(&set, SIGCHLD);
-
- /* NOTE: sched_lock() is not enough for SMP
- * because the child task is running on another CPU
- */
-
-#ifdef CONFIG_SMP
- irqstate_t flags = enter_critical_section();
-#else
- /* Disable pre-emption so that nothing changes while the loop executes */
-
- sched_lock();
-#endif
+ flags = enter_critical_section();
/* Verify that this task actually has children and that the requested PID
* is actually a child of this task.
@@ -495,12 +474,7 @@ pid_t nxsched_waitpid(pid_t pid, int *stat_loc, int options)
ret = pid;
errout:
-#ifdef CONFIG_SMP
leave_critical_section(flags);
-#else
- sched_unlock();
-#endif
-
return ret;
}
#endif /* CONFIG_SCHED_HAVE_PARENT */