record: suggestion for fallback with named pipe creation #1804
Comments
This comment was marked as outdated.
This comment was marked as outdated.
DanielTimLee
changed the title
|
Hi @DanielTimLee, I'm glad to see you here again :) I also saw this problem when testing in an Android phone and it was reported at #1605 (review). Your approach looks reasonable so I may have to test it on my non-rooted Android phone when I have time later. |
|
Please let me know if this patch fits for your problem. |
|
Just figured out that the previous patch induces leak due to multiple xasprintf calls. Sorry for the inconvenience. 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");
+ /* try fallback fifo under tmpfs */
+ if (access(channel, F_OK) != 0) {
+ free(channel);
+ xasprintf(&channel, "/tmp/%s%s", opts->dirname, ".channel");
+ }
+
wd.pid = pid;
wd.pipefd = open(channel, O_RDONLY | O_NONBLOCK);
@@ -2269,8 +2275,13 @@ int command_record(int argc, char *argv[], struct uftrace_opts *opts)
return -1;
xasprintf(&channel, "%s/%s", opts->dirname, ".channel");
- if (mkfifo(channel, 0600) < 0)
- pr_err("cannot create a communication channel");
+ if (mkfifo(channel, 0600) < 0) {
+ free(channel);
+ /* try fallback fifo under tmpfs */
+ xasprintf(&channel, "/tmp/%s%s", opts->dirname, ".channel");
+ if (mkfifo(channel, 0600) < 0)
+ pr_err("cannot create a communication channel");
+ }
}
fflush(stdout);
diff --git a/libmcount/mcount.c b/libmcount/mcount.c
index 6347fab3..8a17c453 100644
--- a/libmcount/mcount.c
+++ b/libmcount/mcount.c
@@ -2259,6 +2259,12 @@ static __used void mcount_startup(void)
char *channel = NULL;
xasprintf(&channel, "%s/%s", dirname, ".channel");
+ /* try fallback fifo under tmpfs */
+ if (access(channel, F_OK) != 0) {
+ free(channel);
+ xasprintf(&channel, "/tmp/%s%s", dirname, ".channel");
+ }
+
pfd = open(channel, O_WRONLY);
free(channel);
} |
|
Thanks @DanielTimLee, even if this cannot be merged this way, you better create a PR and discuss inside. I can't write inline comment this way. I generally agree with this approach, but the only concern is that there is no JFYI, termux folks handle this problem as follows. --- a/cmds/live.c
+++ b/cmds/live.c
@@ -237,7 +237,7 @@
int command_live(int argc, char *argv[], struct uftrace_opts *opts)
{
#define LIVE_NAME "uftrace-live-XXXXXX"
- char template[32] = "/tmp/" LIVE_NAME;
+ char template[strlen("@TERMUX_PREFIX@") + 32] = "@TERMUX_PREFIX@/tmp/" LIVE_NAME;
int fd;
struct sigaction sa = {
.sa_flags = SA_RESETHAND,
@@ -253,7 +253,7 @@
strcpy(template, LIVE_NAME);
if (errno != EPERM && errno != ENOENT)
- pr_err("cannot access to /tmp");
+ pr_err("cannot access to @TERMUX_PREFIX@/tmp");
fd = mkstemp(template);https://github.com/termux/termux-packages/blob/master/packages/uftrace/cmds-live.c.patch I would like to see if there is a general change that we can remove those local patch in termux as well. |
Specifically, some file systems like
vboxsfandFATdo not supportmkfifo(). As a result, the 'uftrace record' fails when creating the.channelfile 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 permittedTo 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>.channelI'm aware that this problem can be simply avoided by executing uftrace
from other directory which doesn't affected by the specified file
systems. However, it would be preferable if uftrace handles by itself or at
least give heads up regarding this limitation.
Regarding this matter, I have created a rough patch. I would
appreciate it if you could review it.
The text was updated successfully, but these errors were encountered: