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/06/18 09:04:13 UTC
[GitHub] [incubator-nuttx] YuuichiNakamura opened a new pull request #1271: Feature/syscall instrumentation
YuuichiNakamura opened a new pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271
## Summary
This PR adds sched_note_syscall_enter/leave hooks for the system call instrumentation.
In FLAT build, syscall wrapper library is created.
In PROTECTED/KERNEL build, insert the hooks in system call proxy library.
## Impact
It should be no impact by default configuration.
To use the feature, CONFIG_SCHED_INSTRUMENTATION_SYSCALL must be enabled.
At this moment, only Arm and RISC-V can enable it.
## Testing
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_SYSCALL=y
CONFIG_SCHED_NOTE_GET=y
CONFIG_SYSTEM_NOTE=y
"note" command displays the system call information.
----------------------------------------------------------------
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] patacongo commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r443061207
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
This absolutely must be reverted!!!!!!!!!!!!!!!!!!!!!!
In the FLAT build, CONFIG_LIB_SYSCALL may or may not be defined. If this CONFIG_LIB_SYSCALL is NOT defined, then this selection will be present in the menus. THAT IS ABSOLUTELY WRONG. There should be no invalid menu selection visible. This is necessary to keep the complexity of the configuration menus down.
And the logic is wrong AND IS A BUG. If some does select CONFIG_SCHED_INSTRUMENTATION_SYSCALL when CONFIG_LIB_SYSCALL is *not* defined then the build will fail. THAT CANNOT BE PERMITTED. We must no permit invalid configuration options that can cause build errors from being selected.
THIS MUST BE REVERTED!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
----------------------------------------------------------------
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] patacongo commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647028975
> > I have no opinion. If someone wants to write such an application they are welcome. However, I will not be doing that.
> > My primary objective was to get the data path working. I have no interest in host applications. I will put something together for testing purpose (which will not be released to Apache because it will be a hack). It will not be ftrace.
> > Originally I posted a question asking for help from the community on this project. No help was offered... but lots of recommendations for things people want me to do. That was not what I was looking for.
> > There is no plan of record or anyone taking responsibility for host based tools.
>
> That is not true! I offered to help although I would like to leverage existing tools as much as possible (doing this well is a LOT of work). I ordered an interface board and it is sitting on my desk. Not all of us can work on this every day or even every week, but that does not mean we are not contributing.
Okay, sorry. I did not understand the extent of your commitment. My only interest is in the OS instrumentation which is deeply affected by this PR (hence, my interest in it) and the real-time transfer of the instrumentation data to the host. If you have a particular interest in ftrace on the host, then go for 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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-648550738
Thanks. I have created the new PR #1288 for new branch.
Then, can I 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] xiaoxiang781216 commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442643767
##########
File path: syscall/syscall_name.c
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * syscall/syscall_name.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>
+
+#ifndef CONFIG_LIB_SYSCALL
+#define CONFIG_LIB_SYSCALL
+#endif
+#include <syscall.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/* System call name tables. This table is indexed by the system call numbers
+ * defined above. Given the system call number, this table provides the
+ * name of the corresponding system function.
+ */
+
+const char *g_syscallname[SYS_nsyscalls + 1] =
Review comment:
Yes, just like g_funcnparms without `__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] xiaoxiang781216 commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442410205
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
$(Q) (cd proxies; $(MKSYSCALL) -p $(CSVFILE);)
$(Q) (cd stubs; $(MKSYSCALL) -s $(CSVFILE);)
+else
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
+ $(Q) (cd wraps; $(MKSYSCALL) -w $(CSVFILE);)
+ $(Q) $(MKSYSCALL) -l $(CSVFILE) > $(SYSCALLLIST:.txt=.h)
+ $(Q) $(call PREPROCESS, $(SYSCALLLIST:.txt=.h), $(SYSCALLLIST))
+ $(Q) sed -i -n -e '/^[^#]/p' $(SYSCALLLIST)
Review comment:
Why we need four steps to generate the link command file?
```
$(Q) $(MKSYSCALL) -l $(CSVFILE) > $(SYSCALLLIST:.txt=.h)
$(Q) $(call PREPROCESS, $(SYSCALLLIST:.txt=.h), $(SYSCALLLIST))
$(Q) sed -i -n -e '/^[^#]/p' $(SYSCALLLIST)
cat $< | sed -e 's/^/--wrap=/' > $@
```
We can define a clever SYSCALL_LOOKUP macro to generate them in one step:
```
--wrap=xxx
--wrap=yyy
```
then we don't need add -l for mksyscall and postprocess again in arch's Makefile.
----------------------------------------------------------------
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] patacongo commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647002587
>
>
> > R> With the hardware solution, the instrumentation data will be encoded and sent out via a parallel port immediately when the call-out occurs. There will be no buffering in RAM; there will be not application driver. Those will all be disabled. The USB parallel solution will be direct instrumentation to hardware.
> > To make that job simpler and fully compatible with the effort you are doing, perhaps we need to have standalone logic to encode instrumentation data to a stream. Perhaps using struct lib_outstream_s as defined in include/nuttx/stream.h. That way, the encoded data can go to either a memory stream or to the hardware parallel interface.
> > Similar rate/content controls will be needed.
>
> Yes, we can extend sched_note.c driver to support the different transport just like syslog(ramlog, itm, serial, file...).
No, you do not understand. With the hardware solution there is nothing buffered in ram, there is no driver, and sched_note.c is not built. sched_note.c is ONLY built if data will will buffered in RAM. None of that logic applies in the hardware solution. Nothing will be buffered in RAM.
You don't understand the architecture.
The path is direct from the instrumentation call-out functions in the OS to a new driver that immediately sends the date to the host via a parallel-to-USB device. CONFIG_SCHED_INSTRUMENTATION_BUFFER will not be enabled. A block diagram would be very space:
CORE OS ->Parallel port driver / USB ->Host driver -> Host Application
When CONFIG_SCHED_INSTRUMENTATION_BUFFER is enabled the path is like:
CORE OS -> RAM -> driver -> embedded application
What you envision is incorrect.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647003582
> > > > With the hardware solution, the instrumentation data will be encoded and sent out via a parallel port immediately when the call-out occurs. There will be no buffering in RAM; there will be not application driver. Those will all be disabled. The USB parallel solution will be direct instrumentation to hardware.
> > > > To make that job simpler and fully compatible with the effort you are doing, perhaps we need to have standalone logic to encode instrumentation data to a stream. Perhaps using struct lib_outstream_s as defined in include/nuttx/stream.h. That way, the encoded data can go to either a memory stream or to the hardware parallel interface.
> > > > Similar rate/content controls will be needed.
> >
> >
> > Yes, we can extend sched_note.c driver to support the different transport just like syslog(ramlog, itm, serial, file...).
>
> No, you do not understand. With the hardware solution there is nothing buffered in ram, there is no driver, and sched_note.c is not built. sched_note.c is ONLY built if data will will buffered in RAM. None of that logic applies in the hardware solution. Nothing will be buffered in RAM.
>
> You don't understand the architecture.
>
> The path is direct from the instrumentation call-out functions in the OS to a new driver that immediately sends the date to the host via a parallel-to-USB device. CONFIG_SCHED_INSTRUMENTATION_BUFFER will not be enabled. A block diagram would be very space:
>
> CORE OS ->Parallel port driver / USB ->Host driver -> Host Application
>
> When CONFIG_SCHED_INSTRUMENTATION_BUFFER is enabled the path is like:
>
> CORE OS -> RAM -> driver -> embedded application
>
> What you envision is incorrect.
I understand your requirement, but we still need a layer between the sched_* callback and the real transport(either USB or ram), because we don't want duplicate these for each transport:
1.The start/stop trace control
2.The runtime filter functionality
3.Format to ftrace for graphic tool
The better path is:
Core OS->sched_note->transport(RAM or USB)
Sorry, I reuse sched_note terminoloy, the old sched_note should become a transport.
----------------------------------------------------------------
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] patacongo commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442962808
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
> @patacongo since the syscall isn't enabled in the many projects, but it's very useful to hook 'syscall' in all situation, @YuuichiNakamura utilize the option '--wrap' provided by ld to hook in this case. This link has more information:
> https://github.com/YuuichiNakamura/nuttx-task-tracer-doc/blob/master/NuttXTaskTracerInternals.md
> So you can think that hook can decouple from syscall totally with --wrap.
I don't fully agree. The menu item must not be visible if CONFIG_LIB_SYCALL is not enabled. That is the requirement. The existing dependency is the simplest and surest way to assure that requirement.
But if the menu selection is never presented if CONFIG_LIB_SYSCALL is not selected through some other mechanism, then we can consider that. The objective is to not burden the end user with many configuration options that do not do anything. The selection must not be present or visible in any way if CONFIG_LIB_SYSCALL is not selected.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442644376
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
Review comment:
Yes, then we use one solution to resolve all build mode.
----------------------------------------------------------------
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] patacongo edited a comment on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647002587
>
>
> > > With the hardware solution, the instrumentation data will be encoded and sent out via a parallel port immediately when the call-out occurs. There will be no buffering in RAM; there will be not application driver. Those will all be disabled. The USB parallel solution will be direct instrumentation to hardware.
> > To make that job simpler and fully compatible with the effort you are doing, perhaps we need to have standalone logic to encode instrumentation data to a stream. Perhaps using struct lib_outstream_s as defined in include/nuttx/stream.h. That way, the encoded data can go to either a memory stream or to the hardware parallel interface.
> > Similar rate/content controls will be needed.
>
> Yes, we can extend sched_note.c driver to support the different transport just like syslog(ramlog, itm, serial, file...).
No, you do not understand. With the hardware solution there is nothing buffered in ram, there is no driver, and sched_note.c is not built. sched_note.c is ONLY built if data will will buffered in RAM. None of that logic applies in the hardware solution. Nothing will be buffered in RAM.
You don't understand the architecture.
The path is direct from the instrumentation call-out functions in the OS to a new driver that immediately sends the date to the host via a parallel-to-USB device. CONFIG_SCHED_INSTRUMENTATION_BUFFER will not be enabled. A block diagram would be very space:
CORE OS ->Parallel port driver / USB ->Host driver -> Host Application
When CONFIG_SCHED_INSTRUMENTATION_BUFFER is enabled the path is like:
CORE OS -> RAM -> driver -> embedded application
What you envision is incorrect.
----------------------------------------------------------------
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] patacongo commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647003628
A different thought:
Currently the logic generates stubs that:
call sched_note_syscall_enter()
Perform the system call
call sched_not_syscall_leave()
These are apparently generated by tools/mksyscall.c. There are potentially hundreds for system call stubs, but on 7 system call functions for 0 through 6 parameters. Would it make more sense to put the instrumentation in those 7 system call functions rather than in the hundreds of system call stubs?
----------------------------------------------------------------
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] btashton commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647025607
> > 3.Format to ftrace for graphic tool
>
> No, this would not be done in the target in the hardware solution. This would be done on the host. But I will not be using ftrace. I will be using a Windows application. I don't do Linux develop; this will be a Windows only solution initially.
Why not use a tool that already supports ftrace and Windows/Linux? I have stayed quite on this since this patch came up, but it is certainly the direction I would support
Is there a technical downside to the ftrace format that you see?
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-646941913
> The user should not be possible to enable syscall instrumentation hooks if system calls are not enabled. The option should never be visible to the user if it does not function.
>
The hook can work even without CONFIG_INSTRUMENTATION_SYSCALL, please read the last paragraph:
https://github.com/YuuichiNakamura/nuttx-task-tracer-doc/blob/master/NuttXTaskTracerInternals.md
@YuuichiNakamura mention this special case clearly.
> Please revert to the original version or provide explanation. If CONFIG_LIB_SYSCALL is not selected, then CONFIG_INSTRUMENTATION_SYSCALL must not be selectable or visiable to the 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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442879887
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
$(Q) (cd proxies; $(MKSYSCALL) -p $(CSVFILE);)
$(Q) (cd stubs; $(MKSYSCALL) -s $(CSVFILE);)
+else
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
+ $(Q) (cd wraps; $(MKSYSCALL) -w $(CSVFILE);)
+ $(Q) $(MKSYSCALL) -l $(CSVFILE) > $(SYSCALLLIST:.txt=.h)
+ $(Q) $(call PREPROCESS, $(SYSCALLLIST:.txt=.h), $(SYSCALLLIST))
+ $(Q) sed -i -n -e '/^[^#]/p' $(SYSCALLLIST)
Review comment:
Oops. Just using "--wrap xxx" instead of "--wrap=xxx" solves it....
GNU linker also accepts the former styles.
I'll try it again, sorry.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442643767
##########
File path: syscall/syscall_name.c
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * syscall/syscall_name.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>
+
+#ifndef CONFIG_LIB_SYSCALL
+#define CONFIG_LIB_SYSCALL
+#endif
+#include <syscall.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/* System call name tables. This table is indexed by the system call numbers
+ * defined above. Given the system call number, this table provides the
+ * name of the corresponding system function.
+ */
+
+const char *g_syscallname[SYS_nsyscalls + 1] =
Review comment:
Yes, just like g_funcnparms without __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] btashton commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647027775
> >
> >
> > > > 3.Format to ftrace for graphic tool
> > >
> > >
> > > No, this would not be done in the target in the hardware solution. This would be done on the host. But I will not be using ftrace. I will be using a Windows application. I don't do Linux develop; this will be a Windows only solution initially.
> >
> > Why not use a tool that already supports ftrace and Windows/Linux? I have stayed quite on this since this patch came up, but it is certainly the direction I would support
> >
> > Is there a technical downside to the ftrace format that you see?
>
> I have no opinion. If someone wants to write such an application they are welcome. However, I will not be doing that.
>
> My primary objective was to get the data path working. I have no interest in host applications. I will put something together for testing purpose (which will not be released to Apache because it will be a hack). It will not be ftrace.
>
> Originally I posted a question asking for help from the community on this project. No help was offered... but lots of recommendations for things people want me to do. That was not what I was looking for.
>
> There is no plan of record or anyone taking responsibility for host based tools.
That is not true! I offered to help although I would like to leverage existing tools as much as possible (doing this well is a LOT of work). I ordered an interface board and it is sitting on my desk. Not all of us can work on this every day or even every week, but that does not mean we are not contributing.
When the ftrace stuff kicked off I wanted to wait until it made it in. Then streaming it out to the host is not hard. It did not make sense to me to work on that beforehand.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647030129
But from the initial NXView email:
https://lists.apache.org/thread.html/r1840904585459f9575877335f6d072e8854234384d1f554c6b26ee52%40%3Cdev.nuttx.apache.org%3E
The UI is the core part of the proposal if my understand is correct.
Since the instrumentation is already suppported, only two work need to be done:
1.Develop a hardware transport layer
2.Develop a desktop analyze tool
From my experience, the 2nd work is much tough than first one. That's why I am very interesting in @YuuichiNakamura ftrace proposal because there arey many tools(either graphic/command or linux/macos/windows) for ftrace. If we output the same format as ftrace, we don't develop any desktop tool at all.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442412819
##########
File path: syscall/syscall_name.c
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * syscall/syscall_name.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>
+
+#ifndef CONFIG_LIB_SYSCALL
+#define CONFIG_LIB_SYSCALL
+#endif
+#include <syscall.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/* System call name tables. This table is indexed by the system call numbers
+ * defined above. Given the system call number, this table provides the
+ * name of the corresponding system function.
+ */
+
+const char *g_syscallname[SYS_nsyscalls + 1] =
Review comment:
Who will use g_syscallname?
Should we change the name to g_funcnames and put the declaration into nuttx/include/sys/syscall.h like other variables?
##########
File path: tools/mksyscall.c
##########
@@ -450,6 +452,27 @@ static void generate_stub(int nfixed, int nparms)
fprintf(stream, ")\n{\n");
+ /* Generate the result variable definition for non-void function */
+
+ if (strcmp(g_parm[RETTYPE_INDEX], "void") != 0)
+ {
+ fprintf(stream, " uintptr_t result;\n\n");
Review comment:
Let's always define result to simplify the code:
```
uintptr_t result = 0;
sched_note_syscall_enter(...);
...
sched_note_syscall_leave(..., result);
return result;
```
##########
File path: syscall/syscall_name.c
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * syscall/syscall_name.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>
+
+#ifndef CONFIG_LIB_SYSCALL
+#define CONFIG_LIB_SYSCALL
+#endif
+#include <syscall.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/* System call name tables. This table is indexed by the system call numbers
+ * defined above. Given the system call number, this table provides the
+ * name of the corresponding system function.
+ */
+
+const char *g_syscallname[SYS_nsyscalls + 1] =
+{
+# define SYSCALL_LOOKUP1(f,n) #f
+# define SYSCALL_LOOKUP(f,n) , #f
+# include <sys/syscall_lookup.h>
+# undef SYSCALL_LOOKUP1
+# undef SYSCALL_LOOKUP
+ , 0
Review comment:
Why need the null terminator?
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
$(Q) (cd proxies; $(MKSYSCALL) -p $(CSVFILE);)
$(Q) (cd stubs; $(MKSYSCALL) -s $(CSVFILE);)
+else
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
+ $(Q) (cd wraps; $(MKSYSCALL) -w $(CSVFILE);)
+ $(Q) $(MKSYSCALL) -l $(CSVFILE) > $(SYSCALLLIST:.txt=.h)
+ $(Q) $(call PREPROCESS, $(SYSCALLLIST:.txt=.h), $(SYSCALLLIST))
+ $(Q) sed -i -n -e '/^[^#]/p' $(SYSCALLLIST)
Review comment:
Why we need four steps to generate the link command file?
```
$(Q) $(MKSYSCALL) -l $(CSVFILE) > $(SYSCALLLIST:.txt=.h)
$(Q) $(call PREPROCESS, $(SYSCALLLIST:.txt=.h), $(SYSCALLLIST))
$(Q) sed -i -n -e '/^[^#]/p' $(SYSCALLLIST)
cat $< | sed -e 's/^/--wrap=/' > $@
```
We can define a clever SYSCALL_LOOKUP macro to generate them in one step:
```
--wrap=xxx
--wrap=yyy
```
then we don't add -l for mksyscall and postprocess again in arch's Makefile.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442688004
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
$(Q) (cd proxies; $(MKSYSCALL) -p $(CSVFILE);)
$(Q) (cd stubs; $(MKSYSCALL) -s $(CSVFILE);)
+else
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
+ $(Q) (cd wraps; $(MKSYSCALL) -w $(CSVFILE);)
+ $(Q) $(MKSYSCALL) -l $(CSVFILE) > $(SYSCALLLIST:.txt=.h)
+ $(Q) $(call PREPROCESS, $(SYSCALLLIST:.txt=.h), $(SYSCALLLIST))
+ $(Q) sed -i -n -e '/^[^#]/p' $(SYSCALLLIST)
Review comment:
I tried to take your way and found that cpp generates the error because `--wrap=xxx` is invalid as C tokens.
It seems should be avoided to handle non-C-source text directly by C preprocessor.
So I suggests the alternative idea to prepare the host command to generate the command line for linker such as:
```
#include <stdio.h>
#include <nuttx/config.h>
#include <arch/arch.h>
#ifndef UP_WRAPOPT
#define UP_WRAPOPT(f) "--wrap=" #f " "
#endif
int main()
{
printf("%s",
#define SYSCALL_LOOKUP1(f,n) UP_WRAPOPT(f)
#define SYSCALL_LOOKUP(f,n) UP_WRAPOPT(f)
#include <sys/syscall_lookup.h>
#undef SYSCALL_LOOKUP1
#undef SYSCALL_LOOKUP
);
}
```
It is built and executed by the host environment.
In my brief trial it seems work as my expectation, but my concern is whether the host tool is permitted to include "nuttx/config.h" and "arch/arch.h" directly.
Are there available for such purpose? Or do you know better way for it?
BTW, I found that similar consideration should be needed for wrapper file because `__wrap_` and `__real_` also depend on GNU linker.
I'll take similar way with mksyscall to generate the wrapper code such as:
```
#include <nuttx/arch.h>
#ifndef UP_WRAPSYM
#define UP_WRAPSYM(s) __wrap_##s
#endif
#ifndef UP_REALSYM
#define UP_REALSYM(s) __real_##s
#endif
int UP_WRAPSYM(open)(FAR const char * parm1, int parm2, ...)
:
```
Thanks for good suggestion.
----------------------------------------------------------------
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] patacongo commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442210048
##########
File path: arch/arm/src/Makefile
##########
@@ -69,6 +69,19 @@ else
NUTTX = "$(TOPDIR)$(DELIM)nuttx$(EXEEXT)"
endif
+# Additional rules for system call wrapper
+
+ifeq ($(CONFIG_BUILD_FLAT),y)
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
Review comment:
Why is this restricted the FLAT build. System calls are seldom used with the FLAT build. Same question below.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-648512447
Thanks for consideration.
- Anyone create the feature branch for this topic.
- I resend same pull request to the new feature branch.
Is this ok for 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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442709270
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
$(Q) (cd proxies; $(MKSYSCALL) -p $(CSVFILE);)
$(Q) (cd stubs; $(MKSYSCALL) -s $(CSVFILE);)
+else
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
+ $(Q) (cd wraps; $(MKSYSCALL) -w $(CSVFILE);)
+ $(Q) $(MKSYSCALL) -l $(CSVFILE) > $(SYSCALLLIST:.txt=.h)
+ $(Q) $(call PREPROCESS, $(SYSCALLLIST:.txt=.h), $(SYSCALLLIST))
+ $(Q) sed -i -n -e '/^[^#]/p' $(SYSCALLLIST)
Review comment:
cpp is a general tool which can handle any text file, for example:
1.This commit process nuttx-names.dat by cpp:
https://github.com/apache/incubator-nuttx/pull/1262/commits/abf77a6dc1bec20b38df2ae2d37bed6109ad125c
2.This commit process shell script by cpp:
https://github.com/apache/incubator-nuttx/pull/793/commits/925d0107451dd50b8637f2a07f1dceb2b90d09d2
The key point is that we have to stop the normal step(compile) after preprocess.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647001511
> R> With the hardware solution, the instrumentation data will be encoded and sent out via a parallel port immediately when the call-out occurs. There will be no buffering in RAM; there will be not application driver. Those will all be disabled. The USB parallel solution will be direct instrumentation to hardware.
>
> To make that job simpler and fully compatible with the effort you are doing, perhaps we need to have standalone logic to encode instrumentation data to a stream. Perhaps using struct lib_outstream_s as defined in include/nuttx/stream.h. That way, the encoded data can go to either a memory stream or to the hardware parallel interface.
>
> Similar rate/content controls will be needed.
Yes, we can extend sched_note.c driver to support the different transport just like syslog(ramlog, itm, serial, file...).
----------------------------------------------------------------
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] patacongo commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-646007116
The nxstyle failure can be be ignored. I would like to have some other people look at this before it is merged since it has mostly impacts on the build-related logic.
----------------------------------------------------------------
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] patacongo commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r443135232
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
> @YuuichiNakamura already support the filter and document here:
> https://github.com/YuuichiNakamura/nuttx-task-tracer-doc/blob/master/NuttXTaskTracer.md
> This filter is done at the runtime(I guess inside the sched_trace.c/sched_note.c driver). So we can:
> 1.Turn on/off the whole syscall instrumentation by SCHED_INSTRUMENTATION_SYSCALL
> 2.Turn on/off the individual syscall from command line at runtime
> The 3rd approach(static filter) only instrument a specific set of syscall(e.g. provide the cut down syscall.csv). But I wonder whether the static filter is required if we already have more powerful runtime filter.
Of course, that #2) CANNOT work for the hardware based solution since there is no user accessible driver in the hardware solution.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647004200
> A different thought:
>
> Currently the logic generates stubs that:
>
> ```
> call sched_note_syscall_enter()
> Perform the system call
> call sched_not_syscall_leave()
> ```
>
> These are apparently generated by tools/mksyscall.c. There are potentially hundreds for system call stubs, but on 7 system call functions for 0 through 6 parameters. Would it make more sense to put the instrumentation in those 7 system call functions rather than in the hundreds of system call stubs?
Yes, it's a good idea, but we need find a solution when syscall isn't enabled. If syscall trace can't work with this case, this feature lose the most value because 99% project don't enable syscall at all.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-646590277
Yes, @YuuichiNakamura already support the filter, but it happen at the runtime not compiler time. Do you want the later?
https://github.com/YuuichiNakamura/nuttx-task-tracer-doc/blob/master/NuttXTaskTracer.md
----------------------------------------------------------------
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] patacongo commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r443061207
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
This absolutely must be reverted!!!!!!!!!!!!!!!!!!!!!!
In the FLAT build, CONFIG_LIB_SYSCALL may or may not be defined. If this CONFIG_LIB_SYSCALL is NOT defined, then this selection will be present in the menus. THAT IS ABSOLUTELY WRONG. There should be no invalid menu selection visible. This is necessary to keep the complexity of the configuration menus down.
And the logic is wrong AND IS A BUG. If someone does select CONFIG_SCHED_INSTRUMENTATION_SYSCALL when CONFIG_LIB_SYSCALL is *not* defined then the dead code will bet built into the system. THAT CANNOT BE PERMITTED. We must not permit invalid configuration options.
THIS MUST BE REVERTED!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442605091
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
Review comment:
The hook and syscall in flat build is mutual exclusive in the current implementation, but the syscall is also useful even in the flat build. How about we always use wrap mechanism for hook in all three build mode? The benefit is that:
1.The desgin and implementation is more simple
2.The hook and syscall can free combination
----------------------------------------------------------------
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] masayuki2009 commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-648522725
Hi @YuuichiNakamura,
> Thanks for consideration.
>
> * Anyone create the feature branch for this topic.
I've just created the 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 a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442631977
##########
File path: tools/mksyscall.c
##########
@@ -450,6 +452,27 @@ static void generate_stub(int nfixed, int nparms)
fprintf(stream, ")\n{\n");
+ /* Generate the result variable definition for non-void function */
+
+ if (strcmp(g_parm[RETTYPE_INDEX], "void") != 0)
+ {
+ fprintf(stream, " uintptr_t result;\n\n");
Review comment:
I got it.
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
Review comment:
Is it mean that the system call is processed as:
application -> syscall proxy -> (syscall handler) -> syscall stub -> **syscall wrapper** -> kernel function
in PROTECTED/KERNEL build?
If so, I'll try to make changes for 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] patacongo commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647606720
>
>
> I have fixed the commented issues on this PR.
>
> For my task trace feature, sched note bufferring and driver code are needed but I feel that it requires re-design because current implementation have no consideration about the hardware solution and existing /dev/note interface.
> There are also another issues such as static trace event filter.
> I think that I have to solve the issues step by step by discussing with you.
Perhaps we should consider merging the changes onto a feature branch. Then we can work through issues and update as needed. When everyone is happy we could merge the feature branch 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] patacongo commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-646976045
> With the hardware solution, the instrumentation data will be encoded and sent out via a parallel port immediately when the call-out occurs. There will be no buffering in RAM; there will be not application driver. Those will all be disabled. The USB parallel solution will be direct instrumentation to hardware.
To make that job simply and fully compatible with the effort you are doing, perhaps we need to have standalone logic to encode instrumentation data to a stream. Perhaps using struct lib_outstream_s as defined in include/nuttx/stream.h. That way, the encoded data can go to either a memory stream or to the hardware parallel interface.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-646548078
@YuuichiNakamura @xiaoxiang781216 - this looks like a great addition and excellent engineering as well as collaboration. Thank you! @YuuichiNakamura Outstanding documentation!
I see CONFIG_SCHED_INSTRUMENTATION_SYSCALL is the "Knob" to turn this on.
I have a question: Is that the only level of granularity of control of the features?
It would be great if this can be used, even on, a resource constrained system. Should we be able to a preserve just a subset and use partial monitoring? Have you considered this?
----------------------------------------------------------------
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 closed pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 closed pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r443101840
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
Yes, in the FLAT build, CONFIG_LIB_SYSCALL may or may not be defined, but SCHED_INSTRUMENTATION_SYSCALL CAN STILL work as expect even without CONFIG_LIB_SYSCALL. That is why @YuuichiNakamura remove the dependence.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647030129
But from the initial NXView email:
https://lists.apache.org/thread.html/r1840904585459f9575877335f6d072e8854234384d1f554c6b26ee52%40%3Cdev.nuttx.apache.org%3E
The UI is the core part of the proposal if my understand is correct.
Since the instrumentation is already suppported, only two work need to be done:
1.Develop a hardware transport layer
2.Develop a desktop analyze tool
From my experience, the 2nd work is much tough than first one. That's why I am very interesting in @YuuichiNakamura ftrace proposal because there arey many tools(either graphic/command or linux/macos/windows) for ftrace. If we output the same format as ftrace, we don't need develop any desktop tool at all.
----------------------------------------------------------------
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] patacongo commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442959624
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
> In my understanding, CONFIG_LIB_SYSCALL is defined in PROTECTED and KERNEL build and not in FLAT build.
> I have made CONFIG_INSTRUMENTATION_SYSCALL feature available in all three build types and it is the reason I removed LIB_SYSCALL.
Incorrect! Your understanding is wrong. System calls are also used in the FLAT build to support external modules. The configuration change you made must be reverted. Please restore the dependency on LIB_SYSCALL. That is required and will make the option operate correctly in all three build modes.
This change cannot be merged unless the dependency is restored.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442928214
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
@patacongo since the syscall isn't enabled in the many projects, but it's very useful to hook 'syscall' in all situation, @YuuichiNakamura utilize the option '--wrap' provided by ld to hook in this case. This link has more information:
https://github.com/YuuichiNakamura/nuttx-task-tracer-doc/blob/master/NuttXTaskTracerInternals.md
So you can think that hook can decouple from syscall totally with --wrap.
----------------------------------------------------------------
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] patacongo edited a comment on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-646976045
> With the hardware solution, the instrumentation data will be encoded and sent out via a parallel port immediately when the call-out occurs. There will be no buffering in RAM; there will be not application driver. Those will all be disabled. The USB parallel solution will be direct instrumentation to hardware.
To make that job simpler and fully compatible with the effort you are doing, perhaps we need to have standalone logic to encode instrumentation data to a stream. Perhaps using struct lib_outstream_s as defined in include/nuttx/stream.h. That way, the encoded data can go to either a memory stream or to the hardware parallel interface.
Similar rate/content controls will be needed. I imagine that the controls that are described for this solution would not be usable since they would operate in parts of software not present in the hardware-based solution. But if we preserve the static, compile-time filter controls, then that should be sufficient.
----------------------------------------------------------------
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] patacongo edited a comment on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-646976045
> With the hardware solution, the instrumentation data will be encoded and sent out via a parallel port immediately when the call-out occurs. There will be no buffering in RAM; there will be not application driver. Those will all be disabled. The USB parallel solution will be direct instrumentation to hardware.
To make that job simpler and fully compatible with the effort you are doing, perhaps we need to have standalone logic to encode instrumentation data to a stream. Perhaps using struct lib_outstream_s as defined in include/nuttx/stream.h. That way, the encoded data can go to either a memory stream or to the hardware parallel interface.
Similar rate/content controls will be needed.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442919415
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
I think the problem will resolve automatically If we decouple hook and 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] xiaoxiang781216 commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r443133917
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
> I think that there there still must be an option to enable the instrumentation in the syscall stubs. We have to be able to generate stubs without the instrumentation. But yes, if it is inside of the syscall stubs, then that part of the logic does not need to be conditioned on CONFIG_LIB_SYSCALL.
>
Yes, it's controlled by SCHED_INSTRUMENTATION_SYSCALL. CONFIG_LIB_SYSCALL enable syscall, SCHED_INSTRUMENTATION_SYSCALL enable instrumentation. But since the instrumentation can work without syscall, the dependence has to be removed.
> But how about the generation of the event handling logic itself?
>
> Some of this comes back to enabling static vs. runtime instrumentation. The static controls are essential for resource limited and for systems with extremely high data rate events.
>
> Better terminology... we need "frontend" and possibly "backend" controls: There are basically two steps:
>
> frontend: instrumentation calls are received and data is buffered in memory.
> backend: The application reads the data from memory.
>
> We need frontend controls to manage how much data is buffered and the rate of buffering. That was accomplished in the past through static configuration settings. If you want to avoid static settings, then you will need other controls to manage the rate that data is buffered.
@YuuichiNakamura already support the filter and document here:
https://github.com/YuuichiNakamura/nuttx-task-tracer-doc/blob/master/NuttXTaskTracer.md
This filter is done at the runtime(I guess inside the sched_trace.c/sched_note.c driver). So we can:
1.Turn on/off the whole syscall instrumentation by SCHED_INSTRUMENTATION_SYSCALL
2.Turn on/off the individual syscall from command line at runtime
The 3rd approach(static filter) only instrument a specific set of syscall(e.g. provide the cut down syscall.csv). But I wonder whether the static filter is required if we already have more powerful runtime 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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442919415
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
I think the problem will resolve automatically If we decouple hook and 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] xiaoxiang781216 commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-646606346
@YuuichiNakamura do you have the binary size difference before and after enabling hook? So we can know how much benefit we can get from the compile time filter. Since the compiler time control is rigid and can't extend at the runtime. If the difference is small, I prefer to only support the runtime 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] patacongo commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442867753
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
This seems incorrect. It should not be possible to enable syscall instrumentation hooks if system calls are not enabled. The option should never be visible to the user if it does not function.
Please revert to the original version or provide explanation. If CONFIG_LIB_SYSCALL is not selected, then CONFIG_INSTRUMENTATION_SYSCALL must not be selectable or visiable to the 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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442652974
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
$(Q) (cd proxies; $(MKSYSCALL) -p $(CSVFILE);)
$(Q) (cd stubs; $(MKSYSCALL) -s $(CSVFILE);)
+else
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
+ $(Q) (cd wraps; $(MKSYSCALL) -w $(CSVFILE);)
+ $(Q) $(MKSYSCALL) -l $(CSVFILE) > $(SYSCALLLIST:.txt=.h)
+ $(Q) $(call PREPROCESS, $(SYSCALLLIST:.txt=.h), $(SYSCALLLIST))
+ $(Q) sed -i -n -e '/^[^#]/p' $(SYSCALLLIST)
Review comment:
The suggestion is reasonable to support the different toolchain, or we can let arch ovevwrite the macro expansion process like this:
```
#include <nuttx/config.h>
#include <syscall.h>
#include <arch.h>
#ifndef UP_WRAP
#define UP_WRAP(f) --wrap=##f
#endif
define SYSCALL_LOOKUP1(f,n) UP_WRAP(f)
define SYSCALL_LOOKUP(f,n) UP_WRAP(f)
#include <sys/syscall_lookup.h>
#undef SYSCALL_LOOKUP1
#undef SYSCALL_LOOKUP
```
----------------------------------------------------------------
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] patacongo commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r443120016
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
I think that there there still must be an option to enable the instrumentation in the syscall stubs. We have to be able to generate stubs without the instrumentation. But yes, if it is inside of the syscall stubs, then that part of the logic does not need to be conditioned on CONFIG_LIB_SYSCALL.
But how about the generation of the event handling logic itself?
Some of this comes back to enabling static vs. runtime instrumentation. The static controls are essential for resource limited and for systems with extremely high data rate events.
Better terminology... we need "frontend" and possibly "backend" controls: There are basically two steps:
frontend: instrumentation calls are received and data is buffered in memory.
backend: The application reads the data from memory.
We need frontend controls to manage how much data is buffered and the rate of buffering. That was accomplished in the past through static configuration settings. If you want to avoid static settings, then you will need other controls to manage the rate that data is buffered.
----------------------------------------------------------------
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] patacongo commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-646974338
Another thought. This all started because I plan to implement a hardware interface via USB to the host to transfer data. That will have to wait a month or so until I get hardware.
In order to do this, we need to retain all controls that enable memory buffering and the driver. Those will not be used in the hardware solution:
With the hardware solution, the instrumentation data will be encoded and sent out via a parallel port immediately when the call-out occurs. There will be no buffering in RAM; there will be not application driver. Those will all be disabled. The USB parallel solution will be direct instrumentation to hardware.
So while you are considering disabling user options, please keep that in mind.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442652974
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
$(Q) (cd proxies; $(MKSYSCALL) -p $(CSVFILE);)
$(Q) (cd stubs; $(MKSYSCALL) -s $(CSVFILE);)
+else
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
+ $(Q) (cd wraps; $(MKSYSCALL) -w $(CSVFILE);)
+ $(Q) $(MKSYSCALL) -l $(CSVFILE) > $(SYSCALLLIST:.txt=.h)
+ $(Q) $(call PREPROCESS, $(SYSCALLLIST:.txt=.h), $(SYSCALLLIST))
+ $(Q) sed -i -n -e '/^[^#]/p' $(SYSCALLLIST)
Review comment:
The suggestion is reasonable to support the different toolchain, or we can let arch ovevwrite the macro expansion process like this:
```
#include <nuttx/config.h>
#include <syscall.h>
#include <arch.h>
#ifndef UP_WRAP
#define UP_WRAP(f) --wrap=##f
#endif
define SYSCALL_LOOKUP1(f,n) UP_WRAP(f)
define SYSCALL_LOOKUP(f,n) UP_WRAP(f)
#include <sys/syscall_lookup.h>
#undef SYSCALL_LOOKUP1
#undef SYSCALL_LOOKUP
```
UP_WRAP is just a demo, maybe we can find a better name.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442256681
##########
File path: arch/arm/src/Makefile
##########
@@ -69,6 +69,19 @@ else
NUTTX = "$(TOPDIR)$(DELIM)nuttx$(EXEEXT)"
endif
+# Additional rules for system call wrapper
+
+ifeq ($(CONFIG_BUILD_FLAT),y)
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
Review comment:
This part is used to create the additional linker command option "--wrap=\<function\>" for gnu linker.
This is needed to supersede the direct kernel function call from application into newly added system call wrapper library.
(see: https://github.com/YuuichiNakamura/nuttx-task-tracer-doc/blob/master/NuttXTaskTracerInternals.md#flat-build)
Only FLAT build requires this treatment because PROTECTED/KERNEL build already have system call stub which can insert sched_note_syscall_enter/leave.
----------------------------------------------------------------
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] patacongo edited a comment on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-646976045
> With the hardware solution, the instrumentation data will be encoded and sent out via a parallel port immediately when the call-out occurs. There will be no buffering in RAM; there will be not application driver. Those will all be disabled. The USB parallel solution will be direct instrumentation to hardware.
To make that job simpler and fully compatible with the effort you are doing, perhaps we need to have standalone logic to encode instrumentation data to a stream. Perhaps using struct lib_outstream_s as defined in include/nuttx/stream.h. That way, the encoded data can go to either a memory stream or to the hardware parallel interface.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442870010
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
$(Q) (cd proxies; $(MKSYSCALL) -p $(CSVFILE);)
$(Q) (cd stubs; $(MKSYSCALL) -s $(CSVFILE);)
+else
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
+ $(Q) (cd wraps; $(MKSYSCALL) -w $(CSVFILE);)
+ $(Q) $(MKSYSCALL) -l $(CSVFILE) > $(SYSCALLLIST:.txt=.h)
+ $(Q) $(call PREPROCESS, $(SYSCALLLIST:.txt=.h), $(SYSCALLLIST))
+ $(Q) sed -i -n -e '/^[^#]/p' $(SYSCALLLIST)
Review comment:
cpp ## operator cannot concatenate arbitrary string.
I found that the C language standard declares that both arguments of the ## operator and the result of the concatenation must be valid preprocessing tokens.
If not, the behavior is undefined in the standard.
In fact, if passing the following code into GNU cpp,
```
#define CONCAT_(x, y) x##y
#define CONCAT(x, y) CONCAT_(x, y)
CONCAT(wrap, xxx)
CONCAT(--wrap=, xxx)
```
The former CONCAT works well but the latter gets the error `error: pasting "=" and "xxx" does not give a valid preprocessing token`.
In your example, abf77a6 concatenates the correct tokens.
925d010 concatenates `CONCAT(/dev/ram, CONFIG_NSH_FATDEVNO)`, works because `CONFIG_NSH_FATDEVNO` has numeric value and the result like "ram0" is the correct token.
(But I think "/dev/ram0" is still incorrect token... Strictly speaking, it seems undefined result in the standard.)
Anyway, in our case, because "--wrap=" is not correct token, I think that we cannot take this way unfortunately.
----------------------------------------------------------------
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] patacongo commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r443132145
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
You still need the CONFIG_LIB_SYSCALL dependency to hide the menu option. It is not needed for the instrumentation, but it is needed to keep garbage selections out of the menus. Otherwise the menu selection will always be available even when it is not appropriate.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647030129
But from the initial NXView email:
https://lists.apache.org/thread.html/r1840904585459f9575877335f6d072e8854234384d1f554c6b26ee52%40%3Cdev.nuttx.apache.org%3E
The UI is the core part of the proposal if my understand is correct.
Since the instrumentation is already suppported, only two work need to be done:
1.Develop a hardware transport layer
2.Develop a desktop analyze tool
From my experience, the 2nd work is much tough than first one. That's why I am very interesting in @YuuichiNakamura ftrace proposal because there are many tools(either graphic/command or linux/macos/windows) for ftrace. If we output the same format as ftrace, we don't need develop any desktop tool at all.
Of course, it's also a valuable addition to trace the 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] patacongo edited a comment on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647002587
>
>
> > > With the hardware solution, the instrumentation data will be encoded and sent out via a parallel port immediately when the call-out occurs. There will be no buffering in RAM; there will be not application driver. Those will all be disabled. The USB parallel solution will be direct instrumentation to hardware.
> > To make that job simpler and fully compatible with the effort you are doing, perhaps we need to have standalone logic to encode instrumentation data to a stream. Perhaps using struct lib_outstream_s as defined in include/nuttx/stream.h. That way, the encoded data can go to either a memory stream or to the hardware parallel interface.
> > Similar rate/content controls will be needed.
>
> Yes, we can extend sched_note.c driver to support the different transport just like syslog(ramlog, itm, serial, file...).
No, you do not understand. With the hardware solution there is nothing buffered in ram, there is no driver, and sched_note.c is not built. sched_note.c is ONLY built if data will will buffered in RAM. None of that logic applies in the hardware solution. Nothing will be buffered in RAM.
You don't understand the architecture.
The path is direct from the instrumentation call-out functions in the OS to a new driver that immediately sends the date to the host via a parallel-to-USB device. CONFIG_SCHED_INSTRUMENTATION_BUFFER will not be enabled. A block diagram would be very space:
CORE OS ->Parallel port driver / USB ->Host driver -> Host Application
When CONFIG_SCHED_INSTRUMENTATION_BUFFER is enabled the path is like:
CORE OS -> RAM -> driver -> embedded application
What way you envision the architecture is incorrect.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442652974
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
$(Q) (cd proxies; $(MKSYSCALL) -p $(CSVFILE);)
$(Q) (cd stubs; $(MKSYSCALL) -s $(CSVFILE);)
+else
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
+ $(Q) (cd wraps; $(MKSYSCALL) -w $(CSVFILE);)
+ $(Q) $(MKSYSCALL) -l $(CSVFILE) > $(SYSCALLLIST:.txt=.h)
+ $(Q) $(call PREPROCESS, $(SYSCALLLIST:.txt=.h), $(SYSCALLLIST))
+ $(Q) sed -i -n -e '/^[^#]/p' $(SYSCALLLIST)
Review comment:
The suggestion is reasonable to support the different toolchain, or we can let arch ovevwrite the macro expansion process like this:
```
#include <nuttx/config.h>
#include <syscall.h>
#include <arch.h>
#ifndef UP_WRAP
#define UP_WRAP(f) --wrap=##f
#endif
define SYSCALL_LOOKUP1(f,n) UP_WRAP(f)
define SYSCALL_LOOKUP(f,n) UP_WRAP(f)
#include <sys/syscall_lookup.h>
#undef SYSCALL_LOOKUP1
#undef SYSCALL_LOOKUP
```
UP_WRAP is just demo, maybe we can find a better name.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647325092
I have fixed the commented issues on this PR.
For my task trace feature, sched note bufferring and driver code are needed but I feel that it requires re-design because current implementation have no consideration about the hardware solution and existing /dev/note interface.
There are also another issues such as static trace event filter.
I think that I have to solve the issues step by step by discussing with you.
----------------------------------------------------------------
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] patacongo commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r443061207
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
This absolutely must be reverted!!!!!!!!!!!!!!!!!!!!!!
In the FLAT build, CONFIG_LIB_SYSCALL may or may not be defined. If this CONFIG_LIB_SYSCALL is NOT defined, then this selection will be present in the menus. THAT IS ABSOLUTELY WRONG. There should be no invalid menu selection visible. This is necessary to keep the complexity of the configuration menus down.
And the logic is wrong AND IS A BUG. If someone does select CONFIG_SCHED_INSTRUMENTATION_SYSCALL when CONFIG_LIB_SYSCALL is *not* defined then the dead code will bet built into the system. THAT CANNOT BE PERMITTED. We must not permit invalid configuration options.
THIS MUST BE REVERTED!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442631758
##########
File path: syscall/Makefile
##########
@@ -76,27 +88,43 @@ $(BIN1): $(PROXY_OBJS)
$(BIN2): $(STUB_OBJS)
$(call ARCHIVE, $@, $(STUB_OBJS))
+$(BIN3): $(WRAP_OBJS)
+ $(call ARCHIVE, $@, $(WRAP_OBJS))
+
+$(SYSCALLIST): .context
+
.depend: Makefile $(SRCS)
- $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) \
+ $(Q) $(MKDEP) $(PROXYDEPPATH) $(STUBDEPPATH) $(WRAPDEPPATH) \
"$(CC)" -- $(CFLAGS) -- $(SRCS) >Make.dep
$(Q) touch $@
depend: .depend
.context: syscall.csv
$(Q) $(MAKE) -C $(TOPDIR)$(DELIM)tools -f Makefile.host mksyscall
+ifeq ($(CONFIG_LIB_SYSCALL),y)
$(Q) (cd proxies; $(MKSYSCALL) -p $(CSVFILE);)
$(Q) (cd stubs; $(MKSYSCALL) -s $(CSVFILE);)
+else
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
+ $(Q) (cd wraps; $(MKSYSCALL) -w $(CSVFILE);)
+ $(Q) $(MKSYSCALL) -l $(CSVFILE) > $(SYSCALLLIST:.txt=.h)
+ $(Q) $(call PREPROCESS, $(SYSCALLLIST:.txt=.h), $(SYSCALLLIST))
+ $(Q) sed -i -n -e '/^[^#]/p' $(SYSCALLLIST)
Review comment:
Because of the post process in arch's Makefile is that the '--wrap' option depends on GNU linker and I didn't want to add such code into arch independent syscall/ directory.
As you commented, the first three steps seems to be able to realize by using SYSCALL_LOOKUP macro without mksyscall -l.
How about this?
- Revert mksyscall change for -l option and fix syscall/Makefile not to use it.
- No change in arch's Makefile
----------------------------------------------------------------
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] patacongo commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647005523
> I understand your requirement, but we still need a layer between the sched_* callback and the real transport(either USB or ram), because we don't want duplicate these for each transport:
It would be as good if as much is common as possible. But the hardware solution has severe performance constaints (as the embedded solution may have severe memory constraints).
> 1.The start/stop trace control
There is no target user interface. The host interface is unidirectional.
> 2.The runtime filter functionality
Probably... but this depends on data rates.
> 3.Format to ftrace for graphic tool
No, this would not be done in the target in the hardware solution. This would be done on the host. But I will not be using ftrace. I will be using a Windows application. I don't do Linux develop; this will be a Windows only solution initially.
The "transport" is a binary byte stream. It has no particular application format. The receiving application can use the data in any way this it chooses.
This is a different project with different end-user objectives.
> The better path is:
> Core OS->sched_note->transport(RAM or USB)
> Sorry, I reuse sched_note terminoloy, the old sched_note should become a transport.
Currently, sched_note only buffers data to RAM. But the binary encoding logic should be sharable.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-649524611
Close, the same patch merge into feature/syscall-instrumentation branch:
https://github.com/apache/incubator-nuttx/pull/1288
----------------------------------------------------------------
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] patacongo commented on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647027075
>
>
> > > 3.Format to ftrace for graphic tool
> >
> >
> > No, this would not be done in the target in the hardware solution. This would be done on the host. But I will not be using ftrace. I will be using a Windows application. I don't do Linux develop; this will be a Windows only solution initially.
>
> Why not use a tool that already supports ftrace and Windows/Linux? I have stayed quite on this since this patch came up, but it is certainly the direction I would support
>
> Is there a technical downside to the ftrace format that you see?
I have no opinion. If someone wants to write such an application they are welcome. However, I will not be doing that.
My primary objective was to get the data path working. I have no interest in host applications. I will put something together for testing purpose (which will not be released to Apache because it will be a hack). It will not be ftrace.
Originally I posted a question asking for help from the community on this project. No help was offered... but lots of recommendations for things people want me to do. That was not what I was looking for.
There is no plan of record or anyone taking responsibility for host based tools.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-647030129
But from the initial NXView email:
https://lists.apache.org/thread.html/r1840904585459f9575877335f6d072e8854234384d1f554c6b26ee52%40%3Cdev.nuttx.apache.org%3E
The UI is the core part of the proposal if my understand is correct.
Since the instrumentation is already suppported, only two work need to be done:
1.Develop a hardware transport layer
2.Develop a desktop analyze tool
From my experience, the 2nd work is much tough than first one. That's why I am very interesting in @YuuichiNakamura ftrace proposal because there arey many tools(either graphic/command or linux/macos/windows) for ftrace.
----------------------------------------------------------------
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] patacongo edited a comment on pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-646976045
R> With the hardware solution, the instrumentation data will be encoded and sent out via a parallel port immediately when the call-out occurs. There will be no buffering in RAM; there will be not application driver. Those will all be disabled. The USB parallel solution will be direct instrumentation to hardware.
To make that job simpler and fully compatible with the effort you are doing, perhaps we need to have standalone logic to encode instrumentation data to a stream. Perhaps using struct lib_outstream_s as defined in include/nuttx/stream.h. That way, the encoded data can go to either a memory stream or to the hardware parallel interface.
Similar rate/content controls will be needed.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442903083
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
In my understanding, CONFIG_LIB_SYSCALL is defined in PROTECTED and KERNEL build and not in FLAT build.
I have made CONFIG_INSTRUMENTATION_SYSCALL feature available in all three build types and it is the reason I removed LIB_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] patacongo commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r443135232
##########
File path: sched/Kconfig
##########
@@ -975,7 +975,7 @@ config SCHED_INSTRUMENTATION_SPINLOCKS
config SCHED_INSTRUMENTATION_SYSCALL
bool "System call monitor hooks"
default n
- depends on LIB_SYSCALL && ARCH_HAVE_SYSCALL_HOOKS
+ depends on ARCH_HAVE_SYSCALL_HOOKS
Review comment:
> @YuuichiNakamura already support the filter and document here:
> https://github.com/YuuichiNakamura/nuttx-task-tracer-doc/blob/master/NuttXTaskTracer.md
> This filter is done at the runtime(I guess inside the sched_trace.c/sched_note.c driver). So we can:
> 1.Turn on/off the whole syscall instrumentation by SCHED_INSTRUMENTATION_SYSCALL
> 2.Turn on/off the individual syscall from command line at runtime
> The 3rd approach(static filter) only instrument a specific set of syscall(e.g. provide the cut down syscall.csv). But I wonder whether the static filter is required if we already have more powerful runtime filter.
Of course, that #2) CANNOT work for the hardware based solution since there is no driver in the hardware solution.
----------------------------------------------------------------
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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#issuecomment-646600886
Yes I think it has to be compile time as 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 #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442631912
##########
File path: syscall/syscall_name.c
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * syscall/syscall_name.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>
+
+#ifndef CONFIG_LIB_SYSCALL
+#define CONFIG_LIB_SYSCALL
+#endif
+#include <syscall.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/* System call name tables. This table is indexed by the system call numbers
+ * defined above. Given the system call number, this table provides the
+ * name of the corresponding system function.
+ */
+
+const char *g_syscallname[SYS_nsyscalls + 1] =
Review comment:
It is intended to be used by the application code which gets the data from /dev/note to convert syscall number into its name.
It's ok to change the variable name and put the declaration into syscall.h. But unlike others, it should not be surrounded by `#ifdef __KERNEL__`
##########
File path: syscall/syscall_name.c
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * syscall/syscall_name.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>
+
+#ifndef CONFIG_LIB_SYSCALL
+#define CONFIG_LIB_SYSCALL
+#endif
+#include <syscall.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/* System call name tables. This table is indexed by the system call numbers
+ * defined above. Given the system call number, this table provides the
+ * name of the corresponding system function.
+ */
+
+const char *g_syscallname[SYS_nsyscalls + 1] =
+{
+# define SYSCALL_LOOKUP1(f,n) #f
+# define SYSCALL_LOOKUP(f,n) , #f
+# include <sys/syscall_lookup.h>
+# undef SYSCALL_LOOKUP1
+# undef SYSCALL_LOOKUP
+ , 0
Review comment:
It was old workaround of the problem in the syscall number definition which already fixed by PR #791.
I'll remove the terminator.
----------------------------------------------------------------
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] patacongo commented on a change in pull request #1271: Feature/syscall instrumentation
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1271:
URL: https://github.com/apache/incubator-nuttx/pull/1271#discussion_r442210048
##########
File path: arch/arm/src/Makefile
##########
@@ -69,6 +69,19 @@ else
NUTTX = "$(TOPDIR)$(DELIM)nuttx$(EXEEXT)"
endif
+# Additional rules for system call wrapper
+
+ifeq ($(CONFIG_BUILD_FLAT),y)
+ifeq ($(CONFIG_SCHED_INSTRUMENTATION_SYSCALL),y)
Review comment:
Why is this restricted the FLAT build. System calls are seldom used with the FLAT build.
----------------------------------------------------------------
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