From 4c6b063463ae48b3509ff8e66cd391f8637a86af Mon Sep 17 00:00:00 2001 From: Csaba Henk Date: Tue, 21 Aug 2018 12:44:54 +0200 Subject: fuse: diagnostic FLUSH interrupt We add dummy interrupt handling for the FLUSH fuse message. It can be enabled by the "--fuse-flush-handle-interrupt" hidden command line option, or "-ofuse-flush-handle-interrupt=yes" mount option. It serves no other than diagnostic & demonstational purposes -- to exercise the interrupt handling framework a bit and to give an usage example. Documentation is also provided that showcases interrupt handling via FLUSH. Change-Id: I522f1e798501d06b74ac3592a5f73c1ab0590c60 updates: #465 Signed-off-by: Csaba Henk --- doc/developer-guide/README.md | 5 ++ doc/developer-guide/fuse-interrupt.md | 130 ++++++++++++++++++++++++++++ glusterfsd/src/glusterfsd.c | 44 ++++++++++ glusterfsd/src/glusterfsd.h | 1 + libglusterfs/src/glusterfs.h | 2 + tests/features/interrupt.t | 67 ++++++++++++++ tests/features/open_and_sleep.c | 27 ++++++ xlators/mount/fuse/src/fuse-bridge.c | 60 ++++++++++++- xlators/mount/fuse/src/fuse-bridge.h | 4 +- xlators/mount/fuse/utils/mount.glusterfs.in | 7 ++ 10 files changed, 345 insertions(+), 2 deletions(-) create mode 100644 doc/developer-guide/fuse-interrupt.md create mode 100644 tests/features/interrupt.t create mode 100644 tests/features/open_and_sleep.c diff --git a/doc/developer-guide/README.md b/doc/developer-guide/README.md index 4c6346e83d4..6c00a4a221b 100644 --- a/doc/developer-guide/README.md +++ b/doc/developer-guide/README.md @@ -59,6 +59,11 @@ Translators - [Storage/posix Translator](./posix.md) - [Compression translator](./network_compression.md) +Fuse +---- + +- [Interrupt handling](./fuse-interrupt.md) + Testing/Debugging ----------------- diff --git a/doc/developer-guide/fuse-interrupt.md b/doc/developer-guide/fuse-interrupt.md new file mode 100644 index 00000000000..f92b5532eaf --- /dev/null +++ b/doc/developer-guide/fuse-interrupt.md @@ -0,0 +1,130 @@ +# Fuse interrupt handling + +## Conventions followed + +- *FUSE* refers to the "wire protocol" between kernel and userspace and + related specifications. +- *fuse* refers to the kernel subsystem and also to the GlusterFs translator. + +## FUSE interrupt handling spec + +The [Linux kernel FUSE documentation](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/filesystems/fuse.txt?h=v4.18#n148) +desrcibes how interrupt handling happens in fuse. + +## Interrupt handling in the fuse translator + +### Declarations + +This document describes the internal API in the fuse translator with which +interrupt can be handled. + +The API being internal (to be used only in fuse-bridge.c; the functions are +not exported to a header file). + +``` +enum fuse_interrupt_state { + INTERRUPT_NONE, + INTERRUPT_SQUELCHED, + INTERRUPT_HANDLED, +}; +typedef enum fuse_interrupt_state fuse_interrupt_state_t; +struct fuse_interrupt_record; +typedef struct fuse_interrupt_record fuse_interrupt_record_t; +typedef void (*fuse_interrupt_handler_t)(xlator_t *this, + fuse_interrupt_record_t *); +struct fuse_interrupt_record { + fuse_in_header_t fuse_in_header; + void *data; + /* + ... + */ +}; + +fuse_interrupt_record_t * +fuse_interrupt_record_new(fuse_in_header_t *finh, + fuse_interrupt_handler_t handler); + +void +fuse_interrupt_record_insert(xlator_t *this, fuse_interrupt_record_t *fir); + +gf_boolean_t +fuse_interrupt_finish_fop(call_frame_t *frame, xlator_t *this, + gf_boolean_t sync, void **datap); + +void +fuse_interrupt_finish_interrupt(xlator_t *this, fuse_interrupt_record_t *fir, + fuse_interrupt_state_t intstat, + gf_boolean_t sync, void **datap); +``` + +The code demonstrates the usage of the API through `fuse_flush()`. (It's a +dummy implementation only for demonstration purposes.) Flush is chosen +because a `FLUSH` interrupt is easy to trigger (see +*tests/features/interrupt.t*). Interrupt handling for flush is switched on +by `--fuse-flush-handle-interrupt` (a hidden glusterfs command line flag). +The flush interrupt handling code is guarded by the +`flush_handle_interrupt` Boolean member of `fuse_private_t`. + +### Usage + +A given FUSE fop can be enabled to handle interrupts via the following +steps: + +- Define a handler function (of type `fuse_interrupt_handler_t`). + It should implement the interrupt handling logic and in the end + call (directly or as async callback) `fuse_interrupt_finish_interrupt()`. + The `intstat` argument to `fuse_interrupt_finish_interrupt` should be + either `INTERRUPT_SQUELCHED` or `INTERRUPT_HANDLED`. + - `INTERRUPT_SQUELCHED` means that we choose not to handle the interrupt + and the fop is going on uninterrupted. + - `INTERRUPT_HANDLED` means that the interrupt was actually handled. In + this case the fop will be answered from interrupt context with errno + `EINTR` (that is, the fop should not send a response to the kernel). + + We return to the `sync` and `datap` arguments later. +- In the `fuse_` function create an interrupt record using + `fuse_interrupt_record_new()`, passing the incoming `fuse_in_header` and + the above handler function to it. + - Arbitrary further data can be referred to via the `data` member of the + interrupt record that is to be passed on from fop context to + interrupt context. +- When it's set up, pass the interrupt record to + `fuse_interrupt_record_insert()`. +- In `fuse__cbk` call `fuse_interrupt_finish_fop()`. + - `fuse_interrupt_finish_fop()` returns a Boolean according to whether the + interrupt was handled. If it was, then the fuse request is already + answered and the stack gets destroyed in `fuse_interrupt_finish_fop` so + `fuse__cbk` can just return (zero). Otherwise follow the standard + cbk logic (answer the fuse request and destroy the stack -- these are + typically accomplished by `fuse_err_cbk()`). +- The last two argument of `fuse_interrupt_finish_fop()` and + `fuse_interrupt_finish_interrupt()` are `gf_boolean_t sync` and + `void **datap`. + - `sync` represents the strategy for freeing the interrupt record. The + interrupt handler and the fop handler are in race to get at the interrupt + record first (interrupt handler for purposes of doing the interrupt + handling, fop handler for purposes of deactivating the interrupt record + upon completion of the fop handling). + - If `sync` is true, then the fop handler will wait for the interrupt + handler to finish and it takes care of freeing. + - If `sync` is false, the loser of the above race will perform freeing. + + Freeing is done within the respective interrupt finish routines, except + for the `data` field of the interrupt record; with respect to that, see + the discussion of the `datap` parameter below. The strategy has to be + consensual, that is, `fuse_interrupt_finish_fop()` and + `fuse_interrupt_finish_interrupt()` must pass the same value for `sync`. + If dismantling the resources associated with the interrupt record is + simple, `sync = _gf_false` is the suggested choice; `sync = _gf_true` can + be useful in the opposite case, when dismantling those resources would + be inconvenient to implement in two places or to enact in non-fop context. + - If `datap` is `NULL`, the `data` member of the interrupt record will be + freed within the interrupt finish routine. If it points to a valid + `void *` pointer, and if caller is doing the cleanup (see `sync` above), + then that pointer will be directed to the `data` member of the interrupt + record and it's up to the caller what it's doing with it. + - If `sync` is true, interrupt handler can use `datap = NULL`, and + fop handler will have `datap` set. + - If `sync` is false, and handlers pass a pointer to a pointer for + `datap`, they should check if the pointed pointer is NULL before + attempting to deal with the data. diff --git a/glusterfsd/src/glusterfsd.c b/glusterfsd/src/glusterfsd.c index dfef7922245..f62fbf5e7c4 100644 --- a/glusterfsd/src/glusterfsd.c +++ b/glusterfsd/src/glusterfsd.c @@ -264,6 +264,9 @@ static struct argp_option gf_options[] = { OPTION_ARG_OPTIONAL, "declare supported granularity of file attribute" " times in nanoseconds"}, + {"fuse-flush-handle-interrupt", ARGP_FUSE_FLUSH_HANDLE_INTERRUPT_KEY, + "BOOL", OPTION_ARG_OPTIONAL | OPTION_HIDDEN, + "handle interrupt in fuse FLUSH handler"}, {0, 0, 0, 0, "Miscellaneous Options:"}, { 0, @@ -643,6 +646,31 @@ set_fuse_mount_options(glusterfs_ctx_t *ctx, dict_t *options) goto err; } } + switch (cmd_args->fuse_flush_handle_interrupt) { + case GF_OPTION_ENABLE: + ret = dict_set_static_ptr(options, "flush-handle-interrupt", "on"); + if (ret < 0) { + gf_msg("glusterfsd", GF_LOG_ERROR, 0, glusterfsd_msg_4, + "failed to set dict value for key " + "flush-handle-interrupt"); + goto err; + } + break; + case GF_OPTION_DISABLE: + ret = dict_set_static_ptr(options, "flush-handle-interrupt", "off"); + if (ret < 0) { + gf_msg("glusterfsd", GF_LOG_ERROR, 0, glusterfsd_msg_4, + "failed to set dict value for key " + "flush-handle-interrupt"); + goto err; + } + break; + case GF_OPTION_DEFERRED: /* default */ + default: + gf_msg_debug("glusterfsd", 0, "fuse-flush-handle-interrupt mode %d", + cmd_args->fuse_flush_handle_interrupt); + break; + } ret = 0; err: @@ -1421,6 +1449,21 @@ parse_opts(int key, char *arg, struct argp_state *state) } break; + + case ARGP_FUSE_FLUSH_HANDLE_INTERRUPT_KEY: + if (!arg) + arg = "yes"; + + if (gf_string2boolean(arg, &b) == 0) { + cmd_args->fuse_flush_handle_interrupt = b; + + break; + } + + argp_failure(state, -1, 0, + "unknown fuse flush handle interrupt setting \"%s\"", + arg); + break; } return 0; } @@ -1728,6 +1771,7 @@ glusterfs_ctx_defaults_init(glusterfs_ctx_t *ctx) cmd_args->fuse_entry_timeout = -1; cmd_args->fopen_keep_cache = GF_OPTION_DEFERRED; cmd_args->kernel_writeback_cache = GF_OPTION_DEFERRED; + cmd_args->fuse_flush_handle_interrupt = GF_OPTION_DEFERRED; if (ctx->mem_acct_enable) cmd_args->mem_acct = 1; diff --git a/glusterfsd/src/glusterfsd.h b/glusterfsd/src/glusterfsd.h index 28d46fc68b9..0042054e138 100644 --- a/glusterfsd/src/glusterfsd.h +++ b/glusterfsd/src/glusterfsd.h @@ -108,6 +108,7 @@ enum argp_option_keys { ARGP_KERNEL_WRITEBACK_CACHE_KEY = 186, ARGP_ATTR_TIMES_GRANULARITY_KEY = 187, ARGP_PRINT_LIBEXECDIR_KEY = 188, + ARGP_FUSE_FLUSH_HANDLE_INTERRUPT_KEY = 189, }; struct _gfd_vol_top_priv { diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h index 90bef74c4c3..93a01cab985 100644 --- a/libglusterfs/src/glusterfs.h +++ b/libglusterfs/src/glusterfs.h @@ -562,6 +562,8 @@ struct _cmd_args { /* FUSE writeback cache support */ int kernel_writeback_cache; uint32_t attr_times_granularity; + + int fuse_flush_handle_interrupt; }; typedef struct _cmd_args cmd_args_t; diff --git a/tests/features/interrupt.t b/tests/features/interrupt.t new file mode 100644 index 00000000000..476d8755d2b --- /dev/null +++ b/tests/features/interrupt.t @@ -0,0 +1,67 @@ +#!/bin/bash + +##Copy this file to tests/bugs before running run.sh (cp extras/test/bug-920583.t tests/bugs/) + +. $(dirname $0)/../include.rc +. $(dirname $0)/../volume.rc + +TESTS_EXPECTED_IN_LOOP=4 + +cleanup; +logdir=`gluster --print-logdir` + +TEST build_tester $(dirname $0)/open_and_sleep.c + +## Start and create a volume +TEST glusterd; +TEST pidof glusterd; + +TEST $CLI volume create $V0 replica 2 stripe 2 $H0:$B0/${V0}{1,2,3,4,5,6,7,8}; + +## Verify volume is is created +EXPECT "$V0" volinfo_field $V0 'Volume Name'; +EXPECT 'Created' volinfo_field $V0 'Status'; + +## Start volume and verify +TEST $CLI volume start $V0; +EXPECT 'Started' volinfo_field $V0 'Status'; + +function log-file-name() +{ + logfilename=$M0".log" + echo ${logfilename:1} | tr / - +} + +log_file=$logdir"/"`log-file-name` + +function test_interrupt { + local handlebool="$1" + local logpattern="$2" + + TEST $GFS --volfile-id=$V0 --volfile-server=$H0 --fuse-flush-handle-interrupt=$handlebool --log-level=DEBUG $M0 + + # If the test helper fails (which is considered a setup error, not failure of the test + # case itself), kill will be invoked without argument, and that will be the actual + # error which is caught. + TEST "./$(dirname $0)/open_and_sleep $M0/testfile | { sleep 0.1; xargs -n1 kill -INT; }" + + TEST "grep -E '$logpattern' $log_file" + # Basic sanity check, making sure filesystem has not crashed. + TEST test -f $M0/testfile +} + +# Theoretically FLUSH might finish before INTERRUPT is handled, +# in which case we'd get the "no handler found" message (but it's unlikely). +test_interrupt yes 'FLUSH.*interrupt handler triggered|INTERRUPT.*no handler found' +EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0 +test_interrupt no 'INTERRUPT.*no handler found' + +## Finish up +TEST $CLI volume stop $V0; +EXPECT 'Stopped' volinfo_field $V0 'Status'; + +TEST $CLI volume delete $V0; +TEST ! $CLI volume info $V0; + +cleanup_tester $(dirname $0)/open_and_sleep; +cleanup; diff --git a/tests/features/open_and_sleep.c b/tests/features/open_and_sleep.c new file mode 100644 index 00000000000..7d0e22a2503 --- /dev/null +++ b/tests/features/open_and_sleep.c @@ -0,0 +1,27 @@ +#include +#include +#include + +int +main(int argc, char **argv) +{ + pid_t pid; + int fd; + + if (argc >= 2) { + fd = open(argv[1], O_RDWR | O_CREAT, 0644); + if (fd == -1) { + fprintf(stderr, "cannot open/create %s\n", argv[1]); + return 1; + } + } + + pid = getpid(); + printf("%d\n", pid); + fflush(stdout); + + for (;;) + sleep(1); + + return 0; +} diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c index e4e894e6c9b..48b8d9df480 100644 --- a/xlators/mount/fuse/src/fuse-bridge.c +++ b/xlators/mount/fuse/src/fuse-bridge.c @@ -1828,6 +1828,21 @@ fuse_err_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int32_t op_ret, return 0; } +static int +fuse_flush_cbk(call_frame_t *frame, void *cookie, xlator_t *this, + int32_t op_ret, int32_t op_errno, dict_t *xdata) +{ + fuse_private_t *priv = this->private; + + if (priv->flush_handle_interrupt) { + if (fuse_interrupt_finish_fop(frame, this, _gf_false, NULL)) { + return 0; + } + } + + return fuse_err_cbk(frame, cookie, this, op_ret, op_errno, xdata); +} + static int fuse_fsync_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int32_t op_ret, int32_t op_errno, struct iatt *prebuf, @@ -3044,7 +3059,19 @@ fuse_lseek(xlator_t *this, fuse_in_header_t *finh, void *msg, void fuse_flush_resume(fuse_state_t *state) { - FUSE_FOP(state, fuse_err_cbk, GF_FOP_FLUSH, flush, state->fd, state->xdata); + FUSE_FOP(state, fuse_flush_cbk, GF_FOP_FLUSH, flush, state->fd, + state->xdata); +} + +static void +fuse_flush_interrupt_handler(xlator_t *this, fuse_interrupt_record_t *fir) +{ + gf_log("glusterfs-fuse", GF_LOG_DEBUG, + "FLUSH unique %" PRIu64 ": interrupt handler triggered", + fir->fuse_in_header.unique); + + fuse_interrupt_finish_interrupt(this, fir, INTERRUPT_HANDLED, _gf_false, + NULL); } static void @@ -3052,6 +3079,7 @@ fuse_flush(xlator_t *this, fuse_in_header_t *finh, void *msg, struct iobuf *iobuf) { struct fuse_flush_in *ffi = msg; + fuse_private_t *priv = NULL; fuse_state_t *state = NULL; fd_t *fd = NULL; @@ -3060,6 +3088,26 @@ fuse_flush(xlator_t *this, fuse_in_header_t *finh, void *msg, fd = FH_TO_FD(ffi->fh); state->fd = fd; + priv = this->private; + if (priv->flush_handle_interrupt) { + fuse_interrupt_record_t *fir = NULL; + + fir = fuse_interrupt_record_new(finh, fuse_flush_interrupt_handler); + if (!fir) { + send_fuse_err(this, finh, ENOMEM); + + gf_log("glusterfs-fuse", GF_LOG_ERROR, + "FLUSH unique %" PRIu64 + ":" + " interrupt record allocation failed", + finh->unique); + free_fuse_state(state); + + return; + } + fuse_interrupt_record_insert(this, fir); + } + fuse_resolve_fd_init(state, &state->resolve, fd); state->lk_owner = ffi->lock_owner; @@ -6301,6 +6349,9 @@ init(xlator_t *this_xl) GF_OPTION_INIT("attr-times-granularity", priv->attr_times_granularity, int32, cleanup_exit); + GF_OPTION_INIT("flush-handle-interrupt", priv->flush_handle_interrupt, bool, + cleanup_exit); + /* user has set only background-qlen, not congestion-threshold, use the fuse kernel driver formula to set congestion. ie, 75% */ if (dict_get(this_xl->options, "background-qlen") && @@ -6608,5 +6659,12 @@ struct volume_options options[] = { .max = 1000000000, .description = "Supported granularity of file attribute times.", }, + { + .key = {"flush-handle-interrupt"}, + .type = GF_OPTION_TYPE_BOOL, + .default_value = "false", + .description = + "Handle iterrupts in FLUSH handler (for testing purposes).", + }, {.key = {NULL}}, }; diff --git a/xlators/mount/fuse/src/fuse-bridge.h b/xlators/mount/fuse/src/fuse-bridge.h index b391af76bac..b997d9e0cbb 100644 --- a/xlators/mount/fuse/src/fuse-bridge.h +++ b/xlators/mount/fuse/src/fuse-bridge.h @@ -161,6 +161,8 @@ struct fuse_private { /* Interrupt subscription */ struct list_head interrupt_list; pthread_mutex_t interrupt_mutex; + + gf_boolean_t flush_handle_interrupt; }; typedef struct fuse_private fuse_private_t; @@ -194,7 +196,7 @@ typedef struct fuse_interrupt_record fuse_interrupt_record_t; typedef void (*fuse_interrupt_handler_t)(xlator_t *this, fuse_interrupt_record_t *); struct fuse_interrupt_record { - struct fuse_in_header fuse_in_header; + fuse_in_header_t fuse_in_header; void *data; gf_boolean_t hit; fuse_interrupt_state_t interrupt_state; diff --git a/xlators/mount/fuse/utils/mount.glusterfs.in b/xlators/mount/fuse/utils/mount.glusterfs.in index 40172fc421d..4a95cd80b87 100755 --- a/xlators/mount/fuse/utils/mount.glusterfs.in +++ b/xlators/mount/fuse/utils/mount.glusterfs.in @@ -281,6 +281,10 @@ start_glusterfs () cmd_line=$(echo "$cmd_line --dump-fuse=$dump_fuse"); fi + if [ -n "$fuse_flush_handle_interrupt" ]; then + cmd_line=$(echo "$cmd_line --fuse-flush-handle-interrupt=$fuse_flush_handle_interrupt"); + fi + if [ -n "$process_name" ]; then cmd_line=$(echo "$cmd_line --process-name fuse.$process_name"); else @@ -541,6 +545,9 @@ with_options() "dump-fuse") dump_fuse=$value ;; + "fuse-flush-handle-interrupt") + fuse_flush_handle_interrupt=$value + ;; "context"|"fscontext"|"defcontext"|"rootcontext") # standard SElinux mount options to pass to the kernel [ -z "$fuse_mountopts" ] || fuse_mountopts="$fuse_mountopts," -- cgit