From fc4bebd605b6a579a4d19c6640aca38057397c77 Mon Sep 17 00:00:00 2001
From: Csaba Henk <csaba@redhat.com>
Date: Tue, 21 Aug 2018 12:44:54 +0200
Subject: [PATCH 527/529] 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.
Upstream: https://review.gluster.org/20876
> Change-Id: I522f1e798501d06b74ac3592a5f73c1ab0590c60
> updates: #465
> Signed-off-by: Csaba Henk <csaba@redhat.com>
Change-Id: I510aff8895a3fe5858ab313c47514de7087d08c1
BUG: 1595246
Signed-off-by: Csaba Henk <csaba@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/162550
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Amar Tumballi Suryanarayan <amarts@redhat.com>
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
---
doc/developer-guide/Developers-Index.md | 5 ++
doc/developer-guide/fuse-interrupt.md | 130 ++++++++++++++++++++++++++++
glusterfsd/src/glusterfsd.c | 53 +++++++++++-
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 | 59 +++++++++++++
xlators/mount/fuse/src/fuse-bridge.h | 4 +-
xlators/mount/fuse/utils/mount.glusterfs.in | 7 ++
10 files changed, 353 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/Developers-Index.md b/doc/developer-guide/Developers-Index.md
index 4c6346e..6c00a4a 100644
--- a/doc/developer-guide/Developers-Index.md
+++ b/doc/developer-guide/Developers-Index.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 0000000..f92b553
--- /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_<FOP>` 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_<FOP>_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_<FOP>_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 2e2cd77..9c536cd 100644
--- a/glusterfsd/src/glusterfsd.c
+++ b/glusterfsd/src/glusterfsd.c
@@ -243,6 +243,9 @@ static struct argp_option gf_options[] = {
OPTION_ARG_OPTIONAL, "disable/enable fuse event-history"},
{"reader-thread-count", ARGP_READER_THREAD_COUNT_KEY, "INTEGER",
OPTION_ARG_OPTIONAL, "set fuse reader thread count"},
+ {"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, }
};
@@ -581,6 +584,38 @@ 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:
@@ -1352,7 +1387,22 @@ no_oom_api:
}
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;
}
@@ -1648,6 +1698,7 @@ glusterfs_ctx_defaults_init (glusterfs_ctx_t *ctx)
cmd_args->fuse_attribute_timeout = -1;
cmd_args->fuse_entry_timeout = -1;
cmd_args->fopen_keep_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 1550a30..28b514a 100644
--- a/glusterfsd/src/glusterfsd.h
+++ b/glusterfsd/src/glusterfsd.h
@@ -101,6 +101,7 @@ enum argp_option_keys {
ARGP_FUSE_EVENT_HISTORY_KEY = 179,
ARGP_READER_THREAD_COUNT_KEY = 180,
ARGP_FUSE_LRU_LIMIT_KEY = 190,
+ ARGP_FUSE_FLUSH_HANDLE_INTERRUPT_KEY = 191,
};
struct _gfd_vol_top_priv {
diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h
index 2690306..9fa066e 100644
--- a/libglusterfs/src/glusterfs.h
+++ b/libglusterfs/src/glusterfs.h
@@ -448,6 +448,8 @@ struct _cmd_args {
char *event_history;
uint32_t reader_thread_count;
+
+ 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 0000000..476d875
--- /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 0000000..da089e9
--- /dev/null
+++ b/tests/features/open_and_sleep.c
@@ -0,0 +1,27 @@
+#include <unistd.h>
+#include <stdio.h>
+#include <fcntl.h>
+
+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 0d4b9db..44c39e4 100644
--- a/xlators/mount/fuse/src/fuse-bridge.c
+++ b/xlators/mount/fuse/src/fuse-bridge.c
@@ -1779,6 +1779,21 @@ fuse_err_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
}
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,
struct iatt *postbuf, dict_t *xdata)
@@ -2961,6 +2976,19 @@ 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
@@ -2968,6 +2996,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;
@@ -2976,6 +3005,27 @@ 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;
@@ -6226,6 +6276,9 @@ init (xlator_t *this_xl)
GF_OPTION_INIT("event-history", priv->event_history, bool,
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") &&
@@ -6552,5 +6605,11 @@ struct volume_options options[] = {
.description = "makes glusterfs invalidate kernel inodes after "
"reaching this limit (0 means 'unlimited')",
},
+ { .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 ba3e000..e18469d 100644
--- a/xlators/mount/fuse/src/fuse-bridge.h
+++ b/xlators/mount/fuse/src/fuse-bridge.h
@@ -157,6 +157,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;
@@ -191,7 +193,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 9a0404f..a3a9fbd 100755
--- a/xlators/mount/fuse/utils/mount.glusterfs.in
+++ b/xlators/mount/fuse/utils/mount.glusterfs.in
@@ -273,6 +273,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 trasnport type is specified, we have to append it to
# volume name, so that it fetches the right client vol file
@@ -524,6 +528,9 @@ with_options()
[ $value = "false" ] ; then
no_root_squash=1;
fi ;;
+ "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,"
--
1.8.3.1