YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r498858021



##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +127,175 @@ static void note_common(FAR struct tcb_s *tcb,
   note->nc_systime[3] = (uint8_t)((systime >> 24) & 0xff);
 }
 
+/****************************************************************************
+ * Name: note_isenabled
+ *
+ * Description:
+ *   Check whether the instrumentation is enabled.
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+static inline int note_isenabled(void)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+  if (!g_note_filter.mode.enable)
+    {
+      return false;
+    }
+
+#ifdef CONFIG_SMP
+  /* Ignore notes that are not in the set of monitored CPUs */
+
+  if ((g_note_filter.mode.cpuset & (1 << this_cpu())) == 0)
+    {
+      /* Not in the set of monitored CPUs.  Do not log the note. */
+
+      return false;
+    }
+#endif
+#endif
+
+  return true;
+}
+
+/****************************************************************************
+ * Name: note_isenabled_syscall
+ *
+ * Description:
+ *   Check whether the syscall instrumentation is enabled.
+ *
+ * Input Parameters:
+ *   nr    - syscall number
+ *   tcb   - The TCB of the thread
+ *   enter - syscall enter/leave flag
+ *
+ * Returned Value:
+ *   True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int note_isenabled_syscall(int nr,
+                                         FAR struct tcb_s *tcb, bool enter)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+  irqstate_t irq_mask;
+#endif
+
+  if (!note_isenabled())
+    {
+      return false;
+    }
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+  /* sched_note_syscall_enter() and sched_note_syscall_leave() will be
+   * called from not only applications, but interrupt handlers and many
+   * kernel APIs. Exclude such situations.
+   */
+
+  if (up_interrupt_context())

Review comment:
       > But if we turn off IRQ trace in the first place, how tracecompass 
detect whether syscall happen inside irq context or not?
   
   If we turn off all IRQ trace, all activities while executing the interrupt 
handler should not be recorded as note. We can realize it by simply fixing here 
to the followings:
   ```
     if (up_interrupt_context() && !(g_note_filter.mode.flag & 
NOTE_FILTER_MODE_FLAG_IRQ))
       {
         return false;
       }
   ```
   
   And if we turn on all IRQ trace, the system call called in the interrupt 
handler should be recoded.
   Because note_irqhandler is also recorded, the app code can maintain a count 
to remove the syscall as you mentioned.
   
   My concern is when we turn off IRQ trace partially. Of course 
sched_note_irqhandler() knows the IRQ number and it can decide not to record 
the note if the IRQ is disabled to trace. But sched_note_syscall_enter/leave() 
cannot decide whether the note should be recorded.
   So I found the idea for it, to add a static variable (or NCPUS array when 
SMP) in sched_note.c to keep the flag whether the running context is interrupt 
handler and the IRQ is disabled to trace. It is set by note_isenabled_irq() and 
referred by note_isenabled_syscall().




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

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


Reply via email to