From f5ea02416c656bba136e742788f2d5ae12b98278 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Mon, 20 Jan 2020 19:44:07 -0500 Subject: [PATCH] fd-tracker: refactor: extract fs_handle interface from fd_tracker MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Make the fs_handle interface a proper abstract interface containing overridable callbacks. The objective of this refactor is to make it possible for lttng_trace_chunk to return fs_handles which are tracked by an fd_tracker (or not) depending on the execution context (which daemon). In effect, the relay daemon will provide a trace chunk with an fd_tracker to use and then rely on the fs_handle interface to track the use of file descriptors. The other daemons using the lttng_trace_chunk interface will use a dummy implementation of fs_handle which basically directly returns the underlying file descriptor and performs the unlink/close operations directly. This makes is possible to share code interacting with files between the various daemons without carrying a plethora of optional parameters in every util. Signed-off-by: Jérémie Galarneau Change-Id: Iaafa0f4442442bdfdaf220ce33a966978877df23 --- src/common/Makefile.am | 3 +- src/common/fd-tracker/fd-tracker.c | 93 +++++++++++++++++++----------- src/common/fs-handle-internal.h | 43 ++++++++++++++ src/common/fs-handle.c | 43 ++++++++++++++ src/common/fs-handle.h | 71 +++++++++++++++++++++++ 5 files changed, 217 insertions(+), 36 deletions(-) create mode 100644 src/common/fs-handle-internal.h create mode 100644 src/common/fs-handle.c create mode 100644 src/common/fs-handle.h diff --git a/src/common/Makefile.am b/src/common/Makefile.am index e7ceefe47..b89ce2a09 100644 --- a/src/common/Makefile.am +++ b/src/common/Makefile.am @@ -63,7 +63,8 @@ libcommon_la_SOURCES = \ utils.c utils.h \ uuid.c uuid.h \ tracker.c \ - waiter.c waiter.h + waiter.c waiter.h \ + fs-handle.h fs-handle-internal.h fs-handle.c if HAVE_ELF_H libcommon_la_SOURCES += \ diff --git a/src/common/fd-tracker/fd-tracker.c b/src/common/fd-tracker/fd-tracker.c index 5e6c8f30e..fd5573930 100644 --- a/src/common/fd-tracker/fd-tracker.c +++ b/src/common/fd-tracker/fd-tracker.c @@ -26,11 +26,12 @@ #include #include -#include "common/defaults.h" -#include "common/error.h" -#include "common/hashtable/hashtable.h" -#include "common/hashtable/utils.h" -#include "common/macros.h" +#include +#include +#include +#include +#include +#include #include "fd-tracker.h" #include "inode.h" @@ -94,7 +95,7 @@ struct open_properties { }; /* - * A fs_handle is not ref-counted. Therefore, it is assumed that a + * A fs_handle_tracked is not ref-counted. Therefore, it is assumed that a * handle is never in-use while it is being reclaimed. It can be * shared by multiple threads, but external synchronization is required * to ensure it is not still being used when it is reclaimed (close method). @@ -102,7 +103,8 @@ struct open_properties { * * The fs_handle lock always nests _within_ the tracker's lock. */ -struct fs_handle { +struct fs_handle_tracked { + struct fs_handle parent; pthread_mutex_t lock; /* * Weak reference to the tracker. All fs_handles are assumed to have @@ -146,18 +148,29 @@ static struct unsuspendable_fd *unsuspendable_fd_create( static int open_from_properties( const char *path, struct open_properties *properties); -static void fs_handle_log(struct fs_handle *handle); -static int fs_handle_suspend(struct fs_handle *handle); -static int fs_handle_restore(struct fs_handle *handle); +static void fs_handle_tracked_log(struct fs_handle_tracked *handle); +static int fs_handle_tracked_suspend(struct fs_handle_tracked *handle); +static int fs_handle_tracked_restore(struct fs_handle_tracked *handle); +static int fs_handle_tracked_get_fd(struct fs_handle *_handle); +static void fs_handle_tracked_put_fd(struct fs_handle *_handle); +static int fs_handle_tracked_unlink(struct fs_handle *_handle); +static int fs_handle_tracked_close(struct fs_handle *_handle); static void fd_tracker_track( - struct fd_tracker *tracker, struct fs_handle *handle); + struct fd_tracker *tracker, struct fs_handle_tracked *handle); static void fd_tracker_untrack( - struct fd_tracker *tracker, struct fs_handle *handle); + struct fd_tracker *tracker, struct fs_handle_tracked *handle); static int fd_tracker_suspend_handles( struct fd_tracker *tracker, unsigned int count); static int fd_tracker_restore_handle( - struct fd_tracker *tracker, struct fs_handle *handle); + struct fd_tracker *tracker, struct fs_handle_tracked *handle); + +static const struct fs_handle fs_handle_tracked_callbacks = { + .get_fd = fs_handle_tracked_get_fd, + .put_fd = fs_handle_tracked_put_fd, + .unlink = fs_handle_tracked_unlink, + .close = fs_handle_tracked_close, +}; /* Match function of the tracker's unsuspendable_fds hash table. */ static int match_fd(struct cds_lfht_node *node, const void *key) @@ -208,7 +221,7 @@ error: return NULL; } -static void fs_handle_log(struct fs_handle *handle) +static void fs_handle_tracked_log(struct fs_handle_tracked *handle) { const char *path; @@ -225,7 +238,7 @@ static void fs_handle_log(struct fs_handle *handle) } /* Tracker lock must be held by the caller. */ -static int fs_handle_suspend(struct fs_handle *handle) +static int fs_handle_tracked_suspend(struct fs_handle_tracked *handle) { int ret = 0; struct stat fs_stat; @@ -283,7 +296,7 @@ end: } /* Caller must hold the tracker and handle's locks. */ -static int fs_handle_restore(struct fs_handle *handle) +static int fs_handle_tracked_restore(struct fs_handle_tracked *handle) { int ret, fd = -1; const char *path = lttng_inode_get_path(handle->inode); @@ -393,7 +406,7 @@ error: void fd_tracker_log(struct fd_tracker *tracker) { - struct fs_handle *handle; + struct fs_handle_tracked *handle; struct unsuspendable_fd *unsuspendable_fd; struct cds_lfht_iter iter; @@ -411,13 +424,13 @@ void fd_tracker_log(struct fd_tracker *tracker) DBG_NO_LOC(" capacity: %u", tracker->capacity); DBG_NO_LOC(" Tracked suspendable file descriptors"); - cds_list_for_each_entry ( + cds_list_for_each_entry( handle, &tracker->active_handles, handles_list_node) { - fs_handle_log(handle); + fs_handle_tracked_log(handle); } - cds_list_for_each_entry (handle, &tracker->suspended_handles, + cds_list_for_each_entry(handle, &tracker->suspended_handles, handles_list_node) { - fs_handle_log(handle); + fs_handle_tracked_log(handle); } if (!SUSPENDABLE_COUNT(tracker)) { DBG_NO_LOC(" None"); @@ -425,7 +438,7 @@ void fd_tracker_log(struct fd_tracker *tracker) DBG_NO_LOC(" Tracked unsuspendable file descriptors"); rcu_read_lock(); - cds_lfht_for_each_entry (tracker->unsuspendable_fds, &iter, + cds_lfht_for_each_entry(tracker->unsuspendable_fds, &iter, unsuspendable_fd, tracker_node) { DBG_NO_LOC(" %s [active, fd %d]", unsuspendable_fd->name ?: "Unnamed", @@ -476,7 +489,7 @@ struct fs_handle *fd_tracker_open_fs_handle(struct fd_tracker *tracker, mode_t *mode) { int ret; - struct fs_handle *handle = NULL; + struct fs_handle_tracked *handle = NULL; struct stat fd_stat; struct open_properties properties = { .flags = flags, @@ -507,6 +520,7 @@ struct fs_handle *fd_tracker_open_fs_handle(struct fd_tracker *tracker, if (!handle) { goto end; } + handle->parent = fs_handle_tracked_callbacks; handle->tracker = tracker; ret = pthread_mutex_init(&handle->lock, NULL); @@ -539,7 +553,7 @@ struct fs_handle *fd_tracker_open_fs_handle(struct fd_tracker *tracker, fd_tracker_track(tracker, handle); end: pthread_mutex_unlock(&tracker->lock); - return handle; + return handle ? &handle->parent : NULL; error: if (handle->inode) { lttng_inode_put(handle->inode); @@ -556,14 +570,14 @@ static int fd_tracker_suspend_handles( struct fd_tracker *tracker, unsigned int count) { unsigned int left_to_close = count; - struct fs_handle *handle, *tmp; + struct fs_handle_tracked *handle, *tmp; - cds_list_for_each_entry_safe (handle, tmp, &tracker->active_handles, + cds_list_for_each_entry_safe(handle, tmp, &tracker->active_handles, handles_list_node) { int ret; fd_tracker_untrack(tracker, handle); - ret = fs_handle_suspend(handle); + ret = fs_handle_tracked_suspend(handle); fd_tracker_track(tracker, handle); if (!ret) { left_to_close--; @@ -742,7 +756,7 @@ end: /* Caller must have taken the tracker's and handle's locks. */ static void fd_tracker_track( - struct fd_tracker *tracker, struct fs_handle *handle) + struct fd_tracker *tracker, struct fs_handle_tracked *handle) { if (handle->fd >= 0) { tracker->count.suspendable.active++; @@ -757,7 +771,7 @@ static void fd_tracker_track( /* Caller must have taken the tracker's and handle's locks. */ static void fd_tracker_untrack( - struct fd_tracker *tracker, struct fs_handle *handle) + struct fd_tracker *tracker, struct fs_handle_tracked *handle) { if (handle->fd >= 0) { tracker->count.suspendable.active--; @@ -769,7 +783,7 @@ static void fd_tracker_untrack( /* Caller must have taken the tracker's and handle's locks. */ static int fd_tracker_restore_handle( - struct fd_tracker *tracker, struct fs_handle *handle) + struct fd_tracker *tracker, struct fs_handle_tracked *handle) { int ret; @@ -780,15 +794,17 @@ static int fd_tracker_restore_handle( goto end; } } - ret = fs_handle_restore(handle); + ret = fs_handle_tracked_restore(handle); end: fd_tracker_track(tracker, handle); return ret ? ret : handle->fd; } -int fs_handle_get_fd(struct fs_handle *handle) +static int fs_handle_tracked_get_fd(struct fs_handle *_handle) { int ret; + struct fs_handle_tracked *handle = + container_of(_handle, struct fs_handle_tracked, parent); /* * TODO This should be optimized as it is a fairly hot path. @@ -826,16 +842,21 @@ end: return ret; } -void fs_handle_put_fd(struct fs_handle *handle) +static void fs_handle_tracked_put_fd(struct fs_handle *_handle) { + struct fs_handle_tracked *handle = + container_of(_handle, struct fs_handle_tracked, parent); + pthread_mutex_lock(&handle->lock); handle->in_use = false; pthread_mutex_unlock(&handle->lock); } -int fs_handle_unlink(struct fs_handle *handle) +static int fs_handle_tracked_unlink(struct fs_handle *_handle) { int ret; + struct fs_handle_tracked *handle = + container_of(_handle, struct fs_handle_tracked, parent); pthread_mutex_lock(&handle->tracker->lock); pthread_mutex_lock(&handle->lock); @@ -845,10 +866,12 @@ int fs_handle_unlink(struct fs_handle *handle) return ret; } -int fs_handle_close(struct fs_handle *handle) +static int fs_handle_tracked_close(struct fs_handle *_handle) { int ret = 0; const char *path = NULL; + struct fs_handle_tracked *handle = + container_of(_handle, struct fs_handle_tracked, parent); if (!handle) { ret = -EINVAL; diff --git a/src/common/fs-handle-internal.h b/src/common/fs-handle-internal.h new file mode 100644 index 000000000..0fe3f8c63 --- /dev/null +++ b/src/common/fs-handle-internal.h @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2020 - Jérémie Galarneau + * + * This library is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by the + * Free Software Foundation; version 2.1 of the License. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#ifndef FS_HANDLE_INTERNAL_H +#define FS_HANDLE_INTERNAL_H + +struct fs_handle; + +/* + * Multiple internal APIs return fs_handles. For the moment, this internal + * interface allows the use of different fs_handle implementations in different + * daemons. For instance, the trace chunk interface returns fs_handles that + * behave diffently depending on whether or not the trace chunk was configured + * to use an fd-tracker. + */ + +typedef int (*fs_handle_get_fd_cb)(struct fs_handle *); +typedef void (*fs_handle_put_fd_cb)(struct fs_handle *); +typedef int (*fs_handle_unlink_cb)(struct fs_handle *); +typedef int (*fs_handle_close_cb)(struct fs_handle *); + +struct fs_handle { + fs_handle_get_fd_cb get_fd; + fs_handle_put_fd_cb put_fd; + fs_handle_unlink_cb unlink; + fs_handle_close_cb close; +}; + +#endif /* FS_HANDLE_INTERNAL_H */ diff --git a/src/common/fs-handle.c b/src/common/fs-handle.c new file mode 100644 index 000000000..e90f06d32 --- /dev/null +++ b/src/common/fs-handle.c @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2020 - Jérémie Galarneau + * + * This library is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by the + * Free Software Foundation; version 2.1 of the License. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include +#include + +LTTNG_HIDDEN +int fs_handle_get_fd(struct fs_handle *handle) +{ + return handle->get_fd(handle); +} + +LTTNG_HIDDEN +void fs_handle_put_fd(struct fs_handle *handle) +{ + return handle->put_fd(handle); +} + +LTTNG_HIDDEN +int fs_handle_unlink(struct fs_handle *handle) +{ + return handle->unlink(handle); +} + +LTTNG_HIDDEN +int fs_handle_close(struct fs_handle *handle) +{ + return handle->close(handle); +} diff --git a/src/common/fs-handle.h b/src/common/fs-handle.h new file mode 100644 index 000000000..e90ba3ca2 --- /dev/null +++ b/src/common/fs-handle.h @@ -0,0 +1,71 @@ +/* + * Copyright (C) 2020 - Jérémie Galarneau + * + * This library is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by the + * Free Software Foundation; version 2.1 of the License. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#ifndef FS_HANDLE_H +#define FS_HANDLE_H + +#include + +struct fs_handle; + +/* + * Marks the handle as the most recently used and marks the 'fd' as + * "in-use". This prevents the tracker from recycling the underlying + * file descriptor while it is actively being used by a thread. + * + * Don't forget that the tracker may be initiating an fd 'suspension' + * from another thread as the need to free an fd slot may arise from any + * thread within the daemon. + * + * Note that a restorable fd should never be held for longer than + * strictly necessary (e.g. the duration of a syscall()). + * + * Returns the fd on success, otherwise a negative value may be returned + * if the restoration of the fd failed. + */ +LTTNG_HIDDEN +int fs_handle_get_fd(struct fs_handle *handle); + +/* + * Used by the caller to signal that it is no longer using the underlying fd and + * that it may be safely suspended. + */ +LTTNG_HIDDEN +void fs_handle_put_fd(struct fs_handle *handle); + +/* + * Unlink the file associated to an fs_handle. Note that the unlink + * operation will not be performed immediately. It will only be performed + * once all references to the underlying file (through other fs_handle objects) + * have been released. + * + * However, note that the file will be renamed so as to provide the observable + * effect of an unlink(), that is removing a name from the filesystem. + * + * Returns 0 on success, otherwise a negative value will be returned + * if the operation failed. + */ +LTTNG_HIDDEN +int fs_handle_unlink(struct fs_handle *handle); + +/* + * Frees the handle and discards the underlying fd. + */ +LTTNG_HIDDEN +int fs_handle_close(struct fs_handle *handle); + +#endif /* FS_HANDLE_H */ -- 2.34.1