You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/03/07 15:54:29 UTC

[GitHub] [incubator-nuttx] anchao opened a new pull request #5689: segger/sysview: add syscall support

anchao opened a new pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689


   ## Summary
   
   segger/sysview: add syscall support
   
   ## Impact
   
   N/A
   
   ## Testing
   
   ci check


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

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



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r821283332



##########
File path: drivers/segger/note_sysview.c
##########
@@ -310,6 +399,42 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_syscall_enter(int nr, int argc, ...)
+{
+  if (sysview_isenabled_syscall(nr) == 0)
+    {
+      return;
+    }
+
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_active) == 0)

Review comment:
       Use the Syscall "0" to identify whether the syscall is enabled, if the host tool is closed abnormally, use this bit to clear the active set.




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

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



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r821282635



##########
File path: drivers/segger/note_sysview.c
##########
@@ -40,12 +40,17 @@
 
 struct sysview_s
 {
-  unsigned int              irq[CONFIG_SMP_NCPUS];
+  unsigned int                 irq[CONFIG_SMP_NCPUS];
 #ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
-  struct note_filter_mode_s mode;
+  struct note_filter_mode_s    mode;
 #endif
 #ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
-  struct note_filter_irq_s  irq_mask;
+  struct note_filter_irq_s     irq_mask;
+  unsigned int                 irq_nest[CONFIG_SMP_NCPUS];

Review comment:
       Done




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

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



[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689


   


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

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



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r822258602



##########
File path: drivers/segger/note_sysview.c
##########
@@ -197,6 +214,45 @@ static bool sysview_isenabled_irq(int irq, bool enter)
 }
 #endif
 
+/****************************************************************************
+ * Name: sysview_isenabled_syscall
+ *
+ * Description:
+ *   Check whether the syscall instrumentation is enabled.
+ *
+ * Input Parameters:
+ *   nr - syscall number
+ *
+ * Returned Value:
+ *   True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int sysview_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+  if (!sysview_isenabled())
+    {
+      return false;
+    }
+
+  /* If the syscall trace is disabled or the syscall number is masked,
+   * do nothing.
+   */
+
+  if (!(g_sysview.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) ||

Review comment:
       Done




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

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



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r821284011



##########
File path: drivers/segger/note_sysview.c
##########
@@ -40,12 +40,17 @@
 
 struct sysview_s
 {
-  unsigned int              irq[CONFIG_SMP_NCPUS];
+  unsigned int                 irq[CONFIG_SMP_NCPUS];
 #ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
-  struct note_filter_mode_s mode;
+  struct note_filter_mode_s    mode;
 #endif
 #ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
-  struct note_filter_irq_s  irq_mask;
+  struct note_filter_irq_s     irq_mask;
+  unsigned int                 irq_nest[CONFIG_SMP_NCPUS];

Review comment:
       Done removed




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

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



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r822264837



##########
File path: drivers/segger/note_sysview.c
##########
@@ -310,6 +366,54 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_syscall_enter(int nr, int argc, ...)
+{
+  if (sysview_isenabled_syscall(nr) == 0)

Review comment:
       Done

##########
File path: boards/sim/sim/sim/configs/segger/defconfig
##########
@@ -62,6 +62,7 @@ CONFIG_SCHED_HAVE_PARENT=y
 CONFIG_SCHED_HPWORK=y
 CONFIG_SCHED_INSTRUMENTATION=y
 CONFIG_SCHED_INSTRUMENTATION_FILTER=y
+CONFIG_SCHED_INSTRUMENTATION_SYSCALL=y

Review comment:
       Done




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

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



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r821282854



##########
File path: drivers/segger/note_sysview.c
##########
@@ -310,6 +399,42 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_syscall_enter(int nr, int argc, ...)
+{
+  if (sysview_isenabled_syscall(nr) == 0)
+    {
+      return;
+    }
+
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_active) == 0)
+    {
+      SEGGER_SYSVIEW_NameMarker(nr, g_funcnames[nr]);
+      NOTE_FILTER_SYSCALLMASK_SET(nr, &g_sysview.syscall_active);
+
+      if (NOTE_FILTER_SYSCALLMASK_ISSET(0, &g_sysview.syscall_active) == 0)

Review comment:
       Use the Syscall "0" to identify whether the syscall is enabled, if the host tool is closed abnormally, use this bit to clear the active set.
   




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

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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r820950224



##########
File path: drivers/segger/note_sysview.c
##########
@@ -40,12 +40,17 @@
 
 struct sysview_s
 {
-  unsigned int              irq[CONFIG_SMP_NCPUS];
+  unsigned int                 irq[CONFIG_SMP_NCPUS];
 #ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
-  struct note_filter_mode_s mode;
+  struct note_filter_mode_s    mode;
 #endif
 #ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
-  struct note_filter_irq_s  irq_mask;
+  struct note_filter_irq_s     irq_mask;
+  unsigned int                 irq_nest[CONFIG_SMP_NCPUS];

Review comment:
       why need record irq nest?

##########
File path: drivers/segger/note_sysview.c
##########
@@ -310,6 +399,42 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_syscall_enter(int nr, int argc, ...)
+{
+  if (sysview_isenabled_syscall(nr) == 0)
+    {
+      return;
+    }
+
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_active) == 0)
+    {
+      SEGGER_SYSVIEW_NameMarker(nr, g_funcnames[nr]);
+      NOTE_FILTER_SYSCALLMASK_SET(nr, &g_sysview.syscall_active);
+
+      if (NOTE_FILTER_SYSCALLMASK_ISSET(0, &g_sysview.syscall_active) == 0)

Review comment:
       why?

##########
File path: drivers/segger/note_sysview.c
##########
@@ -189,6 +202,82 @@ static bool sysview_isenabled_irq(int irq, bool enter)
   if ((g_sysview.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) == 0 ||
       NOTE_FILTER_IRQMASK_ISSET(irq, &g_sysview.irq_mask))
     {
+#ifdef CONFIG_SMP
+      int cpu = this_cpu();
+#else
+      int cpu = 0;
+#endif
+
+      if (enter)
+        {
+          g_sysview.irq_nest[cpu]++;
+        }
+      else
+        {
+          g_sysview.irq_nest[cpu]--;
+        }
+
+      return false;
+    }
+#endif
+
+  return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: sysview_isenabled_syscall
+ *
+ * Description:
+ *   Check whether the syscall instrumentation is enabled.
+ *
+ * Input Parameters:
+ *   nr - syscall number
+ *
+ * Returned Value:
+ *   True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int sysview_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+  if (!sysview_isenabled())
+    {
+      return false;
+    }
+
+  /* Exclude the case of syscall called by the interrupt handler which is
+   * not traced.
+   */
+
+  if (up_interrupt_context())

Review comment:
       why not trace syscall in the interrupt context?

##########
File path: drivers/segger/note_sysview.c
##########
@@ -189,6 +202,82 @@ static bool sysview_isenabled_irq(int irq, bool enter)
   if ((g_sysview.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) == 0 ||
       NOTE_FILTER_IRQMASK_ISSET(irq, &g_sysview.irq_mask))
     {
+#ifdef CONFIG_SMP
+      int cpu = this_cpu();
+#else
+      int cpu = 0;
+#endif
+
+      if (enter)
+        {
+          g_sysview.irq_nest[cpu]++;
+        }
+      else
+        {
+          g_sysview.irq_nest[cpu]--;
+        }
+
+      return false;
+    }
+#endif
+
+  return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: sysview_isenabled_syscall
+ *
+ * Description:
+ *   Check whether the syscall instrumentation is enabled.
+ *
+ * Input Parameters:
+ *   nr - syscall number
+ *
+ * Returned Value:
+ *   True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int sysview_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+  if (!sysview_isenabled())
+    {
+      return false;
+    }
+
+  /* Exclude the case of syscall called by the interrupt handler which is
+   * not traced.
+   */
+
+  if (up_interrupt_context())

Review comment:
       why not trace syscall in the interrupt context?

##########
File path: drivers/segger/note_sysview.c
##########
@@ -40,12 +40,17 @@
 
 struct sysview_s
 {
-  unsigned int              irq[CONFIG_SMP_NCPUS];
+  unsigned int                 irq[CONFIG_SMP_NCPUS];
 #ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
-  struct note_filter_mode_s mode;
+  struct note_filter_mode_s    mode;
 #endif
 #ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
-  struct note_filter_irq_s  irq_mask;
+  struct note_filter_irq_s     irq_mask;
+  unsigned int                 irq_nest[CONFIG_SMP_NCPUS];
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+  struct note_filter_syscall_s syscall_mask;
+  struct note_filter_syscall_s syscall_active;

Review comment:
       change syscall_active to syscall_marker

##########
File path: drivers/segger/note_sysview.c
##########
@@ -40,12 +40,17 @@
 
 struct sysview_s
 {
-  unsigned int              irq[CONFIG_SMP_NCPUS];
+  unsigned int                 irq[CONFIG_SMP_NCPUS];
 #ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
-  struct note_filter_mode_s mode;
+  struct note_filter_mode_s    mode;
 #endif
 #ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
-  struct note_filter_irq_s  irq_mask;
+  struct note_filter_irq_s     irq_mask;
+  unsigned int                 irq_nest[CONFIG_SMP_NCPUS];

Review comment:
       why need record irq nest?

##########
File path: drivers/segger/note_sysview.c
##########
@@ -310,6 +399,42 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_syscall_enter(int nr, int argc, ...)
+{
+  if (sysview_isenabled_syscall(nr) == 0)
+    {
+      return;
+    }
+
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_active) == 0)

Review comment:
       need nr - NOTE_FILTER_SYSCALLMASK_ISSET in all NOTE_FILTER_SYSCALLMASK_xxx

##########
File path: drivers/segger/note_sysview.c
##########
@@ -310,6 +399,42 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_syscall_enter(int nr, int argc, ...)
+{
+  if (sysview_isenabled_syscall(nr) == 0)
+    {
+      return;
+    }
+
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_active) == 0)
+    {
+      SEGGER_SYSVIEW_NameMarker(nr, g_funcnames[nr]);
+      NOTE_FILTER_SYSCALLMASK_SET(nr, &g_sysview.syscall_active);
+
+      if (NOTE_FILTER_SYSCALLMASK_ISSET(0, &g_sysview.syscall_active) == 0)
+        {
+          NOTE_FILTER_SYSCALLMASK_SET(0, &g_sysview.syscall_active);
+        }
+    }
+
+  SEGGER_SYSVIEW_MarkStart(nr);
+}
+
+void sched_note_syscall_leave(int nr, uintptr_t result)
+{
+  if (sysview_isenabled_syscall(nr) == 0)
+    {
+      return;
+    }
+
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_active) != 0)

Review comment:
       why need check

##########
File path: drivers/segger/note_sysview.c
##########
@@ -155,6 +160,14 @@ static bool sysview_isenabled(void)
     }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+  if (!enable &&
+      NOTE_FILTER_SYSCALLMASK_ISSET(0, &g_sysview.syscall_active))
+    {
+      NOTE_FILTER_SYSCALLMASK_ZERO(&g_sysview.syscall_active);

Review comment:
       why change syscall_active in sysview_isenabled?

##########
File path: drivers/segger/note_sysview.c
##########
@@ -310,6 +399,42 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_syscall_enter(int nr, int argc, ...)
+{
+  if (sysview_isenabled_syscall(nr) == 0)
+    {
+      return;
+    }
+
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_active) == 0)
+    {
+      SEGGER_SYSVIEW_NameMarker(nr, g_funcnames[nr]);
+      NOTE_FILTER_SYSCALLMASK_SET(nr, &g_sysview.syscall_active);
+
+      if (NOTE_FILTER_SYSCALLMASK_ISSET(0, &g_sysview.syscall_active) == 0)
+        {
+          NOTE_FILTER_SYSCALLMASK_SET(0, &g_sysview.syscall_active);
+        }
+    }
+
+  SEGGER_SYSVIEW_MarkStart(nr);
+}
+
+void sched_note_syscall_leave(int nr, uintptr_t result)
+{
+  if (sysview_isenabled_syscall(nr) == 0)
+    {
+      return;
+    }
+
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_active) != 0)

