From 2976bb212e7e49d8c9c303f85fc3b83e07cd0010 Mon Sep 17 00:00:00 2001
From: Masayuki Ishikawa <masayuki.ishikawa@gmail.com>
Date: Sat, 20 Mar 2021 09:03:55 +0900
Subject: [PATCH] sched: pthread: Remove a redundant critical section in
 pthread_condclockwsait.c

Summary:
- This commit removes a redundant critical section in pthread_condclockwait.c

Impact:
- None

Testing:
- Tested with ostest with the following configs
  - maix-bit:smp, esp32-devkitc:smp, sabre-6quad:smp
  - spresense:smp, sim:smp
  - maix-bit:nsh, sabre-6quad:nsh
  - sprsesnse:wifi, sim:ostest
- Tested with nxplayer with the folowing configs
  - spresense:wifi_smp, spresense:rndis_smp

Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
---
 sched/pthread/pthread_condclockwait.c | 50 +++++----------------------
 1 file changed, 9 insertions(+), 41 deletions(-)

diff --git a/sched/pthread/pthread_condclockwait.c b/sched/pthread/pthread_condclockwait.c
index 8bc046ef0e..865a40a1ac 100644
--- a/sched/pthread/pthread_condclockwait.c
+++ b/sched/pthread/pthread_condclockwait.c
@@ -154,7 +154,6 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
                            FAR const struct timespec *abstime)
 {
   FAR struct tcb_s *rtcb = this_task();
-  irqstate_t flags;
   sclock_t ticks;
   int mypid = getpid();
   int ret = OK;
@@ -193,14 +192,15 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
     {
       sinfo("Give up mutex...\n");
 
-      /* We must disable pre-emption and interrupts here so that
-       * the time stays valid until the wait begins.   This adds
-       * complexity because we assure that interrupts and
-       * pre-emption are re-enabled correctly.
+      /* NOTE: We do not need a critical section here,
+       * because nxsem_wait() uses a critical section and
+       * pthread_condtimedout() is called from wd_timer()
+       * which uses a critical section for SMP case
        */
 
+      /* REVISIT: Do we need to disable pre-emption? */
+
       sched_lock();
-      flags = enter_critical_section();
 
       /* Convert the timespec to clock ticks.  We must disable pre-
        * emption here so that this time stays valid until the wait
@@ -208,15 +208,7 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
        */
 
       ret = clock_abstime2ticks(clockid, abstime, &ticks);
-      if (ret)
-        {
-          /* Restore interrupts  (pre-emption will be enabled when
-           * we fall through the if/then/else)
-           */
-
-          leave_critical_section(flags);
-        }
-      else
+      if (ret == 0)
         {
           /* Check the absolute time to wait.  If it is now or in the
            * past, then just return with the timedout condition.
@@ -224,12 +216,6 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
 
           if (ticks <= 0)
             {
-              /* Restore interrupts and indicate that we have already
-               * timed out. (pre-emption will be enabled when we fall
-               * through the if/then/else
-               */
-
-              leave_critical_section(flags);
               ret = ETIMEDOUT;
             }
           else
@@ -252,15 +238,7 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
               nlocks     = mutex->nlocks;
 #endif
               ret        = pthread_mutex_give(mutex);
-              if (ret != 0)
-                {
-                  /* Restore interrupts  (pre-emption will be enabled
-                   * when we fall through the if/then/else)
-                   */
-
-                  leave_critical_section(flags);
-                }
-              else
+              if (ret == 0)
                 {
                   /* Start the watchdog */
 
@@ -299,14 +277,6 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
                           ret = status;
                         }
                     }
-
-                  /* The interrupts stay disabled until after we sample
-                   * the errno.  This is because when debug is enabled
-                   * and the console is used for debug output, then the
-                   * errno can be altered by interrupt handling! (bad)
-                   */
-
-                  leave_critical_section(flags);
                 }
 
               /* Reacquire the mutex (retaining the ret). */
@@ -332,9 +302,7 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
             }
         }
 
-      /* Re-enable pre-emption (It is expected that interrupts
-       * have already been re-enabled in the above logic)
-       */
+      /* Re-enable pre-emption */
 
       sched_unlock();
     }