From bb9cecbf67341e03e53dd36b8f69520f3a26f834 Mon Sep 17 00:00:00 2001 From: Matthias Goergens Date: Tue, 5 Mar 2024 15:10:21 +0100 Subject: 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 +++++++++++++++++++++++++++++------------------------------- 1 file changed, 89 insertions(+), 93 deletions(-) (limited to 'lib/mount.c') 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 -- cgit v1.2.3