xiaoxiang781216 commented on code in PR #15130:
URL: https://github.com/apache/nuttx/pull/15130#discussion_r1912461239


##########
include/nuttx/irq.h:
##########
@@ -258,9 +258,19 @@ int irqchain_detach(int irq, xcpt_t isr, FAR void *arg);
  ****************************************************************************/
 
 #ifdef CONFIG_IRQCOUNT
+
+#  if (defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION) && \

Review Comment:
   remove `(defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION) &&`



##########
include/nuttx/irq.h:
##########
@@ -288,9 +298,19 @@ irqstate_t enter_critical_section(void) 
noinstrument_function;
  ****************************************************************************/
 
 #ifdef CONFIG_IRQCOUNT
+
+#  if (defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION) && \

Review Comment:
   remove (defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION) &&



##########
sched/irq/irq_csection.c:
##########
@@ -440,22 +437,37 @@ void leave_critical_section(irqstate_t flags)
        */
 
       DEBUGASSERT(rtcb->irqcount > 0);
-      if (--rtcb->irqcount <= 0)
-        {
-          /* Note that we have left the critical section */
+      --rtcb->irqcount;
+    }
 
-#if CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION >= 0
-          nxsched_critmon_csection(rtcb, false, return_address(0));
+  /* Restore the previous interrupt state. */
+
+  up_irq_restore(flags);
+}
 #endif
-#ifdef CONFIG_SCHED_INSTRUMENTATION_CSECTION
+
+#if CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION >= 0 ||\
+    defined(CONFIG_SCHED_INSTRUMENTATION_CSECTION)
+void leave_critical_section(irqstate_t flags)
+{
+  FAR struct tcb_s *rtcb;
+
+  if (!up_interrupt_context())
+    {
+      rtcb = this_task();
+      if (rtcb->irqcount == 1)
+        {
+#  if CONFIG_SCHED_CRITMONITOR_MAXTIME_CSECTION >= 0
+          nxsched_critmon_csection(rtcb, false, return_address(0));
+#  endif
+#  ifdef CONFIG_SCHED_INSTRUMENTATION_CSECTION
           sched_note_csection(rtcb, false);
-#endif
+#  endif
         }
     }
 
-  /* Restore the previous interrupt state. */
-
-  up_irq_restore(flags);
+  leave_critical_section_wo_note(flags);
 }
 #endif
+
 #endif /* CONFIG_IRQCOUNT */

Review Comment:
   remove `#ifdef CONFIG_IRQCOUNT #endif /* CONFIG_IRQCOUNT  */` since 
Make.defs already check it



##########
sched/wdog/wd_start.c:
##########
@@ -112,10 +112,11 @@ static unsigned int g_wdtimernested;
 static inline_function void wd_expiration(clock_t ticks)
 {
   FAR struct wdog_s *wdog;
+  wdparm_t arg;

Review Comment:
   move after line 117



##########
drivers/note/note_driver.c:
##########
@@ -1393,10 +1393,12 @@ void sched_note_irqhandler(int irq, FAR void *handler, 
bool enter)
 void sched_note_wdog(uint8_t event, FAR void *handler, FAR const void *arg)
 {
   FAR struct note_driver_s **driver;
+  irqstate_t flags;
   struct note_wdog_s note;
   bool formatted = false;
   FAR struct tcb_s *tcb = this_task();
 
+  flags = enter_critical_section_wo_note();

Review Comment:
   but why only sched_note_wdog add the critical section? if the critical 
section is required, let's move to new patch.



##########
sched/wdog/wdog.h:
##########
@@ -64,6 +65,7 @@ extern "C"
  */
 
 extern struct list_node g_wdactivelist;
+extern spinlock_t g_wdog_spinlock;

Review Comment:
   g_wdspinlock



##########
drivers/note/note_driver.c:
##########
@@ -1393,10 +1393,12 @@ void sched_note_irqhandler(int irq, FAR void *handler, 
bool enter)
 void sched_note_wdog(uint8_t event, FAR void *handler, FAR const void *arg)
 {
   FAR struct note_driver_s **driver;
+  irqstate_t flags;

Review Comment:
   move after line 1399



-- 
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

Reply via email to