Re: [PR] sched: remove all spin_lock_irqsave(NULL) [nuttx]

[via GitHub]

anchao commented on code in PR #15324:
URL: https://github.com/apache/nuttx/pull/15324#discussion_r1896376249


##########
sched/clock/clock_gettime.c:
##########
@@ -58,15 +58,15 @@ static clock_t clock_process_runtime(FAR struct tcb_s *tcb)
 
   group = tcb->group;
 
-  flags = spin_lock_irqsave(NULL);
+  flags = spin_lock_irqsave(&group->lock);

Review Comment:
   replace all spin_lock_irqsave to spin_lock_irqsave_wo_note in sched dir



##########
sched/clock/clock_initialize.c:
##########
@@ -373,12 +382,14 @@ void clock_resynchronize(FAR struct timespec *rtc_diff)
 
       /* Add the sleep time to correct system timer */
 
+      flags = enter_critical_section();

Review Comment:
   why?



##########
include/nuttx/spinlock.h:
##########
@@ -618,21 +588,8 @@ static inline_function
 void spin_unlock_irqrestore_wo_note(FAR volatile spinlock_t *lock,
                                     irqstate_t flags)
 {
-  if (NULL == lock)
-    {
-      int me = this_cpu();
-      DEBUGASSERT(0 < g_irq_spin_count[me]);
-      g_irq_spin_count[me]--;
-
-      if (0 == g_irq_spin_count[me])
-        {
-          spin_unlock_wo_note(&g_irq_spin);
-        }
-    }
-  else
-    {
-      spin_unlock_wo_note(lock);
-    }
+  DEBUGASSERT(lock);

Review Comment:
   ```suggestion
   ```



##########
sched/mqueue/mq_msgfree.c:
##########
@@ -69,9 +69,9 @@ void nxmq_free_msg(FAR struct mqueue_msg_s *mqmsg)
        * list from interrupt handlers.
        */
 
-      flags = spin_lock_irqsave(NULL);
+      flags = spin_lock_irqsave(&g_msg_lock);

Review Comment:
   ```suggestion
         flags = spin_lock_irqsave(&g_msgfree_lock);
   ```



##########
sched/signal/sig_default.c:
##########
@@ -546,9 +546,9 @@ _sa_handler_t nxsig_default(FAR struct tcb_s *tcb, int 
signo, bool defaction)
        * atomic (but neither is sigaction()).
        */
 
-      flags = spin_lock_irqsave(NULL);
+      flags = spin_lock_irqsave(&group->lock);
       nxsig_delset(&group->tg_sigdefault, signo);
-      spin_unlock_irqrestore(NULL, flags);
+      spin_unlock_irqrestore(&group->lock, flags);

Review Comment:
   ```suggestion
         spin_unlock_irqrestore(&group->tg_siglock, flags);
   ```



##########
sched/clock/clock_gettime.c:
##########
@@ -58,15 +58,15 @@ static clock_t clock_process_runtime(FAR struct tcb_s *tcb)
 
   group = tcb->group;
 
-  flags = spin_lock_irqsave(NULL);
+  flags = spin_lock_irqsave(&group->lock);
   sq_for_every(&group->tg_members, curr)
     {
       tcb = container_of(curr, struct tcb_s, member);
 
       runtime += tcb->run_time;
     }
 
-  spin_unlock_irqrestore(NULL, flags);
+  spin_unlock_irqrestore(&group->lock, flags);

Review Comment:
   ```suggestion
     spin_unlock_irqrestore(&group->tg_members_lock, flags);
   ```



##########
sched/irq/irq_attach.c:
##########
@@ -67,13 +68,13 @@ int irq_to_ndx(int irq)
 {
   DEBUGASSERT(g_irqmap_count < CONFIG_ARCH_NUSER_INTERRUPTS);
 
-  irqstate_t flags = spin_lock_irqsave(NULL);
+  irqstate_t flags = spin_lock_irqsave(&g_irq_attach_lock);

Review Comment:
   atomic



##########
include/nuttx/spinlock.h:
##########
@@ -451,21 +443,8 @@ irqstate_t spin_lock_irqsave_wo_note(FAR volatile 
spinlock_t *lock)
   irqstate_t ret;

Review Comment:
   ```suggestion
     irqstate_t flags;
   ```



##########
include/nuttx/spinlock.h:
##########
@@ -451,21 +443,8 @@ irqstate_t spin_lock_irqsave_wo_note(FAR volatile 
spinlock_t *lock)
   irqstate_t ret;
   ret = up_irq_save();
 
-  if (NULL == lock)
-    {
-      int me = this_cpu();
-      if (0 == g_irq_spin_count[me])
-        {
-          spin_lock_wo_note(&g_irq_spin);
-        }
-
-      g_irq_spin_count[me]++;
-      DEBUGASSERT(0 != g_irq_spin_count[me]);
-    }
-  else
-    {
-      spin_lock_wo_note(lock);
-    }
+  DEBUGASSERT(lock);

Review Comment:
   ```suggestion
   ```



##########
sched/timer/timer_create.c:
##########
@@ -63,10 +63,10 @@ static FAR struct posix_timer_s *timer_allocate(void)
   /* Try to get a preallocated timer from the free list */
 
 #if CONFIG_PREALLOC_TIMERS > 0
-  flags = spin_lock_irqsave(NULL);
+  flags = spin_lock_irqsave(&g_timers_lock);
   ret   = (FAR struct posix_timer_s *)
     sq_remfirst((FAR sq_queue_t *)&g_freetimers);
-  spin_unlock_irqrestore(NULL, flags);
+  spin_unlock_irqrestore(&g_timers_lock, flags);

Review Comment:
   ```suggestion
     spin_unlock_irqrestore(&g_freetimers_lock, flags);
   ```



-- 
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-unsubscr...@nuttx.apache.org

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