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 2020/07/07 05:18:41 UTC
[GitHub] [incubator-nuttx] YuuichiNakamura opened a new pull request #1377: Add task trace support
YuuichiNakamura opened a new pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377
## Summary
This PR adds the following features for task trace.
- note filter support (enabled by CONFIG_SCHED_INSTRUMENTATION_FILTER)
- /dev/note ioctls to control task trace functions
- split note buffer management from sched_note.c into sched_note_buffer.c and use streams interface for it
- add note buffer clear/rewind functions used by task trace
The task trace is available by using feature/task-tracer branch in the following repository.
https://github.com/YuuichiNakamura/incubator-nuttx-apps.git
(I'll send PR for apps after merging this PR)
## Impact
- New kernel configurations
CONFIG_SCHED_INSTRUMENTATION_FILTER,
CONFIG_SCHED_NOTE_TASKNAME_BUFSIZE are added
- /dev/note function is changed.
(It will be same as the original features if CONFIG_SCHED_INSTRUMENTATION_FILTER is not defined)
## Testing
Tested by using feature/task-tracer branch in the following repository.
https://github.com/YuuichiNakamura/incubator-nuttx-apps.git
Tested by spresense:nsh and maix-bit:nsh by changing the following configurations.
CONFIG_DRIVER_NOTE=y
CONFIG_SCHED_INSTRUMENTATION=y
CONFIG_SCHED_INSTRUMENTATION_BUFFER=y
CONFIG_SCHED_INSTRUMENTATION_FILTER=y
CONFIG_SCHED_INSTRUMENTATION_SYSCALL=y
CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER=y
CONFIG_SCHED_NOTE_GET=y
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r490679202
##########
File path: include/nuttx/sched_note.h
##########
@@ -254,10 +266,78 @@ struct note_irqhandler_s
};
#endif /* CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER */
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
+/* This is the type of the argument passed to the TRIOC_GETMODE and
+ * TRIOC_SETMODE ioctls
+ */
+
+struct note_trace_mode_s
+{
+ bool enable; /* Trace enable */
+ unsigned int flag; /* Trace mode flag */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+#define TRIOC_MODE_FLAG_SYSCALL (1 << 0) /* Enable syscall trace */
+#define TRIOC_MODE_FLAG_SYSCALL_ARGS (1 << 1) /* Enable collecting syscall arguments */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#define TRIOC_MODE_FLAG_IRQ (1 << 2) /* Enable IRQ trace */
+#endif
Review comment:
should we move the macro defintion to the predefine section?
##########
File path: include/nuttx/sched_note.h
##########
@@ -254,10 +266,78 @@ struct note_irqhandler_s
};
#endif /* CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER */
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
+/* This is the type of the argument passed to the TRIOC_GETMODE and
+ * TRIOC_SETMODE ioctls
+ */
+
+struct note_trace_mode_s
Review comment:
should we change all note_trace_ to note_filter_?
##########
File path: include/nuttx/sched_note.h
##########
@@ -254,10 +266,78 @@ struct note_irqhandler_s
};
#endif /* CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER */
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
+/* This is the type of the argument passed to the TRIOC_GETMODE and
+ * TRIOC_SETMODE ioctls
+ */
+
+struct note_trace_mode_s
+{
+ bool enable; /* Trace enable */
+ unsigned int flag; /* Trace mode flag */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+#define TRIOC_MODE_FLAG_SYSCALL (1 << 0) /* Enable syscall trace */
+#define TRIOC_MODE_FLAG_SYSCALL_ARGS (1 << 1) /* Enable collecting syscall arguments */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#define TRIOC_MODE_FLAG_IRQ (1 << 2) /* Enable IRQ trace */
+#endif
+};
+
+/* This is the type of the argument passed to the TRIOC_GETSYSCALLFILTER and
+ * TRIOC_SETSYSCALLFILTER ioctls
+ */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+struct note_trace_syscallfilter_s
+{
+ uint8_t syscall_mask[(SYS_nsyscalls + 7) / 8];
+};
+
+/* Helper macros */
+
+#define SYSCALLMASK_SET(nr, s) \
Review comment:
SYSCALLMASK_ to NOTE_FILTER_SYSCALLMASK_ ?
##########
File path: include/nuttx/sched_note.h
##########
@@ -254,10 +266,78 @@ struct note_irqhandler_s
};
#endif /* CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER */
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
+/* This is the type of the argument passed to the TRIOC_GETMODE and
+ * TRIOC_SETMODE ioctls
+ */
+
+struct note_trace_mode_s
+{
+ bool enable; /* Trace enable */
+ unsigned int flag; /* Trace mode flag */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+#define TRIOC_MODE_FLAG_SYSCALL (1 << 0) /* Enable syscall trace */
+#define TRIOC_MODE_FLAG_SYSCALL_ARGS (1 << 1) /* Enable collecting syscall arguments */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#define TRIOC_MODE_FLAG_IRQ (1 << 2) /* Enable IRQ trace */
+#endif
+};
+
+/* This is the type of the argument passed to the TRIOC_GETSYSCALLFILTER and
+ * TRIOC_SETSYSCALLFILTER ioctls
+ */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+struct note_trace_syscallfilter_s
+{
+ uint8_t syscall_mask[(SYS_nsyscalls + 7) / 8];
+};
+
+/* Helper macros */
+
+#define SYSCALLMASK_SET(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] |= (1 << ((nr) % 8)))
+#define SYSCALLMASK_CLR(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] &= ~(1 << ((nr) % 8)))
+#define SYSCALLMASK_ISSET(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] & (1 << ((nr) % 8)))
+#define SYSCALLMASK_ZERO(s) \
+ memset((s), 0, sizeof(struct note_trace_syscallfilter_s))
+#endif
+
+/* This is the type of the argument passed to the TRIOC_GETIRQFILTER and
+ * TRIOC_SETIRQFILTER ioctls
+ */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+struct note_trace_irqfilter_s
+{
+ uint8_t irq_mask[(NR_IRQS + 7) / 8];
+};
+
+/* Helper macros */
+
+#define IRQMASK_SET(nr, s) \
Review comment:
IRQMASK_ to NOTE_FILTER_IRQMASK_?
##########
File path: sched/sched/sched_note.c
##########
@@ -587,13 +652,25 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
struct note_irqhandler_s note;
FAR struct tcb_s *tcb = this_task();
-#ifdef CONFIG_SMP
- /* Ignore notes that are not in the set of monitored CPUs */
-
- if ((CONFIG_SCHED_INSTRUMENTATION_CPUSET & (1 << this_cpu())) == 0)
+ if (!note_isenabled())
{
- /* Not in the set of monitored CPUs. Do not log the note. */
+ return;
+ }
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ /* If the IRQ trace is disabled or the IRQ number is masked,
+ * do nothing.
+ */
+
+ if (g_note_filter.mode.flag & TRIOC_MODE_FLAG_IRQ)
Review comment:
should we add a new function note_isenabled_irq which call note_isenabled and check IRQMASK_ISSET?
##########
File path: drivers/note/noteram_driver.c
##########
@@ -57,28 +58,33 @@ struct noteram_info_s
{
volatile unsigned int ni_head;
volatile unsigned int ni_tail;
+ volatile unsigned int ni_read;
Review comment:
why we need ni_tail and ni_read two location?
##########
File path: include/nuttx/sched_note.h
##########
@@ -254,10 +266,78 @@ struct note_irqhandler_s
};
#endif /* CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER */
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
+/* This is the type of the argument passed to the TRIOC_GETMODE and
+ * TRIOC_SETMODE ioctls
+ */
+
+struct note_trace_mode_s
+{
+ bool enable; /* Trace enable */
+ unsigned int flag; /* Trace mode flag */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+#define TRIOC_MODE_FLAG_SYSCALL (1 << 0) /* Enable syscall trace */
Review comment:
TRIOC_ to NOTE_FILTER_?
##########
File path: include/nuttx/sched_note.h
##########
@@ -254,10 +266,78 @@ struct note_irqhandler_s
};
#endif /* CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER */
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
+/* This is the type of the argument passed to the TRIOC_GETMODE and
+ * TRIOC_SETMODE ioctls
+ */
+
+struct note_trace_mode_s
+{
+ bool enable; /* Trace enable */
+ unsigned int flag; /* Trace mode flag */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+#define TRIOC_MODE_FLAG_SYSCALL (1 << 0) /* Enable syscall trace */
+#define TRIOC_MODE_FLAG_SYSCALL_ARGS (1 << 1) /* Enable collecting syscall arguments */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#define TRIOC_MODE_FLAG_IRQ (1 << 2) /* Enable IRQ trace */
+#endif
+};
+
+/* This is the type of the argument passed to the TRIOC_GETSYSCALLFILTER and
+ * TRIOC_SETSYSCALLFILTER ioctls
+ */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+struct note_trace_syscallfilter_s
Review comment:
note_filter_syscall_s?
##########
File path: include/nuttx/sched_note.h
##########
@@ -254,10 +266,78 @@ struct note_irqhandler_s
};
#endif /* CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER */
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
+/* This is the type of the argument passed to the TRIOC_GETMODE and
+ * TRIOC_SETMODE ioctls
+ */
+
+struct note_trace_mode_s
+{
+ bool enable; /* Trace enable */
+ unsigned int flag; /* Trace mode flag */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+#define TRIOC_MODE_FLAG_SYSCALL (1 << 0) /* Enable syscall trace */
+#define TRIOC_MODE_FLAG_SYSCALL_ARGS (1 << 1) /* Enable collecting syscall arguments */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#define TRIOC_MODE_FLAG_IRQ (1 << 2) /* Enable IRQ trace */
+#endif
+};
+
+/* This is the type of the argument passed to the TRIOC_GETSYSCALLFILTER and
+ * TRIOC_SETSYSCALLFILTER ioctls
+ */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+struct note_trace_syscallfilter_s
+{
+ uint8_t syscall_mask[(SYS_nsyscalls + 7) / 8];
+};
+
+/* Helper macros */
+
+#define SYSCALLMASK_SET(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] |= (1 << ((nr) % 8)))
+#define SYSCALLMASK_CLR(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] &= ~(1 << ((nr) % 8)))
+#define SYSCALLMASK_ISSET(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] & (1 << ((nr) % 8)))
+#define SYSCALLMASK_ZERO(s) \
+ memset((s), 0, sizeof(struct note_trace_syscallfilter_s))
+#endif
Review comment:
move to predefined section?
##########
File path: include/nuttx/sched_note.h
##########
@@ -254,10 +266,78 @@ struct note_irqhandler_s
};
#endif /* CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER */
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
+/* This is the type of the argument passed to the TRIOC_GETMODE and
+ * TRIOC_SETMODE ioctls
+ */
+
+struct note_trace_mode_s
+{
+ bool enable; /* Trace enable */
+ unsigned int flag; /* Trace mode flag */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+#define TRIOC_MODE_FLAG_SYSCALL (1 << 0) /* Enable syscall trace */
+#define TRIOC_MODE_FLAG_SYSCALL_ARGS (1 << 1) /* Enable collecting syscall arguments */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#define TRIOC_MODE_FLAG_IRQ (1 << 2) /* Enable IRQ trace */
+#endif
+};
+
+/* This is the type of the argument passed to the TRIOC_GETSYSCALLFILTER and
+ * TRIOC_SETSYSCALLFILTER ioctls
+ */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+struct note_trace_syscallfilter_s
+{
+ uint8_t syscall_mask[(SYS_nsyscalls + 7) / 8];
+};
+
+/* Helper macros */
+
+#define SYSCALLMASK_SET(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] |= (1 << ((nr) % 8)))
+#define SYSCALLMASK_CLR(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] &= ~(1 << ((nr) % 8)))
+#define SYSCALLMASK_ISSET(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] & (1 << ((nr) % 8)))
+#define SYSCALLMASK_ZERO(s) \
+ memset((s), 0, sizeof(struct note_trace_syscallfilter_s))
+#endif
+
+/* This is the type of the argument passed to the TRIOC_GETIRQFILTER and
+ * TRIOC_SETIRQFILTER ioctls
+ */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+struct note_trace_irqfilter_s
Review comment:
note_filter_irq_s?
##########
File path: include/nuttx/sched_note.h
##########
@@ -357,6 +437,125 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
void sched_note_add(FAR const void *note, size_t notelen);
+/****************************************************************************
+ * Name: sched_note_trace_start
+ *
+ * Description:
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_start(void);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_stop(void);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
+ struct note_trace_mode_s *newm);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_syscallfilter
+ *
+ * Description:
+ * Set and get syscall trace filter setting
+ * (Same as TRIOC_GETSYSCALLFILTER / TRIOC_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_syscallfilter_s to get
+ * current syscall trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_syscallfilter_s of the
+ * new syscall trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
Review comment:
change to?
```
#if defined(CONFIG_SCHED_INSTRUMENTATION_FILTER) && defined(CONFIG_SCHED_INSTRUMENTATION_SYSCALL)
```
##########
File path: sched/Kconfig
##########
@@ -990,6 +990,14 @@ config SCHED_INSTRUMENTATION_IRQHANDLER
void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
+config SCHED_INSTRUMENTATION_FILTER
+ bool "Instrumenation filter"
+ default n
+ ---help---
+ Enables the filter logic of the instrumentation data. If this option
+ is enabled, the instrumentation is disabled by default and must be
+ enabled explicitly by filter APIs.
Review comment:
should we reword the comment?
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +123,43 @@ 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;
+ }
+#endif
+
+#ifdef CONFIG_SMP
+ /* Ignore notes that are not in the set of monitored CPUs */
+
+ if ((CONFIG_SCHED_INSTRUMENTATION_CPUSET & (1 << this_cpu())) == 0)
Review comment:
should we add cpuset to note_trace_mode_s like other filter?
##########
File path: include/nuttx/sched_note.h
##########
@@ -357,6 +437,125 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
void sched_note_add(FAR const void *note, size_t notelen);
+/****************************************************************************
+ * Name: sched_note_trace_start
+ *
+ * Description:
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_start(void);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_stop(void);
Review comment:
why not reuse sched_note_trace_mode to save two api?
##########
File path: include/nuttx/sched_note.h
##########
@@ -357,6 +437,125 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
void sched_note_add(FAR const void *note, size_t notelen);
+/****************************************************************************
+ * Name: sched_note_trace_start
+ *
+ * Description:
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_start(void);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_stop(void);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
+ struct note_trace_mode_s *newm);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_syscallfilter
+ *
+ * Description:
+ * Set and get syscall trace filter setting
+ * (Same as TRIOC_GETSYSCALLFILTER / TRIOC_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_syscallfilter_s to get
+ * current syscall trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_syscallfilter_s of the
+ * new syscall trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_trace_syscallfilter(struct note_trace_syscallfilter_s *oldf,
Review comment:
sched_note_filter_syscall?
##########
File path: sched/sched/sched_note.c
##########
@@ -25,10 +25,12 @@
#include <nuttx/config.h>
#include <stdint.h>
+#include <stdbool.h>
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:
The same info expose inside arch's xcptcontext:
```
#ifdef CONFIG_LIB_SYSCALL
/* The following array holds the return address and the exc_return value
* needed to return from each nested system call.
*/
uint8_t nsyscalls;
struct xcpt_syscall_s syscall[CONFIG_SYS_NNEST];
#endif
```
But I don't have a good idea how to reuse this info.
##########
File path: include/nuttx/sched_note.h
##########
@@ -357,6 +437,125 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
void sched_note_add(FAR const void *note, size_t notelen);
+/****************************************************************************
+ * Name: sched_note_trace_start
+ *
+ * Description:
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_start(void);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_stop(void);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
+ struct note_trace_mode_s *newm);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_syscallfilter
+ *
+ * Description:
+ * Set and get syscall trace filter setting
+ * (Same as TRIOC_GETSYSCALLFILTER / TRIOC_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_syscallfilter_s to get
+ * current syscall trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_syscallfilter_s of the
+ * new syscall trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_trace_syscallfilter(struct note_trace_syscallfilter_s *oldf,
+ struct note_trace_syscallfilter_s *newf);
+#endif
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_irqfilter
+ *
+ * Description:
+ * Set and get IRQ trace filter setting
+ * (Same as TRIOC_GETIRQFILTER / TRIOC_SETIRQFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_irqfilter_s to get
+ * current IRQ trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_irqfilter_s of the new
+ * IRQ trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+void sched_note_trace_irqfilter(struct note_trace_irqfilter_s *oldf,
Review comment:
sched_note_filter_irq?
##########
File path: sched/sched/sched_note.c
##########
@@ -556,15 +578,58 @@ 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 */
+ if (!note_isenabled())
+ {
+ return;
+ }
- if ((CONFIG_SCHED_INSTRUMENTATION_CPUSET & (1 << this_cpu())) == 0)
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ irqstate_t irq_mask;
+
+ irq_mask = enter_critical_section();
Review comment:
should we add a new function note_isenabled_syscall which call note_isenabled and check SYSCALLMASK_ISSET?
##########
File path: include/nuttx/sched_note.h
##########
@@ -357,6 +437,125 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
void sched_note_add(FAR const void *note, size_t notelen);
+/****************************************************************************
+ * Name: sched_note_trace_start
+ *
+ * Description:
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_start(void);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_stop(void);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
+ struct note_trace_mode_s *newm);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_syscallfilter
+ *
+ * Description:
+ * Set and get syscall trace filter setting
+ * (Same as TRIOC_GETSYSCALLFILTER / TRIOC_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_syscallfilter_s to get
+ * current syscall trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_syscallfilter_s of the
+ * new syscall trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_trace_syscallfilter(struct note_trace_syscallfilter_s *oldf,
+ struct note_trace_syscallfilter_s *newf);
+#endif
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_irqfilter
+ *
+ * Description:
+ * Set and get IRQ trace filter setting
+ * (Same as TRIOC_GETIRQFILTER / TRIOC_SETIRQFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_irqfilter_s to get
+ * current IRQ trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_irqfilter_s of the new
+ * IRQ trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
Review comment:
change to?
```
#if defined(CONFIG_SCHED_INSTRUMENTATION_FILTER) && defined(CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER)
```
##########
File path: include/nuttx/note/notectl_driver.h
##########
@@ -0,0 +1,128 @@
+/****************************************************************************
+ * include/nuttx/note/notectl_driver.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_NOTE_NOTECTL_DRIVER_H
+#define __INCLUDE_NUTTX_NOTE_NOTECTL_DRIVER_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <stdint.h>
+
+#include <nuttx/sched_note.h>
+#include <nuttx/fs/ioctl.h>
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* IOCTL Commands ***********************************************************/
+
+/* TRIOC_START
+ * - Start task tracing
+ * Argument: Ignored
+ * TRIOC_STOP
+ * - Stop task tracing
+ * Argument: Ignored
+ * TRIOC_GETMODE
+ * - Get task trace mode
+ * Argument: A writable pointer to struct note_trace_mode_s
+ * TRIOC_SETMODE
+ * - Set task trace mode
+ * Argument: A read-only pointer to struct note_trace_mode_s
+ * TRIOC_GETSYSCALLFILTER
+ * - Get syscall trace filter setting
+ * Argument: A writable pointer to struct
+ * note_trace_syscallfilter_s
+ * TRIOC_SETSYSCALLFILTER
+ * - Set syscall trace filter setting
+ * Argument: A read-only pointer to struct
+ * note_trace_syscallfilter_s
+ * TRIOC_GETIRQFILTER
+ * - Get IRQ trace filter setting
+ * Argument: A writable pointer to struct
+ * note_trace_irqfilter_s
+ * TRIOC_SETIRQFILTER
+ * - Set IRQ trace filter setting
+ * Argument: A read-only pointer to struct
+ * note_trace_irqfilter_s
+ */
+
+#ifdef CONFIG_DRIVER_NOTECTL
+
+#define _NOTECTLBASE (0xfe00)
+#define _NOTECTLIOCVALID(c) (_IOC_TYPE(c) == _NOTECTLBASE)
+#define _NOTECTLIOC(nr) _IOC(_NOTECTLBASE, nr)
Review comment:
should we move to nuttx/fs/ioctl.h like other driver?
##########
File path: drivers/note/noteram_driver.c
##########
@@ -473,5 +644,7 @@ void sched_note_add(FAR const void *note, size_t notelen)
int noteram_register(void)
{
+ noteram_buffer_clear();
Review comment:
let's keep the trace already save.
##########
File path: include/nuttx/sched_note.h
##########
@@ -254,10 +266,78 @@ struct note_irqhandler_s
};
#endif /* CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER */
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
+/* This is the type of the argument passed to the TRIOC_GETMODE and
+ * TRIOC_SETMODE ioctls
+ */
+
+struct note_trace_mode_s
+{
+ bool enable; /* Trace enable */
+ unsigned int flag; /* Trace mode flag */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+#define TRIOC_MODE_FLAG_SYSCALL (1 << 0) /* Enable syscall trace */
+#define TRIOC_MODE_FLAG_SYSCALL_ARGS (1 << 1) /* Enable collecting syscall arguments */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#define TRIOC_MODE_FLAG_IRQ (1 << 2) /* Enable IRQ trace */
+#endif
+};
+
+/* This is the type of the argument passed to the TRIOC_GETSYSCALLFILTER and
+ * TRIOC_SETSYSCALLFILTER ioctls
+ */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+struct note_trace_syscallfilter_s
+{
+ uint8_t syscall_mask[(SYS_nsyscalls + 7) / 8];
+};
+
+/* Helper macros */
+
+#define SYSCALLMASK_SET(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] |= (1 << ((nr) % 8)))
+#define SYSCALLMASK_CLR(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] &= ~(1 << ((nr) % 8)))
+#define SYSCALLMASK_ISSET(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] & (1 << ((nr) % 8)))
+#define SYSCALLMASK_ZERO(s) \
+ memset((s), 0, sizeof(struct note_trace_syscallfilter_s))
+#endif
+
+/* This is the type of the argument passed to the TRIOC_GETIRQFILTER and
+ * TRIOC_SETIRQFILTER ioctls
+ */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+struct note_trace_irqfilter_s
+{
+ uint8_t irq_mask[(NR_IRQS + 7) / 8];
+};
+
+/* Helper macros */
+
+#define IRQMASK_SET(nr, s) \
+ ((s)->irq_mask[(nr) / 8] |= (1 << ((nr) % 8)))
+#define IRQMASK_CLR(nr, s) \
+ ((s)->irq_mask[(nr) / 8] &= ~(1 << ((nr) % 8)))
+#define IRQMASK_ISSET(nr, s) \
+ ((s)->irq_mask[(nr) / 8] & (1 << ((nr) % 8)))
+#define IRQMASK_ZERO(s) \
+ memset((s), 0, sizeof(struct note_trace_irqfilter_s))
+#endif
+
+#endif /* CONFIG_SCHED_INSTRUMENTATION_FILTER */
+
/****************************************************************************
* Public Function Prototypes
****************************************************************************/
+#if defined(__KERNEL__) || defined(CONFIG_BUILD_FLAT)
Review comment:
don't need?
##########
File path: drivers/note/notectl_driver.c
##########
@@ -263,5 +263,13 @@ static int notectl_ioctl(struct file *filep, int cmd, unsigned long arg)
int notectl_register(void)
{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER_ENABLE_ONBOOT
+ struct note_trace_mode_s mode = {
+ true, CONFIG_SCHED_INSTRUMENTATION_FILTER_ENABLE_ONBOOT_MODE
Review comment:
still too later, need to initialize g_note_filter directly.
##########
File path: drivers/note/noteram_driver.c
##########
@@ -419,6 +569,15 @@ void sched_note_add(FAR const void *note, size_t notelen)
spin_lock_wo_note(&g_noteram_lock);
#endif
+ if (!g_noteram_info.ni_enable)
Review comment:
can we change to no overwrite semantics instead of oneshot? so we need have enable/disable status.
##########
File path: include/nuttx/sched_note.h
##########
@@ -357,6 +437,125 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
void sched_note_add(FAR const void *note, size_t notelen);
+/****************************************************************************
+ * Name: sched_note_trace_start
+ *
+ * Description:
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_start(void);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_stop(void);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
Review comment:
sched_note_filter_mode?
##########
File path: drivers/note/noteram_driver.c
##########
@@ -326,6 +395,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();
+
+ /* Reset the read index of the circular buffer */
+
+ g_noteram_info.ni_read = g_noteram_info.ni_tail;
Review comment:
why do this? we lose the initial trace info which is critical to debuging the boot issue.
##########
File path: include/nuttx/note/notectl_driver.h
##########
@@ -0,0 +1,128 @@
+/****************************************************************************
+ * include/nuttx/note/notectl_driver.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_NOTE_NOTECTL_DRIVER_H
+#define __INCLUDE_NUTTX_NOTE_NOTECTL_DRIVER_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <stdint.h>
+
+#include <nuttx/sched_note.h>
+#include <nuttx/fs/ioctl.h>
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* IOCTL Commands ***********************************************************/
+
+/* TRIOC_START
+ * - Start task tracing
+ * Argument: Ignored
+ * TRIOC_STOP
+ * - Stop task tracing
+ * Argument: Ignored
+ * TRIOC_GETMODE
+ * - Get task trace mode
+ * Argument: A writable pointer to struct note_trace_mode_s
+ * TRIOC_SETMODE
+ * - Set task trace mode
+ * Argument: A read-only pointer to struct note_trace_mode_s
+ * TRIOC_GETSYSCALLFILTER
+ * - Get syscall trace filter setting
+ * Argument: A writable pointer to struct
+ * note_trace_syscallfilter_s
+ * TRIOC_SETSYSCALLFILTER
+ * - Set syscall trace filter setting
+ * Argument: A read-only pointer to struct
+ * note_trace_syscallfilter_s
+ * TRIOC_GETIRQFILTER
+ * - Get IRQ trace filter setting
+ * Argument: A writable pointer to struct
+ * note_trace_irqfilter_s
+ * TRIOC_SETIRQFILTER
+ * - Set IRQ trace filter setting
+ * Argument: A read-only pointer to struct
+ * note_trace_irqfilter_s
+ */
+
+#ifdef CONFIG_DRIVER_NOTECTL
+
+#define _NOTECTLBASE (0xfe00)
+#define _NOTECTLIOCVALID(c) (_IOC_TYPE(c) == _NOTECTLBASE)
+#define _NOTECTLIOC(nr) _IOC(_NOTECTLBASE, nr)
+
+#define TRIOC_START _NOTECTLIOC(0x01)
Review comment:
change to TRIOC_ to NOTECTL_?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura closed pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura closed pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-703401747
@xiaoxiang781216 I have updated the commend and made additional fix.
> @YuuichiNakamura this PR is ready to merge now, how about you change the PR base on the master branch? So we can merge to the master directly.
Then may I push the same patches on the master branch and close this PR?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r452558599
##########
File path: drivers/syslog/note_driver.c
##########
@@ -41,34 +41,62 @@
#include <sys/types.h>
#include <sched.h>
+#include <fcntl.h>
#include <assert.h>
#include <errno.h>
+#include <string.h>
#include <nuttx/sched_note.h>
+#include <nuttx/sched_note_trace.h>
Review comment:
I see. I got it.
##########
File path: include/nuttx/sched_note.h
##########
@@ -348,12 +379,13 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
# define sched_note_irqhandler(i,h,e)
#endif
+void sched_note_message(FAR const char *fmt, ...);
Review comment:
I got. I'll remove sched_note_message features to make another PR for it.
Also ok for the changes of syscall arg/result and irq handler.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-703443253
> @xiaoxiang781216 I have updated the commend and made additional fix.
>
> > @YuuichiNakamura this PR is ready to merge now, how about you change the PR base on the master branch? So we can merge to the master directly.
>
> Then may I push the same patches on the master branch and close this PR?
You can try to rebase your local branch to master and push the change again. If it work, we can reuse this PR, otherwise you have to create a new one.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r495022623
##########
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:
Unfortunately tcb->xcpt.nsyscalls is not fit for my expectation.
It can be used only in PROTECTED and KERNEL build and it counts the nesting level of user mode -> kernel mode transition.
Because my sched_note_syscall_enter/leave implementation hooks the function call in the kernel space directly, the required count is the nesting level of syscall function calls and it also needs in FLAT build too.
("syscall function" means the kernel APIs listed in syscall/syscall.csv)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r485284736
##########
File path: drivers/note/note_driver.c
##########
@@ -384,6 +472,69 @@ static ssize_t note_read(FAR struct file *filep, FAR char *buffer,
return retlen;
}
+/****************************************************************************
+ * Name: note_ioctl
+ ****************************************************************************/
+
+static int note_ioctl(struct file *filep, int cmd, unsigned long arg)
Review comment:
I have reviewed #1731.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r498626169
##########
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:
Yes, this is what I suggest: we can do the filter in the userspace or even on PC before we feed the trace to tracecompass.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-692544687
I will finish review in this week, thanks for patient. Your tools is very critical for performance analysis. Once the kernel side is done, we can start the appilication side change. Also could you integrate your document into the new document system? it will be helpful for people setup the environment.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465486090
##########
File path: sched/sched/sched_note.c
##########
@@ -370,6 +437,10 @@ void sched_note_start(FAR struct tcb_s *tcb)
strncpy(note.nsa_name, tcb->name, CONFIG_TASK_NAME_SIZE + 1);
length = SIZEOF_NOTE_START(namelen + 1);
+
+#ifdef NOTE_FEATURE_TASK_NAME
+ note_taskname_put(tcb->pid, tcb->name);
Review comment:
Different to the existing sched_note feature, in task trace support, the period to collect the note (trace) data and to dump the trace result are completely separated.
So, when dumping the result, the task recorded in the trace data may not exist and its task name string may already freed.
It is the reason why task name and its pid should be saved.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r493668548
##########
File path: drivers/note/noteram_driver.c
##########
@@ -57,28 +58,33 @@ struct noteram_info_s
{
volatile unsigned int ni_head;
volatile unsigned int ni_tail;
+ volatile unsigned int ni_read;
Review comment:
Same as https://github.com/apache/incubator-nuttx/pull/1377#discussion_r493667049
I have wanted to introduce non-destructive read to /dev/note.
It is useful for example no overwrite mode in on-boot instrumentation. After boot, the instrumentation data while boot is kept in the circular buffer and never lost until new instrumentation is started even when we read the data.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499395012
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +137,162 @@ 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.flag & NOTE_FILTER_MODE_FLAG_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
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int note_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* Exclude the case of syscall called by the interrupt handler which is
+ * not traced.
+ */
+
+ if (up_interrupt_context())
+ {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ if (g_note_disabled_irq_nest[cpu] > 0)
+ {
+ return false;
+ }
+#else
+ return false;
+#endif
+ }
+
+ /* If the syscall trace is disabled or the syscall number is masked,
+ * do nothing.
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) ||
+ NOTE_FILTER_SYSCALLMASK_ISSET(nr - CONFIG_SYS_RESERVED,
+ &g_note_filter.syscall_mask))
+ {
+ return false;
+ }
+#endif
+
+ return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: note_isenabled_irqhandler
+ *
+ * Description:
+ * Check whether the interrupt handler instrumentation is enabled.
+ *
+ * Input Parameters:
+ * irq - IRQ number
+ * enter - interrupt enter/leave flag
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+static inline int note_isenabled_irq(int irq, bool enter)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* If the IRQ trace is disabled or the IRQ number is masked, disable
+ * subsequent syscall traces until leaving the interrupt handler
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) ||
+ NOTE_FILTER_IRQMASK_ISSET(irq, &g_note_filter.irq_mask))
+ {
+ irqstate_t irq_mask;
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ irq_mask = enter_critical_section();
Review comment:
But the initial irq is in disable state when cpu jump to the irq handler, onlye the code provided by irq_attach can have the chance to re-enable irq.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r493666398
##########
File path: include/nuttx/sched_note.h
##########
@@ -254,10 +266,78 @@ struct note_irqhandler_s
};
#endif /* CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER */
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
+/* This is the type of the argument passed to the TRIOC_GETMODE and
+ * TRIOC_SETMODE ioctls
+ */
+
+struct note_trace_mode_s
+{
+ bool enable; /* Trace enable */
+ unsigned int flag; /* Trace mode flag */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+#define TRIOC_MODE_FLAG_SYSCALL (1 << 0) /* Enable syscall trace */
+#define TRIOC_MODE_FLAG_SYSCALL_ARGS (1 << 1) /* Enable collecting syscall arguments */
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#define TRIOC_MODE_FLAG_IRQ (1 << 2) /* Enable IRQ trace */
+#endif
+};
+
+/* This is the type of the argument passed to the TRIOC_GETSYSCALLFILTER and
+ * TRIOC_SETSYSCALLFILTER ioctls
+ */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+struct note_trace_syscallfilter_s
+{
+ uint8_t syscall_mask[(SYS_nsyscalls + 7) / 8];
+};
+
+/* Helper macros */
+
+#define SYSCALLMASK_SET(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] |= (1 << ((nr) % 8)))
+#define SYSCALLMASK_CLR(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] &= ~(1 << ((nr) % 8)))
+#define SYSCALLMASK_ISSET(nr, s) \
+ ((s)->syscall_mask[(nr) / 8] & (1 << ((nr) % 8)))
+#define SYSCALLMASK_ZERO(s) \
+ memset((s), 0, sizeof(struct note_trace_syscallfilter_s))
+#endif
+
+/* This is the type of the argument passed to the TRIOC_GETIRQFILTER and
+ * TRIOC_SETIRQFILTER ioctls
+ */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+struct note_trace_irqfilter_s
+{
+ uint8_t irq_mask[(NR_IRQS + 7) / 8];
+};
+
+/* Helper macros */
+
+#define IRQMASK_SET(nr, s) \
+ ((s)->irq_mask[(nr) / 8] |= (1 << ((nr) % 8)))
+#define IRQMASK_CLR(nr, s) \
+ ((s)->irq_mask[(nr) / 8] &= ~(1 << ((nr) % 8)))
+#define IRQMASK_ISSET(nr, s) \
+ ((s)->irq_mask[(nr) / 8] & (1 << ((nr) % 8)))
+#define IRQMASK_ZERO(s) \
+ memset((s), 0, sizeof(struct note_trace_irqfilter_s))
+#endif
+
+#endif /* CONFIG_SCHED_INSTRUMENTATION_FILTER */
+
/****************************************************************************
* Public Function Prototypes
****************************************************************************/
+#if defined(__KERNEL__) || defined(CONFIG_BUILD_FLAT)
Review comment:
I have moved it to beginning of the filter APIs.
I think it is still needed because in the case of PROTECTED and KERNEL build, application code requires constants and struct definitions of this file to control note filters, but no need kernel function declarations because it cannot be called directly.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-697546383
@v01d I have created the issue #1882 for it.
I didn't assigned the milestone for now. Because currently this feature is developed in the separate branch and I cannot decide whether it can be merged in time for the next release. Moreover, not only kernel side but app code is needed for this feature.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-703616679
@xiaoxiang781216 I have created #1936 using same commits.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r494852014
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +127,185 @@ 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
+ if ((g_note_filter.mode.cpuset & (1 << this_cpu())) == 0)
+ {
+ return false;
+ }
+#endif
+
+#else
+
+#ifdef CONFIG_SMP
+ /* Ignore notes that are not in the set of monitored CPUs */
+
+ if ((CONFIG_SCHED_INSTRUMENTATION_CPUSET & (1 << this_cpu())) == 0)
Review comment:
should we remove this code and let the SCHED_INSTRUMENTATION_CPUSET depends on SCHED_INSTRUMENTATION_FILTER?
##########
File path: drivers/note/notectl_driver.c
##########
@@ -0,0 +1,273 @@
+/****************************************************************************
+ * drivers/note/notectl_driver.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <errno.h>
+
+#include <nuttx/fs/fs.h>
+#include <nuttx/note/notectl_driver.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int notectl_ioctl(struct file *filep, int cmd, unsigned long arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations notectl_fops =
+{
+ NULL, /* open */
+ NULL, /* close */
+ NULL, /* read */
+ NULL, /* write */
+ NULL, /* seek */
+ notectl_ioctl, /* ioctl */
+ NULL /* poll */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ , 0 /* unlink */
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: notectl_ioctl
+ ****************************************************************************/
+
+static int notectl_ioctl(struct file *filep, int cmd, unsigned long arg)
+{
+ int ret = -ENOSYS;
+
+ /* Handle the ioctl commands */
+
+ switch (cmd)
+ {
+ /* NOTECTL_START
+ * - Start note recoding
+ * Argument: Ignored
+ */
+
+ case NOTECTL_START:
+ {
+ struct note_filter_mode_s mode;
+
+ sched_note_filter_mode(&mode, NULL);
+ mode.enable = true;
Review comment:
We can do the same thing(NOTECTL_START/NOTECTL_STOP) with NOTECTL_SETMODE, so why not remove these two ioctl?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-662238621
@YuuichiNakamura these patches is ready to merge:
```
commit 02c38e61f402c6d19206ad4ac3e0b788cceab2bc (apache/feature/syscall-instrumentation)
Author: Nakamura, Yuuichi <Yu...@sony.com>
Date: Tue Jun 16 12:16:17 2020 +0900
Syscall instrumentation build system support for Arm and RISC-V
commit 5b96c1ec8804893e8b33ff2d878a904c322bee10
Author: Nakamura, Yuuichi <Yu...@sony.com>
Date: Tue Jun 16 12:16:05 2020 +0900
Create wrapper library for system call instrumentation
From 09296d57b37899e8cf6589c6c08eda9be3476e44 Mon Sep 17 00:00:00 2001
From: "Nakamura, Yuuichi" <Yu...@sony.com>
Date: Tue, 7 Jul 2020 09:30:40 +0900
Subject: [PATCH] Add g_funcnames declaration in syscall.h
```
Since there is some conflict, could you cherrry-pick to mainline?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-688053803
@YuuichiNakamura done, please try it.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499264204
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +137,150 @@ 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.flag & NOTE_FILTER_MODE_FLAG_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
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int note_isenabled_syscall(int nr)
+{
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ /* Exclude the case of syscall called by the interrupt handler which is
+ * not traced.
+ */
+
+ if (up_interrupt_context())
+ {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ if (g_note_disabled_irq[cpu])
+ {
+ return false;
+ }
+#else
+ return false;
+#endif
+ }
+
+ /* If the syscall trace is disabled or the syscall number is masked,
+ * do nothing.
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) ||
+ NOTE_FILTER_SYSCALLMASK_ISSET(nr - CONFIG_SYS_RESERVED,
+ &g_note_filter.syscall_mask))
+ {
+ return false;
+ }
+#endif
+
+ return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: note_isenabled_irqhandler
+ *
+ * Description:
+ * Check whether the interrupt handler instrumentation is enabled.
+ *
+ * Input Parameters:
+ * irq - IRQ number
+ * enter - interrupt enter/leave flag
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+static inline int note_isenabled_irq(int irq, bool enter)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* If the IRQ trace is disabled or the IRQ number is masked, disable
+ * subsequent syscall traces until leaving the interrupt handler
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) ||
+ NOTE_FILTER_IRQMASK_ISSET(irq, &g_note_filter.irq_mask))
+ {
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ g_note_disabled_irq[cpu] = enter;
Review comment:
should we change g_note_disabled_irq to count and increase/decrease it here to handle the interrupt nesting correctly?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499390546
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +137,162 @@ 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.flag & NOTE_FILTER_MODE_FLAG_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
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int note_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* Exclude the case of syscall called by the interrupt handler which is
+ * not traced.
+ */
+
+ if (up_interrupt_context())
+ {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ if (g_note_disabled_irq_nest[cpu] > 0)
+ {
+ return false;
+ }
+#else
+ return false;
+#endif
+ }
+
+ /* If the syscall trace is disabled or the syscall number is masked,
+ * do nothing.
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) ||
+ NOTE_FILTER_SYSCALLMASK_ISSET(nr - CONFIG_SYS_RESERVED,
+ &g_note_filter.syscall_mask))
+ {
+ return false;
+ }
+#endif
+
+ return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: note_isenabled_irqhandler
+ *
+ * Description:
+ * Check whether the interrupt handler instrumentation is enabled.
+ *
+ * Input Parameters:
+ * irq - IRQ number
+ * enter - interrupt enter/leave flag
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+static inline int note_isenabled_irq(int irq, bool enter)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* If the IRQ trace is disabled or the IRQ number is masked, disable
+ * subsequent syscall traces until leaving the interrupt handler
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) ||
+ NOTE_FILTER_IRQMASK_ISSET(irq, &g_note_filter.irq_mask))
+ {
+ irqstate_t irq_mask;
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ irq_mask = enter_critical_section();
Review comment:
If nested interrupts are supported, the interrupt would be enabled here.
But, up_irq_save/restore() would be better because we need only consideration of current running cpu.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-703271553
@xiaoxiang781216 I have updated the commit to follow all comments.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465129927
##########
File path: sched/sched/sched_note.c
##########
@@ -73,41 +127,37 @@ static void note_add(FAR const uint8_t *note, uint8_t notelen);
* Private Data
****************************************************************************/
-static struct note_info_s g_note_info;
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+static struct note_filter_s g_note_filter;
Review comment:
The default value will lose all trace data utils the tools from userspace make the correct setting. The initial trace data is very important to debug the bootup issue. Should we inverse the field meaning to enable all trace by default?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r498665440
##########
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 the handling IRQ is not traced
@xiaoxiang781216 It seems that there are no way to get the IRQ number currently executing inside the interrupt handler.
sched_note_syscall_enter/leave() cannot decide whether the syscall should be recorded or not in the interrupt handler.
So we have to leave this as it is.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r498665527
##########
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:
In this case ni_head should not be updated. Because an incomplete note struct is recorded in the circular buffer if updated. If not updated, the incompletely recorded note will be simply discarded.
But, I found another problem in this case.
If the buffer have 16bytes free space, and 20bytes note and 12bytes note arrives sequentially, the former incoming note is discarded but only the latter note is recorded. This is an unexpected behavior.
I think that we need to add the flag to control whether we record the incoming note or not.
When in not overwrite mode, once we found an incoming note which is greater than the free space, disable the flag to block the following notes.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499252914
##########
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:
@YuuichiNakamura thanks for explanation, all make sense.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-662591821
@YuuichiNakamura please rebase your change to the new feature/syscall-instrumentation branch again.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r494847227
##########
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:
After search the code base, all arch use the same name(nsyscalls), so we use tcb->xcpt.nsyscalls directly which simplify the code a lot.
##########
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:
After search the code base, all arch use the same name(nsyscalls), so we can use tcb->xcpt.nsyscalls directly which simplify the code a lot.
##########
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:
After search the code base, all arch use the same name(nsyscalls), so we can use tcb->xcpt.nsyscalls directly which simplify the code a lot and avoid the dup code.
##########
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:
After search the code base, all arch use the same name(nsyscalls), so we can use tcb->xcpt.nsyscalls directly which simplify the code a lot and avoid the code dup.
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +127,185 @@ 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
+ if ((g_note_filter.mode.cpuset & (1 << this_cpu())) == 0)
+ {
+ return false;
+ }
+#endif
+
+#else
+
+#ifdef CONFIG_SMP
+ /* Ignore notes that are not in the set of monitored CPUs */
+
+ if ((CONFIG_SCHED_INSTRUMENTATION_CPUSET & (1 << this_cpu())) == 0)
Review comment:
should we remove this code and let the SCHED_INSTRUMENTATION_CPUSET depends on SCHED_INSTRUMENTATION_FILTER?
##########
File path: drivers/note/notectl_driver.c
##########
@@ -0,0 +1,273 @@
+/****************************************************************************
+ * drivers/note/notectl_driver.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <errno.h>
+
+#include <nuttx/fs/fs.h>
+#include <nuttx/note/notectl_driver.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int notectl_ioctl(struct file *filep, int cmd, unsigned long arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations notectl_fops =
+{
+ NULL, /* open */
+ NULL, /* close */
+ NULL, /* read */
+ NULL, /* write */
+ NULL, /* seek */
+ notectl_ioctl, /* ioctl */
+ NULL /* poll */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ , 0 /* unlink */
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: notectl_ioctl
+ ****************************************************************************/
+
+static int notectl_ioctl(struct file *filep, int cmd, unsigned long arg)
+{
+ int ret = -ENOSYS;
+
+ /* Handle the ioctl commands */
+
+ switch (cmd)
+ {
+ /* NOTECTL_START
+ * - Start note recoding
+ * Argument: Ignored
+ */
+
+ case NOTECTL_START:
+ {
+ struct note_filter_mode_s mode;
+
+ sched_note_filter_mode(&mode, NULL);
+ mode.enable = true;
Review comment:
We can do the same thing(NOTECTL_START/NOTECTL_STOP) with NOTECTL_SETMODE, so why not remove these two ioctl?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r453415174
##########
File path: arch/arm/src/common/arm_initialize.c
##########
@@ -28,7 +28,7 @@
#include <nuttx/arch.h>
#include <nuttx/board.h>
-#include <nuttx/sched_note.h>
+#include <nuttx/syslog/note_driver.h>
Review comment:
I got it.
But the commit "Fix note driver initialization" cannot be moved into master as it is because file "nuttx/syslog/note_driver.h" is not exist yet in master.
This patch contains the followings:
1. include note_driver.h instead of sched_note.h
2. Remove CONFIG_SCHED_INSTRUMENTATION_BUFFER definition check because we can know whether /dev/note is available by only CONFIG_DRIVER_NOTE.
We need to divide the patch into two.
The latter one and the succeeding "Fix nxstyle warnings" (796f770) can be moved into master.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r485280871
##########
File path: arch/arm/src/common/arm_initialize.c
##########
@@ -163,6 +164,10 @@ void up_initialize(void)
note_register(); /* Non-standard /dev/note */
#endif
+#if defined(CONFIG_DRIVER_NOTECTL)
+ notectl_register(); /* Non-standard /dev/notectl */
Review comment:
@xiaoxiang781216 I have reverted the notectl driver initialization in the all arch files. 54a2a2e
I'll follow your changes completely after #1731 is merged.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-697506558
@xiaoxiang781216 I have followed or answered to all your comments and pushed new commits. Please check it.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499261032
##########
File path: drivers/note/Kconfig
##########
@@ -55,6 +55,14 @@ config DRIVER_NOTERAM_BUFSIZE
---help---
The size of the in-memory, circular instrumentation buffer (in bytes).
+config DRIVER_NOTERAM_DEFAULT_NOOVERWRITE
+ bool "Disable overwrite by default"
+ default n
Review comment:
depend on DRIVER_NOTERAM?
##########
File path: sched/Kconfig
##########
@@ -1000,6 +1000,15 @@ 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_DEFAULT_MODE
+ hex "Default instrumentation filter mode"
+ default 0
+ ---help---
+ Default mode of the instrumentation filter logic.
+ Bit 0 = Enable instrumentation
+ Bit 1 = Enable syscall instrumentation
+ Bit 3 = Enable IRQ instrumentation
Review comment:
should we change NOTE_FILTER_MODE_FLAG_IRQ to:
```
#define NOTE_FILTER_MODE_FLAG_IRQ (1 << 2) /* Enable IRQ instrumentaiton */
``
##########
File path: sched/Kconfig
##########
@@ -1000,6 +1000,15 @@ 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_DEFAULT_MODE
+ hex "Default instrumentation filter mode"
+ default 0
Review comment:
change 0 to 7 to keep the default behaviour same as before?
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +133,150 @@ 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.flag & NOTE_FILTER_MODE_FLAG_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
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int note_isenabled_syscall(int nr)
+{
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
Review comment:
move before line 190 just like note_isenabled_irqhandler?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-657378878
@YuuichiNakamura I just rebase feature/syscall-instrumentation, you can reapply your change on the new base. But after rebase, I found that the remaining two patch is also good to merge into master with the current form:).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465486002
##########
File path: sched/sched/sched_note.c
##########
@@ -361,6 +421,13 @@ void sched_note_start(FAR struct tcb_s *tcb)
int namelen;
#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!g_note_filter.enable)
Review comment:
I have wanted to reduce the performance affect as possible when the trace feature is not used. How about it?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-683584722
Let's wait @patacongo feedback, can you review #1520 and give your feedback too?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499249888
##########
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:
For example, when an interrupt handler is called while some task is running and it issues a syscall (sem_post), the trace log would be like the followings:
```
test-5 [0] 30.000000000: sys_write()
test-5 [0] 30.010000000: sys_write -> 0x10
test-5 [0] 30.030000000: irq_handler_entry: irq=15
test-5 [0] 30.040000000: sys_sem_post()
test-5 [0] 30.050000000: sys_sem_post -> 0x0
test-5 [0] 30.060000000: irq_handler_exit: irq=15
test-5 [0] 30.100000000: sys_write()
test-5 [0] 30.110000000: sys_write -> 0x20
```
If IRQ 15 is masked and only note_isenabled_irqhandler() checks it, the log becomes:
```
test-5 [0] 30.000000000: sys_write()
test-5 [0] 30.010000000: sys_write -> 0x10
test-5 [0] 30.040000000: sys_sem_post()
test-5 [0] 30.050000000: sys_sem_post -> 0x0
test-5 [0] 30.100000000: sys_write()
test-5 [0] 30.110000000: sys_write -> 0x20
```
In spite of the task ID 5 only issues write(), the log seems the task also issues sem_post(). It is very confusing.
So we should couple syscalls which is called by interrupt handler and its IRQ trace. Then we'll get the following result:
```
test-5 [0] 30.000000000: sys_write()
test-5 [0] 30.010000000: sys_write -> 0x10
test-5 [0] 30.100000000: sys_write()
test-5 [0] 30.110000000: sys_write -> 0x20
```
If we don't trace IRQ, this is correct from the point of view of the task ID 5.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r453417539
##########
File path: arch/arm/src/common/arm_initialize.c
##########
@@ -28,7 +28,7 @@
#include <nuttx/arch.h>
#include <nuttx/board.h>
-#include <nuttx/sched_note.h>
+#include <nuttx/syslog/note_driver.h>
Review comment:
Since the change for note_driver.h is minor, it can be incorporated into this patch too.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499415368
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +137,162 @@ 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.flag & NOTE_FILTER_MODE_FLAG_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
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int note_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* Exclude the case of syscall called by the interrupt handler which is
+ * not traced.
+ */
+
+ if (up_interrupt_context())
+ {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ if (g_note_disabled_irq_nest[cpu] > 0)
+ {
+ return false;
+ }
+#else
+ return false;
+#endif
+ }
+
+ /* If the syscall trace is disabled or the syscall number is masked,
+ * do nothing.
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) ||
+ NOTE_FILTER_SYSCALLMASK_ISSET(nr - CONFIG_SYS_RESERVED,
+ &g_note_filter.syscall_mask))
+ {
+ return false;
+ }
+#endif
+
+ return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: note_isenabled_irqhandler
+ *
+ * Description:
+ * Check whether the interrupt handler instrumentation is enabled.
+ *
+ * Input Parameters:
+ * irq - IRQ number
+ * enter - interrupt enter/leave flag
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+static inline int note_isenabled_irq(int irq, bool enter)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* If the IRQ trace is disabled or the IRQ number is masked, disable
+ * subsequent syscall traces until leaving the interrupt handler
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) ||
+ NOTE_FILTER_IRQMASK_ISSET(irq, &g_note_filter.irq_mask))
+ {
+ irqstate_t irq_mask;
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ irq_mask = enter_critical_section();
Review comment:
Yes, no do_irq in arch code will re-enable interrupt, that's why I say only irq handle can enable irq manually now. irq_dispatch is the better place to enable irq before call irq handle but after sched_note_irqhandler in the furture:
```
irq_disaptch(int irq)
{
sched_note_irqhandler(irq, true);
irq_enable();
g_irqvect[irq]();
irq_disable();
sched_note_irqhandler(irq, false);
}
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-702589323
> @YuuichiNakamura, @xiaoxiang781216 this is looking really nice. Is there any documentation with the setup instructions that I can begin testing with?
@davids5 Sorry. https://github.com/YuuichiNakamura/nuttx-task-tracer-doc is the document but it is already outdated and I have to update it. As I answered to @v01d , I'll maintain the documents for the new document system after the branch is merged.
To try it instantly, use https://github.com/YuuichiNakamura/incubator-nuttx-apps.git feature/task-tracer branch as the app code,
and enable the following configs.
```
CONFIG_DRIVER_NOTE=y
CONFIG_DRIVER_NOTECTL=y
CONFIG_SCHED_INSTRUMENTATION=y
CONFIG_SCHED_INSTRUMENTATION_FILTER=y
CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER=y
CONFIG_SCHED_INSTRUMENTATION_SYSCALL=y
```
Then `trace` command is added to nsh. After that,
```
trace mode +s
trace cmd "sleep 1"
trace dump
```
This can get the trace while executing "sleep 1". The log output can be used as the input for tracecompass.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-697520399
> Please add documentation in `Documentaiton` for this new feature
Can I add the documentation after the code is merged?
I'd like to add it by another commit.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465116503
##########
File path: sched/sched/sched_note.c
##########
@@ -186,149 +236,159 @@ void note_spincommon(FAR struct tcb_s *tcb,
#endif
/****************************************************************************
- * Name: note_length
+ * Name: note_add
*
* Description:
- * Length of data currently in circular buffer.
+ * Put the variable length note to the note stream
*
* Input Parameters:
- * None
+ * note - Pointer to the note to be added
+ * notelen - Length of the note
*
* Returned Value:
- * Length of data currently in circular buffer.
+ * None
+ *
+ * Assumptions:
+ * We are within a critical section.
*
****************************************************************************/
-#if defined(CONFIG_SCHED_NOTE_GET) || defined(CONFIG_DEBUG_ASSERTIONS)
-static unsigned int note_length(void)
+static void note_add(FAR const uint8_t *note, uint8_t notelen)
{
- unsigned int head = g_note_info.ni_head;
- unsigned int tail = g_note_info.ni_tail;
+#ifdef CONFIG_SMP
+ /* Ignore notes that are not in the set of monitored CPUs */
- if (tail > head)
+ if ((CONFIG_SCHED_INSTRUMENTATION_CPUSET & (1 << this_cpu())) == 0)
{
- head += CONFIG_SCHED_NOTE_BUFSIZE;
+ /* Not in the set of monitored CPUs. Do not log the note. */
+
+ return;
}
+#endif
- return head - tail;
-}
+#ifdef CONFIG_SMP
+ irqstate_t flags = up_irq_save();
+ spin_lock_wo_note(&g_note_lock);
#endif
+ /* Loop until all bytes have been transferred */
+
+ if (g_sched_note_stream != NULL)
+ {
+ while (notelen > 0)
+ {
+ (g_sched_note_stream)->public.put(&g_sched_note_stream->public,
Review comment:
The performance is poor, how about we make note_add as the interface between the common code and the trransport layer? so the hardware solution and software solution just need implement note_add.
##########
File path: include/nuttx/syslog/note_driver.h
##########
@@ -27,22 +27,283 @@
#include <nuttx/config.h>
+#include <sys/types.h>
+#include <stdint.h>
+
+#include <nuttx/sched_note.h>
+#include <nuttx/fs/ioctl.h>
+
#ifdef CONFIG_SCHED_INSTRUMENTATION
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
+/* IOCTL Commands ***********************************************************/
+
+/* TRIOC_START
+ * - Start task tracing
+ * Argument: Ignored
+ * TRIOC_STOP
+ * - Stop task tracing
+ * Argument: Ignored
+ * TRIOC_GETMODE
+ * - Get task trace mode
+ * Argument: A writable pointer to struct note_trace_mode_s
+ * TRIOC_SETMODE
+ * - Set task trace mode
+ * Argument: A read-only pointer to struct note_trace_mode_s
+ * TRIOC_GETSYSCALLFILTER
+ * - Get syscall trace filter setting
+ * Argument: A writable pointer to struct
+ * note_trace_syscallfilter_s
+ * TRIOC_SETSYSCALLFILTER
+ * - Set syscall trace filter setting
+ * Argument: A read-only pointer to struct
+ * note_trace_syscallfilter_s
+ * TRIOC_GETIRQFILTER
+ * - Get IRQ trace filter setting
+ * Argument: A writable pointer to struct
+ * note_trace_irqfilter_s
+ * TRIOC_SETIRQFILTER
+ * - Set IRQ trace filter setting
+ * Argument: A read-only pointer to struct
+ * note_trace_irqfilter_s
+ * TRIOC_GETTASKNAME
+ * - Get task name string
+ * Argument: A writable pointer to struct note_get_taskname_s
+ * Result: If -ESRCH, the corresponding task name doesn't
+ * exist.
+ */
+
+#if defined(CONFIG_SCHED_INSTRUMENTATION_FILTER) && defined(CONFIG_DRIVER_NOTE)
+
+#define _NOTETRACEBASE (0xfe00)
+#define _NOTETRACEIOCVALID(c) (_IOC_TYPE(c) == _NOTETRACEBASE)
+#define _NOTETRACEIOC(nr) _IOC(_NOTETRACEBASE, nr)
+
+#define TRIOC_START _NOTETRACEIOC(0x01)
+#define TRIOC_STOP _NOTETRACEIOC(0x02)
+#define TRIOC_GETMODE _NOTETRACEIOC(0x03)
+#define TRIOC_SETMODE _NOTETRACEIOC(0x04)
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+#define TRIOC_GETSYSCALLFILTER _NOTETRACEIOC(0x05)
+#define TRIOC_SETSYSCALLFILTER _NOTETRACEIOC(0x06)
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#define TRIOC_GETIRQFILTER _NOTETRACEIOC(0x07)
+#define TRIOC_SETIRQFILTER _NOTETRACEIOC(0x08)
+#endif
+#ifdef NOTE_FEATURE_TASK_NAME
+#define TRIOC_GETTASKNAME _NOTETRACEIOC(0x09)
+#endif
+
+#endif
+
/****************************************************************************
* Public Types
****************************************************************************/
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
Review comment:
should the follow snippet belong nuttx/note.h?
##########
File path: sched/sched/sched_note.c
##########
@@ -361,6 +421,13 @@ void sched_note_start(FAR struct tcb_s *tcb)
int namelen;
#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!g_note_filter.enable)
Review comment:
should we move this check to sched_note_add? so all duplication can be removed.
##########
File path: drivers/syslog/note_driver.c
##########
@@ -79,10 +107,29 @@ static const struct file_operations note_fops =
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: note_open
+ ****************************************************************************/
+
+#ifdef NOTE_FEATURE_BUFFER_REWINDABLE
+static int note_open(FAR struct file *filep)
+{
+ if ((filep->f_oflags & O_ACCMODE) != O_WRONLY)
+ {
+ /* Reset the pointer to read the trace data */
+
+ sched_note_trace_stop();
Review comment:
Why stop the trace? we will lost some trace from here to the next enable.
##########
File path: sched/sched/sched_note.c
##########
@@ -631,144 +924,337 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
}
#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
/****************************************************************************
- * Name: sched_note_get
+ * Name: sched_note_trace_start
*
* Description:
- * Remove the next note from the tail of the circular buffer. The note
- * is also removed from the circular buffer to make room for further notes.
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
*
* Input Parameters:
- * buffer - Location to return the next note
- * buflen - The length of the user provided buffer.
+ * None
*
* Returned Value:
- * On success, the positive, non-zero length of the return note is
- * provided. Zero is returned only if the circular buffer is empty. A
- * negated errno value is returned in the event of any failure.
+ * None
*
****************************************************************************/
-#ifdef CONFIG_SCHED_NOTE_GET
-ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen)
+void sched_note_trace_start(void)
{
- FAR struct note_common_s *note;
- irqstate_t flags;
- unsigned int remaining;
- unsigned int tail;
- ssize_t notelen;
- size_t circlen;
+ irqstate_t irq_mask;
- DEBUGASSERT(buffer != NULL);
- flags = enter_critical_section();
+ irq_mask = enter_critical_section();
- /* Verify that the circular buffer is not empty */
+ g_note_filter.enable = true;
+#ifdef NOTE_FEATURE_TASK_NAME
+ note_taskname_init();
+#endif
- circlen = note_length();
- if (circlen <= 0)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
{
- notelen = 0;
- goto errout_with_csection;
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
}
- /* Get the index to the tail of the circular buffer */
+ leave_critical_section(irq_mask);
+}
- tail = g_note_info.ni_tail;
- DEBUGASSERT(tail < CONFIG_SCHED_NOTE_BUFSIZE);
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- /* Get the length of the note at the tail index */
+void sched_note_trace_stop(void)
+{
+ irqstate_t irq_mask;
- note = (FAR struct note_common_s *)&g_note_info.ni_buffer[tail];
- notelen = note->nc_length;
- DEBUGASSERT(notelen <= circlen);
+ irq_mask = enter_critical_section();
- /* Is the user buffer large enough to hold the note? */
+ g_note_filter.enable = false;
- if (buflen < notelen)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufrewind != NULL)
{
- /* Remove the large note so that we do not get constipated. */
+ (g_sched_note_stream)->bufrewind(g_sched_note_stream);
+ }
+
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- note_remove();
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
+ struct note_trace_mode_s *newm)
+{
+ irqstate_t irq_mask;
- /* and return an error */
+ irq_mask = enter_critical_section();
- notelen = -EFBIG;
- goto errout_with_csection;
+ if (newm != NULL)
+ {
+ sched_note_trace_stop();
+ g_note_filter.mode = *newm;
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
+ {
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
+ }
}
- /* Loop until the note has been transferred to the user buffer */
+ if (oldm != NULL)
+ {
+ *oldm = g_note_filter.mode;
+ }
- remaining = (unsigned int)notelen;
- while (remaining > 0)
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_syscallfilter
+ *
+ * Description:
+ * Set and get syscall trace filter setting
+ * (Same as TRIOC_GETSYSCALLFILTER / TRIOC_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_syscallfilter_s to get
+ * current syscall trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_syscallfilter_s of the
+ * new syscall trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_trace_syscallfilter(struct note_trace_syscallfilter_s *oldf,
+ struct note_trace_syscallfilter_s *newf)
+{
+ irqstate_t irq_mask;
+ int i;
+ int nr;
+
+ irq_mask = enter_critical_section();
+
+ if (newf != NULL)
{
- /* Copy the next byte at the tail index */
+ sched_note_trace_stop();
- *buffer++ = g_note_info.ni_buffer[tail];
+ /* Replace the syscall filter mask by the provided setting */
- /* Adjust indices and counts */
+ memset(g_note_filter.syscall_mask, 0,
+ sizeof g_note_filter.syscall_mask);
+ for (i = 0; i < newf->nr_syscalls; i++)
+ {
+ nr = newf->syscalls[i];
- tail = note_next(tail, 1);
- remaining--;
+ if (nr < SYS_nsyscalls)
+ {
+ BITMASK_SET(g_note_filter.syscall_mask, nr);
+ }
+ }
}
- g_note_info.ni_tail = tail;
+ /* Count the number of filtered syscalls to calculate the requied size to
+ * get the current setting.
+ */
-errout_with_csection:
- leave_critical_section(flags);
- return notelen;
+ nr = 0;
+ for (i = 0; i < SYS_nsyscalls; i++)
+ {
+ if (BITMASK_CHECK(g_note_filter.syscall_mask, i))
+ {
+ nr++;
+ }
+ }
+
+ if (oldf != NULL)
Review comment:
should move before the modification, otherwise we get the latest value not the old one.
##########
File path: sched/sched/sched_note.c
##########
@@ -631,144 +924,337 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
}
#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
/****************************************************************************
- * Name: sched_note_get
+ * Name: sched_note_trace_start
*
* Description:
- * Remove the next note from the tail of the circular buffer. The note
- * is also removed from the circular buffer to make room for further notes.
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
*
* Input Parameters:
- * buffer - Location to return the next note
- * buflen - The length of the user provided buffer.
+ * None
*
* Returned Value:
- * On success, the positive, non-zero length of the return note is
- * provided. Zero is returned only if the circular buffer is empty. A
- * negated errno value is returned in the event of any failure.
+ * None
*
****************************************************************************/
-#ifdef CONFIG_SCHED_NOTE_GET
-ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen)
+void sched_note_trace_start(void)
{
- FAR struct note_common_s *note;
- irqstate_t flags;
- unsigned int remaining;
- unsigned int tail;
- ssize_t notelen;
- size_t circlen;
+ irqstate_t irq_mask;
- DEBUGASSERT(buffer != NULL);
- flags = enter_critical_section();
+ irq_mask = enter_critical_section();
- /* Verify that the circular buffer is not empty */
+ g_note_filter.enable = true;
+#ifdef NOTE_FEATURE_TASK_NAME
+ note_taskname_init();
+#endif
- circlen = note_length();
- if (circlen <= 0)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
{
- notelen = 0;
- goto errout_with_csection;
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
}
- /* Get the index to the tail of the circular buffer */
+ leave_critical_section(irq_mask);
+}
- tail = g_note_info.ni_tail;
- DEBUGASSERT(tail < CONFIG_SCHED_NOTE_BUFSIZE);
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- /* Get the length of the note at the tail index */
+void sched_note_trace_stop(void)
+{
+ irqstate_t irq_mask;
- note = (FAR struct note_common_s *)&g_note_info.ni_buffer[tail];
- notelen = note->nc_length;
- DEBUGASSERT(notelen <= circlen);
+ irq_mask = enter_critical_section();
- /* Is the user buffer large enough to hold the note? */
+ g_note_filter.enable = false;
- if (buflen < notelen)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufrewind != NULL)
{
- /* Remove the large note so that we do not get constipated. */
+ (g_sched_note_stream)->bufrewind(g_sched_note_stream);
+ }
+
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- note_remove();
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
+ struct note_trace_mode_s *newm)
+{
+ irqstate_t irq_mask;
- /* and return an error */
+ irq_mask = enter_critical_section();
- notelen = -EFBIG;
- goto errout_with_csection;
+ if (newm != NULL)
+ {
+ sched_note_trace_stop();
+ g_note_filter.mode = *newm;
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
+ {
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
+ }
}
- /* Loop until the note has been transferred to the user buffer */
+ if (oldm != NULL)
+ {
+ *oldm = g_note_filter.mode;
+ }
- remaining = (unsigned int)notelen;
- while (remaining > 0)
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_syscallfilter
+ *
+ * Description:
+ * Set and get syscall trace filter setting
+ * (Same as TRIOC_GETSYSCALLFILTER / TRIOC_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_syscallfilter_s to get
+ * current syscall trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_syscallfilter_s of the
+ * new syscall trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_trace_syscallfilter(struct note_trace_syscallfilter_s *oldf,
+ struct note_trace_syscallfilter_s *newf)
+{
+ irqstate_t irq_mask;
+ int i;
+ int nr;
+
+ irq_mask = enter_critical_section();
+
+ if (newf != NULL)
{
- /* Copy the next byte at the tail index */
+ sched_note_trace_stop();
- *buffer++ = g_note_info.ni_buffer[tail];
+ /* Replace the syscall filter mask by the provided setting */
- /* Adjust indices and counts */
+ memset(g_note_filter.syscall_mask, 0,
+ sizeof g_note_filter.syscall_mask);
+ for (i = 0; i < newf->nr_syscalls; i++)
+ {
+ nr = newf->syscalls[i];
- tail = note_next(tail, 1);
- remaining--;
+ if (nr < SYS_nsyscalls)
+ {
+ BITMASK_SET(g_note_filter.syscall_mask, nr);
+ }
+ }
}
- g_note_info.ni_tail = tail;
+ /* Count the number of filtered syscalls to calculate the requied size to
+ * get the current setting.
+ */
-errout_with_csection:
- leave_critical_section(flags);
- return notelen;
+ nr = 0;
+ for (i = 0; i < SYS_nsyscalls; i++)
+ {
+ if (BITMASK_CHECK(g_note_filter.syscall_mask, i))
+ {
+ nr++;
+ }
+ }
+
+ if (oldf != NULL)
+ {
+ /* Return the current filter setting */
+
+ int *p = oldf->syscalls;
+ int s = oldf->size;
+ oldf->nr_syscalls = nr;
+
+ for (i = 0; i < SYS_nsyscalls; i++)
+ {
+ if (p == NULL || s <= sizeof(int))
+ {
+ break;
+ }
+
+ if (BITMASK_CHECK(g_note_filter.syscall_mask, i))
+ {
+ *p++ = i;
+ s -= sizeof(int);
+ }
+ }
+ }
+
+ leave_critical_section(irq_mask);
}
#endif
/****************************************************************************
- * Name: sched_note_size
+ * Name: sched_note_trace_irqfilter
*
* Description:
- * Return the size of the next note at the tail of the circular buffer.
+ * Set and get IRQ trace filter setting
+ * (Same as TRIOC_GETIRQFILTER / TRIOC_SETIRQFILTER ioctls)
*
* Input Parameters:
- * None.
+ * oldf - A writable pointer to struct note_trace_irqfilter_s to get
+ * current IRQ trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_irqfilter_s of the new
+ * IRQ trace filter setting
+ * If 0, the setting is not updated.
*
* Returned Value:
- * Zero is returned if the circular buffer is empty. Otherwise, the size
- * of the next note is returned.
+ * None
*
****************************************************************************/
-#ifdef CONFIG_SCHED_NOTE_GET
-ssize_t sched_note_size(void)
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+void sched_note_trace_irqfilter(struct note_trace_irqfilter_s *oldf,
+ struct note_trace_irqfilter_s *newf)
{
- FAR struct note_common_s *note;
- irqstate_t flags;
- unsigned int tail;
- ssize_t notelen;
- size_t circlen;
+ irqstate_t irq_mask;
+ int i;
+ int nr;
- flags = enter_critical_section();
+ irq_mask = enter_critical_section();
- /* Verify that the circular buffer is not empty */
+ if (newf != NULL)
+ {
+ sched_note_trace_stop();
+
+ /* Replace the IRQ filter mask by the provided setting */
+
+ memset(g_note_filter.irqhandler_mask, 0,
+ sizeof g_note_filter.irqhandler_mask);
Review comment:
should we use the same format here to avoid the conversion(number<->bitmap)?
##########
File path: sched/sched/sched_note.c
##########
@@ -370,6 +437,10 @@ void sched_note_start(FAR struct tcb_s *tcb)
strncpy(note.nsa_name, tcb->name, CONFIG_TASK_NAME_SIZE + 1);
length = SIZEOF_NOTE_START(namelen + 1);
+
+#ifdef NOTE_FEATURE_TASK_NAME
+ note_taskname_put(tcb->pid, tcb->name);
Review comment:
If the note already contain task name and pid, why we need save pid and name additionally?
##########
File path: drivers/syslog/note_driver.c
##########
@@ -138,6 +185,231 @@ static ssize_t note_read(FAR struct file *filep, FAR char *buffer,
sched_unlock();
return retlen;
}
+#endif
+
+/****************************************************************************
+ * Name: note_write
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+static ssize_t note_write(FAR struct file *filep, FAR const char *buffer,
Review comment:
why need to implement a dummy note_write?
##########
File path: sched/sched/sched_note.c
##########
@@ -631,144 +924,337 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
}
#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
/****************************************************************************
- * Name: sched_note_get
+ * Name: sched_note_trace_start
*
* Description:
- * Remove the next note from the tail of the circular buffer. The note
- * is also removed from the circular buffer to make room for further notes.
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
*
* Input Parameters:
- * buffer - Location to return the next note
- * buflen - The length of the user provided buffer.
+ * None
*
* Returned Value:
- * On success, the positive, non-zero length of the return note is
- * provided. Zero is returned only if the circular buffer is empty. A
- * negated errno value is returned in the event of any failure.
+ * None
*
****************************************************************************/
-#ifdef CONFIG_SCHED_NOTE_GET
-ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen)
+void sched_note_trace_start(void)
{
- FAR struct note_common_s *note;
- irqstate_t flags;
- unsigned int remaining;
- unsigned int tail;
- ssize_t notelen;
- size_t circlen;
+ irqstate_t irq_mask;
- DEBUGASSERT(buffer != NULL);
- flags = enter_critical_section();
+ irq_mask = enter_critical_section();
- /* Verify that the circular buffer is not empty */
+ g_note_filter.enable = true;
+#ifdef NOTE_FEATURE_TASK_NAME
+ note_taskname_init();
+#endif
- circlen = note_length();
- if (circlen <= 0)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
{
- notelen = 0;
- goto errout_with_csection;
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
}
- /* Get the index to the tail of the circular buffer */
+ leave_critical_section(irq_mask);
+}
- tail = g_note_info.ni_tail;
- DEBUGASSERT(tail < CONFIG_SCHED_NOTE_BUFSIZE);
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- /* Get the length of the note at the tail index */
+void sched_note_trace_stop(void)
+{
+ irqstate_t irq_mask;
- note = (FAR struct note_common_s *)&g_note_info.ni_buffer[tail];
- notelen = note->nc_length;
- DEBUGASSERT(notelen <= circlen);
+ irq_mask = enter_critical_section();
- /* Is the user buffer large enough to hold the note? */
+ g_note_filter.enable = false;
- if (buflen < notelen)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufrewind != NULL)
{
- /* Remove the large note so that we do not get constipated. */
+ (g_sched_note_stream)->bufrewind(g_sched_note_stream);
+ }
+
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- note_remove();
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
+ struct note_trace_mode_s *newm)
+{
+ irqstate_t irq_mask;
- /* and return an error */
+ irq_mask = enter_critical_section();
- notelen = -EFBIG;
- goto errout_with_csection;
+ if (newm != NULL)
+ {
+ sched_note_trace_stop();
+ g_note_filter.mode = *newm;
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
+ {
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
+ }
}
- /* Loop until the note has been transferred to the user buffer */
+ if (oldm != NULL)
+ {
+ *oldm = g_note_filter.mode;
+ }
- remaining = (unsigned int)notelen;
- while (remaining > 0)
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_syscallfilter
+ *
+ * Description:
+ * Set and get syscall trace filter setting
+ * (Same as TRIOC_GETSYSCALLFILTER / TRIOC_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_syscallfilter_s to get
+ * current syscall trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_syscallfilter_s of the
+ * new syscall trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_trace_syscallfilter(struct note_trace_syscallfilter_s *oldf,
+ struct note_trace_syscallfilter_s *newf)
+{
+ irqstate_t irq_mask;
+ int i;
+ int nr;
+
+ irq_mask = enter_critical_section();
+
+ if (newf != NULL)
{
- /* Copy the next byte at the tail index */
+ sched_note_trace_stop();
- *buffer++ = g_note_info.ni_buffer[tail];
+ /* Replace the syscall filter mask by the provided setting */
- /* Adjust indices and counts */
+ memset(g_note_filter.syscall_mask, 0,
+ sizeof g_note_filter.syscall_mask);
+ for (i = 0; i < newf->nr_syscalls; i++)
+ {
+ nr = newf->syscalls[i];
- tail = note_next(tail, 1);
- remaining--;
+ if (nr < SYS_nsyscalls)
+ {
+ BITMASK_SET(g_note_filter.syscall_mask, nr);
+ }
+ }
}
- g_note_info.ni_tail = tail;
+ /* Count the number of filtered syscalls to calculate the requied size to
+ * get the current setting.
+ */
-errout_with_csection:
- leave_critical_section(flags);
- return notelen;
+ nr = 0;
+ for (i = 0; i < SYS_nsyscalls; i++)
+ {
+ if (BITMASK_CHECK(g_note_filter.syscall_mask, i))
+ {
+ nr++;
+ }
+ }
+
+ if (oldf != NULL)
+ {
+ /* Return the current filter setting */
+
+ int *p = oldf->syscalls;
+ int s = oldf->size;
+ oldf->nr_syscalls = nr;
+
+ for (i = 0; i < SYS_nsyscalls; i++)
+ {
+ if (p == NULL || s <= sizeof(int))
+ {
+ break;
+ }
+
+ if (BITMASK_CHECK(g_note_filter.syscall_mask, i))
+ {
+ *p++ = i;
+ s -= sizeof(int);
+ }
+ }
+ }
+
+ leave_critical_section(irq_mask);
}
#endif
/****************************************************************************
- * Name: sched_note_size
+ * Name: sched_note_trace_irqfilter
*
* Description:
- * Return the size of the next note at the tail of the circular buffer.
+ * Set and get IRQ trace filter setting
+ * (Same as TRIOC_GETIRQFILTER / TRIOC_SETIRQFILTER ioctls)
*
* Input Parameters:
- * None.
+ * oldf - A writable pointer to struct note_trace_irqfilter_s to get
+ * current IRQ trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_irqfilter_s of the new
+ * IRQ trace filter setting
+ * If 0, the setting is not updated.
*
* Returned Value:
- * Zero is returned if the circular buffer is empty. Otherwise, the size
- * of the next note is returned.
+ * None
*
****************************************************************************/
-#ifdef CONFIG_SCHED_NOTE_GET
-ssize_t sched_note_size(void)
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+void sched_note_trace_irqfilter(struct note_trace_irqfilter_s *oldf,
+ struct note_trace_irqfilter_s *newf)
{
- FAR struct note_common_s *note;
- irqstate_t flags;
- unsigned int tail;
- ssize_t notelen;
- size_t circlen;
+ irqstate_t irq_mask;
+ int i;
+ int nr;
- flags = enter_critical_section();
+ irq_mask = enter_critical_section();
- /* Verify that the circular buffer is not empty */
+ if (newf != NULL)
+ {
+ sched_note_trace_stop();
+
+ /* Replace the IRQ filter mask by the provided setting */
+
+ memset(g_note_filter.irqhandler_mask, 0,
+ sizeof g_note_filter.irqhandler_mask);
+ for (i = 0; i < newf->nr_irqs; i++)
+ {
+ nr = newf->irqs[i];
+
+ if (nr < NR_IRQS)
+ {
+ BITMASK_SET(g_note_filter.irqhandler_mask, nr);
+ }
+ }
+ }
+
+ /* Count the number of filtered IRQs to calculate the requied size to get
+ * the current setting.
+ */
+
+ nr = 0;
+ for (i = 0; i < NR_IRQS; i++)
+ {
+ if (BITMASK_CHECK(g_note_filter.irqhandler_mask, i))
+ {
+ nr++;
+ }
+ }
- circlen = note_length();
- if (circlen <= 0)
+ if (oldf != NULL)
Review comment:
move befoe the modification.
##########
File path: sched/sched/sched_note.c
##########
@@ -631,144 +924,337 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
}
#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
/****************************************************************************
- * Name: sched_note_get
+ * Name: sched_note_trace_start
*
* Description:
- * Remove the next note from the tail of the circular buffer. The note
- * is also removed from the circular buffer to make room for further notes.
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
*
* Input Parameters:
- * buffer - Location to return the next note
- * buflen - The length of the user provided buffer.
+ * None
*
* Returned Value:
- * On success, the positive, non-zero length of the return note is
- * provided. Zero is returned only if the circular buffer is empty. A
- * negated errno value is returned in the event of any failure.
+ * None
*
****************************************************************************/
-#ifdef CONFIG_SCHED_NOTE_GET
-ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen)
+void sched_note_trace_start(void)
{
- FAR struct note_common_s *note;
- irqstate_t flags;
- unsigned int remaining;
- unsigned int tail;
- ssize_t notelen;
- size_t circlen;
+ irqstate_t irq_mask;
- DEBUGASSERT(buffer != NULL);
- flags = enter_critical_section();
+ irq_mask = enter_critical_section();
- /* Verify that the circular buffer is not empty */
+ g_note_filter.enable = true;
+#ifdef NOTE_FEATURE_TASK_NAME
+ note_taskname_init();
+#endif
- circlen = note_length();
- if (circlen <= 0)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
{
- notelen = 0;
- goto errout_with_csection;
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
}
- /* Get the index to the tail of the circular buffer */
+ leave_critical_section(irq_mask);
+}
- tail = g_note_info.ni_tail;
- DEBUGASSERT(tail < CONFIG_SCHED_NOTE_BUFSIZE);
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- /* Get the length of the note at the tail index */
+void sched_note_trace_stop(void)
+{
+ irqstate_t irq_mask;
- note = (FAR struct note_common_s *)&g_note_info.ni_buffer[tail];
- notelen = note->nc_length;
- DEBUGASSERT(notelen <= circlen);
+ irq_mask = enter_critical_section();
- /* Is the user buffer large enough to hold the note? */
+ g_note_filter.enable = false;
- if (buflen < notelen)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufrewind != NULL)
{
- /* Remove the large note so that we do not get constipated. */
+ (g_sched_note_stream)->bufrewind(g_sched_note_stream);
+ }
+
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- note_remove();
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
+ struct note_trace_mode_s *newm)
+{
+ irqstate_t irq_mask;
- /* and return an error */
+ irq_mask = enter_critical_section();
- notelen = -EFBIG;
- goto errout_with_csection;
+ if (newm != NULL)
+ {
+ sched_note_trace_stop();
+ g_note_filter.mode = *newm;
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
+ {
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
+ }
}
- /* Loop until the note has been transferred to the user buffer */
+ if (oldm != NULL)
+ {
+ *oldm = g_note_filter.mode;
+ }
- remaining = (unsigned int)notelen;
- while (remaining > 0)
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_syscallfilter
+ *
+ * Description:
+ * Set and get syscall trace filter setting
+ * (Same as TRIOC_GETSYSCALLFILTER / TRIOC_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_syscallfilter_s to get
+ * current syscall trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_syscallfilter_s of the
+ * new syscall trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_trace_syscallfilter(struct note_trace_syscallfilter_s *oldf,
+ struct note_trace_syscallfilter_s *newf)
+{
+ irqstate_t irq_mask;
+ int i;
+ int nr;
+
+ irq_mask = enter_critical_section();
+
+ if (newf != NULL)
{
- /* Copy the next byte at the tail index */
+ sched_note_trace_stop();
- *buffer++ = g_note_info.ni_buffer[tail];
+ /* Replace the syscall filter mask by the provided setting */
- /* Adjust indices and counts */
+ memset(g_note_filter.syscall_mask, 0,
+ sizeof g_note_filter.syscall_mask);
+ for (i = 0; i < newf->nr_syscalls; i++)
+ {
+ nr = newf->syscalls[i];
- tail = note_next(tail, 1);
- remaining--;
+ if (nr < SYS_nsyscalls)
+ {
+ BITMASK_SET(g_note_filter.syscall_mask, nr);
Review comment:
should we use the same format here to avoid the conversion(number<->bitmap)?
##########
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:
But should we trace the nest system call too?
##########
File path: drivers/syslog/Kconfig
##########
@@ -64,7 +64,7 @@ endif
config DRIVER_NOTE
bool "Scheduler instrumentation driver"
default n
- depends on SCHED_INSTRUMENTATION_BUFFER && SCHED_NOTE_GET
+ depends on (SCHED_INSTRUMENTATION_BUFFER && SCHED_NOTE_GET) || SCHED_INSTRUMENTATION_FILTER
Review comment:
Ok.
##########
File path: sched/sched/sched_note_buffer.c
##########
@@ -0,0 +1,471 @@
+/****************************************************************************
+ * sched/sched/sched_note_buffer.c
Review comment:
We can merge this file into note_driver to simplify the interaction.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r498625054
##########
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:
@xiaoxiang781216
This is a screenshot of tracecompass to compare the case of removing the nested syscall (upper) and not removing (lower).
The real execution time of putenv() is upper one, but when the nested syscall is not removed, tracecompass misjudges the end of syscall and displays incorrect syscall execution period.
However, I understand your thought that it's very useful to trace the nested syscall.
So I have changed my mind and will propose the alternative idea for it:
- sched_note.c records the syscall notes as it is.
- The app code which translates the note record into ftrace text format counts the nesting level of syscall and removes the nested syscalls for correctly tracecompass display.
Because the note data would be available by not only tracecompass but another tools, it would be better to remove the special treatment for tracecompass from kernel code.
How about this idea?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-692536025
@xiaoxiang781216 Thank you for restructuring the note driver. I have pushed my changes based on the latest note driver.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465486190
##########
File path: sched/sched/sched_note.c
##########
@@ -631,144 +924,337 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
}
#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
/****************************************************************************
- * Name: sched_note_get
+ * Name: sched_note_trace_start
*
* Description:
- * Remove the next note from the tail of the circular buffer. The note
- * is also removed from the circular buffer to make room for further notes.
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
*
* Input Parameters:
- * buffer - Location to return the next note
- * buflen - The length of the user provided buffer.
+ * None
*
* Returned Value:
- * On success, the positive, non-zero length of the return note is
- * provided. Zero is returned only if the circular buffer is empty. A
- * negated errno value is returned in the event of any failure.
+ * None
*
****************************************************************************/
-#ifdef CONFIG_SCHED_NOTE_GET
-ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen)
+void sched_note_trace_start(void)
{
- FAR struct note_common_s *note;
- irqstate_t flags;
- unsigned int remaining;
- unsigned int tail;
- ssize_t notelen;
- size_t circlen;
+ irqstate_t irq_mask;
- DEBUGASSERT(buffer != NULL);
- flags = enter_critical_section();
+ irq_mask = enter_critical_section();
- /* Verify that the circular buffer is not empty */
+ g_note_filter.enable = true;
+#ifdef NOTE_FEATURE_TASK_NAME
+ note_taskname_init();
+#endif
- circlen = note_length();
- if (circlen <= 0)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
{
- notelen = 0;
- goto errout_with_csection;
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
}
- /* Get the index to the tail of the circular buffer */
+ leave_critical_section(irq_mask);
+}
- tail = g_note_info.ni_tail;
- DEBUGASSERT(tail < CONFIG_SCHED_NOTE_BUFSIZE);
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- /* Get the length of the note at the tail index */
+void sched_note_trace_stop(void)
+{
+ irqstate_t irq_mask;
- note = (FAR struct note_common_s *)&g_note_info.ni_buffer[tail];
- notelen = note->nc_length;
- DEBUGASSERT(notelen <= circlen);
+ irq_mask = enter_critical_section();
- /* Is the user buffer large enough to hold the note? */
+ g_note_filter.enable = false;
- if (buflen < notelen)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufrewind != NULL)
{
- /* Remove the large note so that we do not get constipated. */
+ (g_sched_note_stream)->bufrewind(g_sched_note_stream);
+ }
+
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- note_remove();
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
+ struct note_trace_mode_s *newm)
+{
+ irqstate_t irq_mask;
- /* and return an error */
+ irq_mask = enter_critical_section();
- notelen = -EFBIG;
- goto errout_with_csection;
+ if (newm != NULL)
+ {
+ sched_note_trace_stop();
+ g_note_filter.mode = *newm;
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
+ {
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
+ }
}
- /* Loop until the note has been transferred to the user buffer */
+ if (oldm != NULL)
+ {
+ *oldm = g_note_filter.mode;
+ }
- remaining = (unsigned int)notelen;
- while (remaining > 0)
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_syscallfilter
+ *
+ * Description:
+ * Set and get syscall trace filter setting
+ * (Same as TRIOC_GETSYSCALLFILTER / TRIOC_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_syscallfilter_s to get
+ * current syscall trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_syscallfilter_s of the
+ * new syscall trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_trace_syscallfilter(struct note_trace_syscallfilter_s *oldf,
+ struct note_trace_syscallfilter_s *newf)
+{
+ irqstate_t irq_mask;
+ int i;
+ int nr;
+
+ irq_mask = enter_critical_section();
+
+ if (newf != NULL)
{
- /* Copy the next byte at the tail index */
+ sched_note_trace_stop();
- *buffer++ = g_note_info.ni_buffer[tail];
+ /* Replace the syscall filter mask by the provided setting */
- /* Adjust indices and counts */
+ memset(g_note_filter.syscall_mask, 0,
+ sizeof g_note_filter.syscall_mask);
+ for (i = 0; i < newf->nr_syscalls; i++)
+ {
+ nr = newf->syscalls[i];
- tail = note_next(tail, 1);
- remaining--;
+ if (nr < SYS_nsyscalls)
+ {
+ BITMASK_SET(g_note_filter.syscall_mask, nr);
Review comment:
Providing the interface like fd_set used by select() ? It seems good.
##########
File path: sched/sched/sched_note.c
##########
@@ -631,144 +924,337 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
}
#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
/****************************************************************************
- * Name: sched_note_get
+ * Name: sched_note_trace_start
*
* Description:
- * Remove the next note from the tail of the circular buffer. The note
- * is also removed from the circular buffer to make room for further notes.
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
*
* Input Parameters:
- * buffer - Location to return the next note
- * buflen - The length of the user provided buffer.
+ * None
*
* Returned Value:
- * On success, the positive, non-zero length of the return note is
- * provided. Zero is returned only if the circular buffer is empty. A
- * negated errno value is returned in the event of any failure.
+ * None
*
****************************************************************************/
-#ifdef CONFIG_SCHED_NOTE_GET
-ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen)
+void sched_note_trace_start(void)
{
- FAR struct note_common_s *note;
- irqstate_t flags;
- unsigned int remaining;
- unsigned int tail;
- ssize_t notelen;
- size_t circlen;
+ irqstate_t irq_mask;
- DEBUGASSERT(buffer != NULL);
- flags = enter_critical_section();
+ irq_mask = enter_critical_section();
- /* Verify that the circular buffer is not empty */
+ g_note_filter.enable = true;
+#ifdef NOTE_FEATURE_TASK_NAME
+ note_taskname_init();
+#endif
- circlen = note_length();
- if (circlen <= 0)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
{
- notelen = 0;
- goto errout_with_csection;
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
}
- /* Get the index to the tail of the circular buffer */
+ leave_critical_section(irq_mask);
+}
- tail = g_note_info.ni_tail;
- DEBUGASSERT(tail < CONFIG_SCHED_NOTE_BUFSIZE);
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- /* Get the length of the note at the tail index */
+void sched_note_trace_stop(void)
+{
+ irqstate_t irq_mask;
- note = (FAR struct note_common_s *)&g_note_info.ni_buffer[tail];
- notelen = note->nc_length;
- DEBUGASSERT(notelen <= circlen);
+ irq_mask = enter_critical_section();
- /* Is the user buffer large enough to hold the note? */
+ g_note_filter.enable = false;
- if (buflen < notelen)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufrewind != NULL)
{
- /* Remove the large note so that we do not get constipated. */
+ (g_sched_note_stream)->bufrewind(g_sched_note_stream);
+ }
+
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- note_remove();
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
+ struct note_trace_mode_s *newm)
+{
+ irqstate_t irq_mask;
- /* and return an error */
+ irq_mask = enter_critical_section();
- notelen = -EFBIG;
- goto errout_with_csection;
+ if (newm != NULL)
+ {
+ sched_note_trace_stop();
+ g_note_filter.mode = *newm;
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
+ {
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
+ }
}
- /* Loop until the note has been transferred to the user buffer */
+ if (oldm != NULL)
+ {
+ *oldm = g_note_filter.mode;
+ }
- remaining = (unsigned int)notelen;
- while (remaining > 0)
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_syscallfilter
+ *
+ * Description:
+ * Set and get syscall trace filter setting
+ * (Same as TRIOC_GETSYSCALLFILTER / TRIOC_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_syscallfilter_s to get
+ * current syscall trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_syscallfilter_s of the
+ * new syscall trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_trace_syscallfilter(struct note_trace_syscallfilter_s *oldf,
+ struct note_trace_syscallfilter_s *newf)
+{
+ irqstate_t irq_mask;
+ int i;
+ int nr;
+
+ irq_mask = enter_critical_section();
+
+ if (newf != NULL)
{
- /* Copy the next byte at the tail index */
+ sched_note_trace_stop();
- *buffer++ = g_note_info.ni_buffer[tail];
+ /* Replace the syscall filter mask by the provided setting */
- /* Adjust indices and counts */
+ memset(g_note_filter.syscall_mask, 0,
+ sizeof g_note_filter.syscall_mask);
+ for (i = 0; i < newf->nr_syscalls; i++)
+ {
+ nr = newf->syscalls[i];
- tail = note_next(tail, 1);
- remaining--;
+ if (nr < SYS_nsyscalls)
+ {
+ BITMASK_SET(g_note_filter.syscall_mask, nr);
+ }
+ }
}
- g_note_info.ni_tail = tail;
+ /* Count the number of filtered syscalls to calculate the requied size to
+ * get the current setting.
+ */
-errout_with_csection:
- leave_critical_section(flags);
- return notelen;
+ nr = 0;
+ for (i = 0; i < SYS_nsyscalls; i++)
+ {
+ if (BITMASK_CHECK(g_note_filter.syscall_mask, i))
+ {
+ nr++;
+ }
+ }
+
+ if (oldf != NULL)
+ {
+ /* Return the current filter setting */
+
+ int *p = oldf->syscalls;
+ int s = oldf->size;
+ oldf->nr_syscalls = nr;
+
+ for (i = 0; i < SYS_nsyscalls; i++)
+ {
+ if (p == NULL || s <= sizeof(int))
+ {
+ break;
+ }
+
+ if (BITMASK_CHECK(g_note_filter.syscall_mask, i))
+ {
+ *p++ = i;
+ s -= sizeof(int);
+ }
+ }
+ }
+
+ leave_critical_section(irq_mask);
}
#endif
/****************************************************************************
- * Name: sched_note_size
+ * Name: sched_note_trace_irqfilter
*
* Description:
- * Return the size of the next note at the tail of the circular buffer.
+ * Set and get IRQ trace filter setting
+ * (Same as TRIOC_GETIRQFILTER / TRIOC_SETIRQFILTER ioctls)
*
* Input Parameters:
- * None.
+ * oldf - A writable pointer to struct note_trace_irqfilter_s to get
+ * current IRQ trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_irqfilter_s of the new
+ * IRQ trace filter setting
+ * If 0, the setting is not updated.
*
* Returned Value:
- * Zero is returned if the circular buffer is empty. Otherwise, the size
- * of the next note is returned.
+ * None
*
****************************************************************************/
-#ifdef CONFIG_SCHED_NOTE_GET
-ssize_t sched_note_size(void)
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+void sched_note_trace_irqfilter(struct note_trace_irqfilter_s *oldf,
+ struct note_trace_irqfilter_s *newf)
{
- FAR struct note_common_s *note;
- irqstate_t flags;
- unsigned int tail;
- ssize_t notelen;
- size_t circlen;
+ irqstate_t irq_mask;
+ int i;
+ int nr;
- flags = enter_critical_section();
+ irq_mask = enter_critical_section();
- /* Verify that the circular buffer is not empty */
+ if (newf != NULL)
+ {
+ sched_note_trace_stop();
+
+ /* Replace the IRQ filter mask by the provided setting */
+
+ memset(g_note_filter.irqhandler_mask, 0,
+ sizeof g_note_filter.irqhandler_mask);
Review comment:
Same as syscallfiter.
##########
File path: sched/sched/sched_note.c
##########
@@ -631,144 +924,337 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
}
#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
/****************************************************************************
- * Name: sched_note_get
+ * Name: sched_note_trace_start
*
* Description:
- * Remove the next note from the tail of the circular buffer. The note
- * is also removed from the circular buffer to make room for further notes.
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
*
* Input Parameters:
- * buffer - Location to return the next note
- * buflen - The length of the user provided buffer.
+ * None
*
* Returned Value:
- * On success, the positive, non-zero length of the return note is
- * provided. Zero is returned only if the circular buffer is empty. A
- * negated errno value is returned in the event of any failure.
+ * None
*
****************************************************************************/
-#ifdef CONFIG_SCHED_NOTE_GET
-ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen)
+void sched_note_trace_start(void)
{
- FAR struct note_common_s *note;
- irqstate_t flags;
- unsigned int remaining;
- unsigned int tail;
- ssize_t notelen;
- size_t circlen;
+ irqstate_t irq_mask;
- DEBUGASSERT(buffer != NULL);
- flags = enter_critical_section();
+ irq_mask = enter_critical_section();
- /* Verify that the circular buffer is not empty */
+ g_note_filter.enable = true;
+#ifdef NOTE_FEATURE_TASK_NAME
+ note_taskname_init();
+#endif
- circlen = note_length();
- if (circlen <= 0)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
{
- notelen = 0;
- goto errout_with_csection;
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
}
- /* Get the index to the tail of the circular buffer */
+ leave_critical_section(irq_mask);
+}
- tail = g_note_info.ni_tail;
- DEBUGASSERT(tail < CONFIG_SCHED_NOTE_BUFSIZE);
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- /* Get the length of the note at the tail index */
+void sched_note_trace_stop(void)
+{
+ irqstate_t irq_mask;
- note = (FAR struct note_common_s *)&g_note_info.ni_buffer[tail];
- notelen = note->nc_length;
- DEBUGASSERT(notelen <= circlen);
+ irq_mask = enter_critical_section();
- /* Is the user buffer large enough to hold the note? */
+ g_note_filter.enable = false;
- if (buflen < notelen)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufrewind != NULL)
{
- /* Remove the large note so that we do not get constipated. */
+ (g_sched_note_stream)->bufrewind(g_sched_note_stream);
+ }
+
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- note_remove();
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
+ struct note_trace_mode_s *newm)
+{
+ irqstate_t irq_mask;
- /* and return an error */
+ irq_mask = enter_critical_section();
- notelen = -EFBIG;
- goto errout_with_csection;
+ if (newm != NULL)
+ {
+ sched_note_trace_stop();
+ g_note_filter.mode = *newm;
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
+ {
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
+ }
}
- /* Loop until the note has been transferred to the user buffer */
+ if (oldm != NULL)
+ {
+ *oldm = g_note_filter.mode;
+ }
- remaining = (unsigned int)notelen;
- while (remaining > 0)
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_syscallfilter
+ *
+ * Description:
+ * Set and get syscall trace filter setting
+ * (Same as TRIOC_GETSYSCALLFILTER / TRIOC_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_syscallfilter_s to get
+ * current syscall trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_syscallfilter_s of the
+ * new syscall trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_trace_syscallfilter(struct note_trace_syscallfilter_s *oldf,
+ struct note_trace_syscallfilter_s *newf)
+{
+ irqstate_t irq_mask;
+ int i;
+ int nr;
+
+ irq_mask = enter_critical_section();
+
+ if (newf != NULL)
{
- /* Copy the next byte at the tail index */
+ sched_note_trace_stop();
- *buffer++ = g_note_info.ni_buffer[tail];
+ /* Replace the syscall filter mask by the provided setting */
- /* Adjust indices and counts */
+ memset(g_note_filter.syscall_mask, 0,
+ sizeof g_note_filter.syscall_mask);
+ for (i = 0; i < newf->nr_syscalls; i++)
+ {
+ nr = newf->syscalls[i];
- tail = note_next(tail, 1);
- remaining--;
+ if (nr < SYS_nsyscalls)
+ {
+ BITMASK_SET(g_note_filter.syscall_mask, nr);
+ }
+ }
}
- g_note_info.ni_tail = tail;
+ /* Count the number of filtered syscalls to calculate the requied size to
+ * get the current setting.
+ */
-errout_with_csection:
- leave_critical_section(flags);
- return notelen;
+ nr = 0;
+ for (i = 0; i < SYS_nsyscalls; i++)
+ {
+ if (BITMASK_CHECK(g_note_filter.syscall_mask, i))
+ {
+ nr++;
+ }
+ }
+
+ if (oldf != NULL)
Review comment:
Oops, I got it.
##########
File path: sched/sched/sched_note.c
##########
@@ -631,144 +924,337 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter)
}
#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+
/****************************************************************************
- * Name: sched_note_get
+ * Name: sched_note_trace_start
*
* Description:
- * Remove the next note from the tail of the circular buffer. The note
- * is also removed from the circular buffer to make room for further notes.
+ * Start task tracing
+ * (Same as TRIOC_START ioctl)
*
* Input Parameters:
- * buffer - Location to return the next note
- * buflen - The length of the user provided buffer.
+ * None
*
* Returned Value:
- * On success, the positive, non-zero length of the return note is
- * provided. Zero is returned only if the circular buffer is empty. A
- * negated errno value is returned in the event of any failure.
+ * None
*
****************************************************************************/
-#ifdef CONFIG_SCHED_NOTE_GET
-ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen)
+void sched_note_trace_start(void)
{
- FAR struct note_common_s *note;
- irqstate_t flags;
- unsigned int remaining;
- unsigned int tail;
- ssize_t notelen;
- size_t circlen;
+ irqstate_t irq_mask;
- DEBUGASSERT(buffer != NULL);
- flags = enter_critical_section();
+ irq_mask = enter_critical_section();
- /* Verify that the circular buffer is not empty */
+ g_note_filter.enable = true;
+#ifdef NOTE_FEATURE_TASK_NAME
+ note_taskname_init();
+#endif
- circlen = note_length();
- if (circlen <= 0)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
{
- notelen = 0;
- goto errout_with_csection;
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
}
- /* Get the index to the tail of the circular buffer */
+ leave_critical_section(irq_mask);
+}
- tail = g_note_info.ni_tail;
- DEBUGASSERT(tail < CONFIG_SCHED_NOTE_BUFSIZE);
+/****************************************************************************
+ * Name: sched_note_trace_stop
+ *
+ * Description:
+ * Stop task tracing
+ * (Same as TRIOC_STOP ioctl)
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- /* Get the length of the note at the tail index */
+void sched_note_trace_stop(void)
+{
+ irqstate_t irq_mask;
- note = (FAR struct note_common_s *)&g_note_info.ni_buffer[tail];
- notelen = note->nc_length;
- DEBUGASSERT(notelen <= circlen);
+ irq_mask = enter_critical_section();
- /* Is the user buffer large enough to hold the note? */
+ g_note_filter.enable = false;
- if (buflen < notelen)
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufrewind != NULL)
{
- /* Remove the large note so that we do not get constipated. */
+ (g_sched_note_stream)->bufrewind(g_sched_note_stream);
+ }
+
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_mode
+ *
+ * Description:
+ * Set and get task trace mode.
+ * (Same as TRIOC_GETMODE / TRIOC_SETMODE ioctls)
+ *
+ * Input Parameters:
+ * oldm - A writable pointer to struct note_trace_mode_s to get current
+ * trace mode
+ * If 0, no data is written.
+ * newm - A read-only pointer to struct note_trace_mode_s which holds the
+ * new trace mode
+ * If 0, the trace mode is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
- note_remove();
+void sched_note_trace_mode(struct note_trace_mode_s *oldm,
+ struct note_trace_mode_s *newm)
+{
+ irqstate_t irq_mask;
- /* and return an error */
+ irq_mask = enter_critical_section();
- notelen = -EFBIG;
- goto errout_with_csection;
+ if (newm != NULL)
+ {
+ sched_note_trace_stop();
+ g_note_filter.mode = *newm;
+ if (g_sched_note_stream != NULL &&
+ g_sched_note_stream->bufclear != NULL)
+ {
+ (g_sched_note_stream)->bufclear(g_sched_note_stream,
+ (g_note_filter.mode.flag & TRIOC_MODE_FLAG_ONESHOT) != 0);
+ }
}
- /* Loop until the note has been transferred to the user buffer */
+ if (oldm != NULL)
+ {
+ *oldm = g_note_filter.mode;
+ }
- remaining = (unsigned int)notelen;
- while (remaining > 0)
+ leave_critical_section(irq_mask);
+}
+
+/****************************************************************************
+ * Name: sched_note_trace_syscallfilter
+ *
+ * Description:
+ * Set and get syscall trace filter setting
+ * (Same as TRIOC_GETSYSCALLFILTER / TRIOC_SETSYSCALLFILTER ioctls)
+ *
+ * Input Parameters:
+ * oldf - A writable pointer to struct note_trace_syscallfilter_s to get
+ * current syscall trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_syscallfilter_s of the
+ * new syscall trace filter setting
+ * If 0, the setting is not updated.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+void sched_note_trace_syscallfilter(struct note_trace_syscallfilter_s *oldf,
+ struct note_trace_syscallfilter_s *newf)
+{
+ irqstate_t irq_mask;
+ int i;
+ int nr;
+
+ irq_mask = enter_critical_section();
+
+ if (newf != NULL)
{
- /* Copy the next byte at the tail index */
+ sched_note_trace_stop();
- *buffer++ = g_note_info.ni_buffer[tail];
+ /* Replace the syscall filter mask by the provided setting */
- /* Adjust indices and counts */
+ memset(g_note_filter.syscall_mask, 0,
+ sizeof g_note_filter.syscall_mask);
+ for (i = 0; i < newf->nr_syscalls; i++)
+ {
+ nr = newf->syscalls[i];
- tail = note_next(tail, 1);
- remaining--;
+ if (nr < SYS_nsyscalls)
+ {
+ BITMASK_SET(g_note_filter.syscall_mask, nr);
+ }
+ }
}
- g_note_info.ni_tail = tail;
+ /* Count the number of filtered syscalls to calculate the requied size to
+ * get the current setting.
+ */
-errout_with_csection:
- leave_critical_section(flags);
- return notelen;
+ nr = 0;
+ for (i = 0; i < SYS_nsyscalls; i++)
+ {
+ if (BITMASK_CHECK(g_note_filter.syscall_mask, i))
+ {
+ nr++;
+ }
+ }
+
+ if (oldf != NULL)
+ {
+ /* Return the current filter setting */
+
+ int *p = oldf->syscalls;
+ int s = oldf->size;
+ oldf->nr_syscalls = nr;
+
+ for (i = 0; i < SYS_nsyscalls; i++)
+ {
+ if (p == NULL || s <= sizeof(int))
+ {
+ break;
+ }
+
+ if (BITMASK_CHECK(g_note_filter.syscall_mask, i))
+ {
+ *p++ = i;
+ s -= sizeof(int);
+ }
+ }
+ }
+
+ leave_critical_section(irq_mask);
}
#endif
/****************************************************************************
- * Name: sched_note_size
+ * Name: sched_note_trace_irqfilter
*
* Description:
- * Return the size of the next note at the tail of the circular buffer.
+ * Set and get IRQ trace filter setting
+ * (Same as TRIOC_GETIRQFILTER / TRIOC_SETIRQFILTER ioctls)
*
* Input Parameters:
- * None.
+ * oldf - A writable pointer to struct note_trace_irqfilter_s to get
+ * current IRQ trace filter setting
+ * If 0, no data is written.
+ * newf - A read-only pointer to struct note_trace_irqfilter_s of the new
+ * IRQ trace filter setting
+ * If 0, the setting is not updated.
*
* Returned Value:
- * Zero is returned if the circular buffer is empty. Otherwise, the size
- * of the next note is returned.
+ * None
*
****************************************************************************/
-#ifdef CONFIG_SCHED_NOTE_GET
-ssize_t sched_note_size(void)
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+void sched_note_trace_irqfilter(struct note_trace_irqfilter_s *oldf,
+ struct note_trace_irqfilter_s *newf)
{
- FAR struct note_common_s *note;
- irqstate_t flags;
- unsigned int tail;
- ssize_t notelen;
- size_t circlen;
+ irqstate_t irq_mask;
+ int i;
+ int nr;
- flags = enter_critical_section();
+ irq_mask = enter_critical_section();
- /* Verify that the circular buffer is not empty */
+ if (newf != NULL)
+ {
+ sched_note_trace_stop();
+
+ /* Replace the IRQ filter mask by the provided setting */
+
+ memset(g_note_filter.irqhandler_mask, 0,
+ sizeof g_note_filter.irqhandler_mask);
+ for (i = 0; i < newf->nr_irqs; i++)
+ {
+ nr = newf->irqs[i];
+
+ if (nr < NR_IRQS)
+ {
+ BITMASK_SET(g_note_filter.irqhandler_mask, nr);
+ }
+ }
+ }
+
+ /* Count the number of filtered IRQs to calculate the requied size to get
+ * the current setting.
+ */
+
+ nr = 0;
+ for (i = 0; i < NR_IRQS; i++)
+ {
+ if (BITMASK_CHECK(g_note_filter.irqhandler_mask, i))
+ {
+ nr++;
+ }
+ }
- circlen = note_length();
- if (circlen <= 0)
+ if (oldf != NULL)
Review comment:
Same as syscallfilter.
##########
File path: sched/sched/sched_note_buffer.c
##########
@@ -0,0 +1,471 @@
+/****************************************************************************
+ * sched/sched/sched_note_buffer.c
Review comment:
If merged, the exiting sched note interface `sched_note_get()` is moved into note_driver. Is it OK?
##########
File path: sched/sched/sched_note.c
##########
@@ -73,41 +127,37 @@ static void note_add(FAR const uint8_t *note, uint8_t notelen);
* Private Data
****************************************************************************/
-static struct note_info_s g_note_info;
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+static struct note_filter_s g_note_filter;
Review comment:
I understood the situation you want to use.
How about preparing new kernel configuration such as SCHED_INSTRUMENTATION_FILTER_DEFAULT ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465485896
##########
File path: include/nuttx/syslog/note_driver.h
##########
@@ -27,22 +27,283 @@
#include <nuttx/config.h>
+#include <sys/types.h>
+#include <stdint.h>
+
+#include <nuttx/sched_note.h>
+#include <nuttx/fs/ioctl.h>
+
#ifdef CONFIG_SCHED_INSTRUMENTATION
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
+/* IOCTL Commands ***********************************************************/
+
+/* TRIOC_START
+ * - Start task tracing
+ * Argument: Ignored
+ * TRIOC_STOP
+ * - Stop task tracing
+ * Argument: Ignored
+ * TRIOC_GETMODE
+ * - Get task trace mode
+ * Argument: A writable pointer to struct note_trace_mode_s
+ * TRIOC_SETMODE
+ * - Set task trace mode
+ * Argument: A read-only pointer to struct note_trace_mode_s
+ * TRIOC_GETSYSCALLFILTER
+ * - Get syscall trace filter setting
+ * Argument: A writable pointer to struct
+ * note_trace_syscallfilter_s
+ * TRIOC_SETSYSCALLFILTER
+ * - Set syscall trace filter setting
+ * Argument: A read-only pointer to struct
+ * note_trace_syscallfilter_s
+ * TRIOC_GETIRQFILTER
+ * - Get IRQ trace filter setting
+ * Argument: A writable pointer to struct
+ * note_trace_irqfilter_s
+ * TRIOC_SETIRQFILTER
+ * - Set IRQ trace filter setting
+ * Argument: A read-only pointer to struct
+ * note_trace_irqfilter_s
+ * TRIOC_GETTASKNAME
+ * - Get task name string
+ * Argument: A writable pointer to struct note_get_taskname_s
+ * Result: If -ESRCH, the corresponding task name doesn't
+ * exist.
+ */
+
+#if defined(CONFIG_SCHED_INSTRUMENTATION_FILTER) && defined(CONFIG_DRIVER_NOTE)
+
+#define _NOTETRACEBASE (0xfe00)
+#define _NOTETRACEIOCVALID(c) (_IOC_TYPE(c) == _NOTETRACEBASE)
+#define _NOTETRACEIOC(nr) _IOC(_NOTETRACEBASE, nr)
+
+#define TRIOC_START _NOTETRACEIOC(0x01)
+#define TRIOC_STOP _NOTETRACEIOC(0x02)
+#define TRIOC_GETMODE _NOTETRACEIOC(0x03)
+#define TRIOC_SETMODE _NOTETRACEIOC(0x04)
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+#define TRIOC_GETSYSCALLFILTER _NOTETRACEIOC(0x05)
+#define TRIOC_SETSYSCALLFILTER _NOTETRACEIOC(0x06)
+#endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#define TRIOC_GETIRQFILTER _NOTETRACEIOC(0x07)
+#define TRIOC_SETIRQFILTER _NOTETRACEIOC(0x08)
+#endif
+#ifdef NOTE_FEATURE_TASK_NAME
+#define TRIOC_GETTASKNAME _NOTETRACEIOC(0x09)
+#endif
+
+#endif
+
/****************************************************************************
* Public Types
****************************************************************************/
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
Review comment:
You mean that syslog/note_driver.h should contain only /dev/note specific definitions and the definitions which are used by both note_driver.c and sched_note.c should be in nuttx/sched_note.h ?
I got it.
##########
File path: sched/sched/sched_note.c
##########
@@ -186,149 +236,159 @@ void note_spincommon(FAR struct tcb_s *tcb,
#endif
/****************************************************************************
- * Name: note_length
+ * Name: note_add
*
* Description:
- * Length of data currently in circular buffer.
+ * Put the variable length note to the note stream
*
* Input Parameters:
- * None
+ * note - Pointer to the note to be added
+ * notelen - Length of the note
*
* Returned Value:
- * Length of data currently in circular buffer.
+ * None
+ *
+ * Assumptions:
+ * We are within a critical section.
*
****************************************************************************/
-#if defined(CONFIG_SCHED_NOTE_GET) || defined(CONFIG_DEBUG_ASSERTIONS)
-static unsigned int note_length(void)
+static void note_add(FAR const uint8_t *note, uint8_t notelen)
{
- unsigned int head = g_note_info.ni_head;
- unsigned int tail = g_note_info.ni_tail;
+#ifdef CONFIG_SMP
+ /* Ignore notes that are not in the set of monitored CPUs */
- if (tail > head)
+ if ((CONFIG_SCHED_INSTRUMENTATION_CPUSET & (1 << this_cpu())) == 0)
{
- head += CONFIG_SCHED_NOTE_BUFSIZE;
+ /* Not in the set of monitored CPUs. Do not log the note. */
+
+ return;
}
+#endif
- return head - tail;
-}
+#ifdef CONFIG_SMP
+ irqstate_t flags = up_irq_save();
+ spin_lock_wo_note(&g_note_lock);
#endif
+ /* Loop until all bytes have been transferred */
+
+ if (g_sched_note_stream != NULL)
+ {
+ while (notelen > 0)
+ {
+ (g_sched_note_stream)->public.put(&g_sched_note_stream->public,
Review comment:
Previously I have proposed the similar solution
https://lists.apache.org/x/thread.html/r61db41b432470e43f1c63d4401b6b74b1ec7f9e4a1b3f50df6d71c7d@%3Cdev.nuttx.apache.org%3E
and have been recommended to use stream interface.
https://lists.apache.org/x/thread.html/rdbc397be6e54783cf3b3433afae06c3068f8526b0bf6318d6431847a@%3Cdev.nuttx.apache.org%3E
Then how about adding write() like interface to outstream which Greg had recommended in the latter mail?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-683584722
@YuuichiNakamura let's wait @patacongo feedback, can you review #1520 and give your feedback too?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-688038947
@xiaoxiang781216 Thank you!
At first, would you rebase feature/syscall-instrumentation branch to the master?
Then I'll force push my latest patches into the branch.
Yes, I also considered to provide a separate device for filter control. I already prepared it.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465485597
##########
File path: drivers/syslog/note_driver.c
##########
@@ -79,10 +107,29 @@ static const struct file_operations note_fops =
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: note_open
+ ****************************************************************************/
+
+#ifdef NOTE_FEATURE_BUFFER_REWINDABLE
+static int note_open(FAR struct file *filep)
+{
+ if ((filep->f_oflags & O_ACCMODE) != O_WRONLY)
+ {
+ /* Reset the pointer to read the trace data */
+
+ sched_note_trace_stop();
Review comment:
Different to the existing sched_note feature, in task trace support, the period to collect the note (trace) data and to dump the trace result are completely separated.
While dumping the trace result the trace recording is stopped because task activity itself to dump the result affects the trace data.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r453418790
##########
File path: arch/arm/src/common/arm_initialize.c
##########
@@ -28,7 +28,7 @@
#include <nuttx/arch.h>
#include <nuttx/board.h>
-#include <nuttx/sched_note.h>
+#include <nuttx/syslog/note_driver.h>
Review comment:
The major complexity come from:
1.Redesign the buffer management
2.New IOCTL for the note driver
For these changes, we need push to feature branch before it's ready. But other change is either the bug fix or compatible enhancement, we can directly merge into mainline after review.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-661693218
@xiaoxiang781216 How about my changes?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-692523356
@YuuichiNakamura sorry for delay, the note source code restrucuture is done, could you rebase your patch again? I have updated feature/syscall-instrumentation as the master.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-688021749
@YuuichiNakamura could you rebase your patch? also do you consider to use a separated device node to control the filter?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r458532816
##########
File path: include/nuttx/sched_note.h
##########
@@ -256,15 +267,31 @@ struct note_syscall_leave_s
struct note_irqhandler_s
{
struct note_common_s nih_cmn; /* Common note parameters */
- int nih_irq; /* IRQ number */
+ uint8_t nih_irq; /* IRQ number */
Review comment:
The change for note_irqhandler_s, note_syscall_leave_s and note_syscall_enter_s could move to another patch and apply to the master directly.
##########
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:
Nobody use this variable?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-693106984
> @v01d Thank you for advice. The document for this feature https://github.com/YuuichiNakamura/nuttx-task-tracer-doc is outdated and needs update. I'll refine to rst format and try to integrate into Documentation folder later.
That is some very nice looking documentation. The rest of the documentation will be jelous :wink:
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r494847227
##########
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:
After search the code base, all arch use the same name(nsyscalls), so we can use tcb->xcpt.nsyscalls directly which simplify the code a lot.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r494847227
##########
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:
After search the code base, all arch use the same name(nsyscalls), so we can use tcb->xcpt.nsyscalls directly which simplify the code a lot and avoid the code dup.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r458822696
##########
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:
sched_note_syscall_enter() and _leave() use it.
It is used to exclude the nested system call in the trace data (ex. sched_lock/unlock called by another kernel APIs)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-683516063
@xiaoxiang781216 I'm very sorry for late reply.
I have force pushed the branch to follow the comment below:
https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465485517
https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465485896
https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465486190
https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465486220
https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465486260
https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465486273
Should I wait for the further updates until #1520 is merged?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r484884535
##########
File path: arch/arm/src/common/arm_initialize.c
##########
@@ -163,6 +164,10 @@ void up_initialize(void)
note_register(); /* Non-standard /dev/note */
#endif
+#if defined(CONFIG_DRIVER_NOTECTL)
+ notectl_register(); /* Non-standard /dev/notectl */
Review comment:
with the PR: https://github.com/apache/incubator-nuttx/pull/1731
we can move notectl_register to drivers/note/note_driver.c, to avoid touch the arch file every time the new transport or filter drivrer is added.
##########
File path: drivers/note/note_driver.c
##########
@@ -384,6 +472,69 @@ static ssize_t note_read(FAR struct file *filep, FAR char *buffer,
return retlen;
}
+/****************************************************************************
+ * Name: note_ioctl
+ ****************************************************************************/
+
+static int note_ioctl(struct file *filep, int cmd, unsigned long arg)
Review comment:
@YuuichiNakamura could you review PR https://github.com/apache/incubator-nuttx/pull/1731? which make the driver structure more clean.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465485517
##########
File path: drivers/syslog/note_driver.c
##########
@@ -138,6 +185,231 @@ static ssize_t note_read(FAR struct file *filep, FAR char *buffer,
sched_unlock();
return retlen;
}
+#endif
+
+/****************************************************************************
+ * Name: note_write
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+static ssize_t note_write(FAR struct file *filep, FAR const char *buffer,
Review comment:
It is the remaining part when I had removed sched_note_message support commented at https://github.com/apache/incubator-nuttx/pull/1377#discussion_r451710712 .
I'll remove note_write completely.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r498873881
##########
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:
I am wondering why we must couple syscall and irq trace. Can we only check syscall flag/mask in note_isenabled_syscall and irq flag/mask in note_isenabled_irqhandler? If the tool(e.g. tracecompass) can't handle the complex case correctly, the tool in userspace can filter out the combination with the command line option.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r453342052
##########
File path: arch/arm/src/common/arm_initialize.c
##########
@@ -28,7 +28,7 @@
#include <nuttx/arch.h>
#include <nuttx/board.h>
-#include <nuttx/sched_note.h>
+#include <nuttx/syslog/note_driver.h>
Review comment:
Move just bofore line 37?
##########
File path: arch/arm/src/common/arm_initialize.c
##########
@@ -28,7 +28,7 @@
#include <nuttx/arch.h>
#include <nuttx/board.h>
-#include <nuttx/sched_note.h>
+#include <nuttx/syslog/note_driver.h>
Review comment:
And it's better to send this patch for master:
Fix note driver initialization
So we can merge the general change first and reduce the review complexity.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-702581808
@YuuichiNakamura, @xiaoxiang781216 this is looking really nice. Is there any documentation with the setup instructions that I can begin testing with?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-703276571
@YuuichiNakamura this PR is ready to merge now, how about you change the PR base on the master branch? So we can merge to the master directly.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-698958024
@xiaoxiang781216 I have updated the commit to follow https://github.com/apache/incubator-nuttx/pull/1377#discussion_r494852014 and https://github.com/apache/incubator-nuttx/pull/1377#discussion_r494864249 .
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-668406404
@xiaoxiang781216 How about my changes? I think I have answered all your comments.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r494847227
##########
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:
After search the code base, all arch use the same name(nsyscalls), so we use tcb->xcpt.nsyscalls directly which simplify the code a lot.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-703453416
> > @xiaoxiang781216 I have updated the commend and made additional fix.
> > > @YuuichiNakamura this PR is ready to merge now, how about you change the PR base on the master branch? So we can merge to the master directly.
> >
> >
> > Then may I push the same patches on the master branch and close this PR?
>
> You can try to rebase your local branch to master and push the change again. If it work, we can reuse this PR, otherwise you have to create a new one.
I got. I'll try it.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-662177893
Sorry. I should have informed you it is ready for review.
I think 09296d5 should be here because it depends on 5b96c1e in feature/syscall-instrumentation branch.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-654653350
- The reason of "Check" failure is the design of syscall.h.
- I have already fixed two "Build" failures in the latest commit 0b49bee . But the build process for the latest code doesn't run yet.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-692719567
> Also could you integrate your document into the new document system? it will be helpful for people setup the environment.
Yes, please do. I was waiting for the "dust to settle" on this change to mention that. I understand this involves documenting an API so please add it to "Architecture API" inside the reference. If you think there should also be some guide that exceeds the API reference, you can add one under "Guides"
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499395012
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +137,162 @@ 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.flag & NOTE_FILTER_MODE_FLAG_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
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int note_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* Exclude the case of syscall called by the interrupt handler which is
+ * not traced.
+ */
+
+ if (up_interrupt_context())
+ {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ if (g_note_disabled_irq_nest[cpu] > 0)
+ {
+ return false;
+ }
+#else
+ return false;
+#endif
+ }
+
+ /* If the syscall trace is disabled or the syscall number is masked,
+ * do nothing.
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) ||
+ NOTE_FILTER_SYSCALLMASK_ISSET(nr - CONFIG_SYS_RESERVED,
+ &g_note_filter.syscall_mask))
+ {
+ return false;
+ }
+#endif
+
+ return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: note_isenabled_irqhandler
+ *
+ * Description:
+ * Check whether the interrupt handler instrumentation is enabled.
+ *
+ * Input Parameters:
+ * irq - IRQ number
+ * enter - interrupt enter/leave flag
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+static inline int note_isenabled_irq(int irq, bool enter)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* If the IRQ trace is disabled or the IRQ number is masked, disable
+ * subsequent syscall traces until leaving the interrupt handler
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) ||
+ NOTE_FILTER_IRQMASK_ISSET(irq, &g_note_filter.irq_mask))
+ {
+ irqstate_t irq_mask;
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ irq_mask = enter_critical_section();
Review comment:
But the initial irq is in disable state when cpu jump to the irq handler, onlye the code provided by irq_attach have the chance to re-enable irq.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-693315240
@YuuichiNakamura - Thank you for this great contribution. @xiaoxiang781216 Thank you for your efforts as well. Once the docs are done I would be happy to test the set up.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r498801563
##########
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:
> The same reason to https://github.com/apache/incubator-nuttx/pull/1377/files#r498625054
> tracecompass cannot handle the syscall in interrupt handler correctly.
>
> So I'll take the similar solution to nested syscalls. Remove the syscall inside the interrupt handler by app code.
>
> But I'll not record the syscall when:
>
> * The syscall is issued in the interrupt handler
> * But the handling IRQ is not traced
>
> Because in such case we cannot distinguish whether the syscall is called in the app code or interrupt handler.
But if we turn off IRQ trace in the first place, how tracecompass detect whether syscall happen inside irq context or not?
> > * But the handling IRQ is not traced
>
> @xiaoxiang781216 It seems that there are no way to get the IRQ number currently executing inside the interrupt handler.
> sched_note_syscall_enter/leave() cannot decide whether the syscall should be recorded or not in the interrupt handler.
> So we have to leave this as it is.
The post process tool could maintain a count which:
1. Increase the count in the record of sched_note_irq_enter
2. Decrease the count in the reord of sched_note_irq_leave
3. Filter out the syscall if the count isn't equal to zero
Yes, the user can disable all or partial irq trace, but tracecompass should handle the syscall record correctly if there isn't irq related record between them, regardless wether it's due to either no irq happen at all or irq is filtered by user.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499415368
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +137,162 @@ 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.flag & NOTE_FILTER_MODE_FLAG_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
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int note_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* Exclude the case of syscall called by the interrupt handler which is
+ * not traced.
+ */
+
+ if (up_interrupt_context())
+ {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ if (g_note_disabled_irq_nest[cpu] > 0)
+ {
+ return false;
+ }
+#else
+ return false;
+#endif
+ }
+
+ /* If the syscall trace is disabled or the syscall number is masked,
+ * do nothing.
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) ||
+ NOTE_FILTER_SYSCALLMASK_ISSET(nr - CONFIG_SYS_RESERVED,
+ &g_note_filter.syscall_mask))
+ {
+ return false;
+ }
+#endif
+
+ return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: note_isenabled_irqhandler
+ *
+ * Description:
+ * Check whether the interrupt handler instrumentation is enabled.
+ *
+ * Input Parameters:
+ * irq - IRQ number
+ * enter - interrupt enter/leave flag
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+static inline int note_isenabled_irq(int irq, bool enter)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* If the IRQ trace is disabled or the IRQ number is masked, disable
+ * subsequent syscall traces until leaving the interrupt handler
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) ||
+ NOTE_FILTER_IRQMASK_ISSET(irq, &g_note_filter.irq_mask))
+ {
+ irqstate_t irq_mask;
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ irq_mask = enter_critical_section();
Review comment:
Yes, no do_irq in arch code will re-enable interrupt, that's why I say only irq handle can enable irq manually now. irq_dispatch is the better place to enable irq before call irq handle but after sched_note_irqhandler in the furture.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-668656385
@YuuichiNakamura sorry for the late reply, I am busy to review the internal module in recent weeks. Since we will have many transport implementation, does it make sense to move these files:
arch/sim/src/sim/up_schednote.c
drivers/syslog/note_driver.c
sched/sched/sched_note_buffer.c
to a new drivers folder(e.g.):
drivers/note/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499416834
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +137,162 @@ 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.flag & NOTE_FILTER_MODE_FLAG_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
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int note_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* Exclude the case of syscall called by the interrupt handler which is
+ * not traced.
+ */
+
+ if (up_interrupt_context())
+ {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ if (g_note_disabled_irq_nest[cpu] > 0)
+ {
+ return false;
+ }
+#else
+ return false;
+#endif
+ }
+
+ /* If the syscall trace is disabled or the syscall number is masked,
+ * do nothing.
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) ||
+ NOTE_FILTER_SYSCALLMASK_ISSET(nr - CONFIG_SYS_RESERVED,
+ &g_note_filter.syscall_mask))
+ {
+ return false;
+ }
+#endif
+
+ return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: note_isenabled_irqhandler
+ *
+ * Description:
+ * Check whether the interrupt handler instrumentation is enabled.
+ *
+ * Input Parameters:
+ * irq - IRQ number
+ * enter - interrupt enter/leave flag
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+static inline int note_isenabled_irq(int irq, bool enter)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* If the IRQ trace is disabled or the IRQ number is masked, disable
+ * subsequent syscall traces until leaving the interrupt handler
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) ||
+ NOTE_FILTER_IRQMASK_ISSET(irq, &g_note_filter.irq_mask))
+ {
+ irqstate_t irq_mask;
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ irq_mask = enter_critical_section();
Review comment:
I got it. Thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-661716175
> @xiaoxiang781216 How about my changes?
Is It ready for review? Sorry, I will look this by today. But first I think this patch can send to master directly:
https://github.com/apache/incubator-nuttx/pull/1377/commits/09296d57b37899e8cf6589c6c08eda9be3476e44
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465485665
##########
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:
From the application developer's point of view, what and when system calls are used is important, but the internal of the system call is not.
And, the most important reason is, the trace viewer "tracecompass" cannot handle nested system call well.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499349844
##########
File path: drivers/note/noteram_driver.c
##########
@@ -413,12 +564,22 @@ void sched_note_add(FAR const void *note, size_t notelen)
FAR const char *buf = note;
unsigned int head;
unsigned int next;
+ irqstate_t flags;
+ flags = up_irq_save();
#ifdef CONFIG_SMP
- irqstate_t flags = up_irq_save();
spin_lock_wo_note(&g_noteram_lock);
#endif
+ if (g_noteram_info.ni_overwrite == NOTERAM_MODE_OVERWRITE_OVERFLOW)
Review comment:
I have also fixed here.
- ni_overwrite flag must be checked in the locked region.
- up_irq_save() is needed in not SMP case.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r498801563
##########
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:
> The same reason to https://github.com/apache/incubator-nuttx/pull/1377/files#r498625054
> tracecompass cannot handle the syscall in interrupt handler correctly.
>
> So I'll take the similar solution to nested syscalls. Remove the syscall inside the interrupt handler by app code.
>
> But I'll not record the syscall when:
>
> * The syscall is issued in the interrupt handler
> * But the handling IRQ is not traced
>
> Because in such case we cannot distinguish whether the syscall is called in the app code or interrupt handler.
But if we turn off IRQ trace in the first place, how tracecompass detect whether syscall happen inside irq context or not?
> > * But the handling IRQ is not traced
>
> @xiaoxiang781216 It seems that there are no way to get the IRQ number currently executing inside the interrupt handler.
> sched_note_syscall_enter/leave() cannot decide whether the syscall should be recorded or not in the interrupt handler.
> So we have to leave this as it is.
The post process tool could maintain a count which:
1. Increase the count in the record of sched_note_irq_enter
2. Decrease the count in the reord of sched_note_irq_leave
3. Filter out the syscall if the count isn't equal to zero
Yes, the user can disable all or partial irq trace, but tracecompass should handle the syscall record correctly if there isn't irq related record between them, regardless wether it's due to either no irq happen at all or irq is filtered by user.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r452558692
##########
File path: include/nuttx/sched_note.h
##########
@@ -391,27 +423,62 @@ ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen);
ssize_t sched_note_size(void);
#endif
+/****************************************************************************
+ * Name: sched_note_buffer_clear
+ *
+ * Description:
+ * Clear all contents of the circular buffer.
+ *
+ * Input Parameters:
+ * oneshot - Oneshot mode flag. If true, the buffer put operation stops
+ * when whole buffer is used.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+#ifdef NOTE_FEATURE_BUFFER_REWINDABLE
+void sched_note_buffer_clear(int oneshot);
Review comment:
It is because the function is in sched_note_buffer.c because it manages the note buffer, but the trace start/stop function in sched_note.c need it.
The reason of sched_note_buffer_rewind() is same, too. (I also think it is not good design...)
The another idea for it is:
- Provide the struct which inherits lib_outstream_s. It contains the function pointers to manage the buffer (clear/rewind).
- sched_note_buffer.c sets the function pointer of sched_note_buffer_clear/rewind() to the new struct.
- sched_note.c calls the function if the pointer is not NULL.
##########
File path: include/nuttx/sched_note.h
##########
@@ -391,27 +423,62 @@ ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen);
ssize_t sched_note_size(void);
#endif
+/****************************************************************************
+ * Name: sched_note_buffer_clear
+ *
+ * Description:
+ * Clear all contents of the circular buffer.
+ *
+ * Input Parameters:
+ * oneshot - Oneshot mode flag. If true, the buffer put operation stops
+ * when whole buffer is used.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+#ifdef NOTE_FEATURE_BUFFER_REWINDABLE
+void sched_note_buffer_clear(int oneshot);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_buffer_rewind
+ *
+ * Description:
+ * Rewind the read pointer of the circular buffer.
+ *
+ * Input Parameters:
+ * None.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+#ifdef NOTE_FEATURE_BUFFER_REWINDABLE
+void sched_note_buffer_rewind(void);
+#endif
+
/****************************************************************************
* Name: note_register
*
* Description:
* Register a serial driver at /dev/note that can be used by an
- * application to read data from the circular not buffer.
+ * application to read data from the circular note buffer.
*
* Input Parameters:
* None.
*
* Returned Value:
- * Zero is returned if the circular buffer is empty. Otherwise, a negated
- * errno value is returned.
+ * Zero on succress. A negated errno value is returned on a failure.
*
****************************************************************************/
-#if defined(CONFIG_SCHED_INSTRUMENTATION_BUFFER) && \
- defined(CONFIG_DRIVER_NOTE)
+#ifdef CONFIG_DRIVER_NOTE
int note_register(void);
Review comment:
I got it.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r452558506
##########
File path: arch/arm/src/common/arm_initialize.c
##########
@@ -159,8 +159,7 @@ void up_initialize(void)
loop_register(); /* Standard /dev/loop */
#endif
-#if defined(CONFIG_SCHED_INSTRUMENTATION_BUFFER) && \
Review comment:
I got it.
##########
File path: drivers/syslog/Kconfig
##########
@@ -64,7 +64,7 @@ endif
config DRIVER_NOTE
bool "Scheduler instrumentation driver"
default n
- depends on SCHED_INSTRUMENTATION_BUFFER && SCHED_NOTE_GET
+ depends on (SCHED_INSTRUMENTATION_BUFFER && SCHED_NOTE_GET) || SCHED_INSTRUMENTATION_FILTER
Review comment:
No. SCHED_INSTRUMENTATION decides only whether note hooks are inserted in the kernel.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-693104819
@xiaoxiang781216 Thank you for your great effort and support.
For application side, I always maintain my app repository to synchronize with changes on kernel side.
(https://github.com/YuuichiNakamura/incubator-nuttx-apps.git feature/task-tracer branch)
I'll issue the PR for it to app repository after this PR is merged.
@v01d Thank you for advice. The document for this feature https://github.com/YuuichiNakamura/nuttx-task-tracer-doc is outdated and needs update. I'll refine to rst format and try to integrate into Documentation folder later.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-663009231
@xiaoxiang781216 Thank you. I have rebased my changes.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r451713748
##########
File path: include/nuttx/sched_note.h
##########
@@ -255,16 +274,28 @@ struct note_syscall_leave_s
struct note_irqhandler_s
{
- struct note_common_s nih_cmn; /* Common note parameters */
- int nih_irq; /* IRQ number */
+ struct note_common_s nih_cmn; /* Common note parameters */
+ uint8_t nih_irq; /* IRQ number */
+ uint8_t nih_handler[sizeof(void *)]; /* Address of the interrupt handler */
Review comment:
It's better to move nih_handler related change to another patch, so we can merge it to mainline directly. Also please don't forget to update the apps/system/sched_note/.
##########
File path: include/nuttx/sched_note.h
##########
@@ -391,27 +423,62 @@ ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen);
ssize_t sched_note_size(void);
#endif
+/****************************************************************************
+ * Name: sched_note_buffer_clear
+ *
+ * Description:
+ * Clear all contents of the circular buffer.
+ *
+ * Input Parameters:
+ * oneshot - Oneshot mode flag. If true, the buffer put operation stops
+ * when whole buffer is used.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+#ifdef NOTE_FEATURE_BUFFER_REWINDABLE
+void sched_note_buffer_clear(int oneshot);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_buffer_rewind
+ *
+ * Description:
+ * Rewind the read pointer of the circular buffer.
+ *
+ * Input Parameters:
+ * None.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+#ifdef NOTE_FEATURE_BUFFER_REWINDABLE
+void sched_note_buffer_rewind(void);
+#endif
+
/****************************************************************************
* Name: note_register
*
* Description:
* Register a serial driver at /dev/note that can be used by an
- * application to read data from the circular not buffer.
+ * application to read data from the circular note buffer.
*
* Input Parameters:
* None.
*
* Returned Value:
- * Zero is returned if the circular buffer is empty. Otherwise, a negated
- * errno value is returned.
+ * Zero on succress. A negated errno value is returned on a failure.
*
****************************************************************************/
-#if defined(CONFIG_SCHED_INSTRUMENTATION_BUFFER) && \
- defined(CONFIG_DRIVER_NOTE)
+#ifdef CONFIG_DRIVER_NOTE
int note_register(void);
Review comment:
move to note_driver.h?
##########
File path: include/nuttx/sched_note.h
##########
@@ -348,12 +379,13 @@ void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
# define sched_note_irqhandler(i,h,e)
#endif
+void sched_note_message(FAR const char *fmt, ...);
Review comment:
It's better to move sched_note_message related change to another patch, so we can merge it to mainline directly. Also please don't forget to update the apps/system/sched_note/.
##########
File path: include/nuttx/sched_note.h
##########
@@ -236,17 +248,24 @@ struct note_spinlock_s
#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
/* This is the specific form of the NOTE_SYSCALL_ENTER/LEAVE notes */
+#define MAX_SYSCALL_ARGS 6
+#define SIZEOF_NOTE_SYSCALL_ENTER(n) (sizeof(struct note_common_s) + \
+ sizeof(uint8_t) + sizeof(uint8_t) + \
+ (sizeof(uintptr_t) * (n)))
+
struct note_syscall_enter_s
{
- struct note_common_s nsc_cmn; /* Common note parameters */
- int nsc_nr; /* System call number */
+ struct note_common_s nsc_cmn; /* Common note parameters */
+ uint8_t nsc_nr; /* System call number */
+ uint8_t nsc_argc; /* Number of system call arguments */
+ uint8_t nsc_args[sizeof(uintptr_t) * MAX_SYSCALL_ARGS]; /* System call arguments */
};
struct note_syscall_leave_s
{
- struct note_common_s nsc_cmn; /* Common note parameters */
- uintptr_t nsc_result; /* Result of the system call */
- int nsc_nr; /* System call number */
+ struct note_common_s nsc_cmn; /* Common note parameters */
+ uint8_t nsc_nr; /* System call number */
+ uint8_t nsc_result[sizeof(uintptr_t)]; /* Result of the system call */
Review comment:
It's better to move arg/result saving change to another patch, so we can merge it to mainline directly. Also please don't forget to update the apps/system/sched_note/.
##########
File path: drivers/syslog/Kconfig
##########
@@ -64,7 +64,7 @@ endif
config DRIVER_NOTE
bool "Scheduler instrumentation driver"
default n
- depends on SCHED_INSTRUMENTATION_BUFFER && SCHED_NOTE_GET
+ depends on (SCHED_INSTRUMENTATION_BUFFER && SCHED_NOTE_GET) || SCHED_INSTRUMENTATION_FILTER
Review comment:
Is it enough to depend on SCHED_INSTRUMENTATION?
##########
File path: include/nuttx/sched_note.h
##########
@@ -391,27 +423,62 @@ ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen);
ssize_t sched_note_size(void);
#endif
+/****************************************************************************
+ * Name: sched_note_buffer_clear
+ *
+ * Description:
+ * Clear all contents of the circular buffer.
+ *
+ * Input Parameters:
+ * oneshot - Oneshot mode flag. If true, the buffer put operation stops
+ * when whole buffer is used.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+#ifdef NOTE_FEATURE_BUFFER_REWINDABLE
+void sched_note_buffer_clear(int oneshot);
Review comment:
Why we expose the internal function?
##########
File path: include/nuttx/sched_note_trace.h
##########
@@ -0,0 +1,311 @@
+/****************************************************************************
+ * include/nuttx/sched_note_trace.h
Review comment:
Change to note_driver.h and put to include/nuttx/syslog/
##########
File path: include/nuttx/sched_note.h
##########
@@ -391,27 +423,62 @@ ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen);
ssize_t sched_note_size(void);
#endif
+/****************************************************************************
+ * Name: sched_note_buffer_clear
+ *
+ * Description:
+ * Clear all contents of the circular buffer.
+ *
+ * Input Parameters:
+ * oneshot - Oneshot mode flag. If true, the buffer put operation stops
+ * when whole buffer is used.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+#ifdef NOTE_FEATURE_BUFFER_REWINDABLE
+void sched_note_buffer_clear(int oneshot);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_buffer_rewind
+ *
+ * Description:
+ * Rewind the read pointer of the circular buffer.
+ *
+ * Input Parameters:
+ * None.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+#ifdef NOTE_FEATURE_BUFFER_REWINDABLE
+void sched_note_buffer_rewind(void);
Review comment:
Why we expose the internal function?
##########
File path: include/nuttx/sched_note.h
##########
@@ -391,27 +423,62 @@ ssize_t sched_note_get(FAR uint8_t *buffer, size_t buflen);
ssize_t sched_note_size(void);
#endif
+/****************************************************************************
+ * Name: sched_note_buffer_clear
+ *
+ * Description:
+ * Clear all contents of the circular buffer.
+ *
+ * Input Parameters:
+ * oneshot - Oneshot mode flag. If true, the buffer put operation stops
+ * when whole buffer is used.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+#ifdef NOTE_FEATURE_BUFFER_REWINDABLE
+void sched_note_buffer_clear(int oneshot);
+#endif
+
+/****************************************************************************
+ * Name: sched_note_buffer_rewind
+ *
+ * Description:
+ * Rewind the read pointer of the circular buffer.
+ *
+ * Input Parameters:
+ * None.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+#ifdef NOTE_FEATURE_BUFFER_REWINDABLE
+void sched_note_buffer_rewind(void);
Review comment:
We should make sched_note_size and sched_note_get as static function too.
##########
File path: arch/arm/src/common/arm_initialize.c
##########
@@ -159,8 +159,7 @@ void up_initialize(void)
loop_register(); /* Standard /dev/loop */
#endif
-#if defined(CONFIG_SCHED_INSTRUMENTATION_BUFFER) && \
Review comment:
Other arch also need change too.
##########
File path: drivers/syslog/note_driver.c
##########
@@ -41,34 +41,62 @@
#include <sys/types.h>
#include <sched.h>
+#include <fcntl.h>
#include <assert.h>
#include <errno.h>
+#include <string.h>
#include <nuttx/sched_note.h>
+#include <nuttx/sched_note_trace.h>
Review comment:
Should move the addition of sched_note_trace.h from commit "Add task trace support" to here.
Is the name note_driver.h better than sched_note_trace.h? and put it to nuttx/syslog.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465490743
##########
File path: sched/sched/sched_note.c
##########
@@ -370,6 +437,10 @@ void sched_note_start(FAR struct tcb_s *tcb)
strncpy(note.nsa_name, tcb->name, CONFIG_TASK_NAME_SIZE + 1);
length = SIZEOF_NOTE_START(namelen + 1);
+
+#ifdef NOTE_FEATURE_TASK_NAME
+ note_taskname_put(tcb->pid, tcb->name);
Review comment:
Sorry, my comment didn't answer to your question.
It is because only note_start_s has the task name information. We can observe the task name only for newly created tasks and cannot for existing tasks.
Adding task name member to note_suspend_s and note_resume_s solves it but it increases the trace data size considerably because the task name is recorded for each context switches.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r493667049
##########
File path: drivers/note/noteram_driver.c
##########
@@ -326,6 +395,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();
+
+ /* Reset the read index of the circular buffer */
+
+ g_noteram_info.ni_read = g_noteram_info.ni_tail;
Review comment:
It doesn't lose the information. Just rewinding the read pointer.
Previously the tail pointer of the circular buffer goes forward in noteram_get(). The data in the circular buffer could be read from /dev/note only once.
I have added new read pointer and made it to use instead of tail pointer in noteram_get(). So when we reopen /dev/note, we can get the same data.
The tail pointer is modified only when an overwrite in the circular buffer occurs.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499410087
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +137,162 @@ 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.flag & NOTE_FILTER_MODE_FLAG_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
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int note_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* Exclude the case of syscall called by the interrupt handler which is
+ * not traced.
+ */
+
+ if (up_interrupt_context())
+ {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ if (g_note_disabled_irq_nest[cpu] > 0)
+ {
+ return false;
+ }
+#else
+ return false;
+#endif
+ }
+
+ /* If the syscall trace is disabled or the syscall number is masked,
+ * do nothing.
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) ||
+ NOTE_FILTER_SYSCALLMASK_ISSET(nr - CONFIG_SYS_RESERVED,
+ &g_note_filter.syscall_mask))
+ {
+ return false;
+ }
+#endif
+
+ return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: note_isenabled_irqhandler
+ *
+ * Description:
+ * Check whether the interrupt handler instrumentation is enabled.
+ *
+ * Input Parameters:
+ * irq - IRQ number
+ * enter - interrupt enter/leave flag
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+static inline int note_isenabled_irq(int irq, bool enter)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* If the IRQ trace is disabled or the IRQ number is masked, disable
+ * subsequent syscall traces until leaving the interrupt handler
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) ||
+ NOTE_FILTER_IRQMASK_ISSET(irq, &g_note_filter.irq_mask))
+ {
+ irqstate_t irq_mask;
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ irq_mask = enter_critical_section();
Review comment:
By referring https://cwiki.apache.org/confluence/display/NUTTX/Nested+Interrupts, it seems that irq_dispatch() is called with interrupt-enabled when nested interrupts are supported.
But I didn't find a concrete example because almost every arch doesn't support the nested interrupt...
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-688070489
@xiaoxiang781216 I have force pushed new patches.
- 8f6567a Add new device /dev/notectl for filter control
- a5ff46b Add new configs to follow https://github.com/apache/incubator-nuttx/pull/1377#discussion_r465129927
- Remove task name buffer feature once for simplify the patches. I'll provide the feature by another commit.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-703511681
@YuuichiNakamura it seem that the PR mess up:(.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r498627532
##########
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:
The same reason to https://github.com/apache/incubator-nuttx/pull/1377/files#r498625054
tracecompass cannot handle the syscall in interrupt handler correctly.
So I'll take the similar solution to nested syscalls. Remove the syscall inside the interrupt handler by app code.
But I'll not record the syscall when:
- The syscall is issued in the interrupt handler
- But the handling IRQ is not traced
Because in such case we cannot distinguish whether the syscall is called in the app code or interrupt handler.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-697522699
> > Please add documentation in `Documentaiton` for this new feature
>
> Can I add the documentation after the code is merged?
> I'd like to add it by another commit.
Okay. Can you then open an issue for that and assign it to the 10 milestone? It is important that we document this if it will be part of next release
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r493666745
##########
File path: include/nuttx/note/notectl_driver.h
##########
@@ -0,0 +1,128 @@
+/****************************************************************************
+ * include/nuttx/note/notectl_driver.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_NOTE_NOTECTL_DRIVER_H
+#define __INCLUDE_NUTTX_NOTE_NOTECTL_DRIVER_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <stdint.h>
+
+#include <nuttx/sched_note.h>
+#include <nuttx/fs/ioctl.h>
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* IOCTL Commands ***********************************************************/
+
+/* TRIOC_START
+ * - Start task tracing
+ * Argument: Ignored
+ * TRIOC_STOP
+ * - Stop task tracing
+ * Argument: Ignored
+ * TRIOC_GETMODE
+ * - Get task trace mode
+ * Argument: A writable pointer to struct note_trace_mode_s
+ * TRIOC_SETMODE
+ * - Set task trace mode
+ * Argument: A read-only pointer to struct note_trace_mode_s
+ * TRIOC_GETSYSCALLFILTER
+ * - Get syscall trace filter setting
+ * Argument: A writable pointer to struct
+ * note_trace_syscallfilter_s
+ * TRIOC_SETSYSCALLFILTER
+ * - Set syscall trace filter setting
+ * Argument: A read-only pointer to struct
+ * note_trace_syscallfilter_s
+ * TRIOC_GETIRQFILTER
+ * - Get IRQ trace filter setting
+ * Argument: A writable pointer to struct
+ * note_trace_irqfilter_s
+ * TRIOC_SETIRQFILTER
+ * - Set IRQ trace filter setting
+ * Argument: A read-only pointer to struct
+ * note_trace_irqfilter_s
+ */
+
+#ifdef CONFIG_DRIVER_NOTECTL
+
+#define _NOTECTLBASE (0xfe00)
+#define _NOTECTLIOCVALID(c) (_IOC_TYPE(c) == _NOTECTLBASE)
+#define _NOTECTLIOC(nr) _IOC(_NOTECTLBASE, nr)
Review comment:
I have moved it.
I have added new ioctl base number to the existing ioctls.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r458825091
##########
File path: include/nuttx/sched_note.h
##########
@@ -256,15 +267,31 @@ struct note_syscall_leave_s
struct note_irqhandler_s
{
struct note_common_s nih_cmn; /* Common note parameters */
- int nih_irq; /* IRQ number */
+ uint8_t nih_irq; /* IRQ number */
Review comment:
I got it. I'll create another PR for it to the master.
And create the PR to cherry-pick 02c38e6 5b96c1e 09296d5.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] YuuichiNakamura commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r498665909
##########
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:
Because the filter mode contains the following variables:
1. global instrumentation enable/disable
2. syscall/irq instrumentation enable flag
SCHED_INSTRUMENTATION_FILTER_ENABLE_ONBOOT defines the default value of 1.
SCHED_INSTRUMENTATION_FILTER_ENABLE_ONBOOT_MODE defines 2. (The name may be confusing.)
@xiaoxiang781216 Do you mean that these two variables should be integrated to one mode variable and provide only one configuration which defines the default value of the mode variable?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499379874
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +137,162 @@ 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.flag & NOTE_FILTER_MODE_FLAG_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
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int note_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* Exclude the case of syscall called by the interrupt handler which is
+ * not traced.
+ */
+
+ if (up_interrupt_context())
+ {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ if (g_note_disabled_irq_nest[cpu] > 0)
+ {
+ return false;
+ }
+#else
+ return false;
+#endif
+ }
+
+ /* If the syscall trace is disabled or the syscall number is masked,
+ * do nothing.
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) ||
+ NOTE_FILTER_SYSCALLMASK_ISSET(nr - CONFIG_SYS_RESERVED,
+ &g_note_filter.syscall_mask))
+ {
+ return false;
+ }
+#endif
+
+ return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: note_isenabled_irqhandler
+ *
+ * Description:
+ * Check whether the interrupt handler instrumentation is enabled.
+ *
+ * Input Parameters:
+ * irq - IRQ number
+ * enter - interrupt enter/leave flag
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+static inline int note_isenabled_irq(int irq, bool enter)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* If the IRQ trace is disabled or the IRQ number is masked, disable
+ * subsequent syscall traces until leaving the interrupt handler
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) ||
+ NOTE_FILTER_IRQMASK_ISSET(irq, &g_note_filter.irq_mask))
+ {
+ irqstate_t irq_mask;
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ irq_mask = enter_critical_section();
Review comment:
don't need? the interrupt should already disable in the current state.
##########
File path: sched/Kconfig
##########
@@ -990,6 +990,25 @@ config SCHED_INSTRUMENTATION_IRQHANDLER
void sched_note_irqhandler(int irq, FAR void *handler, bool enter);
+config SCHED_INSTRUMENTATION_FILTER
+ bool "Instrumenation filter"
+ default n
+ ---help---
+ Enables the filter logic for the instrumentation. If this option
+ is enabled, the instrumentation data passed to sched_note_add()
+ can be filtered by syscall and IRQ number.
+ The filter logic can be configured by sched_note_filter APIs defined in
+ include/nuttx/sched_note.h.
+
+config SCHED_INSTRUMENTATION_FILTER_DEFAULT_MODE
+ hex "Default instrumentation filter mode"
+ default 0x7
Review comment:
add depend on SCHED_INSTRUMENTATION_FILTER
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r499415368
##########
File path: sched/sched/sched_note.c
##########
@@ -100,6 +137,162 @@ 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.flag & NOTE_FILTER_MODE_FLAG_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
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SYSCALL
+static inline int note_isenabled_syscall(int nr)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* Exclude the case of syscall called by the interrupt handler which is
+ * not traced.
+ */
+
+ if (up_interrupt_context())
+ {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ if (g_note_disabled_irq_nest[cpu] > 0)
+ {
+ return false;
+ }
+#else
+ return false;
+#endif
+ }
+
+ /* If the syscall trace is disabled or the syscall number is masked,
+ * do nothing.
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_SYSCALL) ||
+ NOTE_FILTER_SYSCALLMASK_ISSET(nr - CONFIG_SYS_RESERVED,
+ &g_note_filter.syscall_mask))
+ {
+ return false;
+ }
+#endif
+
+ return true;
+}
+#endif
+
+/****************************************************************************
+ * Name: note_isenabled_irqhandler
+ *
+ * Description:
+ * Check whether the interrupt handler instrumentation is enabled.
+ *
+ * Input Parameters:
+ * irq - IRQ number
+ * enter - interrupt enter/leave flag
+ *
+ * Returned Value:
+ * True is returned if the instrumentation is enabled.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER
+static inline int note_isenabled_irq(int irq, bool enter)
+{
+#ifdef CONFIG_SCHED_INSTRUMENTATION_FILTER
+ if (!note_isenabled())
+ {
+ return false;
+ }
+
+ /* If the IRQ trace is disabled or the IRQ number is masked, disable
+ * subsequent syscall traces until leaving the interrupt handler
+ */
+
+ if (!(g_note_filter.mode.flag & NOTE_FILTER_MODE_FLAG_IRQ) ||
+ NOTE_FILTER_IRQMASK_ISSET(irq, &g_note_filter.irq_mask))
+ {
+ irqstate_t irq_mask;
+#ifdef CONFIG_SMP
+ int cpu = this_cpu();
+#else
+ int cpu = 0;
+#endif
+
+ irq_mask = enter_critical_section();
Review comment:
Yes, no do_irq in arch code will re-enable interrupt, that's why I say only irq handle can enable irq manually now. irq_dispatch is the better place to enable irq before call irq handle but after sched_note_irqhandler in the future:
```
irq_disaptch(int irq)
{
sched_note_irqhandler(irq, true);
irq_enable();
g_irqvect[irq]();
irq_disable();
sched_note_irqhandler(irq, false);
}
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r498810165
##########
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:
How about we change enable field to a flag part of flag field? so we can control the initial state by one Kconfig option.
##########
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:
> In this case ni_head should not be updated. Because an incomplete note struct is recorded in the circular buffer if updated. If not updated, the incompletely recorded note will be simply discarded.
>
Ok.
> But, I found another problem in this case.
> If the buffer have 16bytes free space, and 20bytes note and 12bytes note arrives sequentially, the former incoming note is discarded but only the latter note is recorded. This is an unexpected behavior.
> I think that we need to add the flag to control whether we record the incoming note or not.
> When in not overwrite mode, once we found an incoming note which is greater than the free space, disable the flag to block the following notes.
maybe we can change ni_overwrite to tristate(disable, enable, overflow).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#issuecomment-662238621
@YuuichiNakamura these patches is ready to merge:
```
commit 02c38e61f402c6d19206ad4ac3e0b788cceab2bc (apache/feature/syscall-instrumentation)
Author: Nakamura, Yuuichi <Yu...@sony.com>
Date: Tue Jun 16 12:16:17 2020 +0900
Syscall instrumentation build system support for Arm and RISC-V
commit 5b96c1ec8804893e8b33ff2d878a904c322bee10
Author: Nakamura, Yuuichi <Yu...@sony.com>
Date: Tue Jun 16 12:16:05 2020 +0900
Create wrapper library for system call instrumentation
From 09296d57b37899e8cf6589c6c08eda9be3476e44 Mon Sep 17 00:00:00 2001
From: "Nakamura, Yuuichi" <Yu...@sony.com>
Date: Tue, 7 Jul 2020 09:30:40 +0900
Subject: [PATCH] Add g_funcnames declaration in syscall.h
```
Since there is some conflict, could you help cherrry-pick to master?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1377: Add task trace support
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1377:
URL: https://github.com/apache/incubator-nuttx/pull/1377#discussion_r494847227
##########
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:
After search the code base, all arch use the same name(nsyscalls), so we can use tcb->xcpt.nsyscalls directly which simplify the code a lot and avoid the dup code.
----------------------------------------------------------------
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:
users@infra.apache.org