You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/03/30 14:54:13 UTC
[GitHub] [incubator-nuttx] anchao opened a new pull request #5915: sched/note: add support of trace section mark
anchao opened a new pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915
## Summary
sched/note: add support of trace section mark
The implementation of this feature is based on android systrace:
https://source.android.com/devices/tech/debug/ftrace
Application developers are more concerned about the performance of
the specified application section,
added two APIs to implement performance measurement:
void sched_note_begin(FAR const char *str);
void sched_note_end(FAR const char *str);
or
SCHED_NOTE_BEGIN(); /* defined to sched_note_begin(__FUNCTION__) */
SCHED_NOTE_END(); /* defined to sched_note_end(__FUNCTION__) */
Signed-off-by: chao.an <an...@xiaomi.com>
## Impact
N/A
## Testing
https://source.android.com/devices/tech/debug/ftrace
https://github.com/catapult-project/catapult/
test code:
```
void ccc(void)
{
SCHED_NOTE_BEGIN();
usleep(10);
SCHED_NOTE_END();
}
void bbb(void)
{
SCHED_NOTE_BEGIN();
ccc();
SCHED_NOTE_END();
}
void aaa(void)
{
SCHED_NOTE_BEGIN();
bbb();
SCHED_NOTE_END();
}
int main(int argc, FAR char *argv[])
{
while (1)
{
SCHED_NOTE_BEGIN();
aaa();
SCHED_NOTE_END();
}
return 0;
}
```
## How To Use
https://nuttx.apache.org/docs/latest/guides/tasktraceuser.html
1. enable sched instrumentation
```
CONFIG_DRIVER_NOTERAM=y
...
CONFIG_SCHED_INSTRUMENTATION=y
CONFIG_SCHED_INSTRUMENTATION_SWITCH=y
CONFIG_SCHED_INSTRUMENTATION_SYSCALL=y
CONFIG_SCHED_INSTRUMENTATION_IRQHANDLER=y
CONFIG_SCHED_INSTRUMENTATION_DUMP=y
```
2. Capture sched note info
```
nsh> trace switch +
nsh> trace syscall +
nsh> trace start
nsh> trace dump
```
3. convert to systrace
https://source.android.com/devices/tech/debug/ftrace
If you captured your systrace with --no-compress, this is in the html file in the section beginning with BEGIN TRACE.
If not, run html2trace from the [Catapult tree](https://github.com/catapult-project/catapult/tree/master/) (tracing/bin/html2trace) to uncompress the trace.
```
$ git clone git@github.com:catapult-project/catapult.git
$ cd catapult/
$ ./tracing/bin/trace2html trace.txt
trace.html
```
[systrace.txt](https://github.com/apache/incubator-nuttx/files/8381565/systrace.txt)
4. open the trace html via web browser:
`$ google-chrome trace.html`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r839709765
##########
File path: sched/sched/sched_note.c
##########
@@ -107,6 +107,67 @@ static unsigned int g_note_disabled_irq_nest[CONFIG_SMP_NCPUS];
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: sched_note_format
+ *
+ * Description:
+ * swap the data to unified format
+ *
+ ****************************************************************************/
+
+static inline void sched_note_format(FAR uint8_t *dst,
Review comment:
Done
##########
File path: sched/sched/sched_note.c
##########
@@ -107,6 +107,67 @@ static unsigned int g_note_disabled_irq_nest[CONFIG_SMP_NCPUS];
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: sched_note_format
Review comment:
Done
##########
File path: sched/sched/sched_note.c
##########
@@ -144,25 +205,15 @@ static void note_common(FAR struct tcb_s *tcb,
#ifdef CONFIG_SMP
Review comment:
Done
##########
File path: sched/sched/sched_note.c
##########
@@ -107,6 +107,67 @@ static unsigned int g_note_disabled_irq_nest[CONFIG_SMP_NCPUS];
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: sched_note_format
+ *
+ * Description:
+ * swap the data to unified format
+ *
+ ****************************************************************************/
+
+static inline void sched_note_format(FAR uint8_t *dst,
+ FAR void *src, size_t len)
+{
+ uint64_t u64;
+ uint32_t u32;
+ uint16_t u16;
+ uint8_t u8;
+
+ switch (len)
+ {
+ case 8:
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r840368303
##########
File path: include/nuttx/sched_note.h
##########
@@ -105,16 +105,38 @@
memset((s), 0, sizeof(struct note_filter_irq_s))
#endif
-/* Note dump module tag definitions */
-
#ifdef CONFIG_SCHED_INSTRUMENTATION_DUMP
-# define NOTE_MODULE(a, b, c, d) \
- ((uint32_t)((a) & 0xff) | \
- ((uint32_t)((b) & 0xff) << 8) | \
- ((uint32_t)((c) & 0xff) << 16) | \
- ((uint32_t)((d) & 0xff) << 24))
+# define SCHED_NOTE_LABEL__(x, y) x ## y
+# define SCHED_NOTE_LABEL_(x, y) SCHED_NOTE_LABEL__(x, y)
+# define SCHED_NOTE_LABEL \
+ SCHED_NOTE_LABEL_(sched_note_here, __LINE__)
+# define SCHED_NOTE_IP \
+ ({SCHED_NOTE_LABEL: (uintptr_t)&&SCHED_NOTE_LABEL;})
Review comment:
I mean that `&&label` itself is a GNU extension.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#issuecomment-1086625249
Ok, it's good ideal. let's give it's a general name, because getting the current instruction is very useful in other debugging 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#issuecomment-1086249983
> @xiaoxiang781216 I think that the merge was done too early.
Sorry, I would think that all issues have been addressed.
> I'm not sure if SDCC or other non-GCC compilers supports GNU extensions.
It isn't a problem at least it doesn't break the build even the compiler(e.g. SDCC) doesn't support the extension, because the special usage only exist in a macro. If you don't really use and expend the macro, all is fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r839765941
##########
File path: sched/sched/sched_note.c
##########
@@ -789,18 +800,8 @@ void sched_note_syscall_enter(int nr, int argc, ...)
for (i = 0; i < argc; i++)
{
arg = (uintptr_t)va_arg(ap, uintptr_t);
- *args++ = (uint8_t)(arg & 0xff);
- *args++ = (uint8_t)((arg >> 8) & 0xff);
-#if UINTPTR_MAX > UINT16_MAX
- *args++ = (uint8_t)((arg >> 16) & 0xff);
- *args++ = (uint8_t)((arg >> 24) & 0xff);
-#if UINTPTR_MAX > UINT32_MAX
- *args++ = (uint8_t)((arg >> 32) & 0xff);
- *args++ = (uint8_t)((arg >> 40) & 0xff);
- *args++ = (uint8_t)((arg >> 48) & 0xff);
- *args++ = (uint8_t)((arg >> 56) & 0xff);
-#endif
-#endif
+ sched_note_flatten(args, &arg, sizeof(uintptr_t));
Review comment:
Done
##########
File path: sched/sched/sched_note.c
##########
@@ -825,20 +826,9 @@ void sched_note_syscall_leave(int nr, uintptr_t result)
note_common(tcb, ¬e.nsc_cmn, sizeof(struct note_syscall_leave_s),
NOTE_SYSCALL_LEAVE);
DEBUGASSERT(nr <= UCHAR_MAX);
- note.nsc_nr = nr;
-
- note.nsc_result[0] = (uint8_t)(result & 0xff);
- note.nsc_result[1] = (uint8_t)((result >> 8) & 0xff);
-#if UINTPTR_MAX > UINT16_MAX
- note.nsc_result[2] = (uint8_t)((result >> 16) & 0xff);
- note.nsc_result[3] = (uint8_t)((result >> 24) & 0xff);
-#if UINTPTR_MAX > UINT32_MAX
- note.nsc_result[4] = (uint8_t)((result >> 32) & 0xff);
- note.nsc_result[5] = (uint8_t)((result >> 40) & 0xff);
- note.nsc_result[6] = (uint8_t)((result >> 48) & 0xff);
- note.nsc_result[7] = (uint8_t)((result >> 56) & 0xff);
-#endif
-#endif
+ note.nsc_nr = nr;
+
+ sched_note_flatten(note.nsc_result, &result, sizeof(uintptr_t));
Review comment:
Done
##########
File path: sched/sched/sched_note.c
##########
@@ -419,20 +443,8 @@ static void note_spincommon(FAR struct tcb_s *tcb,
note_common(tcb, ¬e.nsp_cmn, sizeof(struct note_spinlock_s), type);
- note.nsp_spinlock[0] = (uint8_t)((uintptr_t)spinlock & 0xff);
- note.nsp_spinlock[1] = (uint8_t)(((uintptr_t)spinlock >> 8) & 0xff);
-#if UINTPTR_MAX > UINT16_MAX
- note.nsp_spinlock[2] = (uint8_t)(((uintptr_t)spinlock >> 16) & 0xff);
- note.nsp_spinlock[3] = (uint8_t)(((uintptr_t)spinlock >> 24) & 0xff);
-#if UINTPTR_MAX > UINT32_MAX
- note.nsp_spinlock[4] = (uint8_t)(((uintptr_t)spinlock >> 32) & 0xff);
- note.nsp_spinlock[5] = (uint8_t)(((uintptr_t)spinlock >> 40) & 0xff);
- note.nsp_spinlock[6] = (uint8_t)(((uintptr_t)spinlock >> 48) & 0xff);
- note.nsp_spinlock[7] = (uint8_t)(((uintptr_t)spinlock >> 56) & 0xff);
-#endif
-#endif
-
- note.nsp_value = (uint8_t)*spinlock;
+ sched_note_flatten(note.nsp_spinlock, &spinlock, sizeof(uintptr_t));
Review comment:
Done
##########
File path: sched/sched/sched_note.c
##########
@@ -133,7 +167,7 @@ static void note_common(FAR struct tcb_s *tcb,
clock_systime_timespec(&ts);
#else
- uint32_t systime = (uint32_t)clock_systime_ticks();
+ clock_t systime = (clock_t)clock_systime_ticks();
Review comment:
Done
##########
File path: include/nuttx/sched_note.h
##########
@@ -113,8 +113,29 @@
((uint32_t)((b) & 0xff) << 8) | \
((uint32_t)((c) & 0xff) << 16) | \
((uint32_t)((d) & 0xff) << 24))
+
+# define SCHED_NOTE_IP \
+ ({__label__ __sched_note_here; __sched_note_here: (uintptr_t)&&__sched_note_here;})
Review comment:
Done
##########
File path: include/nuttx/sched_note.h
##########
@@ -188,11 +188,11 @@ struct note_common_s
#ifdef CONFIG_SMP
uint8_t nc_cpu; /* CPU thread/task running on */
#endif
- uint8_t nc_pid[2]; /* ID of the thread/task */
+ uint8_t nc_pid[sizeof(pid_t)]; /* ID of the thread/task */
#ifdef CONFIG_SCHED_INSTRUMENTATION_HIRES
- uint8_t nc_systime_sec[4]; /* Time when note was buffered (sec) */
- uint8_t nc_systime_nsec[4]; /* Time when note was buffered (nsec) */
+ uint8_t nc_systime_sec[sizeof(time_t)]; /* Time when note was buffered (sec) */
+ uint8_t nc_systime_nsec[sizeof(long)]; /* Time when note was buffered (nsec) */
#else
uint8_t nc_systime[4]; /* Time when note was buffered */
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r840001219
##########
File path: include/nuttx/sched_note.h
##########
@@ -105,16 +105,32 @@
memset((s), 0, sizeof(struct note_filter_irq_s))
#endif
-/* Note dump module tag definitions */
-
#ifdef CONFIG_SCHED_INSTRUMENTATION_DUMP
-# define NOTE_MODULE(a, b, c, d) \
- ((uint32_t)((a) & 0xff) | \
- ((uint32_t)((b) & 0xff) << 8) | \
- ((uint32_t)((c) & 0xff) << 16) | \
- ((uint32_t)((d) & 0xff) << 24))
+# define SCHED_NOTE_IP \
+ ({__label__ sched_note_here; sched_note_here: (uintptr_t)&&sched_note_here;})
Review comment:
I think `__label__` is a GCC extension only. What about other compilers?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#issuecomment-1086181845
@xiaoxiang781216 I think that the merge was done too early. I'm not sure if SDCC or other non-GCC compilers supports GNU extensions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r840368865
##########
File path: include/nuttx/sched_note.h
##########
@@ -105,16 +105,38 @@
memset((s), 0, sizeof(struct note_filter_irq_s))
#endif
-/* Note dump module tag definitions */
-
#ifdef CONFIG_SCHED_INSTRUMENTATION_DUMP
-# define NOTE_MODULE(a, b, c, d) \
- ((uint32_t)((a) & 0xff) | \
- ((uint32_t)((b) & 0xff) << 8) | \
- ((uint32_t)((c) & 0xff) << 16) | \
- ((uint32_t)((d) & 0xff) << 24))
+# define SCHED_NOTE_LABEL__(x, y) x ## y
+# define SCHED_NOTE_LABEL_(x, y) SCHED_NOTE_LABEL__(x, y)
+# define SCHED_NOTE_LABEL \
+ SCHED_NOTE_LABEL_(sched_note_here, __LINE__)
+# define SCHED_NOTE_IP \
+ ({SCHED_NOTE_LABEL: (uintptr_t)&&SCHED_NOTE_LABEL;})
Review comment:
I think I know why. Because we need a unique value
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#issuecomment-1086249983
> @xiaoxiang781216 I think that the merge was done too early.
Sorry, I would think that all issue has been addressed.
> I'm not sure if SDCC or other non-GCC compilers supports GNU extensions.
It isn't a problem at least it doesn't break the build even the compiler(e.g. SDCC) doesn't support the extension, because the special usage only exist in a macro. If you don't really use and expend the macro, all is fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#issuecomment-1086621014
Np, but I will issue a PR and move `SCHED_NOTE_IP` to `compiler.h` and rename to `uniqueid_get`. Just to keep compiler dependent implementation in a single place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r840367589
##########
File path: include/nuttx/sched_note.h
##########
@@ -105,16 +105,38 @@
memset((s), 0, sizeof(struct note_filter_irq_s))
#endif
-/* Note dump module tag definitions */
-
#ifdef CONFIG_SCHED_INSTRUMENTATION_DUMP
-# define NOTE_MODULE(a, b, c, d) \
- ((uint32_t)((a) & 0xff) | \
- ((uint32_t)((b) & 0xff) << 8) | \
- ((uint32_t)((c) & 0xff) << 16) | \
- ((uint32_t)((d) & 0xff) << 24))
+# define SCHED_NOTE_LABEL__(x, y) x ## y
+# define SCHED_NOTE_LABEL_(x, y) SCHED_NOTE_LABEL__(x, y)
+# define SCHED_NOTE_LABEL \
+ SCHED_NOTE_LABEL_(sched_note_here, __LINE__)
+# define SCHED_NOTE_IP \
+ ({SCHED_NOTE_LABEL: (uintptr_t)&&SCHED_NOTE_LABEL;})
Review comment:
This is a good improvement, but still I have a question why
```
# define SCHED_NOTE_STRING(buf) \
sched_note_string(__LINE__, buf)
```
should not work? I mean why do we need `&&`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r839710250
##########
File path: sched/sched/sched_note.c
##########
@@ -107,6 +107,67 @@ static unsigned int g_note_disabled_irq_nest[CONFIG_SMP_NCPUS];
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: sched_note_format
+ *
+ * Description:
+ * swap the data to unified format
+ *
+ ****************************************************************************/
+
+static inline void sched_note_format(FAR uint8_t *dst,
+ FAR void *src, size_t len)
+{
+ uint64_t u64;
+ uint32_t u32;
+ uint16_t u16;
+ uint8_t u8;
+
+ switch (len)
+ {
+ case 8:
+ {
+ u64 = *(uint64_t *)src;
+
+ dst[7] = (uint8_t)((u64 >> 56) & 0xff);
+ dst[6] = (uint8_t)((u64 >> 48) & 0xff);
+ dst[5] = (uint8_t)((u64 >> 40) & 0xff);
+ dst[4] = (uint8_t)((u64 >> 32) & 0xff);
+
+ u32 = u64 & 0xffffffff;
+ src = &u32;
+ }
+
+ case 4:
+ {
+ u32 = *(uint32_t *)src;
+
+ dst[3] = (uint8_t)((u32 >> 24) & 0xff);
+ dst[2] = (uint8_t)((u32 >> 16) & 0xff);
+
+ u16 = u32 & 0xffff;
+ src = &u16;
+ }
+
+ case 2:
+ {
+ u16 = *(uint16_t *)src;
+
+ dst[1] = (uint8_t)((u16 >> 8) & 0xff);
+
+ u8 = u16 & 0xff;
+ src = &u8;
+ }
+
+ case 1:
+ {
+ u8 = *(uint8_t *)src;
+
+ dst[0] = (uint8_t)(u8 & 0xff);
+ }
+ }
Review comment:
Done
##########
File path: sched/sched/sched_note.c
##########
@@ -107,6 +107,67 @@ static unsigned int g_note_disabled_irq_nest[CONFIG_SMP_NCPUS];
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: sched_note_format
+ *
+ * Description:
+ * swap the data to unified format
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r840203982
##########
File path: include/nuttx/sched_note.h
##########
@@ -105,16 +105,32 @@
memset((s), 0, sizeof(struct note_filter_irq_s))
#endif
-/* Note dump module tag definitions */
-
#ifdef CONFIG_SCHED_INSTRUMENTATION_DUMP
-# define NOTE_MODULE(a, b, c, d) \
- ((uint32_t)((a) & 0xff) | \
- ((uint32_t)((b) & 0xff) << 8) | \
- ((uint32_t)((c) & 0xff) << 16) | \
- ((uint32_t)((d) & 0xff) << 24))
+# define SCHED_NOTE_IP \
+ ({__label__ sched_note_here; sched_note_here: (uintptr_t)&&sched_note_here;})
Review comment:
This seems dose not to work, __label__ can ensure the uniqueness of the current label, it can be guaranteed that the definition will not be duplicate in the code scope:
`: error: duplicate label ‘sched_note_here’`
this practice is very common, you can see that the linux kernel uses this extension a lot
https://github.com/torvalds/linux/blob/master/include/linux/instruction_pointer.h#L6
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r840254423
##########
File path: include/nuttx/sched_note.h
##########
@@ -105,16 +105,38 @@
memset((s), 0, sizeof(struct note_filter_irq_s))
#endif
-/* Note dump module tag definitions */
-
#ifdef CONFIG_SCHED_INSTRUMENTATION_DUMP
-# define NOTE_MODULE(a, b, c, d) \
- ((uint32_t)((a) & 0xff) | \
- ((uint32_t)((b) & 0xff) << 8) | \
- ((uint32_t)((c) & 0xff) << 16) | \
- ((uint32_t)((d) & 0xff) << 24))
+# define SCHED_NOTE_PASTE_(x, y) x ## y
+# define SCHED_NOTE_PASTE(x, y) SCHED_NOTE_PASTE_(x, y)
Review comment:
let's merge SCHED_NOTE_PASTE and SCHED_NOTE_PASTE_ into one:
```
#define SCHED_NOTE_LABEL_(x, y) x ## 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r840370220
##########
File path: include/nuttx/sched_note.h
##########
@@ -105,16 +105,38 @@
memset((s), 0, sizeof(struct note_filter_irq_s))
#endif
-/* Note dump module tag definitions */
-
#ifdef CONFIG_SCHED_INSTRUMENTATION_DUMP
-# define NOTE_MODULE(a, b, c, d) \
- ((uint32_t)((a) & 0xff) | \
- ((uint32_t)((b) & 0xff) << 8) | \
- ((uint32_t)((c) & 0xff) << 16) | \
- ((uint32_t)((d) & 0xff) << 24))
+# define SCHED_NOTE_LABEL__(x, y) x ## y
+# define SCHED_NOTE_LABEL_(x, y) SCHED_NOTE_LABEL__(x, y)
+# define SCHED_NOTE_LABEL \
+ SCHED_NOTE_LABEL_(sched_note_here, __LINE__)
+# define SCHED_NOTE_IP \
+ ({SCHED_NOTE_LABEL: (uintptr_t)&&SCHED_NOTE_LABEL;})
Review comment:
Let me think a bit. Because from compiler compatibility point of view both `__label__` or reworked mens the same if `&&` is used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r840373544
##########
File path: include/nuttx/sched_note.h
##########
@@ -105,16 +105,38 @@
memset((s), 0, sizeof(struct note_filter_irq_s))
#endif
-/* Note dump module tag definitions */
-
#ifdef CONFIG_SCHED_INSTRUMENTATION_DUMP
-# define NOTE_MODULE(a, b, c, d) \
- ((uint32_t)((a) & 0xff) | \
- ((uint32_t)((b) & 0xff) << 8) | \
- ((uint32_t)((c) & 0xff) << 16) | \
- ((uint32_t)((d) & 0xff) << 24))
+# define SCHED_NOTE_LABEL__(x, y) x ## y
+# define SCHED_NOTE_LABEL_(x, y) SCHED_NOTE_LABEL__(x, y)
+# define SCHED_NOTE_LABEL \
+ SCHED_NOTE_LABEL_(sched_note_here, __LINE__)
+# define SCHED_NOTE_IP \
+ ({SCHED_NOTE_LABEL: (uintptr_t)&&SCHED_NOTE_LABEL;})
Review comment:
https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r839744392
##########
File path: include/nuttx/sched_note.h
##########
@@ -113,8 +113,29 @@
((uint32_t)((b) & 0xff) << 8) | \
((uint32_t)((c) & 0xff) << 16) | \
((uint32_t)((d) & 0xff) << 24))
+
+# define SCHED_NOTE_IP \
+ ({__label__ __sched_note_here; __sched_note_here: (uintptr_t)&&__sched_note_here;})
Review comment:
remove __ prefix?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r840213082
##########
File path: include/nuttx/sched_note.h
##########
@@ -105,16 +105,32 @@
memset((s), 0, sizeof(struct note_filter_irq_s))
#endif
-/* Note dump module tag definitions */
-
#ifdef CONFIG_SCHED_INSTRUMENTATION_DUMP
-# define NOTE_MODULE(a, b, c, d) \
- ((uint32_t)((a) & 0xff) | \
- ((uint32_t)((b) & 0xff) << 8) | \
- ((uint32_t)((c) & 0xff) << 16) | \
- ((uint32_t)((d) & 0xff) << 24))
+# define SCHED_NOTE_IP \
+ ({__label__ sched_note_here; sched_note_here: (uintptr_t)&&sched_note_here;})
Review comment:
Improved again, seems works fine.
```
# define _SCHED_NOTE_PASTE(x, y) x ## y
# define SCHED_NOTE_PASTE(x, y) _SCHED_NOTE_PASTE(x, y)
# define SCHED_NOTE_LABEL \
SCHED_NOTE_PASTE(sched_note_here, __LINE__)
# define SCHED_NOTE_IP \
({SCHED_NOTE_LABEL: (uintptr_t)&&SCHED_NOTE_LABEL;})
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r839742024
##########
File path: include/nuttx/sched_note.h
##########
@@ -188,11 +188,11 @@ struct note_common_s
#ifdef CONFIG_SMP
uint8_t nc_cpu; /* CPU thread/task running on */
#endif
- uint8_t nc_pid[2]; /* ID of the thread/task */
+ uint8_t nc_pid[sizeof(pid_t)]; /* ID of the thread/task */
#ifdef CONFIG_SCHED_INSTRUMENTATION_HIRES
- uint8_t nc_systime_sec[4]; /* Time when note was buffered (sec) */
- uint8_t nc_systime_nsec[4]; /* Time when note was buffered (nsec) */
+ uint8_t nc_systime_sec[sizeof(time_t)]; /* Time when note was buffered (sec) */
+ uint8_t nc_systime_nsec[sizeof(long)]; /* Time when note was buffered (nsec) */
#else
uint8_t nc_systime[4]; /* Time when note was buffered */
Review comment:
sizeof(clock_t)
##########
File path: sched/sched/sched_note.c
##########
@@ -144,25 +178,15 @@ static void note_common(FAR struct tcb_s *tcb,
#ifdef CONFIG_SMP
note->nc_cpu = tcb->cpu;
#endif
- note->nc_pid[0] = (uint8_t)(tcb->pid & 0xff);
- note->nc_pid[1] = (uint8_t)((tcb->pid >> 8) & 0xff);
+ sched_note_flatten(note->nc_pid, &tcb->pid, sizeof(tcb->pid));
#ifdef CONFIG_SCHED_INSTRUMENTATION_HIRES
- note->nc_systime_nsec[0] = (uint8_t)(ts.tv_nsec & 0xff);
- note->nc_systime_nsec[1] = (uint8_t)((ts.tv_nsec >> 8) & 0xff);
- note->nc_systime_nsec[2] = (uint8_t)((ts.tv_nsec >> 16) & 0xff);
- note->nc_systime_nsec[3] = (uint8_t)((ts.tv_nsec >> 24) & 0xff);
- note->nc_systime_sec[0] = (uint8_t)(ts.tv_sec & 0xff);
- note->nc_systime_sec[1] = (uint8_t)((ts.tv_sec >> 8) & 0xff);
- note->nc_systime_sec[2] = (uint8_t)((ts.tv_sec >> 16) & 0xff);
- note->nc_systime_sec[3] = (uint8_t)((ts.tv_sec >> 24) & 0xff);
+ sched_note_flatten(note->nc_systime_nsec, &ts.tv_nsec, sizeof(ts.tv_nsec));
+ sched_note_flatten(note->nc_systime_sec, &ts.tv_sec, sizeof(ts.tv_sec));
#else
/* Save the LS 32-bits of the system timer in little endian order */
- note->nc_systime[0] = (uint8_t)(systime & 0xff);
- note->nc_systime[1] = (uint8_t)((systime >> 8) & 0xff);
- note->nc_systime[2] = (uint8_t)((systime >> 16) & 0xff);
- note->nc_systime[3] = (uint8_t)((systime >> 24) & 0xff);
+ sched_note_flatten(note->nc_systime, &systime, sizeof(systime));
Review comment:
change systime to clock_t
##########
File path: include/nuttx/sched_note.h
##########
@@ -188,11 +188,11 @@ struct note_common_s
#ifdef CONFIG_SMP
uint8_t nc_cpu; /* CPU thread/task running on */
#endif
- uint8_t nc_pid[2]; /* ID of the thread/task */
+ uint8_t nc_pid[sizeof(pid_t)]; /* ID of the thread/task */
Review comment:
merge to previous patch
##########
File path: sched/sched/sched_note.c
##########
@@ -144,25 +205,15 @@ static void note_common(FAR struct tcb_s *tcb,
#ifdef CONFIG_SMP
Review comment:
Not change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r840370220
##########
File path: include/nuttx/sched_note.h
##########
@@ -105,16 +105,38 @@
memset((s), 0, sizeof(struct note_filter_irq_s))
#endif
-/* Note dump module tag definitions */
-
#ifdef CONFIG_SCHED_INSTRUMENTATION_DUMP
-# define NOTE_MODULE(a, b, c, d) \
- ((uint32_t)((a) & 0xff) | \
- ((uint32_t)((b) & 0xff) << 8) | \
- ((uint32_t)((c) & 0xff) << 16) | \
- ((uint32_t)((d) & 0xff) << 24))
+# define SCHED_NOTE_LABEL__(x, y) x ## y
+# define SCHED_NOTE_LABEL_(x, y) SCHED_NOTE_LABEL__(x, y)
+# define SCHED_NOTE_LABEL \
+ SCHED_NOTE_LABEL_(sched_note_here, __LINE__)
+# define SCHED_NOTE_IP \
+ ({SCHED_NOTE_LABEL: (uintptr_t)&&SCHED_NOTE_LABEL;})
Review comment:
Let me think a bit. Because from compiler compatibility point of view both `__label__` or reworked mens the same if `&&` is used. I have nothing against the Linux, just want to make sure that compatibility with non GCC compilers will be kept
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r839570395
##########
File path: sched/sched/sched_note.c
##########
@@ -107,6 +107,67 @@ static unsigned int g_note_disabled_irq_nest[CONFIG_SMP_NCPUS];
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: sched_note_format
Review comment:
sched_note_copy
##########
File path: sched/sched/sched_note.c
##########
@@ -107,6 +107,67 @@ static unsigned int g_note_disabled_irq_nest[CONFIG_SMP_NCPUS];
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: sched_note_format
+ *
+ * Description:
+ * swap the data to unified format
Review comment:
Copy the data in the little endian layout
##########
File path: sched/sched/sched_note.c
##########
@@ -107,6 +107,67 @@ static unsigned int g_note_disabled_irq_nest[CONFIG_SMP_NCPUS];
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: sched_note_format
+ *
+ * Description:
+ * swap the data to unified format
+ *
+ ****************************************************************************/
+
+static inline void sched_note_format(FAR uint8_t *dst,
Review comment:
format->copy
##########
File path: sched/sched/sched_note.c
##########
@@ -107,6 +107,67 @@ static unsigned int g_note_disabled_irq_nest[CONFIG_SMP_NCPUS];
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: sched_note_format
+ *
+ * Description:
+ * swap the data to unified format
+ *
+ ****************************************************************************/
+
+static inline void sched_note_format(FAR uint8_t *dst,
+ FAR void *src, size_t len)
+{
+ uint64_t u64;
+ uint32_t u32;
+ uint16_t u16;
+ uint8_t u8;
+
+ switch (len)
+ {
+ case 8:
+ {
+ u64 = *(uint64_t *)src;
+
+ dst[7] = (uint8_t)((u64 >> 56) & 0xff);
+ dst[6] = (uint8_t)((u64 >> 48) & 0xff);
+ dst[5] = (uint8_t)((u64 >> 40) & 0xff);
+ dst[4] = (uint8_t)((u64 >> 32) & 0xff);
+
+ u32 = u64 & 0xffffffff;
+ src = &u32;
+ }
+
+ case 4:
+ {
+ u32 = *(uint32_t *)src;
+
+ dst[3] = (uint8_t)((u32 >> 24) & 0xff);
+ dst[2] = (uint8_t)((u32 >> 16) & 0xff);
+
+ u16 = u32 & 0xffff;
+ src = &u16;
+ }
+
+ case 2:
+ {
+ u16 = *(uint16_t *)src;
+
+ dst[1] = (uint8_t)((u16 >> 8) & 0xff);
+
+ u8 = u16 & 0xff;
+ src = &u8;
+ }
+
+ case 1:
+ {
+ u8 = *(uint8_t *)src;
+
+ dst[0] = (uint8_t)(u8 & 0xff);
+ }
+ }
Review comment:
add:
```
default:
DEBGUASSERT(FALSE);
break;
```
##########
File path: sched/sched/sched_note.c
##########
@@ -144,25 +205,15 @@ static void note_common(FAR struct tcb_s *tcb,
#ifdef CONFIG_SMP
Review comment:
remove the extra space before =
##########
File path: sched/sched/sched_note.c
##########
@@ -419,19 +470,7 @@ static void note_spincommon(FAR struct tcb_s *tcb,
note_common(tcb, ¬e.nsp_cmn, sizeof(struct note_spinlock_s), type);
- note.nsp_spinlock[0] = (uint8_t)((uintptr_t)spinlock & 0xff);
- note.nsp_spinlock[1] = (uint8_t)(((uintptr_t)spinlock >> 8) & 0xff);
-#if UINTPTR_MAX > UINT16_MAX
- note.nsp_spinlock[2] = (uint8_t)(((uintptr_t)spinlock >> 16) & 0xff);
- note.nsp_spinlock[3] = (uint8_t)(((uintptr_t)spinlock >> 24) & 0xff);
-#if UINTPTR_MAX > UINT32_MAX
- note.nsp_spinlock[4] = (uint8_t)(((uintptr_t)spinlock >> 32) & 0xff);
- note.nsp_spinlock[5] = (uint8_t)(((uintptr_t)spinlock >> 40) & 0xff);
- note.nsp_spinlock[6] = (uint8_t)(((uintptr_t)spinlock >> 48) & 0xff);
- note.nsp_spinlock[7] = (uint8_t)(((uintptr_t)spinlock >> 56) & 0xff);
-#endif
-#endif
-
+ sched_note_format(note.nsp_spinlock, &spinlock, sizeof(uintptr_t));
note.nsp_value = (uint8_t)*spinlock;
Review comment:
ditto
##########
File path: sched/sched/sched_note.c
##########
@@ -107,6 +107,67 @@ static unsigned int g_note_disabled_irq_nest[CONFIG_SMP_NCPUS];
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: sched_note_format
+ *
+ * Description:
+ * swap the data to unified format
+ *
+ ****************************************************************************/
+
+static inline void sched_note_format(FAR uint8_t *dst,
+ FAR void *src, size_t len)
+{
+ uint64_t u64;
+ uint32_t u32;
+ uint16_t u16;
+ uint8_t u8;
+
+ switch (len)
+ {
+ case 8:
Review comment:
check CONFIG_HAVE_LONGLONG?
##########
File path: sched/sched/sched_note.c
##########
@@ -107,6 +107,67 @@ static unsigned int g_note_disabled_irq_nest[CONFIG_SMP_NCPUS];
* Private Functions
****************************************************************************/
+/****************************************************************************
+ * Name: sched_note_format
+ *
+ * Description:
+ * swap the data to unified format
+ *
+ ****************************************************************************/
+
+static inline void sched_note_format(FAR uint8_t *dst,
+ FAR void *src, size_t len)
+{
+ uint64_t u64;
+ uint32_t u32;
+ uint16_t u16;
+ uint8_t u8;
+
+ switch (len)
+ {
+ case 8:
+ {
Review comment:
don't need {}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r839753085
##########
File path: sched/sched/sched_note.c
##########
@@ -133,7 +167,7 @@ static void note_common(FAR struct tcb_s *tcb,
clock_systime_timespec(&ts);
#else
- uint32_t systime = (uint32_t)clock_systime_ticks();
+ clock_t systime = (clock_t)clock_systime_ticks();
Review comment:
remove the cast
##########
File path: sched/sched/sched_note.c
##########
@@ -419,20 +443,8 @@ static void note_spincommon(FAR struct tcb_s *tcb,
note_common(tcb, ¬e.nsp_cmn, sizeof(struct note_spinlock_s), type);
- note.nsp_spinlock[0] = (uint8_t)((uintptr_t)spinlock & 0xff);
- note.nsp_spinlock[1] = (uint8_t)(((uintptr_t)spinlock >> 8) & 0xff);
-#if UINTPTR_MAX > UINT16_MAX
- note.nsp_spinlock[2] = (uint8_t)(((uintptr_t)spinlock >> 16) & 0xff);
- note.nsp_spinlock[3] = (uint8_t)(((uintptr_t)spinlock >> 24) & 0xff);
-#if UINTPTR_MAX > UINT32_MAX
- note.nsp_spinlock[4] = (uint8_t)(((uintptr_t)spinlock >> 32) & 0xff);
- note.nsp_spinlock[5] = (uint8_t)(((uintptr_t)spinlock >> 40) & 0xff);
- note.nsp_spinlock[6] = (uint8_t)(((uintptr_t)spinlock >> 48) & 0xff);
- note.nsp_spinlock[7] = (uint8_t)(((uintptr_t)spinlock >> 56) & 0xff);
-#endif
-#endif
-
- note.nsp_value = (uint8_t)*spinlock;
+ sched_note_flatten(note.nsp_spinlock, &spinlock, sizeof(uintptr_t));
Review comment:
sizeof(&spinlock)
##########
File path: sched/sched/sched_note.c
##########
@@ -825,20 +826,9 @@ void sched_note_syscall_leave(int nr, uintptr_t result)
note_common(tcb, ¬e.nsc_cmn, sizeof(struct note_syscall_leave_s),
NOTE_SYSCALL_LEAVE);
DEBUGASSERT(nr <= UCHAR_MAX);
- note.nsc_nr = nr;
-
- note.nsc_result[0] = (uint8_t)(result & 0xff);
- note.nsc_result[1] = (uint8_t)((result >> 8) & 0xff);
-#if UINTPTR_MAX > UINT16_MAX
- note.nsc_result[2] = (uint8_t)((result >> 16) & 0xff);
- note.nsc_result[3] = (uint8_t)((result >> 24) & 0xff);
-#if UINTPTR_MAX > UINT32_MAX
- note.nsc_result[4] = (uint8_t)((result >> 32) & 0xff);
- note.nsc_result[5] = (uint8_t)((result >> 40) & 0xff);
- note.nsc_result[6] = (uint8_t)((result >> 48) & 0xff);
- note.nsc_result[7] = (uint8_t)((result >> 56) & 0xff);
-#endif
-#endif
+ note.nsc_nr = nr;
+
+ sched_note_flatten(note.nsc_result, &result, sizeof(uintptr_t));
Review comment:
sizeof(result)
##########
File path: sched/sched/sched_note.c
##########
@@ -789,18 +800,8 @@ void sched_note_syscall_enter(int nr, int argc, ...)
for (i = 0; i < argc; i++)
{
arg = (uintptr_t)va_arg(ap, uintptr_t);
- *args++ = (uint8_t)(arg & 0xff);
- *args++ = (uint8_t)((arg >> 8) & 0xff);
-#if UINTPTR_MAX > UINT16_MAX
- *args++ = (uint8_t)((arg >> 16) & 0xff);
- *args++ = (uint8_t)((arg >> 24) & 0xff);
-#if UINTPTR_MAX > UINT32_MAX
- *args++ = (uint8_t)((arg >> 32) & 0xff);
- *args++ = (uint8_t)((arg >> 40) & 0xff);
- *args++ = (uint8_t)((arg >> 48) & 0xff);
- *args++ = (uint8_t)((arg >> 56) & 0xff);
-#endif
-#endif
+ sched_note_flatten(args, &arg, sizeof(uintptr_t));
Review comment:
sizeof(arg)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5915: sched/note: add support of trace section mark
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5915:
URL: https://github.com/apache/incubator-nuttx/pull/5915#discussion_r840004619
##########
File path: include/nuttx/sched_note.h
##########
@@ -105,16 +105,32 @@
memset((s), 0, sizeof(struct note_filter_irq_s))
#endif
-/* Note dump module tag definitions */
-
#ifdef CONFIG_SCHED_INSTRUMENTATION_DUMP
-# define NOTE_MODULE(a, b, c, d) \
- ((uint32_t)((a) & 0xff) | \
- ((uint32_t)((b) & 0xff) << 8) | \
- ((uint32_t)((c) & 0xff) << 16) | \
- ((uint32_t)((d) & 0xff) << 24))
+# define SCHED_NOTE_IP \
+ ({__label__ sched_note_here; sched_note_here: (uintptr_t)&&sched_note_here;})
Review comment:
Maybe we can reuse `__LINE__` macro to generate unique labels? In this case we can still keep `sched_note_string` with `uint32_t` parameter
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org