xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r497448048
##########
File path: sched/sched/sched_note.c
##########
@@ -528,16 +646,12 @@ void sched_note_syscall_enter(int nr, int argc, ...)
struct note_syscall_enter_s note;
FAR struct tcb_s *tcb = this_task();
-#ifdef CONFIG_SMP
- /* Ignore notes that are not in the set of monitored CPUs */
+ nr -= CONFIG_SYS_RESERVED;
Review comment:
let move into note_isenabled_syscall?
##########
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:
why not record the syscall inside the interrupt handing?
##########
File path: drivers/note/noteram_driver.c
##########
@@ -326,6 +396,24 @@ static ssize_t noteram_size(void)
return notelen;
}
+/****************************************************************************
+ * Name: noteram_open
+ ****************************************************************************/
+
+static int noteram_open(FAR struct file *filep)
+{
+ irqstate_t flags;
+
+ flags = enter_critical_section();
Review comment:
don't need?
##########
File path: include/nuttx/sched.h
##########
@@ -748,6 +748,10 @@ struct tcb_s
#if CONFIG_TASK_NAME_SIZE > 0
char name[CONFIG_TASK_NAME_SIZE + 1]; /* Task name (with NUL terminator)
*/
#endif
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+ int syscall_nest; /* Syscall nest level */
Review comment:
I am still thinking that it's very useful to trace the nested syscall
because it could show the real sequence. Could you explain what's wrong with
tracecompass in this case. BTW, we can filter out the nested syscall if
tracecompass can't handle it correctly.
##########
File path: sched/Kconfig
##########
@@ -1000,6 +1000,26 @@ config SCHED_INSTRUMENTATION_FILTER
The filter logic can be configured by sched_note_filter APIs
defined in
include/nuttx/sched_note.h.
+config SCHED_INSTRUMENTATION_FILTER_ENABLE_ONBOOT
Review comment:
why need an additional config? it's simple to directly add
SCHED_INSTRUMENTATION_FILTER_MODE and
SCHED_INSTRUMENTATION_FILTER_ENABLE_ONBOOT_NOOVERWRITE.
##########
File path: drivers/note/noteram_driver.c
##########
@@ -435,6 +586,17 @@ void sched_note_add(FAR const void *note, size_t notelen)
next = noteram_next(head, 1);
if (next == g_noteram_info.ni_tail)
{
+ if (!g_noteram_info.ni_overwrite)
+ {
+ /* Stop recording if not in overwrite mode */
+
+#ifdef CONFIG_SMP
+ spin_unlock_wo_note(&g_noteram_lock);
+ up_irq_restore(flags);
+#endif
+ return;
Review comment:
should we break here to otherwise we will forget to update ni_head at
line 451?
##########
File path: sched/sched/sched_note.c
##########
@@ -556,16 +670,12 @@ void sched_note_syscall_leave(int nr, uintptr_t result)
struct note_syscall_leave_s note;
FAR struct tcb_s *tcb = this_task();
-#ifdef CONFIG_SMP
- /* Ignore notes that are not in the set of monitored CPUs */
+ nr -= CONFIG_SYS_RESERVED;
Review comment:
let's move into note_isenabled_syscall?
##########
File path: sched/Kconfig
##########
@@ -1000,6 +1000,26 @@ config SCHED_INSTRUMENTATION_FILTER
The filter logic can be configured by sched_note_filter APIs
defined in
include/nuttx/sched_note.h.
+config SCHED_INSTRUMENTATION_FILTER_ENABLE_ONBOOT
+ bool "Instrumenation filter enable on boot"
+ default n
+ ---help---
+ Enables the on-boot instrumentation when the filter logic is
available.
+
+config SCHED_INSTRUMENTATION_FILTER_ENABLE_ONBOOT_NOOVERWRITE
Review comment:
move to drivers/note/Kconfig
----------------------------------------------------------------
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
