[incubator-nuttx] 01/02: sched: semaphore: Remove a redundant critical section in nxsem_clockwait()

[xi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git

commit 65dec5d10ac57abc9a15aadf076d693ab5208f69
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Tue Mar 16 08:06:16 2021 +0900

    sched: semaphore: Remove a redundant critical section in nxsem_clockwait()
    
    Summary:
    - This commit removes a redundant critical section in nxsem_clockwait()
    
    Impact:
    - None
    
    Testing:
    - Tested with ostest with the following configurations
    - maix-bit:smp (QEMU), sim:smp, esp32-devkitc:smp (QEMU)
    - sabre-6quad:smp (QEMU), spresense:smp
    - maix-bit:nsh (QEMU), sim:ostest, esp32-devkitc:ostest (QEMU)
    - sabre-6quad:nsh (QEMU), spresense:wifi
    
    Signed-off-by: Masayuki Ishikawa <Ma...@jp.sony.com>
---
 sched/semaphore/sem_clockwait.c | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/sched/semaphore/sem_clockwait.c b/sched/semaphore/sem_clockwait.c
index 0db0270..7269bfa 100644
--- a/sched/semaphore/sem_clockwait.c
+++ b/sched/semaphore/sem_clockwait.c
@@ -91,7 +91,6 @@ int nxsem_clockwait(FAR sem_t *sem, clockid_t clockid,
                     FAR const struct timespec *abstime)
 {
   FAR struct tcb_s *rtcb = this_task();
-  irqstate_t flags;
   sclock_t ticks;
   int status;
   int ret = ERROR;
@@ -109,16 +108,11 @@ int nxsem_clockwait(FAR sem_t *sem, clockid_t clockid,
     }
 #endif
 
-  /* We will disable interrupts until we have completed the semaphore
-   * wait.  We need to do this (as opposed to just disabling pre-emption)
-   * because there could be interrupt handlers that are asynchronously
-   * posting semaphores and to prevent race conditions with watchdog
-   * timeout.  This is not too bad because interrupts will be re-
-   * enabled while we are blocked waiting for the semaphore.
+  /* NOTE: We do not need a critical section here, because
+   * nxsem_wait() and nxsem_timeout() use a critical section
+   * in the functions.
    */
 
-  flags = enter_critical_section();
-
   /* Try to take the semaphore without waiting. */
 
   ret = nxsem_trywait(sem);
@@ -126,7 +120,7 @@ int nxsem_clockwait(FAR sem_t *sem, clockid_t clockid,
     {
       /* We got it! */
 
-      goto success_with_irqdisabled;
+      goto out;
     }
 
   /* We will have to wait for the semaphore.  Make sure that we were provided
@@ -136,7 +130,7 @@ int nxsem_clockwait(FAR sem_t *sem, clockid_t clockid,
   if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
     {
       ret = -EINVAL;
-      goto errout_with_irqdisabled;
+      goto out;
     }
 
   /* Convert the timespec to clock ticks.  We must have interrupts
@@ -153,7 +147,7 @@ int nxsem_clockwait(FAR sem_t *sem, clockid_t clockid,
   if (status == OK && ticks <= 0)
     {
       ret = -ETIMEDOUT;
-      goto errout_with_irqdisabled;
+      goto out;
     }
 
   /* Handle any time-related errors */
@@ -161,7 +155,7 @@ int nxsem_clockwait(FAR sem_t *sem, clockid_t clockid,
   if (status != OK)
     {
       ret = -status;
-      goto errout_with_irqdisabled;
+      goto out;
     }
 
   /* Start the watchdog */
@@ -178,11 +172,7 @@ int nxsem_clockwait(FAR sem_t *sem, clockid_t clockid,
 
   wd_cancel(&rtcb->waitdog);
 
-  /* We can now restore interrupts and delete the watchdog */
-
-success_with_irqdisabled:
-errout_with_irqdisabled:
-  leave_critical_section(flags);
+out:
   return ret;
 }