-
Notifications
You must be signed in to change notification settings - Fork 478
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
record: suggestion for fallback with named pipe creation #1805
base: master
Are you sure you want to change the base?
record: suggestion for fallback with named pipe creation #1805
Conversation
Specifically, some file systems like vboxsf and FAT do not support mkfifo(). As a result, the 'uftrace record' fails when creating the .channel file for communication. # uftrace record -vvv -P . ./spintest uftrace: running uftrace v0.14-21-gafed ( x86_64 dwarf python3 luajit tui perf sched dynamic ) uftrace: checking binary ./spintest uftrace: removing uftrace.data.old directory uftrace: /home/vagrant/uftrace/cmds/record.c:2273:command_record ERROR: cannot create a communication channel: Operation not permitted This patch, creates a fallback channel through 'tmpfs', which does support mkfifo(). In this suggestion, the .channel file will be named as the following. '/tmp/<dirname>.channel' Signed-off-by: Daniel T. Lee <[email protected]>
Currently, compiler generates mulitple dependency file with its own unique identifiers. This commit ignores auxiliary dependency files as well. libtraceevent/.event-parse.d.12994 libtraceevent/.event-plugin.d.12995 libtraceevent/.kbuffer-parse.d.13005 libtraceevent/.parse-filter.d.12999 libtraceevent/.parse-utils.d.13003 libtraceevent/.trace-seq.d.12998 Signed-off-by: Daniel T. Lee <[email protected]>
To pick up from where we left off, #1804 (comment) Using anon pipe might be the case, but as the commit 0b50230 states it's a no-go option.
Other IPC mechanism may work but performance matters. One approach that could be done without explicitly specifying such specific case is using macros. diff --git a/cmds/record.c b/cmds/record.c
index ec40ca17..05e3d930 100644
--- a/cmds/record.c
+++ b/cmds/record.c
@@ -2095,6 +2095,12 @@ int do_main_loop(int ready, struct uftrace_opts *opts, int pid)
xasprintf(&channel, "%s/%s", opts->dirname, ".channel");
+#ifndef CHANNEL_PREFIX
+#define CHANNEL_PREFIX "/tmp"
+#endif
+ if (access(channel, F_OK) != 0)
+ xasprintf(&channel, CHANNEL_PREFIX "/%s%s", opts->dirname, ".channel");
+
wd.pid = pid;
wd.pipefd = open(channel, O_RDONLY | O_NONBLOCK); cc … -DCHANNEL_PREFIX="@TERMUX_PREFIX@/tmp" … This could be work as a trade-off for adding additional macros relevant to build target. Opinions would be much appreciated. |
Thanks for the upload. I will need to test again in my android phone but need a bit more time. Please wait a little bit more. |
Originally it used a (unnamed) pipe by passing a file descriptor via "UFTRACE_PIPE" environ variable. You can check the git history to enable it as a fallback. |
I might not following this properly, but, Isn't the fallback to (unnamed) pipe will bring up the problem again with the possibility of "pipe ... closed by a target process" as stated at the commit 0b50230 ?
(I'm not sure what was the exact situation that leads to the close of the (unnamed) pipe back in 0b50230 (v0.9.3).) |
For some daemon programs, they want to close all existing file descriptors before start. But I guess it's a rare condition and should be ok to pass a pipe fd for most cases. |
We introduced named pipe when we found that there is no renderer process tracing data in chrome browser tracing. |
Ah, forgot about that. Yeah so it won't work if the target process active closes all existing file descriptors usually for security reasons. Then I'm ok to fallback to the temp directory, but we can keep the (unnamed) pipe as a last resort (for example, if the temp directory also refuses name pipes). |
So far as we speak, the following two approaches should be implemented with this patch:
And regarding the first matter, I have question about specifying the path. As stated earlier #1805 (comment) , |
Specifically, some file systems like
vboxsf
andFAT
do not supportmkfifo()
. As a result, the 'uftrace record' fails when creating the.channel
file for communication.# uftrace record -vvv -P . ./spintest uftrace: running uftrace v0.14-21-gafed ( x86_64 dwarf python3 luajit tui perf sched dynamic ) uftrace: checking binary ./spintest uftrace: removing uftrace.data.old directory uftrace: /home/vagrant/uftrace/cmds/record.c:2273:command_record ERROR: cannot create a communication channel: Operation not permitted
To address this issue, one proposed solution is creating a fallback
channel through
tmpfs
, which does support mkfifo(). In thissuggestion, the .channel file will be named as the following.
/tmp/<dirname>.channel
Close #1804