From bb9cecbf67341e03e53dd36b8f69520f3a26f834 Mon Sep 17 00:00:00 2001 From: Matthias Goergens Date: Tue, 5 Mar 2024 15:10:21 +0100 Subject: [PATCH] Use posix_spawn instead of fork+exec Client code might allocate a lot of memory before starting the mount. Fork is slow for processes that are using a lot of memory. But posix_spawn fixes that. Another issue with fork is if the process is also doing RDMA - this might lead to data corruption, as least if memory used for RDMA is not marked with MADV_DONTFORK. At least with linux kernels before 5.12. Also see https://blog.nelhage.com/post/a-cursed-bug/ for more details Change by Bernd: This also prepares the new fusermount option "--comm-fd", but keeps the previous way to pass the parameter as env variable. In a future release (exact data to be determined) we are going to remove usage of the env variable and will switch to the new parameter. --- lib/mount.c | 182 +++++++++++++++++++++++----------------------- util/fusermount.c | 15 ++-- 2 files changed, 99 insertions(+), 98 deletions(-) diff --git a/lib/mount.c b/lib/mount.c index f98a8bb..2ffb05c 100644 --- a/lib/mount.c +++ b/lib/mount.c @@ -8,6 +8,9 @@ See the file COPYING.LIB. */ +/* For environ */ +#define _GNU_SOURCE + #include "fuse_config.h" #include "fuse_i.h" #include "fuse_misc.h" @@ -22,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -44,10 +48,6 @@ #define FUSERMOUNT_PROG "fusermount3" #define FUSE_COMMFD_ENV "_FUSE_COMMFD" -#ifndef HAVE_FORK -#define fork() vfork() -#endif - #ifndef MS_DIRSYNC #define MS_DIRSYNC 128 #endif @@ -117,20 +117,17 @@ static const struct fuse_opt fuse_mount_opts[] = { FUSE_OPT_END }; -static void exec_fusermount(const char *argv[]) -{ - execv(FUSERMOUNT_DIR "/" FUSERMOUNT_PROG, (char **) argv); - execvp(FUSERMOUNT_PROG, (char **) argv); -} - void fuse_mount_version(void) { - int pid = fork(); - if (!pid) { - const char *argv[] = { FUSERMOUNT_PROG, "--version", NULL }; - exec_fusermount(argv); - _exit(1); - } else if (pid != -1) + char const *const argv[] = {FUSERMOUNT_PROG, "--version", NULL}; + pid_t pid; + int status = posix_spawn(&pid, FUSERMOUNT_DIR "/" FUSERMOUNT_PROG, NULL, + NULL, (char * const *) argv, environ) + && posix_spawnp(&pid, FUSERMOUNT_PROG, NULL, + NULL, (char * const *) argv, environ); + if(status != 0) + perror("posix_spawn"); + else waitpid(pid, NULL, 0); } @@ -269,7 +266,7 @@ static int receive_fd(int fd) void fuse_kern_unmount(const char *mountpoint, int fd) { int res; - int pid; + pid_t pid; if (fd != -1) { struct pollfd pfd; @@ -301,23 +298,24 @@ void fuse_kern_unmount(const char *mountpoint, int fd) if (res == 0) return; - pid = fork(); - if(pid == -1) + char const * const argv[] = + { FUSERMOUNT_PROG, "--unmount", "--quiet", "--lazy", + "--", mountpoint, NULL }; + int status = posix_spawn( + &pid, FUSERMOUNT_DIR "/" FUSERMOUNT_PROG, NULL, NULL, (char * const *) argv, environ) + && posix_spawnp( + &pid, FUSERMOUNT_PROG, NULL, NULL, (char * const *) argv, environ); + if(status != 0) { + perror("posix_spawn"); return; - - if(pid == 0) { - const char *argv[] = { FUSERMOUNT_PROG, "-u", "-q", "-z", - "--", mountpoint, NULL }; - - exec_fusermount(argv); - _exit(1); } waitpid(pid, NULL, 0); } static int setup_auto_unmount(const char *mountpoint, int quiet) { - int fds[2], pid; + int fds[2]; + pid_t pid; int res; if (!mountpoint) { @@ -331,55 +329,55 @@ static int setup_auto_unmount(const char *mountpoint, int quiet) return -1; } - pid = fork(); - if(pid == -1) { - perror("fuse: fork() failed"); - close(fds[0]); - close(fds[1]); - return -1; + char arg_fd_entry[30]; + snprintf(arg_fd_entry, sizeof(arg_fd_entry), "%i", fds[0]); + setenv(FUSE_COMMFD_ENV, arg_fd_entry, 1); + + char const *const argv[] = { + FUSERMOUNT_PROG, + "--auto-unmount", + "--", + mountpoint, + NULL, + }; + + // TODO: add error handling for all manipulations of action. + posix_spawn_file_actions_t action; + posix_spawn_file_actions_init(&action); + + if (quiet) { + posix_spawn_file_actions_addclose(&action, 1); + posix_spawn_file_actions_addclose(&action, 2); } + posix_spawn_file_actions_addclose(&action, fds[1]); - if(pid == 0) { - char env[10]; - const char *argv[32]; - int a = 0; - - if (quiet) { - int fd = open("/dev/null", O_RDONLY); - if (fd != -1) { - dup2(fd, 1); - dup2(fd, 2); - } - } - - argv[a++] = FUSERMOUNT_PROG; - argv[a++] = "--auto-unmount"; - argv[a++] = "--"; - argv[a++] = mountpoint; - argv[a++] = NULL; + int status = posix_spawn(&pid, FUSERMOUNT_DIR "/" FUSERMOUNT_PROG, &action, + NULL, (char *const *) argv, environ) + && posix_spawnp(&pid, FUSERMOUNT_PROG, &action, + NULL, (char *const *) argv, environ); + posix_spawn_file_actions_destroy(&action); + if(status != 0) { + close(fds[0]); close(fds[1]); - fcntl(fds[0], F_SETFD, 0); - snprintf(env, sizeof(env), "%i", fds[0]); - setenv(FUSE_COMMFD_ENV, env, 1); - exec_fusermount(argv); - perror("fuse: failed to exec fusermount3"); - _exit(1); + perror("fuse: posix_spawnp() of fusermount3 failed"); + return -1; } - + // passed to child now, so can close here. close(fds[0]); // Now fusermount3 will only exit when fds[1] closes automatically when our // process exits. return 0; + // Note: fds[1] is leakend and doesn't get FD_CLOEXEC } static int fuse_mount_fusermount(const char *mountpoint, struct mount_opts *mo, const char *opts, int quiet) { - int fds[2], pid; + int fds[2]; + pid_t pid; int res; - int rv; if (!mountpoint) { fuse_log(FUSE_LOG_ERR, "fuse: missing mountpoint parameter\n"); @@ -392,47 +390,45 @@ static int fuse_mount_fusermount(const char *mountpoint, struct mount_opts *mo, return -1; } - pid = fork(); - if(pid == -1) { - perror("fuse: fork() failed"); - close(fds[0]); - close(fds[1]); - return -1; - } + char arg_fd_entry[30]; + snprintf(arg_fd_entry, sizeof(arg_fd_entry), "%i", fds[0]); + setenv(FUSE_COMMFD_ENV, arg_fd_entry, 1); - if(pid == 0) { - char env[10]; - const char *argv[32]; - int a = 0; + char const *const argv[] = { + FUSERMOUNT_PROG, + "-o", opts ? opts : "", + "--", + mountpoint, + NULL, + }; - if (quiet) { - int fd = open("/dev/null", O_RDONLY); - if (fd != -1) { - dup2(fd, 1); - dup2(fd, 2); - } - } - argv[a++] = FUSERMOUNT_PROG; - if (opts) { - argv[a++] = "-o"; - argv[a++] = opts; - } - argv[a++] = "--"; - argv[a++] = mountpoint; - argv[a++] = NULL; + posix_spawn_file_actions_t action; + posix_spawn_file_actions_init(&action); + + if (quiet) { + posix_spawn_file_actions_addclose(&action, 1); + posix_spawn_file_actions_addclose(&action, 2); + } + posix_spawn_file_actions_addclose(&action, fds[1]); + int status = posix_spawn(&pid, FUSERMOUNT_DIR "/" FUSERMOUNT_PROG, &action, + NULL, (char *const *) argv, environ) + && posix_spawnp(&pid, FUSERMOUNT_PROG, &action, + NULL, (char *const *) argv, environ); + posix_spawn_file_actions_destroy(&action); + + if(status != 0) { + close(fds[0]); close(fds[1]); - fcntl(fds[0], F_SETFD, 0); - snprintf(env, sizeof(env), "%i", fds[0]); - setenv(FUSE_COMMFD_ENV, env, 1); - exec_fusermount(argv); - perror("fuse: failed to exec fusermount3"); - _exit(1); + perror("fuse: posix_spawnp() of fusermount3 failed"); + return -1; } + // passed to child now, so can close here. close(fds[0]); - rv = receive_fd(fds[1]); + + int rv = receive_fd(fds[1]); if (!mo->auto_unmount) { /* with auto_unmount option fusermount3 will not exit until diff --git a/util/fusermount.c b/util/fusermount.c index 06f5f56..5716c76 100644 --- a/util/fusermount.c +++ b/util/fusermount.c @@ -1454,7 +1454,7 @@ int main(int argc, char *argv[]) static int unmount = 0; static int lazy = 0; static int quiet = 0; - char *commfd; + char *commfd = NULL; int cfd; const char *opts = ""; const char *type = NULL; @@ -1462,14 +1462,15 @@ int main(int argc, char *argv[]) static const struct option long_opts[] = { {"unmount", no_argument, NULL, 'u'}, - // Note: auto-unmount deliberately does not have a short version. - // It's meant for internal use by mount.c's setup_auto_unmount. - {"auto-unmount", no_argument, NULL, 'U'}, {"lazy", no_argument, NULL, 'z'}, {"quiet", no_argument, NULL, 'q'}, {"help", no_argument, NULL, 'h'}, {"version", no_argument, NULL, 'V'}, {"options", required_argument, NULL, 'o'}, + // Note: auto-unmount and comm-fd don't have short versions. + // They'ne meant for internal use by mount.c + {"auto-unmount", no_argument, NULL, 'U'}, + {"comm-fd", required_argument, NULL, 'c'}, {0, 0, 0, 0}}; progname = strdup(argc > 0 ? argv[0] : "fusermount"); @@ -1501,6 +1502,9 @@ int main(int argc, char *argv[]) auto_unmount = 1; setup_auto_unmount_only = 1; break; + case 'c': + commfd = optarg; + break; case 'z': lazy = 1; break; @@ -1547,7 +1551,8 @@ int main(int argc, char *argv[]) if (!setup_auto_unmount_only && unmount) goto do_unmount; - commfd = getenv(FUSE_COMMFD_ENV); + if(commfd == NULL) + commfd = getenv(FUSE_COMMFD_ENV); if (commfd == NULL) { fprintf(stderr, "%s: old style mounting not supported\n", progname); -- 2.30.2