Review comment:
       why need check

##########
File path: drivers/segger/note_sysview.c
##########
@@ -40,12 +40,17 @@
 
 struct sysview_s
 {
-  unsigned int              irq[CONFIG_SMP_NCPUS];
+  unsigned int                 irq[CONFIG_SMP_NCPUS];
 #ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
-  struct note_filter_mode_s mode;
+  struct note_filter_mode_s    mode;
 #endif
 #ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
-  struct note_filter_irq_s  irq_mask;
+  struct note_filter_irq_s     irq_mask;
+  unsigned int                 irq_nest[CONFIG_SMP_NCPUS];
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+  struct note_filter_syscall_s syscall_mask;
+  struct note_filter_syscall_s syscall_active;

Review comment:
       change syscall_active to syscall_marker

##########
File path: drivers/segger/note_sysview.c
##########
@@ -310,6 +399,42 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_syscall_enter(int nr, int argc, ...)
+{
+  if (sysview_isenabled_syscall(nr) == 0)
+    {
+      return;
+    }
+
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_active) == 0)

Review comment:
       need nr - NOTE_FILTER_SYSCALLMASK_ISSET in all NOTE_FILTER_SYSCALLMASK_xxx

##########
File path: drivers/segger/note_sysview.c
##########
@@ -310,6 +399,42 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_syscall_enter(int nr, int argc, ...)
+{
+  if (sysview_isenabled_syscall(nr) == 0)
+    {
+      return;
+    }
+
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_active) == 0)
+    {
+      SEGGER_SYSVIEW_NameMarker(nr, g_funcnames[nr]);
+      NOTE_FILTER_SYSCALLMASK_SET(nr, &g_sysview.syscall_active);
+
+      if (NOTE_FILTER_SYSCALLMASK_ISSET(0, &g_sysview.syscall_active) == 0)

Review comment:
       why?

##########
File path: drivers/segger/note_sysview.c
##########
@@ -155,6 +160,14 @@ static bool sysview_isenabled(void)
     }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+  if (!enable &&
+      NOTE_FILTER_SYSCALLMASK_ISSET(0, &g_sysview.syscall_active))
+    {
+      NOTE_FILTER_SYSCALLMASK_ZERO(&g_sysview.syscall_active);

Review comment:
       why change syscall_active in sysview_isenabled?




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

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



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r822260473



##########
File path: drivers/segger/note_sysview.c
##########
@@ -490,4 +594,50 @@ void sched_note_filter_irq(struct note_filter_irq_s *oldf,
 }
 #endif
 
+/****************************************************************************
+ * Name: sched_note_filter_syscall
+ *
+ * Description:
+ *   Set and get syscall filter setting
+ *   (Same as NOTECTL_GETSYSCALLFILTER / NOTECTL_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ *   oldf - A writable pointer to struct note_filter_syscall_s to get
+ *          current syscall filter setting
+ *          If 0, no data is written.
+ *   newf - A read-only pointer to struct note_filter_syscall_s of the
+ *          new syscall filter setting
+ *          If 0, the setting is not updated.
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_filter_syscall(struct note_filter_syscall_s *oldf,
+                               struct note_filter_syscall_s *newf)
+{
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave(NULL);

Review comment:
       we will plan to unified filter to common code on next PR, which will be a separate patch




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

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



[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r821544929



##########
File path: drivers/segger/note_sysview.c
##########
@@ -490,4 +594,50 @@ void sched_note_filter_irq(struct note_filter_irq_s *oldf,
 }
 #endif
 
+/****************************************************************************
+ * Name: sched_note_filter_syscall
+ *
+ * Description:
+ *   Set and get syscall filter setting
+ *   (Same as NOTECTL_GETSYSCALLFILTER / NOTECTL_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ *   oldf - A writable pointer to struct note_filter_syscall_s to get
+ *          current syscall filter setting
+ *          If 0, no data is written.
+ *   newf - A read-only pointer to struct note_filter_syscall_s of the
+ *          new syscall filter setting
+ *          If 0, the setting is not updated.
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_filter_syscall(struct note_filter_syscall_s *oldf,
+                               struct note_filter_syscall_s *newf)
+{
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave(NULL);

Review comment:
       Should we introduce a separate `segger_spinlock` instance?

##########
File path: drivers/segger/note_sysview.c
##########
@@ -197,6 +214,45 @@ static bool sysview_isenabled_irq(int irq, bool enter)
 }
 #endif
 
+/****************************************************************************
+ * Name: sysview_isenabled_syscall
+ *
+ * Description:
+ *   Check whether the syscall instrumentation is enabled.
+ *
+ * Input Parameters:
+ *   nr - syscall number
+ *
+ * Returned Value:
+ *   True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int sysview_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+  if (!sysview_isenabled())
+    {
+      return false;
+    }
+
+  /* If the syscall trace is disabled or the syscall number is masked,
+   * do nothing.
+   */
+
+  if (!(g_sysview.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) ||

Review comment:
       ```suggestion
     if ((g_sysview.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) == 0 ||
   ```




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

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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r822275106



##########
File path: tools/ci/testlist/sim-02.dat
##########
@@ -8,3 +8,7 @@
 
 # macOS doesn't have X11
 -Darwin,sim:touchscreen
+
+# macOS doesn't support --wrap flag

Review comment:
       move before line 9 to keep the order




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

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



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r821283125



##########
File path: drivers/segger/note_sysview.c
##########
@@ -189,6 +202,82 @@ static bool sysview_isenabled_irq(int irq, bool enter)
   if ((g_sysview.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) == 0 ||
       NOTE_FILTER_IRQMASK_ISSET(irq, &g_sysview.irq_mask))
     {
+#ifdef CONFIG_SMP
+      int cpu = this_cpu();
+#else
+      int cpu = 0;
+#endif
+
+      if (enter)
+        {
+          g_sysview.irq_nest[cpu]++;
+        }
+      else
+        {
+          g_sysview.irq_nest[cpu]--;
+        }
+
+      return false;
+    }
+#endif
+
+  return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: sysview_isenabled_syscall
+ *
+ * Description:
+ *   Check whether the syscall instrumentation is enabled.
+ *
+ * Input Parameters:
+ *   nr - syscall number
+ *
+ * Returned Value:
+ *   True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int sysview_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+  if (!sysview_isenabled())
+    {
+      return false;
+    }
+
+  /* Exclude the case of syscall called by the interrupt handler which is
+   * not traced.
+   */
+
+  if (up_interrupt_context())

Review comment:
       Done, removed




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

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



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r822268252



##########
File path: drivers/segger/note_sysview.c
##########
@@ -155,11 +159,24 @@ static bool sysview_isenabled(void)
     }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+  /* Use the Syscall "0" to identify whether the syscall is enabled,

Review comment:
       The 0th syscall is a special symbol (_exit()), most calls focus only on exit()
   
   https://github.com/apache/incubator-nuttx/blob/master/include/sys/syscall_lookup.h#L27




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

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



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r821282912



##########
File path: drivers/segger/note_sysview.c
##########
@@ -310,6 +399,42 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_syscall_enter(int nr, int argc, ...)
+{
+  if (sysview_isenabled_syscall(nr) == 0)
+    {
+      return;
+    }
+
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_active) == 0)
+    {
+      SEGGER_SYSVIEW_NameMarker(nr, g_funcnames[nr]);
+      NOTE_FILTER_SYSCALLMASK_SET(nr, &g_sysview.syscall_active);
+
+      if (NOTE_FILTER_SYSCALLMASK_ISSET(0, &g_sysview.syscall_active) == 0)

Review comment:
       Use the Syscall "0" to identify whether the syscall is enabled, if the host tool is closed abnormally, use this bit to clear the active set.




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

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



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r821283908



##########
File path: drivers/segger/note_sysview.c
##########
@@ -40,12 +40,17 @@
 
 struct sysview_s
 {
-  unsigned int              irq[CONFIG_SMP_NCPUS];
+  unsigned int                 irq[CONFIG_SMP_NCPUS];
 #ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
-  struct note_filter_mode_s mode;
+  struct note_filter_mode_s    mode;
 #endif
 #ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
-  struct note_filter_irq_s  irq_mask;
+  struct note_filter_irq_s     irq_mask;
+  unsigned int                 irq_nest[CONFIG_SMP_NCPUS];
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+  struct note_filter_syscall_s syscall_mask;
+  struct note_filter_syscall_s syscall_active;

Review comment:
       Done




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

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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5689: segger/sysview: add syscall support

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5689:
URL: https://github.com/apache/incubator-nuttx/pull/5689#discussion_r822242616



##########
File path: drivers/segger/note_sysview.c
##########
@@ -310,6 +366,54 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_syscall_enter(int nr, int argc, ...)
+{
+  if (sysview_isenabled_syscall(nr) == 0)

Review comment:
       let's sub nr by CONFIG_SYS_RESERVED at the beginning to ensure all place use the correct value.

##########
File path: boards/sim/sim/sim/configs/segger/defconfig
##########
@@ -62,6 +62,7 @@ CONFIG_SCHED_HAVE_PARENT=y
 CONFIG_SCHED_HPWORK=y
 CONFIG_SCHED_INSTRUMENTATION=y
 CONFIG_SCHED_INSTRUMENTATION_FILTER=y
+CONFIG_SCHED_INSTRUMENTATION_SYSCALL=y

Review comment:
       let's modify https://github.com/apache/incubator-nuttx/blob/master/tools/ci/testlist/sim-02.dat to remove segger from macOS build.

##########
File path: drivers/segger/note_sysview.c
##########
@@ -155,11 +159,24 @@ static bool sysview_isenabled(void)
     }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+  /* Use the Syscall "0" to identify whether the syscall is enabled,

Review comment:
       but how to handle if nobody call syscall#0?

##########
File path: drivers/segger/note_sysview.c
##########
@@ -310,6 +366,54 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_syscall_enter(int nr, int argc, ...)
+{
+  if (sysview_isenabled_syscall(nr) == 0)
+    {
+      return;
+    }
+
+  /* Set the name marker if the current syscall nr is not active */
+
+  if (NOTE_FILTER_SYSCALLMASK_ISSET(nr, &g_sysview.syscall_marker) == 0)
+    {
+      /* Set the name marker */
+
+      SEGGER_SYSVIEW_NameMarker(nr, g_funcnames[nr]);
+
+      /* Mark the syscall active */
+
+      NOTE_FILTER_SYSCALLMASK_SET(nr, &g_sysview.syscall_marker);
+
+      /* Use the Syscall "0" to identify whether the syscall is enabled,
+       * if the host tool is closed abnormally, use this bit to clear
+       * the active set.
+       */
+
+      if (NOTE_FILTER_SYSCALLMASK_ISSET(0, &g_sysview.syscall_marker) == 0)
+        {
+          NOTE_FILTER_SYSCALLMASK_SET(0, &g_sysview.syscall_marker);
+        }
+    }
+
+  SEGGER_SYSVIEW_MarkStart(nr);
+}
+
+void sched_note_syscall_leave(int nr, uintptr_t result)
+{
+  if (sysview_isenabled_syscall(nr) == 0)

Review comment:
       ditto for nr




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

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