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 17:54:37 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5689: segger/sysview: add syscall support
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