From dd95933f8371a8c81ffc1dc5b306f2263f1ff808 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Thu, 30 Jan 2020 11:45:07 -0500 Subject: [PATCH] relayd: track directory handles through the fd-tracker MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Track directory handles through the fd-tracker as unsuspendable file descriptors. New fd-tracker utils are introduced to wrap the creation and registration of the file descriptors to the fd-tracker. Note that file descriptors only need to be tracked when the dirfd-backed implementation of the directory handles is used. Signed-off-by: Jérémie Galarneau Change-Id: I754af1943f5d6f02a6219d48c8fc4b8106de1c13 --- src/bin/lttng-relayd/session.c | 6 +- src/common/compat/directory-handle.c | 4 + src/common/compat/directory-handle.h | 8 ++ src/common/fd-tracker/fd-tracker.c | 34 ++++++- src/common/fd-tracker/inode.c | 14 ++- src/common/fd-tracker/inode.h | 7 +- src/common/fd-tracker/utils.c | 136 ++++++++++++++++++++++++++- src/common/fd-tracker/utils.h | 17 ++++ src/common/trace-chunk.c | 34 +++++-- 9 files changed, 240 insertions(+), 20 deletions(-) diff --git a/src/bin/lttng-relayd/session.c b/src/bin/lttng-relayd/session.c index 3dcb1cd33..daae9a55b 100644 --- a/src/bin/lttng-relayd/session.c +++ b/src/bin/lttng-relayd/session.c @@ -9,11 +9,11 @@ #define _LGPL_SOURCE #include -#include +#include +#include #include #include #include -#include #include #include @@ -196,7 +196,7 @@ static struct lttng_directory_handle *session_create_output_directory_handle( goto end; } - handle = lttng_directory_handle_create(full_session_path); + handle = fd_tracker_create_directory_handle(the_fd_tracker, full_session_path); end: pthread_mutex_unlock(&session->lock); free(full_session_path); diff --git a/src/common/compat/directory-handle.c b/src/common/compat/directory-handle.c index eb1cd926e..f0139be29 100644 --- a/src/common/compat/directory-handle.c +++ b/src/common/compat/directory-handle.c @@ -181,6 +181,10 @@ void lttng_directory_handle_release(struct urcu_ref *ref) struct lttng_directory_handle *handle = container_of(ref, struct lttng_directory_handle, ref); + if (handle->destroy_cb) { + handle->destroy_cb(handle, handle->destroy_cb_data); + } + if (handle->dirfd == AT_FDCWD || handle->dirfd == -1) { goto end; } diff --git a/src/common/compat/directory-handle.h b/src/common/compat/directory-handle.h index 35e91f71c..574a09d74 100644 --- a/src/common/compat/directory-handle.h +++ b/src/common/compat/directory-handle.h @@ -26,10 +26,18 @@ enum lttng_directory_handle_rmdir_recursive_flags { * or a directory file descriptors, depending on the platform's capabilities. */ #ifdef COMPAT_DIRFD + +struct lttng_directory_handle; + +typedef void (*lttng_directory_handle_destroy_cb)( + struct lttng_directory_handle *handle, void *data); + struct lttng_directory_handle { struct urcu_ref ref; ino_t directory_inode; int dirfd; + lttng_directory_handle_destroy_cb destroy_cb; + void *destroy_cb_data; }; static inline diff --git a/src/common/fd-tracker/fd-tracker.c b/src/common/fd-tracker/fd-tracker.c index 417859abe..188abfb6d 100644 --- a/src/common/fd-tracker/fd-tracker.c +++ b/src/common/fd-tracker/fd-tracker.c @@ -210,7 +210,7 @@ static void fs_handle_tracked_log(struct fs_handle_tracked *handle) const char *path; pthread_mutex_lock(&handle->lock); - lttng_inode_get_location(handle->inode, NULL, &path); + lttng_inode_borrow_location(handle->inode, NULL, &path); if (handle->fd >= 0) { DBG_NO_LOC(" %s [active, fd %d%s]", path, handle->fd, @@ -230,7 +230,8 @@ static int fs_handle_tracked_suspend(struct fs_handle_tracked *handle) const struct lttng_directory_handle *node_directory_handle; pthread_mutex_lock(&handle->lock); - lttng_inode_get_location(handle->inode, &node_directory_handle, &path); + lttng_inode_borrow_location( + handle->inode, &node_directory_handle, &path); assert(handle->fd >= 0); if (handle->in_use) { /* This handle can't be suspended as it is currently in use. */ @@ -288,7 +289,8 @@ static int fs_handle_tracked_restore(struct fs_handle_tracked *handle) const char *path; const struct lttng_directory_handle *node_directory_handle; - lttng_inode_get_location(handle->inode, &node_directory_handle, &path); + lttng_inode_borrow_location( + handle->inode, &node_directory_handle, &path); assert(handle->fd == -1); assert(path); @@ -882,6 +884,7 @@ static int fs_handle_tracked_close(struct fs_handle *_handle) const char *path = NULL; struct fs_handle_tracked *handle = container_of(_handle, struct fs_handle_tracked, parent); + struct lttng_directory_handle *inode_directory_handle = NULL; if (!handle) { ret = -EINVAL; @@ -891,7 +894,29 @@ static int fs_handle_tracked_close(struct fs_handle *_handle) pthread_mutex_lock(&handle->tracker->lock); pthread_mutex_lock(&handle->lock); if (handle->inode) { - lttng_inode_get_location(handle->inode, NULL, &path); + lttng_inode_borrow_location(handle->inode, NULL, &path); + /* + * Here a reference to the inode's directory handle is acquired + * to prevent the last reference to it from being released while + * the tracker's lock is taken. + * + * If this wasn't done, the directory handle could attempt to + * close its underlying directory file descriptor, which would + * attempt to lock the tracker's lock, resulting in a deadlock. + * + * Since a new reference to the directory handle is taken within + * the scope of this function, it is not possible for the last + * reference to the inode's location directory handle to be + * released during the call to lttng_inode_put(). + * + * We wait until the tracker's lock is released to release the + * reference. Hence, the call to the tracker is delayed just + * enough to not attempt to recursively acquire the tracker's + * lock twice. + */ + inode_directory_handle = + lttng_inode_get_location_directory_handle( + handle->inode); } fd_tracker_untrack(handle->tracker, handle); if (handle->fd >= 0) { @@ -912,6 +937,7 @@ static int fs_handle_tracked_close(struct fs_handle *_handle) pthread_mutex_destroy(&handle->lock); pthread_mutex_unlock(&handle->tracker->lock); free(handle); + lttng_directory_handle_put(inode_directory_handle); end: return ret; } diff --git a/src/common/fd-tracker/inode.c b/src/common/fd-tracker/inode.c index 0e1ebc6ff..727f6141a 100644 --- a/src/common/fd-tracker/inode.c +++ b/src/common/fd-tracker/inode.c @@ -280,7 +280,19 @@ void lttng_inode_put(struct lttng_inode *inode) urcu_ref_put(&inode->ref, lttng_inode_release); } -void lttng_inode_get_location(struct lttng_inode *inode, +struct lttng_directory_handle *lttng_inode_get_location_directory_handle( + struct lttng_inode *inode) +{ + if (inode->location.directory_handle) { + const bool reference_acquired = lttng_directory_handle_get( + inode->location.directory_handle); + + assert(reference_acquired); + } + return inode->location.directory_handle; +} + +void lttng_inode_borrow_location(struct lttng_inode *inode, const struct lttng_directory_handle **out_directory_handle, const char **out_path) { diff --git a/src/common/fd-tracker/inode.h b/src/common/fd-tracker/inode.h index d69f354ea..7e95c0726 100644 --- a/src/common/fd-tracker/inode.h +++ b/src/common/fd-tracker/inode.h @@ -14,6 +14,7 @@ struct lttng_inode; struct lttng_inode_registry; struct lttng_unlinked_file_directory; +struct lttng_directory_handle; /* * The unlinked file pool is protected by the fd-tracker's lock. @@ -41,10 +42,14 @@ struct lttng_inode *lttng_inode_registry_get_inode( void lttng_inode_registry_destroy(struct lttng_inode_registry *registry); -void lttng_inode_get_location(struct lttng_inode *inode, +void lttng_inode_borrow_location(struct lttng_inode *inode, const struct lttng_directory_handle **out_directory_handle, const char **out_path); +/* Returns a new reference to the inode's location directory handle. */ +struct lttng_directory_handle *lttng_inode_get_location_directory_handle( + struct lttng_inode *inode); + int lttng_inode_rename(struct lttng_inode *inode, struct lttng_directory_handle *old_directory_handle, const char *old_path, diff --git a/src/common/fd-tracker/utils.c b/src/common/fd-tracker/utils.c index 1f71fd496..fe2cdcc4e 100644 --- a/src/common/fd-tracker/utils.c +++ b/src/common/fd-tracker/utils.c @@ -5,29 +5,35 @@ * */ +#include #include #include +#include #include #include #include -static int open_pipe_cloexec(void *data, int *fds) +static +int open_pipe_cloexec(void *data, int *fds) { return utils_create_pipe_cloexec(fds); } -static int close_pipe(void *data, int *pipe) +static +int close_pipe(void *data, int *pipe) { utils_close_pipe(pipe); pipe[0] = pipe[1] = -1; return 0; } +LTTNG_HIDDEN int fd_tracker_util_close_fd(void *unused, int *fd) { return close(*fd); } +LTTNG_HIDDEN int fd_tracker_util_pipe_open_cloexec( struct fd_tracker *tracker, const char *name, int *pipe) { @@ -53,8 +59,134 @@ end: return ret; } +LTTNG_HIDDEN int fd_tracker_util_pipe_close(struct fd_tracker *tracker, int *pipe) { return fd_tracker_close_unsuspendable_fd( tracker, pipe, 2, close_pipe, NULL); } + +struct open_directory_handle_args { + const struct lttng_directory_handle *in_handle; + struct lttng_directory_handle *ret_handle; + const char *path; +}; + +static +int open_directory_handle(void *_args, int *out_fds) +{ + int ret = 0; + struct open_directory_handle_args *args = _args; + struct lttng_directory_handle *new_handle = NULL; + + new_handle = args->in_handle ? + lttng_directory_handle_create_from_handle( + args->path, args->in_handle) : + lttng_directory_handle_create(args->path); + if (!new_handle) { + ret = -errno; + goto end; + } + + args->ret_handle = new_handle; + + /* + * Reserved to indicate that the handle does not use a handle; there is + * nothing to track. We want to indicate an error to the fd-tracker so + * that it doesn't attempt to track the file descriptor, but also want + * the caller to retrieve the newly-created handle. + * + * Calling this a hack is a fair assessment. + */ + if (!lttng_directory_handle_uses_fd(new_handle)) { + ret = ENOTSUP; + } else { +#ifdef COMPAT_DIRFD + *out_fds = new_handle->dirfd; +#else + abort(); +#endif + + } +end: + return ret; +} + +#ifdef COMPAT_DIRFD +static +int fd_close(void *unused, int *in_fds) +{ + const int ret = close(in_fds[0]); + + in_fds[0] = -1; + return ret; +} + +static +void directory_handle_destroy( + struct lttng_directory_handle *handle, void *data) +{ + struct fd_tracker *tracker = data; + const int ret = fd_tracker_close_unsuspendable_fd( + tracker, &handle->dirfd, 1, fd_close, NULL); + + if (ret) { + ERR("Failed to untrack directory handle file descriptor"); + } +} +#endif + +LTTNG_HIDDEN +struct lttng_directory_handle *fd_tracker_create_directory_handle( + struct fd_tracker *tracker, const char *path) +{ + return fd_tracker_create_directory_handle_from_handle( + tracker, NULL, path); +} + +LTTNG_HIDDEN +struct lttng_directory_handle *fd_tracker_create_directory_handle_from_handle( + struct fd_tracker *tracker, + struct lttng_directory_handle *in_handle, + const char *path) +{ + int ret; + int dirfd = -1; + char *handle_name = NULL; + char cwd_path[LTTNG_PATH_MAX] = "working directory"; + struct lttng_directory_handle *new_handle = NULL; + struct open_directory_handle_args open_args = { + .in_handle = in_handle, + .path = path, + }; + + if (!path) { + if (!getcwd(cwd_path, sizeof(cwd_path))) { + PERROR("Failed to get current working directory to name directory handle"); + goto end; + } + } + + ret = asprintf(&handle_name, "Directory handle to %s", + path ? path : cwd_path); + if (ret < 0) { + PERROR("Failed to format directory handle name"); + goto end; + } + + ret = fd_tracker_open_unsuspendable_fd(tracker, &dirfd, + (const char **) &handle_name, 1, open_directory_handle, + &open_args); + if (ret && ret != ENOTSUP) { + ERR("Failed to open directory handle to %s through the fd tracker", path ? path : cwd_path); + } + new_handle = open_args.ret_handle; + +#ifdef COMPAT_DIRFD + new_handle->destroy_cb = directory_handle_destroy; + new_handle->destroy_cb_data = tracker; +#endif +end: + free(handle_name); + return new_handle; +} diff --git a/src/common/fd-tracker/utils.h b/src/common/fd-tracker/utils.h index 617ee49bb..202c4a1b8 100644 --- a/src/common/fd-tracker/utils.h +++ b/src/common/fd-tracker/utils.h @@ -8,7 +8,9 @@ #ifndef FD_TRACKER_UTILS_H #define FD_TRACKER_UTILS_H +#include #include +#include struct lttng_poll_event; @@ -16,24 +18,39 @@ struct lttng_poll_event; * Utility implementing a close_fd callback which receives one file descriptor * and closes it, returning close()'s return value. */ +LTTNG_HIDDEN int fd_tracker_util_close_fd(void *, int *fd); /* * Create a pipe and track its underlying fds. */ +LTTNG_HIDDEN int fd_tracker_util_pipe_open_cloexec( struct fd_tracker *tracker, const char *name, int *pipe); +LTTNG_HIDDEN int fd_tracker_util_pipe_close(struct fd_tracker *tracker, int *pipe); /* * Create a poll event and track its underlying fd, if applicable. */ +LTTNG_HIDDEN int fd_tracker_util_poll_create(struct fd_tracker *tracker, const char *name, struct lttng_poll_event *events, int size, int flags); +LTTNG_HIDDEN int fd_tracker_util_poll_clean( struct fd_tracker *tracker, struct lttng_poll_event *events); +LTTNG_HIDDEN +struct lttng_directory_handle *fd_tracker_create_directory_handle( + struct fd_tracker *tracker, const char *path); + +LTTNG_HIDDEN +struct lttng_directory_handle *fd_tracker_create_directory_handle_from_handle( + struct fd_tracker *tracker, + struct lttng_directory_handle *handle, + const char *path); + #endif /* FD_TRACKER_UTILS_H */ diff --git a/src/common/trace-chunk.c b/src/common/trace-chunk.c index f84671252..63bfb6c52 100644 --- a/src/common/trace-chunk.c +++ b/src/common/trace-chunk.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -781,9 +782,14 @@ enum lttng_trace_chunk_status lttng_trace_chunk_rename_path_no_lock( status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto end; } - rename_directory = lttng_directory_handle_create_from_handle( - path, - chunk->session_output_directory); + rename_directory = chunk->fd_tracker ? + fd_tracker_create_directory_handle_from_handle( + chunk->fd_tracker, + chunk->session_output_directory, + path) : + lttng_directory_handle_create_from_handle( + path, + chunk->session_output_directory); if (!rename_directory) { ERR("Failed to get handle to trace chunk rename directory"); status = LTTNG_TRACE_CHUNK_STATUS_ERROR; @@ -1039,9 +1045,14 @@ enum lttng_trace_chunk_status lttng_trace_chunk_set_as_owner( goto end; } chunk_directory_handle = - lttng_directory_handle_create_from_handle( - chunk->path, - session_output_directory); + chunk->fd_tracker ? + fd_tracker_create_directory_handle_from_handle( + chunk->fd_tracker, + session_output_directory, + chunk->path) : + lttng_directory_handle_create_from_handle( + chunk->path, + session_output_directory); if (!chunk_directory_handle) { /* The function already logs on all error paths. */ status = LTTNG_TRACE_CHUNK_STATUS_ERROR; @@ -1572,9 +1583,14 @@ int lttng_trace_chunk_move_to_completed_post_release( goto end; } - archived_chunks_directory = lttng_directory_handle_create_from_handle( - DEFAULT_ARCHIVED_TRACE_CHUNKS_DIRECTORY, - trace_chunk->session_output_directory); + archived_chunks_directory = trace_chunk->fd_tracker ? + fd_tracker_create_directory_handle_from_handle( + trace_chunk->fd_tracker, + trace_chunk->session_output_directory, + DEFAULT_ARCHIVED_TRACE_CHUNKS_DIRECTORY) : + lttng_directory_handle_create_from_handle( + DEFAULT_ARCHIVED_TRACE_CHUNKS_DIRECTORY, + trace_chunk->session_output_directory); if (!archived_chunks_directory) { PERROR("Failed to get handle to archived trace chunks directory"); ret = -1; -- 2.34.1