You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "hujun260 (via GitHub)" <gi...@apache.org> on 2024/05/04 08:45:11 UTC

[PR] sched/signal: There is no need to use sched_[un]lock [nuttx]

hujun260 opened a new pull request, #12283:
URL: https://github.com/apache/nuttx/pull/12283

   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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] sched/signal: There is no need to use sched_[un]lock [nuttx]

Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #12283:
URL: https://github.com/apache/nuttx/pull/12283#issuecomment-2094678162

   @hujun260 
   As I commented in https://github.com/apache/nuttx/pull/12140#issuecomment-2062948378
   You replaced replaced sched_lock with critical section in sig_default.c, so you should create a separate commit for it.
   Also, you moved leave_critical_section() into sig_pselect.c and sig_suspend.c, so we need a clearer description for it.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] sched/signal: There is no need to use sched_[un]lock [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #12283:
URL: https://github.com/apache/nuttx/pull/12283#issuecomment-2094248632

   @hujun260 some ltp test case fail: https://github.com/apache/nuttx/actions/runs/8949322132/job/24583521866?pr=12283


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] sched/signal: There is no need to use sched_[un]lock [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12283:
URL: https://github.com/apache/nuttx/pull/12283#discussion_r1590003270


##########
sched/signal/sig_tgkill.c:
##########
@@ -85,14 +85,9 @@ int tgkill(pid_t pid, pid_t tid, int signo)
 
   if (!GOOD_SIGNO(signo))
     {
-      ret = -EINVAL;
-      goto errout;
+      return -EINVAL;

Review Comment:
   let keep jump to errout to set errno.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] sched/signal: There is no need to use sched_[un]lock [nuttx]

Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #12283:
URL: https://github.com/apache/nuttx/pull/12283#discussion_r1592106494


##########
sched/signal/sig_suspend.c:
##########
@@ -154,7 +153,6 @@ int sigsuspend(FAR const sigset_t *set)
       nxsig_unmask_pendingsignal();
     }
 
-  sched_unlock();

Review Comment:
   ditto ,context switch on line 129
   
   https://github.com/apache/nuttx/blob/4ffad2656e6a35d4837e653a3ec65de0336f3cd1/sched/sched/sched_removereadytorun.c#L228



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] sched/signal: There is no need to use sched_[un]lock [nuttx]

Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #12283:
URL: https://github.com/apache/nuttx/pull/12283#discussion_r1590457357


##########
sched/signal/sig_default.c:
##########
@@ -327,7 +323,7 @@ static void nxsig_stop_task(int signo)
   /* Then, finally, suspend this the final thread of the task group */
 
   nxsched_suspend(rtcb);

Review Comment:
   `nxsched_suspend()` and `nxsem_post()` line 318 will trigger context switch



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org