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 From 290c65b1ad4b8a42a9af507b411df480400d9aff Mon Sep 17 00:00:00 2001 From: Bernd Schubert Date: Tue, 5 Mar 2024 15:47:50 +0100 Subject: posix_spawn style updates - This adds a wrapper function for the call sequence of posix_spawn and posix_spawnp. - Replaces perror() with fuse_log - the latter can be redirected through the file system log function and gives better end user friendly output - other minor changes, like variable renames - no functional change --- lib/mount.c | 96 ++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 64 insertions(+), 32 deletions(-) (limited to 'lib/mount.c') diff --git a/lib/mount.c b/lib/mount.c index 2ffb05c..3fbbe72 100644 --- a/lib/mount.c +++ b/lib/mount.c @@ -117,18 +117,51 @@ static const struct fuse_opt fuse_mount_opts[] = { FUSE_OPT_END }; -void fuse_mount_version(void) +/* + * Running fusermount by calling 'posix_spawn' + * + * @param out_pid might be NULL + */ +static int fusermount_posix_spawn(posix_spawn_file_actions_t *action, + char const * const argv[], pid_t *out_pid) { - char const *const argv[] = {FUSERMOUNT_PROG, "--version", NULL}; + const char *full_path = FUSERMOUNT_DIR "/" FUSERMOUNT_PROG; 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"); + + /* See man 7 environ for the global environ pointer */ + + /* first try the install path */ + int status = posix_spawn(&pid, full_path, action, NULL, + (char * const *) argv, environ); + if (status != 0) { + /* if that fails, try a system install */ + status = posix_spawnp(&pid, FUSERMOUNT_PROG, action, NULL, + (char * const *) argv, environ); + } + + if (status != 0) { + fuse_log(FUSE_LOG_ERR, + "On calling fusermount posix_spawn failed: %s\n", + strerror(status)); + return -status; + } + + if (out_pid) + *out_pid = pid; else waitpid(pid, NULL, 0); + + return 0; +} + +void fuse_mount_version(void) +{ + char const *const argv[] = {FUSERMOUNT_PROG, "--version", NULL}; + int status = fusermount_posix_spawn(NULL, argv, NULL); + + if(status != 0) + fuse_log(FUSE_LOG_ERR, "Running '%s --version' failed", + FUSERMOUNT_PROG); } struct mount_flags { @@ -246,7 +279,7 @@ static int receive_fd(int fd) while(((rv = recvmsg(fd, &msg, 0)) == -1) && errno == EINTR); if (rv == -1) { - perror("recvmsg"); + fuse_log(FUSE_LOG_ERR, "recvmsg failed: %s", strerror(errno)); return -1; } if(!rv) { @@ -266,7 +299,6 @@ static int receive_fd(int fd) void fuse_kern_unmount(const char *mountpoint, int fd) { int res; - pid_t pid; if (fd != -1) { struct pollfd pfd; @@ -301,15 +333,12 @@ void fuse_kern_unmount(const char *mountpoint, int fd) 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); + int status = fusermount_posix_spawn(NULL, argv, NULL); if(status != 0) { - perror("posix_spawn"); + fuse_log(FUSE_LOG_ERR, "Spawaning %s to unumount failed", + FUSERMOUNT_PROG); return; } - waitpid(pid, NULL, 0); } static int setup_auto_unmount(const char *mountpoint, int quiet) @@ -325,7 +354,8 @@ static int setup_auto_unmount(const char *mountpoint, int quiet) res = socketpair(PF_UNIX, SOCK_STREAM, 0, fds); if(res == -1) { - perror("fuse: socketpair() failed"); + fuse_log(FUSE_LOG_ERR, "Setting up auto-unmountsocketpair() failed", + strerror(errno)); return -1; } @@ -351,16 +381,18 @@ static int setup_auto_unmount(const char *mountpoint, int quiet) } 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); + /* + * auto-umount runs in the background - it is not waiting for the + * process + */ + int status = fusermount_posix_spawn(&action, argv, &pid); + posix_spawn_file_actions_destroy(&action); if(status != 0) { close(fds[0]); close(fds[1]); - perror("fuse: posix_spawnp() of fusermount3 failed"); + fuse_log(FUSE_LOG_ERR, "fuse: Setting up auto-unmount failed"); return -1; } // passed to child now, so can close here. @@ -386,7 +418,8 @@ static int fuse_mount_fusermount(const char *mountpoint, struct mount_opts *mo, res = socketpair(PF_UNIX, SOCK_STREAM, 0, fds); if(res == -1) { - perror("fuse: socketpair() failed"); + fuse_log(FUSE_LOG_ERR, "Running %s: socketpair() failed: %s\n", + FUSERMOUNT_PROG, strerror(errno)); return -1; } @@ -412,23 +445,22 @@ static int fuse_mount_fusermount(const char *mountpoint, struct mount_opts *mo, } 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); + int status = fusermount_posix_spawn(&action, argv, &pid); + posix_spawn_file_actions_destroy(&action); if(status != 0) { close(fds[0]); close(fds[1]); - perror("fuse: posix_spawnp() of fusermount3 failed"); + fuse_log(FUSE_LOG_ERR, "posix_spawnp() for %s failed", + FUSERMOUNT_PROG, strerror(errno)); return -1; } // passed to child now, so can close here. close(fds[0]); - int rv = receive_fd(fds[1]); + int fd = receive_fd(fds[1]); if (!mo->auto_unmount) { /* with auto_unmount option fusermount3 will not exit until @@ -437,10 +469,10 @@ static int fuse_mount_fusermount(const char *mountpoint, struct mount_opts *mo, waitpid(pid, NULL, 0); /* bury zombie */ } - if (rv >= 0) - fcntl(rv, F_SETFD, FD_CLOEXEC); + if (fd >= 0) + fcntl(fd, F_SETFD, FD_CLOEXEC); - return rv; + return fd; } #ifndef O_CLOEXEC -- cgit v1.2.3