* [PATCH v12 1/2] fuse: add kernel-enforced timeout option for requests
2025-01-22 21:55 [PATCH v12 0/2] fuse: add kernel-enforced request timeout option Joanne Koong
@ 2025-01-22 21:55 ` Joanne Koong
2025-01-22 21:55 ` [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls Joanne Koong
` (2 subsequent siblings)
3 siblings, 0 replies; 24+ messages in thread
From: Joanne Koong @ 2025-01-22 21:55 UTC (permalink / raw)
To: miklos, linux-fsdevel
Cc: bernd.schubert, jefflexu, laoar.shao, jlayton, senozhatsky, tfiga,
bgeffon, etmartin4313, kernel-team
There are situations where fuse servers can become unresponsive or
stuck, for example if the server is deadlocked. Currently, there's no
good way to detect if a server is stuck and needs to be killed manually.
This commit adds an option for enforcing a timeout (in seconds) for
requests where if the timeout elapses without the server responding to
the request, the connection will be automatically aborted.
Please note that these timeouts are not 100% precise. For example, the
request may take roughly an extra FUSE_TIMEOUT_TIMER_FREQ seconds beyond
the requested timeout due to internal implementation, in order to
mitigate overhead.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dev.c | 101 ++++++++++++++++++++++++++++++++++++++
fs/fuse/dev_uring.c | 27 ++++++++++
fs/fuse/dev_uring_i.h | 6 +++
fs/fuse/fuse_dev_i.h | 3 ++
fs/fuse/fuse_i.h | 17 +++++++
fs/fuse/inode.c | 17 ++++++-
include/uapi/linux/fuse.h | 10 +++-
7 files changed, 178 insertions(+), 3 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 5b5f789b37eb..a9d8c739229d 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -32,6 +32,103 @@ MODULE_ALIAS("devname:fuse");
static struct kmem_cache *fuse_req_cachep;
+/* Frequency (in seconds) of request timeout checks, if opted into */
+#define FUSE_TIMEOUT_TIMER_FREQ 15
+
+const unsigned long fuse_timeout_timer_freq =
+ secs_to_jiffies(FUSE_TIMEOUT_TIMER_FREQ);
+
+bool fuse_request_expired(struct fuse_conn *fc, struct list_head *list)
+{
+ struct fuse_req *req;
+
+ req = list_first_entry_or_null(list, struct fuse_req, list);
+ if (!req)
+ return false;
+ return time_is_before_jiffies(req->create_time + fc->timeout.req_timeout);
+}
+
+bool fuse_fpq_processing_expired(struct fuse_conn *fc, struct list_head *processing)
+{
+ int i;
+
+ for (i = 0; i < FUSE_PQ_HASH_SIZE; i++)
+ if (fuse_request_expired(fc, &processing[i]))
+ return true;
+
+ return false;
+}
+
+/*
+ * Check if any requests aren't being completed by the time the request timeout
+ * elapses. To do so, we:
+ * - check the fiq pending list
+ * - check the bg queue
+ * - check the fpq io and processing lists
+ *
+ * To make this fast, we only check against the head request on each list since
+ * these are generally queued in order of creation time (eg newer requests get
+ * queued to the tail). We might miss a few edge cases (eg requests transitioning
+ * between lists, re-sent requests at the head of the pending list having a
+ * later creation time than other requests on that list, etc.) but that is fine
+ * since if the request never gets fulfilled, it will eventually be caught.
+ */
+void fuse_check_timeout(struct work_struct *work)
+{
+ struct delayed_work *dwork = to_delayed_work(work);
+ struct fuse_conn *fc = container_of(dwork, struct fuse_conn,
+ timeout.work);
+ struct fuse_iqueue *fiq = &fc->iq;
+ struct fuse_dev *fud;
+ struct fuse_pqueue *fpq;
+ bool expired = false;
+
+ if (!atomic_read(&fc->num_waiting))
+ goto out;
+
+ spin_lock(&fiq->lock);
+ expired = fuse_request_expired(fc, &fiq->pending);
+ spin_unlock(&fiq->lock);
+ if (expired)
+ goto abort_conn;
+
+ spin_lock(&fc->bg_lock);
+ expired = fuse_request_expired(fc, &fc->bg_queue);
+ spin_unlock(&fc->bg_lock);
+ if (expired)
+ goto abort_conn;
+
+ spin_lock(&fc->lock);
+ if (!fc->connected) {
+ spin_unlock(&fc->lock);
+ return;
+ }
+ list_for_each_entry(fud, &fc->devices, entry) {
+ fpq = &fud->pq;
+ spin_lock(&fpq->lock);
+ if (fuse_request_expired(fc, &fpq->io) ||
+ fuse_fpq_processing_expired(fc, fpq->processing)) {
+ spin_unlock(&fpq->lock);
+ spin_unlock(&fc->lock);
+ goto abort_conn;
+ }
+
+ spin_unlock(&fpq->lock);
+ }
+ spin_unlock(&fc->lock);
+
+ if (fuse_uring_request_expired(fc))
+ goto abort_conn;
+
+out:
+ queue_delayed_work(system_wq, &fc->timeout.work,
+ fuse_timeout_timer_freq);
+ return;
+
+abort_conn:
+ fuse_abort_conn(fc);
+}
+
static void fuse_request_init(struct fuse_mount *fm, struct fuse_req *req)
{
INIT_LIST_HEAD(&req->list);
@@ -40,6 +137,7 @@ static void fuse_request_init(struct fuse_mount *fm, struct fuse_req *req)
refcount_set(&req->count, 1);
__set_bit(FR_PENDING, &req->flags);
req->fm = fm;
+ req->create_time = jiffies;
}
static struct fuse_req *fuse_request_alloc(struct fuse_mount *fm, gfp_t flags)
@@ -2270,6 +2368,9 @@ void fuse_abort_conn(struct fuse_conn *fc)
LIST_HEAD(to_end);
unsigned int i;
+ if (fc->timeout.req_timeout)
+ cancel_delayed_work(&fc->timeout.work);
+
/* Background queuing checks fc->connected under bg_lock */
spin_lock(&fc->bg_lock);
fc->connected = 0;
diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
index 5f10f3880d5a..4f935b11e96f 100644
--- a/fs/fuse/dev_uring.c
+++ b/fs/fuse/dev_uring.c
@@ -140,6 +140,33 @@ void fuse_uring_abort_end_requests(struct fuse_ring *ring)
}
}
+bool fuse_uring_request_expired(struct fuse_conn *fc)
+{
+ struct fuse_ring *ring = fc->ring;
+ struct fuse_ring_queue *queue;
+ int qid;
+
+ if (!ring)
+ return false;
+
+ for (qid = 0; qid < ring->nr_queues; qid++) {
+ queue = READ_ONCE(ring->queues[qid]);
+ if (!queue)
+ continue;
+
+ spin_lock(&queue->lock);
+ if (fuse_request_expired(fc, &queue->fuse_req_queue) ||
+ fuse_request_expired(fc, &queue->fuse_req_bg_queue) ||
+ fuse_fpq_processing_expired(fc, queue->fpq.processing)) {
+ spin_unlock(&queue->lock);
+ return true;
+ }
+ spin_unlock(&queue->lock);
+ }
+
+ return false;
+}
+
void fuse_uring_destruct(struct fuse_conn *fc)
{
struct fuse_ring *ring = fc->ring;
diff --git a/fs/fuse/dev_uring_i.h b/fs/fuse/dev_uring_i.h
index 2102b3d0c1ae..a37991d17d34 100644
--- a/fs/fuse/dev_uring_i.h
+++ b/fs/fuse/dev_uring_i.h
@@ -142,6 +142,7 @@ void fuse_uring_abort_end_requests(struct fuse_ring *ring);
int fuse_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags);
void fuse_uring_queue_fuse_req(struct fuse_iqueue *fiq, struct fuse_req *req);
bool fuse_uring_queue_bq_req(struct fuse_req *req);
+bool fuse_uring_request_expired(struct fuse_conn *fc);
static inline void fuse_uring_abort(struct fuse_conn *fc)
{
@@ -200,6 +201,11 @@ static inline bool fuse_uring_ready(struct fuse_conn *fc)
return false;
}
+static inline bool fuse_uring_request_expired(struct fuse_conn *fc)
+{
+ return false;
+}
+
#endif /* CONFIG_FUSE_IO_URING */
#endif /* _FS_FUSE_DEV_URING_I_H */
diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
index 429661ae0654..5bb91cac400f 100644
--- a/fs/fuse/fuse_dev_i.h
+++ b/fs/fuse/fuse_dev_i.h
@@ -63,5 +63,8 @@ void fuse_dev_queue_forget(struct fuse_iqueue *fiq,
struct fuse_forget_link *forget);
void fuse_dev_queue_interrupt(struct fuse_iqueue *fiq, struct fuse_req *req);
+bool fuse_request_expired(struct fuse_conn *fc, struct list_head *list);
+bool fuse_fpq_processing_expired(struct fuse_conn *fc, struct list_head *processing);
+
#endif
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 886c3af21958..1321cc4ed2ab 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -44,6 +44,9 @@
/** Number of dentries for each connection in the control filesystem */
#define FUSE_CTL_NUM_DENTRIES 5
+/** Frequency (in jiffies) of request timeout checks, if opted into */
+extern const unsigned long fuse_timeout_timer_freq;
+
/** Maximum of max_pages received in init_out */
extern unsigned int fuse_max_pages_limit;
@@ -442,6 +445,8 @@ struct fuse_req {
#ifdef CONFIG_FUSE_IO_URING
void *ring_entry;
#endif
+ /** When (in jiffies) the request was created */
+ unsigned long create_time;
};
struct fuse_iqueue;
@@ -935,6 +940,15 @@ struct fuse_conn {
/** uring connection information*/
struct fuse_ring *ring;
#endif
+
+ /** Only used if the connection opts into request timeouts */
+ struct {
+ /* Worker for checking if any requests have timed out */
+ struct delayed_work work;
+
+ /* Request timeout (in jiffies). 0 = no timeout */
+ unsigned int req_timeout;
+ } timeout;
};
/*
@@ -1216,6 +1230,9 @@ void fuse_request_end(struct fuse_req *req);
void fuse_abort_conn(struct fuse_conn *fc);
void fuse_wait_aborted(struct fuse_conn *fc);
+/* Check if any requests timed out */
+void fuse_check_timeout(struct work_struct *work);
+
/**
* Invalidate inode attributes
*/
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index e9db2cb8c150..79ebeb60015c 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -979,6 +979,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
fc->user_ns = get_user_ns(user_ns);
fc->max_pages = FUSE_DEFAULT_MAX_PAGES_PER_REQ;
fc->max_pages_limit = fuse_max_pages_limit;
+ fc->timeout.req_timeout = 0;
if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
fuse_backing_files_init(fc);
@@ -1007,6 +1008,8 @@ void fuse_conn_put(struct fuse_conn *fc)
if (IS_ENABLED(CONFIG_FUSE_DAX))
fuse_dax_conn_free(fc);
+ if (fc->timeout.req_timeout)
+ cancel_delayed_work_sync(&fc->timeout.work);
if (fiq->ops->release)
fiq->ops->release(fiq);
put_pid_ns(fc->pid_ns);
@@ -1257,6 +1260,14 @@ static void process_init_limits(struct fuse_conn *fc, struct fuse_init_out *arg)
spin_unlock(&fc->bg_lock);
}
+static void set_request_timeout(struct fuse_conn *fc, unsigned int timeout)
+{
+ fc->timeout.req_timeout = secs_to_jiffies(timeout);
+ INIT_DELAYED_WORK(&fc->timeout.work, fuse_check_timeout);
+ queue_delayed_work(system_wq, &fc->timeout.work,
+ fuse_timeout_timer_freq);
+}
+
struct fuse_init_args {
struct fuse_args args;
struct fuse_init_in in;
@@ -1392,6 +1403,9 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
}
if (flags & FUSE_OVER_IO_URING && fuse_uring_enabled())
fc->io_uring = 1;
+
+ if ((flags & FUSE_REQUEST_TIMEOUT) && arg->request_timeout)
+ set_request_timeout(fc, arg->request_timeout);
} else {
ra_pages = fc->max_read / PAGE_SIZE;
fc->no_lock = 1;
@@ -1439,7 +1453,8 @@ void fuse_send_init(struct fuse_mount *fm)
FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_INIT_EXT |
FUSE_SECURITY_CTX | FUSE_CREATE_SUPP_GROUP |
FUSE_HAS_EXPIRE_ONLY | FUSE_DIRECT_IO_ALLOW_MMAP |
- FUSE_NO_EXPORT_SUPPORT | FUSE_HAS_RESEND | FUSE_ALLOW_IDMAP;
+ FUSE_NO_EXPORT_SUPPORT | FUSE_HAS_RESEND | FUSE_ALLOW_IDMAP |
+ FUSE_REQUEST_TIMEOUT;
#ifdef CONFIG_FUSE_DAX
if (fm->fc->dax)
flags |= FUSE_MAP_ALIGNMENT;
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 5e0eb41d967e..42db04c0c5b1 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -229,6 +229,9 @@
* - FUSE_URING_IN_OUT_HEADER_SZ
* - FUSE_URING_OP_IN_OUT_SZ
* - enum fuse_uring_cmd
+ *
+ * 7.43
+ * - add FUSE_REQUEST_TIMEOUT
*/
#ifndef _LINUX_FUSE_H
@@ -435,6 +438,8 @@ struct fuse_file_lock {
* of the request ID indicates resend requests
* FUSE_ALLOW_IDMAP: allow creation of idmapped mounts
* FUSE_OVER_IO_URING: Indicate that client supports io-uring
+ * FUSE_REQUEST_TIMEOUT: kernel supports timing out requests.
+ * init_out.request_timeout contains the timeout (in secs)
*/
#define FUSE_ASYNC_READ (1 << 0)
#define FUSE_POSIX_LOCKS (1 << 1)
@@ -477,11 +482,11 @@ struct fuse_file_lock {
#define FUSE_PASSTHROUGH (1ULL << 37)
#define FUSE_NO_EXPORT_SUPPORT (1ULL << 38)
#define FUSE_HAS_RESEND (1ULL << 39)
-
/* Obsolete alias for FUSE_DIRECT_IO_ALLOW_MMAP */
#define FUSE_DIRECT_IO_RELAX FUSE_DIRECT_IO_ALLOW_MMAP
#define FUSE_ALLOW_IDMAP (1ULL << 40)
#define FUSE_OVER_IO_URING (1ULL << 41)
+#define FUSE_REQUEST_TIMEOUT (1ULL << 42)
/**
* CUSE INIT request/reply flags
@@ -909,7 +914,8 @@ struct fuse_init_out {
uint16_t map_alignment;
uint32_t flags2;
uint32_t max_stack_depth;
- uint32_t unused[6];
+ uint16_t request_timeout;
+ uint16_t unused[11];
};
#define CUSE_INIT_INFO_MAX 4096
--
2.43.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-22 21:55 [PATCH v12 0/2] fuse: add kernel-enforced request timeout option Joanne Koong
2025-01-22 21:55 ` [PATCH v12 1/2] fuse: add kernel-enforced timeout option for requests Joanne Koong
@ 2025-01-22 21:55 ` Joanne Koong
2025-01-23 9:20 ` Luis Henriques
2025-01-22 22:53 ` [PATCH v12 0/2] fuse: add kernel-enforced request timeout option Jeff Layton
2025-02-25 2:04 ` Sergey Senozhatsky
3 siblings, 1 reply; 24+ messages in thread
From: Joanne Koong @ 2025-01-22 21:55 UTC (permalink / raw)
To: miklos, linux-fsdevel
Cc: bernd.schubert, jefflexu, laoar.shao, jlayton, senozhatsky, tfiga,
bgeffon, etmartin4313, kernel-team, Bernd Schubert, Josef Bacik
Introduce two new sysctls, "default_request_timeout" and
"max_request_timeout". These control how long (in seconds) a server can
take to reply to a request. If the server does not reply by the timeout,
then the connection will be aborted. The upper bound on these sysctl
values is 65535.
"default_request_timeout" sets the default timeout if no timeout is
specified by the fuse server on mount. 0 (default) indicates no default
timeout should be enforced. If the server did specify a timeout, then
default_request_timeout will be ignored.
"max_request_timeout" sets the max amount of time the server may take to
reply to a request. 0 (default) indicates no maximum timeout. If
max_request_timeout is set and the fuse server attempts to set a
timeout greater than max_request_timeout, the system will use
max_request_timeout as the timeout. Similarly, if default_request_timeout
is greater than max_request_timeout, the system will use
max_request_timeout as the timeout. If the server does not request a
timeout and default_request_timeout is set to 0 but max_request_timeout
is set, then the timeout will be max_request_timeout.
Please note that these timeouts are not 100% precise. The request may
take roughly an extra FUSE_TIMEOUT_TIMER_FREQ seconds beyond the set max
timeout due to how it's internally implemented.
$ sysctl -a | grep fuse.default_request_timeout
fs.fuse.default_request_timeout = 0
$ echo 65536 | sudo tee /proc/sys/fs/fuse/default_request_timeout
tee: /proc/sys/fs/fuse/default_request_timeout: Invalid argument
$ echo 65535 | sudo tee /proc/sys/fs/fuse/default_request_timeout
65535
$ sysctl -a | grep fuse.default_request_timeout
fs.fuse.default_request_timeout = 65535
$ echo 0 | sudo tee /proc/sys/fs/fuse/default_request_timeout
0
$ sysctl -a | grep fuse.default_request_timeout
fs.fuse.default_request_timeout = 0
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
Reviewed-by: Bernd Schubert <bschubert@ddn.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
Documentation/admin-guide/sysctl/fs.rst | 25 ++++++++++++++++++++++
fs/fuse/fuse_i.h | 10 +++++++++
fs/fuse/inode.c | 28 +++++++++++++++++++++++--
fs/fuse/sysctl.c | 24 +++++++++++++++++++++
4 files changed, 85 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/sysctl/fs.rst b/Documentation/admin-guide/sysctl/fs.rst
index f5ec6c9312e1..35aeb30bed8b 100644
--- a/Documentation/admin-guide/sysctl/fs.rst
+++ b/Documentation/admin-guide/sysctl/fs.rst
@@ -347,3 +347,28 @@ filesystems:
``/proc/sys/fs/fuse/max_pages_limit`` is a read/write file for
setting/getting the maximum number of pages that can be used for servicing
requests in FUSE.
+
+``/proc/sys/fs/fuse/default_request_timeout`` is a read/write file for
+setting/getting the default timeout (in seconds) for a fuse server to
+reply to a kernel-issued request in the event where the server did not
+specify a timeout at mount. If the server set a timeout,
+then default_request_timeout will be ignored. The default
+"default_request_timeout" is set to 0. 0 indicates no default timeout.
+The maximum value that can be set is 65535.
+
+``/proc/sys/fs/fuse/max_request_timeout`` is a read/write file for
+setting/getting the maximum timeout (in seconds) for a fuse server to
+reply to a kernel-issued request. A value greater than 0 automatically opts
+the server into a timeout that will be set to at most "max_request_timeout",
+even if the server did not specify a timeout and default_request_timeout is
+set to 0. If max_request_timeout is greater than 0 and the server set a timeout
+greater than max_request_timeout or default_request_timeout is set to a value
+greater than max_request_timeout, the system will use max_request_timeout as the
+timeout. 0 indicates no max request timeout. The maximum value that can be set
+is 65535.
+
+For timeouts, if the server does not respond to the request by the time
+the set timeout elapses, then the connection to the fuse server will be aborted.
+Please note that the timeouts are not 100% precise (eg you may set 60 seconds but
+the timeout may kick in after 70 seconds). The upper margin of error for the
+timeout is roughly FUSE_TIMEOUT_TIMER_FREQ seconds.
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 1321cc4ed2ab..e5114831798f 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -49,6 +49,16 @@ extern const unsigned long fuse_timeout_timer_freq;
/** Maximum of max_pages received in init_out */
extern unsigned int fuse_max_pages_limit;
+/*
+ * Default timeout (in seconds) for the server to reply to a request
+ * before the connection is aborted, if no timeout was specified on mount.
+ */
+extern unsigned int fuse_default_req_timeout;
+/*
+ * Max timeout (in seconds) for the server to reply to a request before
+ * the connection is aborted.
+ */
+extern unsigned int fuse_max_req_timeout;
/** List of active connections */
extern struct list_head fuse_conn_list;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 79ebeb60015c..4e36d99fae52 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -37,6 +37,9 @@ DEFINE_MUTEX(fuse_mutex);
static int set_global_limit(const char *val, const struct kernel_param *kp);
unsigned int fuse_max_pages_limit = 256;
+/* default is no timeout */
+unsigned int fuse_default_req_timeout = 0;
+unsigned int fuse_max_req_timeout = 0;
unsigned max_user_bgreq;
module_param_call(max_user_bgreq, set_global_limit, param_get_uint,
@@ -1268,6 +1271,24 @@ static void set_request_timeout(struct fuse_conn *fc, unsigned int timeout)
fuse_timeout_timer_freq);
}
+static void init_server_timeout(struct fuse_conn *fc, unsigned int timeout)
+{
+ if (!timeout && !fuse_max_req_timeout && !fuse_default_req_timeout)
+ return;
+
+ if (!timeout)
+ timeout = fuse_default_req_timeout;
+
+ if (fuse_max_req_timeout) {
+ if (timeout)
+ timeout = min(fuse_max_req_timeout, timeout);
+ else
+ timeout = fuse_max_req_timeout;
+ }
+
+ set_request_timeout(fc, timeout);
+}
+
struct fuse_init_args {
struct fuse_args args;
struct fuse_init_in in;
@@ -1286,6 +1307,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
ok = false;
else {
unsigned long ra_pages;
+ unsigned int timeout = 0;
process_init_limits(fc, arg);
@@ -1404,14 +1426,16 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
if (flags & FUSE_OVER_IO_URING && fuse_uring_enabled())
fc->io_uring = 1;
- if ((flags & FUSE_REQUEST_TIMEOUT) && arg->request_timeout)
- set_request_timeout(fc, arg->request_timeout);
+ if (flags & FUSE_REQUEST_TIMEOUT)
+ timeout = arg->request_timeout;
} else {
ra_pages = fc->max_read / PAGE_SIZE;
fc->no_lock = 1;
fc->no_flock = 1;
}
+ init_server_timeout(fc, timeout);
+
fm->sb->s_bdi->ra_pages =
min(fm->sb->s_bdi->ra_pages, ra_pages);
fc->minor = arg->minor;
diff --git a/fs/fuse/sysctl.c b/fs/fuse/sysctl.c
index b272bb333005..3d542ef9d889 100644
--- a/fs/fuse/sysctl.c
+++ b/fs/fuse/sysctl.c
@@ -13,6 +13,12 @@ static struct ctl_table_header *fuse_table_header;
/* Bound by fuse_init_out max_pages, which is a u16 */
static unsigned int sysctl_fuse_max_pages_limit = 65535;
+/*
+ * fuse_init_out request timeouts are u16.
+ * This goes up to ~18 hours, which is plenty for a timeout.
+ */
+static unsigned int sysctl_fuse_req_timeout_limit = 65535;
+
static struct ctl_table fuse_sysctl_table[] = {
{
.procname = "max_pages_limit",
@@ -23,6 +29,24 @@ static struct ctl_table fuse_sysctl_table[] = {
.extra1 = SYSCTL_ONE,
.extra2 = &sysctl_fuse_max_pages_limit,
},
+ {
+ .procname = "default_request_timeout",
+ .data = &fuse_default_req_timeout,
+ .maxlen = sizeof(fuse_default_req_timeout),
+ .mode = 0644,
+ .proc_handler = proc_douintvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = &sysctl_fuse_req_timeout_limit,
+ },
+ {
+ .procname = "max_request_timeout",
+ .data = &fuse_max_req_timeout,
+ .maxlen = sizeof(fuse_max_req_timeout),
+ .mode = 0644,
+ .proc_handler = proc_douintvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = &sysctl_fuse_req_timeout_limit,
+ },
};
int fuse_sysctl_register(void)
--
2.43.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-22 21:55 ` [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls Joanne Koong
@ 2025-01-23 9:20 ` Luis Henriques
2025-01-23 14:28 ` Miklos Szeredi
2025-01-23 17:18 ` Joanne Koong
0 siblings, 2 replies; 24+ messages in thread
From: Luis Henriques @ 2025-01-23 9:20 UTC (permalink / raw)
To: Joanne Koong
Cc: miklos, linux-fsdevel, bernd.schubert, jefflexu, laoar.shao,
jlayton, senozhatsky, tfiga, bgeffon, etmartin4313, kernel-team,
Bernd Schubert, Josef Bacik
Hi Joanne,
On Wed, Jan 22 2025, Joanne Koong wrote:
> Introduce two new sysctls, "default_request_timeout" and
> "max_request_timeout". These control how long (in seconds) a server can
> take to reply to a request. If the server does not reply by the timeout,
> then the connection will be aborted. The upper bound on these sysctl
> values is 65535.
>
> "default_request_timeout" sets the default timeout if no timeout is
> specified by the fuse server on mount. 0 (default) indicates no default
> timeout should be enforced. If the server did specify a timeout, then
> default_request_timeout will be ignored.
>
> "max_request_timeout" sets the max amount of time the server may take to
> reply to a request. 0 (default) indicates no maximum timeout. If
> max_request_timeout is set and the fuse server attempts to set a
> timeout greater than max_request_timeout, the system will use
> max_request_timeout as the timeout. Similarly, if default_request_timeout
> is greater than max_request_timeout, the system will use
> max_request_timeout as the timeout. If the server does not request a
> timeout and default_request_timeout is set to 0 but max_request_timeout
> is set, then the timeout will be max_request_timeout.
>
> Please note that these timeouts are not 100% precise. The request may
> take roughly an extra FUSE_TIMEOUT_TIMER_FREQ seconds beyond the set max
> timeout due to how it's internally implemented.
>
> $ sysctl -a | grep fuse.default_request_timeout
> fs.fuse.default_request_timeout = 0
>
> $ echo 65536 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> tee: /proc/sys/fs/fuse/default_request_timeout: Invalid argument
>
> $ echo 65535 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> 65535
>
> $ sysctl -a | grep fuse.default_request_timeout
> fs.fuse.default_request_timeout = 65535
>
> $ echo 0 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> 0
>
> $ sysctl -a | grep fuse.default_request_timeout
> fs.fuse.default_request_timeout = 0
>
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> Reviewed-by: Bernd Schubert <bschubert@ddn.com>
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
> Documentation/admin-guide/sysctl/fs.rst | 25 ++++++++++++++++++++++
> fs/fuse/fuse_i.h | 10 +++++++++
> fs/fuse/inode.c | 28 +++++++++++++++++++++++--
> fs/fuse/sysctl.c | 24 +++++++++++++++++++++
> 4 files changed, 85 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/admin-guide/sysctl/fs.rst b/Documentation/admin-guide/sysctl/fs.rst
> index f5ec6c9312e1..35aeb30bed8b 100644
> --- a/Documentation/admin-guide/sysctl/fs.rst
> +++ b/Documentation/admin-guide/sysctl/fs.rst
> @@ -347,3 +347,28 @@ filesystems:
> ``/proc/sys/fs/fuse/max_pages_limit`` is a read/write file for
> setting/getting the maximum number of pages that can be used for servicing
> requests in FUSE.
> +
> +``/proc/sys/fs/fuse/default_request_timeout`` is a read/write file for
> +setting/getting the default timeout (in seconds) for a fuse server to
> +reply to a kernel-issued request in the event where the server did not
> +specify a timeout at mount. If the server set a timeout,
> +then default_request_timeout will be ignored. The default
> +"default_request_timeout" is set to 0. 0 indicates no default timeout.
> +The maximum value that can be set is 65535.
> +
> +``/proc/sys/fs/fuse/max_request_timeout`` is a read/write file for
> +setting/getting the maximum timeout (in seconds) for a fuse server to
> +reply to a kernel-issued request. A value greater than 0 automatically opts
> +the server into a timeout that will be set to at most "max_request_timeout",
> +even if the server did not specify a timeout and default_request_timeout is
> +set to 0. If max_request_timeout is greater than 0 and the server set a timeout
> +greater than max_request_timeout or default_request_timeout is set to a value
> +greater than max_request_timeout, the system will use max_request_timeout as the
> +timeout. 0 indicates no max request timeout. The maximum value that can be set
> +is 65535.
> +
> +For timeouts, if the server does not respond to the request by the time
> +the set timeout elapses, then the connection to the fuse server will be aborted.
> +Please note that the timeouts are not 100% precise (eg you may set 60 seconds but
> +the timeout may kick in after 70 seconds). The upper margin of error for the
> +timeout is roughly FUSE_TIMEOUT_TIMER_FREQ seconds.
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 1321cc4ed2ab..e5114831798f 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -49,6 +49,16 @@ extern const unsigned long fuse_timeout_timer_freq;
>
> /** Maximum of max_pages received in init_out */
> extern unsigned int fuse_max_pages_limit;
> +/*
> + * Default timeout (in seconds) for the server to reply to a request
> + * before the connection is aborted, if no timeout was specified on mount.
> + */
> +extern unsigned int fuse_default_req_timeout;
> +/*
> + * Max timeout (in seconds) for the server to reply to a request before
> + * the connection is aborted.
> + */
> +extern unsigned int fuse_max_req_timeout;
>
> /** List of active connections */
> extern struct list_head fuse_conn_list;
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 79ebeb60015c..4e36d99fae52 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -37,6 +37,9 @@ DEFINE_MUTEX(fuse_mutex);
> static int set_global_limit(const char *val, const struct kernel_param *kp);
>
> unsigned int fuse_max_pages_limit = 256;
> +/* default is no timeout */
> +unsigned int fuse_default_req_timeout = 0;
> +unsigned int fuse_max_req_timeout = 0;
>
> unsigned max_user_bgreq;
> module_param_call(max_user_bgreq, set_global_limit, param_get_uint,
> @@ -1268,6 +1271,24 @@ static void set_request_timeout(struct fuse_conn *fc, unsigned int timeout)
> fuse_timeout_timer_freq);
> }
>
> +static void init_server_timeout(struct fuse_conn *fc, unsigned int timeout)
> +{
> + if (!timeout && !fuse_max_req_timeout && !fuse_default_req_timeout)
> + return;
> +
> + if (!timeout)
> + timeout = fuse_default_req_timeout;
> +
> + if (fuse_max_req_timeout) {
> + if (timeout)
> + timeout = min(fuse_max_req_timeout, timeout);
Sorry for this late comment (this is v12 already!), but I wonder if it
would be worth to use clamp() instead of min(). Limiting this value to
the range [FUSE_TIMEOUT_TIMER_FREQ, fuxe_max_req_timeout] would prevent
accidentally setting the timeout to a very low value.
There are also some white-spaces/tabs issues with the other patch, which
are worth fixing before merging. Other than that, feel free to add my
Reviewed-by: Luis Henriques <luis@igalia.com>
Cheers,
--
Luís
> + else
> + timeout = fuse_max_req_timeout;
> + }
> +
> + set_request_timeout(fc, timeout);
> +}
> +
> struct fuse_init_args {
> struct fuse_args args;
> struct fuse_init_in in;
> @@ -1286,6 +1307,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
> ok = false;
> else {
> unsigned long ra_pages;
> + unsigned int timeout = 0;
>
> process_init_limits(fc, arg);
>
> @@ -1404,14 +1426,16 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
> if (flags & FUSE_OVER_IO_URING && fuse_uring_enabled())
> fc->io_uring = 1;
>
> - if ((flags & FUSE_REQUEST_TIMEOUT) && arg->request_timeout)
> - set_request_timeout(fc, arg->request_timeout);
> + if (flags & FUSE_REQUEST_TIMEOUT)
> + timeout = arg->request_timeout;
> } else {
> ra_pages = fc->max_read / PAGE_SIZE;
> fc->no_lock = 1;
> fc->no_flock = 1;
> }
>
> + init_server_timeout(fc, timeout);
> +
> fm->sb->s_bdi->ra_pages =
> min(fm->sb->s_bdi->ra_pages, ra_pages);
> fc->minor = arg->minor;
> diff --git a/fs/fuse/sysctl.c b/fs/fuse/sysctl.c
> index b272bb333005..3d542ef9d889 100644
> --- a/fs/fuse/sysctl.c
> +++ b/fs/fuse/sysctl.c
> @@ -13,6 +13,12 @@ static struct ctl_table_header *fuse_table_header;
> /* Bound by fuse_init_out max_pages, which is a u16 */
> static unsigned int sysctl_fuse_max_pages_limit = 65535;
>
> +/*
> + * fuse_init_out request timeouts are u16.
> + * This goes up to ~18 hours, which is plenty for a timeout.
> + */
> +static unsigned int sysctl_fuse_req_timeout_limit = 65535;
> +
> static struct ctl_table fuse_sysctl_table[] = {
> {
> .procname = "max_pages_limit",
> @@ -23,6 +29,24 @@ static struct ctl_table fuse_sysctl_table[] = {
> .extra1 = SYSCTL_ONE,
> .extra2 = &sysctl_fuse_max_pages_limit,
> },
> + {
> + .procname = "default_request_timeout",
> + .data = &fuse_default_req_timeout,
> + .maxlen = sizeof(fuse_default_req_timeout),
> + .mode = 0644,
> + .proc_handler = proc_douintvec_minmax,
> + .extra1 = SYSCTL_ZERO,
> + .extra2 = &sysctl_fuse_req_timeout_limit,
> + },
> + {
> + .procname = "max_request_timeout",
> + .data = &fuse_max_req_timeout,
> + .maxlen = sizeof(fuse_max_req_timeout),
> + .mode = 0644,
> + .proc_handler = proc_douintvec_minmax,
> + .extra1 = SYSCTL_ZERO,
> + .extra2 = &sysctl_fuse_req_timeout_limit,
> + },
> };
>
> int fuse_sysctl_register(void)
> --
> 2.43.5
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 9:20 ` Luis Henriques
@ 2025-01-23 14:28 ` Miklos Szeredi
2025-01-23 14:41 ` Bernd Schubert
2025-01-23 17:18 ` Joanne Koong
1 sibling, 1 reply; 24+ messages in thread
From: Miklos Szeredi @ 2025-01-23 14:28 UTC (permalink / raw)
To: Luis Henriques
Cc: Joanne Koong, linux-fsdevel, bernd.schubert, jefflexu, laoar.shao,
jlayton, senozhatsky, tfiga, bgeffon, etmartin4313, kernel-team,
Bernd Schubert, Josef Bacik
On Thu, 23 Jan 2025 at 10:21, Luis Henriques <luis@igalia.com> wrote:
>
> Hi Joanne,
>
> On Wed, Jan 22 2025, Joanne Koong wrote:
>
> > Introduce two new sysctls, "default_request_timeout" and
> > "max_request_timeout". These control how long (in seconds) a server can
> > take to reply to a request. If the server does not reply by the timeout,
> > then the connection will be aborted. The upper bound on these sysctl
> > values is 65535.
> >
> > "default_request_timeout" sets the default timeout if no timeout is
> > specified by the fuse server on mount. 0 (default) indicates no default
> > timeout should be enforced. If the server did specify a timeout, then
> > default_request_timeout will be ignored.
> >
> > "max_request_timeout" sets the max amount of time the server may take to
> > reply to a request. 0 (default) indicates no maximum timeout. If
> > max_request_timeout is set and the fuse server attempts to set a
> > timeout greater than max_request_timeout, the system will use
> > max_request_timeout as the timeout. Similarly, if default_request_timeout
> > is greater than max_request_timeout, the system will use
> > max_request_timeout as the timeout. If the server does not request a
> > timeout and default_request_timeout is set to 0 but max_request_timeout
> > is set, then the timeout will be max_request_timeout.
> >
> > Please note that these timeouts are not 100% precise. The request may
> > take roughly an extra FUSE_TIMEOUT_TIMER_FREQ seconds beyond the set max
> > timeout due to how it's internally implemented.
> >
> > $ sysctl -a | grep fuse.default_request_timeout
> > fs.fuse.default_request_timeout = 0
> >
> > $ echo 65536 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> > tee: /proc/sys/fs/fuse/default_request_timeout: Invalid argument
> >
> > $ echo 65535 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> > 65535
> >
> > $ sysctl -a | grep fuse.default_request_timeout
> > fs.fuse.default_request_timeout = 65535
> >
> > $ echo 0 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> > 0
> >
> > $ sysctl -a | grep fuse.default_request_timeout
> > fs.fuse.default_request_timeout = 0
> >
> > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> > Reviewed-by: Bernd Schubert <bschubert@ddn.com>
> > Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> > Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Thanks, applied and pushed with some cleanups including Luis's clamp idea.
Miklos
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 14:28 ` Miklos Szeredi
@ 2025-01-23 14:41 ` Bernd Schubert
2025-01-23 14:54 ` Miklos Szeredi
2025-01-23 16:51 ` Joanne Koong
0 siblings, 2 replies; 24+ messages in thread
From: Bernd Schubert @ 2025-01-23 14:41 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Joanne Koong, linux-fsdevel, bernd.schubert, jefflexu, laoar.shao,
jlayton, senozhatsky, tfiga, bgeffon, etmartin4313, kernel-team,
Josef Bacik, Luis Henriques
On 1/23/25 15:28, Miklos Szeredi wrote:
> On Thu, 23 Jan 2025 at 10:21, Luis Henriques <luis@igalia.com> wrote:
>>
>> Hi Joanne,
>>
>> On Wed, Jan 22 2025, Joanne Koong wrote:
>>
>>> Introduce two new sysctls, "default_request_timeout" and
>>> "max_request_timeout". These control how long (in seconds) a server can
>>> take to reply to a request. If the server does not reply by the timeout,
>>> then the connection will be aborted. The upper bound on these sysctl
>>> values is 65535.
>>>
>>> "default_request_timeout" sets the default timeout if no timeout is
>>> specified by the fuse server on mount. 0 (default) indicates no default
>>> timeout should be enforced. If the server did specify a timeout, then
>>> default_request_timeout will be ignored.
>>>
>>> "max_request_timeout" sets the max amount of time the server may take to
>>> reply to a request. 0 (default) indicates no maximum timeout. If
>>> max_request_timeout is set and the fuse server attempts to set a
>>> timeout greater than max_request_timeout, the system will use
>>> max_request_timeout as the timeout. Similarly, if default_request_timeout
>>> is greater than max_request_timeout, the system will use
>>> max_request_timeout as the timeout. If the server does not request a
>>> timeout and default_request_timeout is set to 0 but max_request_timeout
>>> is set, then the timeout will be max_request_timeout.
>>>
>>> Please note that these timeouts are not 100% precise. The request may
>>> take roughly an extra FUSE_TIMEOUT_TIMER_FREQ seconds beyond the set max
>>> timeout due to how it's internally implemented.
>>>
>>> $ sysctl -a | grep fuse.default_request_timeout
>>> fs.fuse.default_request_timeout = 0
>>>
>>> $ echo 65536 | sudo tee /proc/sys/fs/fuse/default_request_timeout
>>> tee: /proc/sys/fs/fuse/default_request_timeout: Invalid argument
>>>
>>> $ echo 65535 | sudo tee /proc/sys/fs/fuse/default_request_timeout
>>> 65535
>>>
>>> $ sysctl -a | grep fuse.default_request_timeout
>>> fs.fuse.default_request_timeout = 65535
>>>
>>> $ echo 0 | sudo tee /proc/sys/fs/fuse/default_request_timeout
>>> 0
>>>
>>> $ sysctl -a | grep fuse.default_request_timeout
>>> fs.fuse.default_request_timeout = 0
>>>
>>> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
>>> Reviewed-by: Bernd Schubert <bschubert@ddn.com>
>>> Reviewed-by: Josef Bacik <josef@toxicpanda.com>
>>> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
>
> Thanks, applied and pushed with some cleanups including Luis's clamp idea.
Hi Miklos,
I don't think the timeouts do work with io-uring yet, I'm not sure
yet if I have time to work on that today or tomorrow (on something
else right now, I can try, but no promises).
How shall we handle it, if I don't manage in time?
Thanks,
Bernd
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 14:41 ` Bernd Schubert
@ 2025-01-23 14:54 ` Miklos Szeredi
2025-01-23 18:42 ` Bernd Schubert
2025-01-23 16:51 ` Joanne Koong
1 sibling, 1 reply; 24+ messages in thread
From: Miklos Szeredi @ 2025-01-23 14:54 UTC (permalink / raw)
To: Bernd Schubert
Cc: Joanne Koong, linux-fsdevel, bernd.schubert, jefflexu, laoar.shao,
jlayton, senozhatsky, tfiga, bgeffon, etmartin4313, kernel-team,
Josef Bacik, Luis Henriques
On Thu, 23 Jan 2025 at 15:41, Bernd Schubert <bschubert@ddn.com> wrote:
> I don't think the timeouts do work with io-uring yet, I'm not sure
> yet if I have time to work on that today or tomorrow (on something
> else right now, I can try, but no promises).
>
> How shall we handle it, if I don't manage in time?
Okay. Let's just put the timeout patches on hold until this is resolved.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 14:54 ` Miklos Szeredi
@ 2025-01-23 18:42 ` Bernd Schubert
0 siblings, 0 replies; 24+ messages in thread
From: Bernd Schubert @ 2025-01-23 18:42 UTC (permalink / raw)
To: Miklos Szeredi, Bernd Schubert
Cc: Joanne Koong, linux-fsdevel, jefflexu, laoar.shao, jlayton,
senozhatsky, tfiga, bgeffon, etmartin4313, kernel-team,
Josef Bacik, Luis Henriques
On 1/23/25 15:54, Miklos Szeredi wrote:
> On Thu, 23 Jan 2025 at 15:41, Bernd Schubert <bschubert@ddn.com> wrote:
>
>> I don't think the timeouts do work with io-uring yet, I'm not sure
>> yet if I have time to work on that today or tomorrow (on something
>> else right now, I can try, but no promises).
>>
>> How shall we handle it, if I don't manage in time?
>
> Okay. Let's just put the timeout patches on hold until this is resolved.
Sorry, my fault, I had missed that Joanne had already rebased to linux-next
and already handles uring queues.
Thanks,
Bernd
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 14:41 ` Bernd Schubert
2025-01-23 14:54 ` Miklos Szeredi
@ 2025-01-23 16:51 ` Joanne Koong
2025-01-23 17:19 ` Bernd Schubert
1 sibling, 1 reply; 24+ messages in thread
From: Joanne Koong @ 2025-01-23 16:51 UTC (permalink / raw)
To: Bernd Schubert
Cc: Miklos Szeredi, linux-fsdevel, bernd.schubert, jefflexu,
laoar.shao, jlayton, senozhatsky, tfiga, bgeffon, etmartin4313,
kernel-team, Josef Bacik, Luis Henriques
On Thu, Jan 23, 2025 at 6:42 AM Bernd Schubert <bschubert@ddn.com> wrote:
>
>
>
> On 1/23/25 15:28, Miklos Szeredi wrote:
> > On Thu, 23 Jan 2025 at 10:21, Luis Henriques <luis@igalia.com> wrote:
> >>
> >> Hi Joanne,
> >>
> >> On Wed, Jan 22 2025, Joanne Koong wrote:
> >>
> >>> Introduce two new sysctls, "default_request_timeout" and
> >>> "max_request_timeout". These control how long (in seconds) a server can
> >>> take to reply to a request. If the server does not reply by the timeout,
> >>> then the connection will be aborted. The upper bound on these sysctl
> >>> values is 65535.
> >>>
> >>> "default_request_timeout" sets the default timeout if no timeout is
> >>> specified by the fuse server on mount. 0 (default) indicates no default
> >>> timeout should be enforced. If the server did specify a timeout, then
> >>> default_request_timeout will be ignored.
> >>>
> >>> "max_request_timeout" sets the max amount of time the server may take to
> >>> reply to a request. 0 (default) indicates no maximum timeout. If
> >>> max_request_timeout is set and the fuse server attempts to set a
> >>> timeout greater than max_request_timeout, the system will use
> >>> max_request_timeout as the timeout. Similarly, if default_request_timeout
> >>> is greater than max_request_timeout, the system will use
> >>> max_request_timeout as the timeout. If the server does not request a
> >>> timeout and default_request_timeout is set to 0 but max_request_timeout
> >>> is set, then the timeout will be max_request_timeout.
> >>>
> >>> Please note that these timeouts are not 100% precise. The request may
> >>> take roughly an extra FUSE_TIMEOUT_TIMER_FREQ seconds beyond the set max
> >>> timeout due to how it's internally implemented.
> >>>
> >>> $ sysctl -a | grep fuse.default_request_timeout
> >>> fs.fuse.default_request_timeout = 0
> >>>
> >>> $ echo 65536 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> >>> tee: /proc/sys/fs/fuse/default_request_timeout: Invalid argument
> >>>
> >>> $ echo 65535 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> >>> 65535
> >>>
> >>> $ sysctl -a | grep fuse.default_request_timeout
> >>> fs.fuse.default_request_timeout = 65535
> >>>
> >>> $ echo 0 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> >>> 0
> >>>
> >>> $ sysctl -a | grep fuse.default_request_timeout
> >>> fs.fuse.default_request_timeout = 0
> >>>
> >>> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> >>> Reviewed-by: Bernd Schubert <bschubert@ddn.com>
> >>> Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> >>> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> >
> > Thanks, applied and pushed with some cleanups including Luis's clamp idea.
>
> Hi Miklos,
>
> I don't think the timeouts do work with io-uring yet, I'm not sure
> yet if I have time to work on that today or tomorrow (on something
> else right now, I can try, but no promises).
Hi Bernd,
What are your thoughts on what is missing on the io-uring side for
timeouts? If a request times out, it will abort the connection and
AFAICT, the abort logic should already be fine for io-uring, as users
can currently abort the connection through the sysfs interface and
there's no internal difference in aborting through sysfs vs timeouts.
Thanks,
Joanne
>
> How shall we handle it, if I don't manage in time?
>
>
> Thanks,
> Bernd
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 16:51 ` Joanne Koong
@ 2025-01-23 17:19 ` Bernd Schubert
2025-01-23 17:48 ` Joanne Koong
0 siblings, 1 reply; 24+ messages in thread
From: Bernd Schubert @ 2025-01-23 17:19 UTC (permalink / raw)
To: Joanne Koong, Bernd Schubert
Cc: Miklos Szeredi, linux-fsdevel, jefflexu, laoar.shao, jlayton,
senozhatsky, tfiga, bgeffon, etmartin4313, kernel-team,
Josef Bacik, Luis Henriques
Hi Joanne,
>>> Thanks, applied and pushed with some cleanups including Luis's clamp idea.
>>
>> Hi Miklos,
>>
>> I don't think the timeouts do work with io-uring yet, I'm not sure
>> yet if I have time to work on that today or tomorrow (on something
>> else right now, I can try, but no promises).
>
> Hi Bernd,
>
> What are your thoughts on what is missing on the io-uring side for
> timeouts? If a request times out, it will abort the connection and
> AFAICT, the abort logic should already be fine for io-uring, as users
> can currently abort the connection through the sysfs interface and
> there's no internal difference in aborting through sysfs vs timeouts.
>
in fuse_check_timeout() it iterates over each fud and then fpq.
In dev_uring.c fpq is is per queue but unrelated to fud. In current
fuse-io-uring fud is not cloned anymore - using fud won't work.
And Requests are also not queued at all on the other list
fuse_check_timeout() is currently checking.
Also, with a ring per core, maybe better to use
a per queue check that is core bound? I.e. zero locking overhead?
And I think we can also avoid iterating over hash lists (queue->fpq),
but can use the 'ent_in_userspace' list.
We need to iterate over these other entry queues anyway:
ent_w_req_queue
fuse_req_bg_queue
ent_commit_queue
And we also need to iterate over
fuse_req_queue
fuse_req_bg_queue
Thanks,
Bernd
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 17:19 ` Bernd Schubert
@ 2025-01-23 17:48 ` Joanne Koong
2025-01-23 18:06 ` Bernd Schubert
0 siblings, 1 reply; 24+ messages in thread
From: Joanne Koong @ 2025-01-23 17:48 UTC (permalink / raw)
To: Bernd Schubert
Cc: Bernd Schubert, Miklos Szeredi, linux-fsdevel, jefflexu,
laoar.shao, jlayton, senozhatsky, tfiga, bgeffon, etmartin4313,
kernel-team, Josef Bacik, Luis Henriques
On Thu, Jan 23, 2025 at 9:19 AM Bernd Schubert
<bernd.schubert@fastmail.fm> wrote:
>
> Hi Joanne,
>
> >>> Thanks, applied and pushed with some cleanups including Luis's clamp idea.
> >>
> >> Hi Miklos,
> >>
> >> I don't think the timeouts do work with io-uring yet, I'm not sure
> >> yet if I have time to work on that today or tomorrow (on something
> >> else right now, I can try, but no promises).
> >
> > Hi Bernd,
> >
> > What are your thoughts on what is missing on the io-uring side for
> > timeouts? If a request times out, it will abort the connection and
> > AFAICT, the abort logic should already be fine for io-uring, as users
> > can currently abort the connection through the sysfs interface and
> > there's no internal difference in aborting through sysfs vs timeouts.
> >
>
> in fuse_check_timeout() it iterates over each fud and then fpq.
> In dev_uring.c fpq is is per queue but unrelated to fud. In current
> fuse-io-uring fud is not cloned anymore - using fud won't work.
> And Requests are also not queued at all on the other list
> fuse_check_timeout() is currently checking.
In the io-uring case, there still can be fuds and their associated
fpqs given that /dev/fuse can be used still, no? So wouldn't the
io-uring case still need this logic in fuse_check_timeout() for
checking requests going through /dev/fuse?
>
> Also, with a ring per core, maybe better to use
> a per queue check that is core bound? I.e. zero locking overhead?
How do you envision a queue check that bypasses grabbing the
queue->lock? The timeout handler could be triggered on any core, so
I'm not seeing how it could be core bound.
> And I think we can also avoid iterating over hash lists (queue->fpq),
> but can use the 'ent_in_userspace' list.
>
> We need to iterate over these other entry queues anyway:
>
> ent_w_req_queue
> fuse_req_bg_queue
> ent_commit_queue
>
Why do we need to iterate through the ent lists (ent_w_req_queue and
ent_commit_queue)? AFAICT, in io-uring a request is either on the
fuse_req_queue/fuse_req_bg_queue or on the fpq->processing list. Even
when an entry has been queued to ent_w_req_queue or ent_commit_queue,
the request itself is still queued on
fuse_req_queue/fuse_req_bg_queue/fpq->processing. I'm not sure I
understand why we still need to look at the ent lists?
>
> And we also need to iterate over
>
> fuse_req_queue
> fuse_req_bg_queue
Why do we need to iterate through fuse_req_queue and
fuse_req_bg_queue? fuse_uring_request_expired() checks the head of
fuse_req_queue and fuse_req_bg_queue and given that requests are added
to fuse_req_queue/fuse_req_bg_queue sequentially (eg added to the tail
of these lists), why isn't this enough?
If it's helpful, I can resubmit this patch series so that the io-uring
changes are isolated to its own patch (eg have patch 1 and 2 from the
original series and then have patch 3 be the io-uring changes).
Thanks,
Joanne
>
>
>
> Thanks,
> Bernd
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 17:48 ` Joanne Koong
@ 2025-01-23 18:06 ` Bernd Schubert
2025-01-23 18:32 ` Joanne Koong
0 siblings, 1 reply; 24+ messages in thread
From: Bernd Schubert @ 2025-01-23 18:06 UTC (permalink / raw)
To: Joanne Koong
Cc: Bernd Schubert, Miklos Szeredi, linux-fsdevel, jefflexu,
laoar.shao, jlayton, senozhatsky, tfiga, bgeffon, etmartin4313,
kernel-team, Josef Bacik, Luis Henriques
On 1/23/25 18:48, Joanne Koong wrote:
> On Thu, Jan 23, 2025 at 9:19 AM Bernd Schubert
> <bernd.schubert@fastmail.fm> wrote:
>>
>> Hi Joanne,
>>
>>>>> Thanks, applied and pushed with some cleanups including Luis's clamp idea.
>>>>
>>>> Hi Miklos,
>>>>
>>>> I don't think the timeouts do work with io-uring yet, I'm not sure
>>>> yet if I have time to work on that today or tomorrow (on something
>>>> else right now, I can try, but no promises).
>>>
>>> Hi Bernd,
>>>
>>> What are your thoughts on what is missing on the io-uring side for
>>> timeouts? If a request times out, it will abort the connection and
>>> AFAICT, the abort logic should already be fine for io-uring, as users
>>> can currently abort the connection through the sysfs interface and
>>> there's no internal difference in aborting through sysfs vs timeouts.
>>>
>>
>> in fuse_check_timeout() it iterates over each fud and then fpq.
>> In dev_uring.c fpq is is per queue but unrelated to fud. In current
>> fuse-io-uring fud is not cloned anymore - using fud won't work.
>> And Requests are also not queued at all on the other list
>> fuse_check_timeout() is currently checking.
>
> In the io-uring case, there still can be fuds and their associated
> fpqs given that /dev/fuse can be used still, no? So wouldn't the
> io-uring case still need this logic in fuse_check_timeout() for
> checking requests going through /dev/fuse?
Yes, these need to be additionally checked.
>
>>
>> Also, with a ring per core, maybe better to use
>> a per queue check that is core bound? I.e. zero locking overhead?
>
> How do you envision a queue check that bypasses grabbing the
> queue->lock? The timeout handler could be triggered on any core, so
> I'm not seeing how it could be core bound.
I don't want to bypass it, but maybe each queue could have its own
workq and timeout checker? And then use queue_delayed_work_on()?
>
>> And I think we can also avoid iterating over hash lists (queue->fpq),
>> but can use the 'ent_in_userspace' list.
>>
>> We need to iterate over these other entry queues anyway:
>>
>> ent_w_req_queue
>> fuse_req_bg_queue
>> ent_commit_queue
>>
>
> Why do we need to iterate through the ent lists (ent_w_req_queue and
> ent_commit_queue)? AFAICT, in io-uring a request is either on the
> fuse_req_queue/fuse_req_bg_queue or on the fpq->processing list. Even
> when an entry has been queued to ent_w_req_queue or ent_commit_queue,
> the request itself is still queued on
> fuse_req_queue/fuse_req_bg_queue/fpq->processing. I'm not sure I
> understand why we still need to look at the ent lists?
Yeah you are right, we could avoid ent_w_req_queue and ent_commit_queue
if we use fpq->processing, but processing consists of 256 lists -
overhead is much smaller by using the entry lists?
>
>>
>> And we also need to iterate over
>>
>> fuse_req_queue
>> fuse_req_bg_queue
>
> Why do we need to iterate through fuse_req_queue and
> fuse_req_bg_queue? fuse_uring_request_expired() checks the head of
> fuse_req_queue and fuse_req_bg_queue and given that requests are added
> to fuse_req_queue/fuse_req_bg_queue sequentially (eg added to the tail
> of these lists), why isn't this enough?
I admit I'm a bit lost with that question. Aren't you pointing out
the same lists as I do?
>
>
> If it's helpful, I can resubmit this patch series so that the io-uring
> changes are isolated to its own patch (eg have patch 1 and 2 from the
> original series and then have patch 3 be the io-uring changes).
Sounds good to me.
Thanks,
Bernd
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 18:06 ` Bernd Schubert
@ 2025-01-23 18:32 ` Joanne Koong
2025-01-23 18:40 ` Bernd Schubert
0 siblings, 1 reply; 24+ messages in thread
From: Joanne Koong @ 2025-01-23 18:32 UTC (permalink / raw)
To: Bernd Schubert
Cc: Bernd Schubert, Miklos Szeredi, linux-fsdevel, jefflexu,
laoar.shao, jlayton, senozhatsky, tfiga, bgeffon, etmartin4313,
kernel-team, Josef Bacik, Luis Henriques
On Thu, Jan 23, 2025 at 10:06 AM Bernd Schubert
<bernd.schubert@fastmail.fm> wrote:
>
>
>
> On 1/23/25 18:48, Joanne Koong wrote:
> > On Thu, Jan 23, 2025 at 9:19 AM Bernd Schubert
> > <bernd.schubert@fastmail.fm> wrote:
> >>
> >> Hi Joanne,
> >>
> >>>>> Thanks, applied and pushed with some cleanups including Luis's clamp idea.
> >>>>
> >>>> Hi Miklos,
> >>>>
> >>>> I don't think the timeouts do work with io-uring yet, I'm not sure
> >>>> yet if I have time to work on that today or tomorrow (on something
> >>>> else right now, I can try, but no promises).
> >>>
> >>> Hi Bernd,
> >>>
> >>> What are your thoughts on what is missing on the io-uring side for
> >>> timeouts? If a request times out, it will abort the connection and
> >>> AFAICT, the abort logic should already be fine for io-uring, as users
> >>> can currently abort the connection through the sysfs interface and
> >>> there's no internal difference in aborting through sysfs vs timeouts.
> >>>
> >>
> >> in fuse_check_timeout() it iterates over each fud and then fpq.
> >> In dev_uring.c fpq is is per queue but unrelated to fud. In current
> >> fuse-io-uring fud is not cloned anymore - using fud won't work.
> >> And Requests are also not queued at all on the other list
> >> fuse_check_timeout() is currently checking.
> >
> > In the io-uring case, there still can be fuds and their associated
> > fpqs given that /dev/fuse can be used still, no? So wouldn't the
> > io-uring case still need this logic in fuse_check_timeout() for
> > checking requests going through /dev/fuse?
>
> Yes, these need to be additionally checked.
>
> >
> >>
> >> Also, with a ring per core, maybe better to use
> >> a per queue check that is core bound? I.e. zero locking overhead?
> >
> > How do you envision a queue check that bypasses grabbing the
> > queue->lock? The timeout handler could be triggered on any core, so
> > I'm not seeing how it could be core bound.
>
> I don't want to bypass it, but maybe each queue could have its own
> workq and timeout checker? And then use queue_delayed_work_on()?
>
>
> >
> >> And I think we can also avoid iterating over hash lists (queue->fpq),
> >> but can use the 'ent_in_userspace' list.
> >>
> >> We need to iterate over these other entry queues anyway:
> >>
> >> ent_w_req_queue
> >> fuse_req_bg_queue
> >> ent_commit_queue
> >>
> >
> > Why do we need to iterate through the ent lists (ent_w_req_queue and
> > ent_commit_queue)? AFAICT, in io-uring a request is either on the
> > fuse_req_queue/fuse_req_bg_queue or on the fpq->processing list. Even
> > when an entry has been queued to ent_w_req_queue or ent_commit_queue,
> > the request itself is still queued on
> > fuse_req_queue/fuse_req_bg_queue/fpq->processing. I'm not sure I
> > understand why we still need to look at the ent lists?
>
> Yeah you are right, we could avoid ent_w_req_queue and ent_commit_queue
> if we use fpq->processing, but processing consists of 256 lists -
> overhead is much smaller by using the entry lists?
>
>
> >
> >>
> >> And we also need to iterate over
> >>
> >> fuse_req_queue
> >> fuse_req_bg_queue
> >
> > Why do we need to iterate through fuse_req_queue and
> > fuse_req_bg_queue? fuse_uring_request_expired() checks the head of
> > fuse_req_queue and fuse_req_bg_queue and given that requests are added
> > to fuse_req_queue/fuse_req_bg_queue sequentially (eg added to the tail
> > of these lists), why isn't this enough?
>
> I admit I'm a bit lost with that question. Aren't you pointing out
> the same lists as I do?
>
Oh, I thought your comment was saying that we need to "iterate" over
it (eg go through every request on the lists)? It currently already
checks the fuse_req_queue and fuse_req_bg_queue lists (in
fuse_uring_request_expired() which gets invoked in the
fuse_check_timeout() timeout handler).
Maybe the fuse_uring_request_expired() addition got missed - I tried
to call this out in the cover letter changelog, as I had to rebase
this patchset on top of the io-uring patches, so I added this function
in to make it work for io-uring. I believe this suffices for now for
the io uring case (with future optimizations that can be added)?
Thanks,
Joanne
> >
> >
> > If it's helpful, I can resubmit this patch series so that the io-uring
> > changes are isolated to its own patch (eg have patch 1 and 2 from the
> > original series and then have patch 3 be the io-uring changes).
>
> Sounds good to me.
>
>
> Thanks,
> Bernd
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 18:32 ` Joanne Koong
@ 2025-01-23 18:40 ` Bernd Schubert
2025-01-23 23:20 ` Joanne Koong
0 siblings, 1 reply; 24+ messages in thread
From: Bernd Schubert @ 2025-01-23 18:40 UTC (permalink / raw)
To: Joanne Koong
Cc: Bernd Schubert, Miklos Szeredi, linux-fsdevel, jefflexu,
laoar.shao, jlayton, senozhatsky, tfiga, bgeffon, etmartin4313,
kernel-team, Josef Bacik, Luis Henriques
On 1/23/25 19:32, Joanne Koong wrote:
> On Thu, Jan 23, 2025 at 10:06 AM Bernd Schubert
> <bernd.schubert@fastmail.fm> wrote:
>>
>>
>>
>> On 1/23/25 18:48, Joanne Koong wrote:
>>> On Thu, Jan 23, 2025 at 9:19 AM Bernd Schubert
>>> <bernd.schubert@fastmail.fm> wrote:
>>>>
>>>> Hi Joanne,
>>>>
>>>>>>> Thanks, applied and pushed with some cleanups including Luis's clamp idea.
>>>>>>
>>>>>> Hi Miklos,
>>>>>>
>>>>>> I don't think the timeouts do work with io-uring yet, I'm not sure
>>>>>> yet if I have time to work on that today or tomorrow (on something
>>>>>> else right now, I can try, but no promises).
>>>>>
>>>>> Hi Bernd,
>>>>>
>>>>> What are your thoughts on what is missing on the io-uring side for
>>>>> timeouts? If a request times out, it will abort the connection and
>>>>> AFAICT, the abort logic should already be fine for io-uring, as users
>>>>> can currently abort the connection through the sysfs interface and
>>>>> there's no internal difference in aborting through sysfs vs timeouts.
>>>>>
>>>>
>>>> in fuse_check_timeout() it iterates over each fud and then fpq.
>>>> In dev_uring.c fpq is is per queue but unrelated to fud. In current
>>>> fuse-io-uring fud is not cloned anymore - using fud won't work.
>>>> And Requests are also not queued at all on the other list
>>>> fuse_check_timeout() is currently checking.
>>>
>>> In the io-uring case, there still can be fuds and their associated
>>> fpqs given that /dev/fuse can be used still, no? So wouldn't the
>>> io-uring case still need this logic in fuse_check_timeout() for
>>> checking requests going through /dev/fuse?
>>
>> Yes, these need to be additionally checked.
>>
>>>
>>>>
>>>> Also, with a ring per core, maybe better to use
>>>> a per queue check that is core bound? I.e. zero locking overhead?
>>>
>>> How do you envision a queue check that bypasses grabbing the
>>> queue->lock? The timeout handler could be triggered on any core, so
>>> I'm not seeing how it could be core bound.
>>
>> I don't want to bypass it, but maybe each queue could have its own
>> workq and timeout checker? And then use queue_delayed_work_on()?
>>
>>
>>>
>>>> And I think we can also avoid iterating over hash lists (queue->fpq),
>>>> but can use the 'ent_in_userspace' list.
>>>>
>>>> We need to iterate over these other entry queues anyway:
>>>>
>>>> ent_w_req_queue
>>>> fuse_req_bg_queue
>>>> ent_commit_queue
>>>>
>>>
>>> Why do we need to iterate through the ent lists (ent_w_req_queue and
>>> ent_commit_queue)? AFAICT, in io-uring a request is either on the
>>> fuse_req_queue/fuse_req_bg_queue or on the fpq->processing list. Even
>>> when an entry has been queued to ent_w_req_queue or ent_commit_queue,
>>> the request itself is still queued on
>>> fuse_req_queue/fuse_req_bg_queue/fpq->processing. I'm not sure I
>>> understand why we still need to look at the ent lists?
>>
>> Yeah you are right, we could avoid ent_w_req_queue and ent_commit_queue
>> if we use fpq->processing, but processing consists of 256 lists -
>> overhead is much smaller by using the entry lists?
>>
>>
>>>
>>>>
>>>> And we also need to iterate over
>>>>
>>>> fuse_req_queue
>>>> fuse_req_bg_queue
>>>
>>> Why do we need to iterate through fuse_req_queue and
>>> fuse_req_bg_queue? fuse_uring_request_expired() checks the head of
>>> fuse_req_queue and fuse_req_bg_queue and given that requests are added
>>> to fuse_req_queue/fuse_req_bg_queue sequentially (eg added to the tail
>>> of these lists), why isn't this enough?
>>
>> I admit I'm a bit lost with that question. Aren't you pointing out
>> the same lists as I do?
>>
>
> Oh, I thought your comment was saying that we need to "iterate" over
> it (eg go through every request on the lists)? It currently already
> checks the fuse_req_queue and fuse_req_bg_queue lists (in
> fuse_uring_request_expired() which gets invoked in the
> fuse_check_timeout() timeout handler).
>
> Maybe the fuse_uring_request_expired() addition got missed - I tried
> to call this out in the cover letter changelog, as I had to rebase
> this patchset on top of the io-uring patches, so I added this function
> in to make it work for io-uring. I believe this suffices for now for
> the io uring case (with future optimizations that can be added)?
Ah sorry, that is me, I had missed you had already rebased it to
io-uring.
So we are good to land this version.
Just would be good if we could optimize this soon - our test systems
have 96 cores - 24576 list heads to check... I won't manage to work
on it today and probably also not tomorrow, but by Monday I should
have an optimized version ready.
Thanks,
Bernd
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 18:40 ` Bernd Schubert
@ 2025-01-23 23:20 ` Joanne Koong
0 siblings, 0 replies; 24+ messages in thread
From: Joanne Koong @ 2025-01-23 23:20 UTC (permalink / raw)
To: Bernd Schubert
Cc: Bernd Schubert, Miklos Szeredi, linux-fsdevel, jefflexu,
laoar.shao, jlayton, senozhatsky, tfiga, bgeffon, etmartin4313,
kernel-team, Josef Bacik, Luis Henriques
On Thu, Jan 23, 2025 at 10:40 AM Bernd Schubert
<bernd.schubert@fastmail.fm> wrote:
>
>
>
> On 1/23/25 19:32, Joanne Koong wrote:
> > On Thu, Jan 23, 2025 at 10:06 AM Bernd Schubert
> > <bernd.schubert@fastmail.fm> wrote:
> >>
> >>
> >>
> >> On 1/23/25 18:48, Joanne Koong wrote:
> >>> On Thu, Jan 23, 2025 at 9:19 AM Bernd Schubert
> >>> <bernd.schubert@fastmail.fm> wrote:
> >>>>
> >>>> Hi Joanne,
> >>>>
> >>>>>>> Thanks, applied and pushed with some cleanups including Luis's clamp idea.
> >>>>>>
> >>>>>> Hi Miklos,
> >>>>>>
> >>>>>> I don't think the timeouts do work with io-uring yet, I'm not sure
> >>>>>> yet if I have time to work on that today or tomorrow (on something
> >>>>>> else right now, I can try, but no promises).
> >>>>>
> >>>>> Hi Bernd,
> >>>>>
> >>>>> What are your thoughts on what is missing on the io-uring side for
> >>>>> timeouts? If a request times out, it will abort the connection and
> >>>>> AFAICT, the abort logic should already be fine for io-uring, as users
> >>>>> can currently abort the connection through the sysfs interface and
> >>>>> there's no internal difference in aborting through sysfs vs timeouts.
> >>>>>
> >>>>
> >>>> in fuse_check_timeout() it iterates over each fud and then fpq.
> >>>> In dev_uring.c fpq is is per queue but unrelated to fud. In current
> >>>> fuse-io-uring fud is not cloned anymore - using fud won't work.
> >>>> And Requests are also not queued at all on the other list
> >>>> fuse_check_timeout() is currently checking.
> >>>
> >>> In the io-uring case, there still can be fuds and their associated
> >>> fpqs given that /dev/fuse can be used still, no? So wouldn't the
> >>> io-uring case still need this logic in fuse_check_timeout() for
> >>> checking requests going through /dev/fuse?
> >>
> >> Yes, these need to be additionally checked.
> >>
> >>>
> >>>>
> >>>> Also, with a ring per core, maybe better to use
> >>>> a per queue check that is core bound? I.e. zero locking overhead?
> >>>
> >>> How do you envision a queue check that bypasses grabbing the
> >>> queue->lock? The timeout handler could be triggered on any core, so
> >>> I'm not seeing how it could be core bound.
> >>
> >> I don't want to bypass it, but maybe each queue could have its own
> >> workq and timeout checker? And then use queue_delayed_work_on()?
> >>
> >>
> >>>
> >>>> And I think we can also avoid iterating over hash lists (queue->fpq),
> >>>> but can use the 'ent_in_userspace' list.
> >>>>
> >>>> We need to iterate over these other entry queues anyway:
> >>>>
> >>>> ent_w_req_queue
> >>>> fuse_req_bg_queue
> >>>> ent_commit_queue
> >>>>
> >>>
> >>> Why do we need to iterate through the ent lists (ent_w_req_queue and
> >>> ent_commit_queue)? AFAICT, in io-uring a request is either on the
> >>> fuse_req_queue/fuse_req_bg_queue or on the fpq->processing list. Even
> >>> when an entry has been queued to ent_w_req_queue or ent_commit_queue,
> >>> the request itself is still queued on
> >>> fuse_req_queue/fuse_req_bg_queue/fpq->processing. I'm not sure I
> >>> understand why we still need to look at the ent lists?
> >>
> >> Yeah you are right, we could avoid ent_w_req_queue and ent_commit_queue
> >> if we use fpq->processing, but processing consists of 256 lists -
> >> overhead is much smaller by using the entry lists?
> >>
> >>
> >>>
> >>>>
> >>>> And we also need to iterate over
> >>>>
> >>>> fuse_req_queue
> >>>> fuse_req_bg_queue
> >>>
> >>> Why do we need to iterate through fuse_req_queue and
> >>> fuse_req_bg_queue? fuse_uring_request_expired() checks the head of
> >>> fuse_req_queue and fuse_req_bg_queue and given that requests are added
> >>> to fuse_req_queue/fuse_req_bg_queue sequentially (eg added to the tail
> >>> of these lists), why isn't this enough?
> >>
> >> I admit I'm a bit lost with that question. Aren't you pointing out
> >> the same lists as I do?
> >>
> >
> > Oh, I thought your comment was saying that we need to "iterate" over
> > it (eg go through every request on the lists)? It currently already
> > checks the fuse_req_queue and fuse_req_bg_queue lists (in
> > fuse_uring_request_expired() which gets invoked in the
> > fuse_check_timeout() timeout handler).
> >
> > Maybe the fuse_uring_request_expired() addition got missed - I tried
> > to call this out in the cover letter changelog, as I had to rebase
> > this patchset on top of the io-uring patches, so I added this function
> > in to make it work for io-uring. I believe this suffices for now for
> > the io uring case (with future optimizations that can be added)?
>
>
> Ah sorry, that is me, I had missed you had already rebased it to
> io-uring.
>
> So we are good to land this version.
> Just would be good if we could optimize this soon - our test systems
> have 96 cores - 24576 list heads to check... I won't manage to work
> on it today and probably also not tomorrow, but by Monday I should
> have an optimized version ready.
>
Oh wow, 96 cores!! I'll submit an optimized version of this today or
tomorrow then, if that makes things easier for you.
Thanks,
Joanne
> Thanks,
> Bernd
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 9:20 ` Luis Henriques
2025-01-23 14:28 ` Miklos Szeredi
@ 2025-01-23 17:18 ` Joanne Koong
2025-01-23 21:54 ` Luis Henriques
1 sibling, 1 reply; 24+ messages in thread
From: Joanne Koong @ 2025-01-23 17:18 UTC (permalink / raw)
To: Luis Henriques
Cc: miklos, linux-fsdevel, bernd.schubert, jefflexu, laoar.shao,
jlayton, senozhatsky, tfiga, bgeffon, etmartin4313, kernel-team,
Bernd Schubert, Josef Bacik
On Thu, Jan 23, 2025 at 1:21 AM Luis Henriques <luis@igalia.com> wrote:
>
> Hi Joanne,
>
> On Wed, Jan 22 2025, Joanne Koong wrote:
>
> > Introduce two new sysctls, "default_request_timeout" and
> > "max_request_timeout". These control how long (in seconds) a server can
> > take to reply to a request. If the server does not reply by the timeout,
> > then the connection will be aborted. The upper bound on these sysctl
> > values is 65535.
> >
> > "default_request_timeout" sets the default timeout if no timeout is
> > specified by the fuse server on mount. 0 (default) indicates no default
> > timeout should be enforced. If the server did specify a timeout, then
> > default_request_timeout will be ignored.
> >
> > "max_request_timeout" sets the max amount of time the server may take to
> > reply to a request. 0 (default) indicates no maximum timeout. If
> > max_request_timeout is set and the fuse server attempts to set a
> > timeout greater than max_request_timeout, the system will use
> > max_request_timeout as the timeout. Similarly, if default_request_timeout
> > is greater than max_request_timeout, the system will use
> > max_request_timeout as the timeout. If the server does not request a
> > timeout and default_request_timeout is set to 0 but max_request_timeout
> > is set, then the timeout will be max_request_timeout.
> >
> > Please note that these timeouts are not 100% precise. The request may
> > take roughly an extra FUSE_TIMEOUT_TIMER_FREQ seconds beyond the set max
> > timeout due to how it's internally implemented.
> >
> > $ sysctl -a | grep fuse.default_request_timeout
> > fs.fuse.default_request_timeout = 0
> >
> > $ echo 65536 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> > tee: /proc/sys/fs/fuse/default_request_timeout: Invalid argument
> >
> > $ echo 65535 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> > 65535
> >
> > $ sysctl -a | grep fuse.default_request_timeout
> > fs.fuse.default_request_timeout = 65535
> >
> > $ echo 0 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> > 0
> >
> > $ sysctl -a | grep fuse.default_request_timeout
> > fs.fuse.default_request_timeout = 0
> >
> > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> > Reviewed-by: Bernd Schubert <bschubert@ddn.com>
> > Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> > Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> > ---
> > Documentation/admin-guide/sysctl/fs.rst | 25 ++++++++++++++++++++++
> > fs/fuse/fuse_i.h | 10 +++++++++
> > fs/fuse/inode.c | 28 +++++++++++++++++++++++--
> > fs/fuse/sysctl.c | 24 +++++++++++++++++++++
> > 4 files changed, 85 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/sysctl/fs.rst b/Documentation/admin-guide/sysctl/fs.rst
> > index f5ec6c9312e1..35aeb30bed8b 100644
> > --- a/Documentation/admin-guide/sysctl/fs.rst
> > +++ b/Documentation/admin-guide/sysctl/fs.rst
> > @@ -347,3 +347,28 @@ filesystems:
> > ``/proc/sys/fs/fuse/max_pages_limit`` is a read/write file for
> > setting/getting the maximum number of pages that can be used for servicing
> > requests in FUSE.
> > +
> > +``/proc/sys/fs/fuse/default_request_timeout`` is a read/write file for
> > +setting/getting the default timeout (in seconds) for a fuse server to
> > +reply to a kernel-issued request in the event where the server did not
> > +specify a timeout at mount. If the server set a timeout,
> > +then default_request_timeout will be ignored. The default
> > +"default_request_timeout" is set to 0. 0 indicates no default timeout.
> > +The maximum value that can be set is 65535.
> > +
> > +``/proc/sys/fs/fuse/max_request_timeout`` is a read/write file for
> > +setting/getting the maximum timeout (in seconds) for a fuse server to
> > +reply to a kernel-issued request. A value greater than 0 automatically opts
> > +the server into a timeout that will be set to at most "max_request_timeout",
> > +even if the server did not specify a timeout and default_request_timeout is
> > +set to 0. If max_request_timeout is greater than 0 and the server set a timeout
> > +greater than max_request_timeout or default_request_timeout is set to a value
> > +greater than max_request_timeout, the system will use max_request_timeout as the
> > +timeout. 0 indicates no max request timeout. The maximum value that can be set
> > +is 65535.
> > +
> > +For timeouts, if the server does not respond to the request by the time
> > +the set timeout elapses, then the connection to the fuse server will be aborted.
> > +Please note that the timeouts are not 100% precise (eg you may set 60 seconds but
> > +the timeout may kick in after 70 seconds). The upper margin of error for the
> > +timeout is roughly FUSE_TIMEOUT_TIMER_FREQ seconds.
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index 1321cc4ed2ab..e5114831798f 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -49,6 +49,16 @@ extern const unsigned long fuse_timeout_timer_freq;
> >
> > /** Maximum of max_pages received in init_out */
> > extern unsigned int fuse_max_pages_limit;
> > +/*
> > + * Default timeout (in seconds) for the server to reply to a request
> > + * before the connection is aborted, if no timeout was specified on mount.
> > + */
> > +extern unsigned int fuse_default_req_timeout;
> > +/*
> > + * Max timeout (in seconds) for the server to reply to a request before
> > + * the connection is aborted.
> > + */
> > +extern unsigned int fuse_max_req_timeout;
> >
> > /** List of active connections */
> > extern struct list_head fuse_conn_list;
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index 79ebeb60015c..4e36d99fae52 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -37,6 +37,9 @@ DEFINE_MUTEX(fuse_mutex);
> > static int set_global_limit(const char *val, const struct kernel_param *kp);
> >
> > unsigned int fuse_max_pages_limit = 256;
> > +/* default is no timeout */
> > +unsigned int fuse_default_req_timeout = 0;
> > +unsigned int fuse_max_req_timeout = 0;
> >
> > unsigned max_user_bgreq;
> > module_param_call(max_user_bgreq, set_global_limit, param_get_uint,
> > @@ -1268,6 +1271,24 @@ static void set_request_timeout(struct fuse_conn *fc, unsigned int timeout)
> > fuse_timeout_timer_freq);
> > }
> >
> > +static void init_server_timeout(struct fuse_conn *fc, unsigned int timeout)
> > +{
> > + if (!timeout && !fuse_max_req_timeout && !fuse_default_req_timeout)
> > + return;
> > +
> > + if (!timeout)
> > + timeout = fuse_default_req_timeout;
> > +
> > + if (fuse_max_req_timeout) {
> > + if (timeout)
> > + timeout = min(fuse_max_req_timeout, timeout);
>
> Sorry for this late comment (this is v12 already!), but I wonder if it
> would be worth to use clamp() instead of min(). Limiting this value to
> the range [FUSE_TIMEOUT_TIMER_FREQ, fuxe_max_req_timeout] would prevent
> accidentally setting the timeout to a very low value.
I don't think we need to clamp to FUSE_TIMEOUT_TIMER_FREQ. If the user
sets a timeout value lower than FUSE_TIMEOUT_TIMER_FREQ (15 secs), imo
that should be supported. For example, if the user sets the timeout to
10 seconds, then the connection should be aborted if the request takes
13 seconds. I don't see why we need to have the min bound be
FUSE_TIMEOUT_TIMER_FREQ. imo, the user-specified timeout value and
FUSE_TIMEOUT_TIMER_FREQ value are orthogonal. But I also don't feel
strongly about this and am fine with whichever way we go.
Thanks,
Joanne
>
> There are also some white-spaces/tabs issues with the other patch, which
> are worth fixing before merging. Other than that, feel free to add my
>
> Reviewed-by: Luis Henriques <luis@igalia.com>
>
> Cheers,
> --
> Luís
>
> > + else
> > + timeout = fuse_max_req_timeout;
> > + }
> > +
> > + set_request_timeout(fc, timeout);
> > +}
> > +
> > struct fuse_init_args {
> > struct fuse_args args;
> > struct fuse_init_in in;
> > @@ -1286,6 +1307,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
> > ok = false;
> > else {
> > unsigned long ra_pages;
> > + unsigned int timeout = 0;
> >
> > process_init_limits(fc, arg);
> >
> > @@ -1404,14 +1426,16 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
> > if (flags & FUSE_OVER_IO_URING && fuse_uring_enabled())
> > fc->io_uring = 1;
> >
> > - if ((flags & FUSE_REQUEST_TIMEOUT) && arg->request_timeout)
> > - set_request_timeout(fc, arg->request_timeout);
> > + if (flags & FUSE_REQUEST_TIMEOUT)
> > + timeout = arg->request_timeout;
> > } else {
> > ra_pages = fc->max_read / PAGE_SIZE;
> > fc->no_lock = 1;
> > fc->no_flock = 1;
> > }
> >
> > + init_server_timeout(fc, timeout);
> > +
> > fm->sb->s_bdi->ra_pages =
> > min(fm->sb->s_bdi->ra_pages, ra_pages);
> > fc->minor = arg->minor;
> > diff --git a/fs/fuse/sysctl.c b/fs/fuse/sysctl.c
> > index b272bb333005..3d542ef9d889 100644
> > --- a/fs/fuse/sysctl.c
> > +++ b/fs/fuse/sysctl.c
> > @@ -13,6 +13,12 @@ static struct ctl_table_header *fuse_table_header;
> > /* Bound by fuse_init_out max_pages, which is a u16 */
> > static unsigned int sysctl_fuse_max_pages_limit = 65535;
> >
> > +/*
> > + * fuse_init_out request timeouts are u16.
> > + * This goes up to ~18 hours, which is plenty for a timeout.
> > + */
> > +static unsigned int sysctl_fuse_req_timeout_limit = 65535;
> > +
> > static struct ctl_table fuse_sysctl_table[] = {
> > {
> > .procname = "max_pages_limit",
> > @@ -23,6 +29,24 @@ static struct ctl_table fuse_sysctl_table[] = {
> > .extra1 = SYSCTL_ONE,
> > .extra2 = &sysctl_fuse_max_pages_limit,
> > },
> > + {
> > + .procname = "default_request_timeout",
> > + .data = &fuse_default_req_timeout,
> > + .maxlen = sizeof(fuse_default_req_timeout),
> > + .mode = 0644,
> > + .proc_handler = proc_douintvec_minmax,
> > + .extra1 = SYSCTL_ZERO,
> > + .extra2 = &sysctl_fuse_req_timeout_limit,
> > + },
> > + {
> > + .procname = "max_request_timeout",
> > + .data = &fuse_max_req_timeout,
> > + .maxlen = sizeof(fuse_max_req_timeout),
> > + .mode = 0644,
> > + .proc_handler = proc_douintvec_minmax,
> > + .extra1 = SYSCTL_ZERO,
> > + .extra2 = &sysctl_fuse_req_timeout_limit,
> > + },
> > };
> >
> > int fuse_sysctl_register(void)
> > --
> > 2.43.5
> >
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 17:18 ` Joanne Koong
@ 2025-01-23 21:54 ` Luis Henriques
2025-01-23 23:15 ` Joanne Koong
0 siblings, 1 reply; 24+ messages in thread
From: Luis Henriques @ 2025-01-23 21:54 UTC (permalink / raw)
To: Joanne Koong
Cc: miklos, linux-fsdevel, bernd.schubert, jefflexu, laoar.shao,
jlayton, senozhatsky, tfiga, bgeffon, etmartin4313, kernel-team,
Bernd Schubert, Josef Bacik
Hi Joanne,
On Thu, Jan 23 2025, Joanne Koong wrote:
> On Thu, Jan 23, 2025 at 1:21 AM Luis Henriques <luis@igalia.com> wrote:
>>
>> Hi Joanne,
>>
>> On Wed, Jan 22 2025, Joanne Koong wrote:
>>
>> > Introduce two new sysctls, "default_request_timeout" and
>> > "max_request_timeout". These control how long (in seconds) a server can
>> > take to reply to a request. If the server does not reply by the timeout,
>> > then the connection will be aborted. The upper bound on these sysctl
>> > values is 65535.
>> >
>> > "default_request_timeout" sets the default timeout if no timeout is
>> > specified by the fuse server on mount. 0 (default) indicates no default
>> > timeout should be enforced. If the server did specify a timeout, then
>> > default_request_timeout will be ignored.
>> >
>> > "max_request_timeout" sets the max amount of time the server may take to
>> > reply to a request. 0 (default) indicates no maximum timeout. If
>> > max_request_timeout is set and the fuse server attempts to set a
>> > timeout greater than max_request_timeout, the system will use
>> > max_request_timeout as the timeout. Similarly, if default_request_timeout
>> > is greater than max_request_timeout, the system will use
>> > max_request_timeout as the timeout. If the server does not request a
>> > timeout and default_request_timeout is set to 0 but max_request_timeout
>> > is set, then the timeout will be max_request_timeout.
>> >
>> > Please note that these timeouts are not 100% precise. The request may
>> > take roughly an extra FUSE_TIMEOUT_TIMER_FREQ seconds beyond the set max
>> > timeout due to how it's internally implemented.
>> >
>> > $ sysctl -a | grep fuse.default_request_timeout
>> > fs.fuse.default_request_timeout = 0
>> >
>> > $ echo 65536 | sudo tee /proc/sys/fs/fuse/default_request_timeout
>> > tee: /proc/sys/fs/fuse/default_request_timeout: Invalid argument
>> >
>> > $ echo 65535 | sudo tee /proc/sys/fs/fuse/default_request_timeout
>> > 65535
>> >
>> > $ sysctl -a | grep fuse.default_request_timeout
>> > fs.fuse.default_request_timeout = 65535
>> >
>> > $ echo 0 | sudo tee /proc/sys/fs/fuse/default_request_timeout
>> > 0
>> >
>> > $ sysctl -a | grep fuse.default_request_timeout
>> > fs.fuse.default_request_timeout = 0
>> >
>> > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
>> > Reviewed-by: Bernd Schubert <bschubert@ddn.com>
>> > Reviewed-by: Josef Bacik <josef@toxicpanda.com>
>> > Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
>> > ---
>> > Documentation/admin-guide/sysctl/fs.rst | 25 ++++++++++++++++++++++
>> > fs/fuse/fuse_i.h | 10 +++++++++
>> > fs/fuse/inode.c | 28 +++++++++++++++++++++++--
>> > fs/fuse/sysctl.c | 24 +++++++++++++++++++++
>> > 4 files changed, 85 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/Documentation/admin-guide/sysctl/fs.rst b/Documentation/admin-guide/sysctl/fs.rst
>> > index f5ec6c9312e1..35aeb30bed8b 100644
>> > --- a/Documentation/admin-guide/sysctl/fs.rst
>> > +++ b/Documentation/admin-guide/sysctl/fs.rst
>> > @@ -347,3 +347,28 @@ filesystems:
>> > ``/proc/sys/fs/fuse/max_pages_limit`` is a read/write file for
>> > setting/getting the maximum number of pages that can be used for servicing
>> > requests in FUSE.
>> > +
>> > +``/proc/sys/fs/fuse/default_request_timeout`` is a read/write file for
>> > +setting/getting the default timeout (in seconds) for a fuse server to
>> > +reply to a kernel-issued request in the event where the server did not
>> > +specify a timeout at mount. If the server set a timeout,
>> > +then default_request_timeout will be ignored. The default
>> > +"default_request_timeout" is set to 0. 0 indicates no default timeout.
>> > +The maximum value that can be set is 65535.
>> > +
>> > +``/proc/sys/fs/fuse/max_request_timeout`` is a read/write file for
>> > +setting/getting the maximum timeout (in seconds) for a fuse server to
>> > +reply to a kernel-issued request. A value greater than 0 automatically opts
>> > +the server into a timeout that will be set to at most "max_request_timeout",
>> > +even if the server did not specify a timeout and default_request_timeout is
>> > +set to 0. If max_request_timeout is greater than 0 and the server set a timeout
>> > +greater than max_request_timeout or default_request_timeout is set to a value
>> > +greater than max_request_timeout, the system will use max_request_timeout as the
>> > +timeout. 0 indicates no max request timeout. The maximum value that can be set
>> > +is 65535.
>> > +
>> > +For timeouts, if the server does not respond to the request by the time
>> > +the set timeout elapses, then the connection to the fuse server will be aborted.
>> > +Please note that the timeouts are not 100% precise (eg you may set 60 seconds but
>> > +the timeout may kick in after 70 seconds). The upper margin of error for the
>> > +timeout is roughly FUSE_TIMEOUT_TIMER_FREQ seconds.
>> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
>> > index 1321cc4ed2ab..e5114831798f 100644
>> > --- a/fs/fuse/fuse_i.h
>> > +++ b/fs/fuse/fuse_i.h
>> > @@ -49,6 +49,16 @@ extern const unsigned long fuse_timeout_timer_freq;
>> >
>> > /** Maximum of max_pages received in init_out */
>> > extern unsigned int fuse_max_pages_limit;
>> > +/*
>> > + * Default timeout (in seconds) for the server to reply to a request
>> > + * before the connection is aborted, if no timeout was specified on mount.
>> > + */
>> > +extern unsigned int fuse_default_req_timeout;
>> > +/*
>> > + * Max timeout (in seconds) for the server to reply to a request before
>> > + * the connection is aborted.
>> > + */
>> > +extern unsigned int fuse_max_req_timeout;
>> >
>> > /** List of active connections */
>> > extern struct list_head fuse_conn_list;
>> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
>> > index 79ebeb60015c..4e36d99fae52 100644
>> > --- a/fs/fuse/inode.c
>> > +++ b/fs/fuse/inode.c
>> > @@ -37,6 +37,9 @@ DEFINE_MUTEX(fuse_mutex);
>> > static int set_global_limit(const char *val, const struct kernel_param *kp);
>> >
>> > unsigned int fuse_max_pages_limit = 256;
>> > +/* default is no timeout */
>> > +unsigned int fuse_default_req_timeout = 0;
>> > +unsigned int fuse_max_req_timeout = 0;
>> >
>> > unsigned max_user_bgreq;
>> > module_param_call(max_user_bgreq, set_global_limit, param_get_uint,
>> > @@ -1268,6 +1271,24 @@ static void set_request_timeout(struct fuse_conn *fc, unsigned int timeout)
>> > fuse_timeout_timer_freq);
>> > }
>> >
>> > +static void init_server_timeout(struct fuse_conn *fc, unsigned int timeout)
>> > +{
>> > + if (!timeout && !fuse_max_req_timeout && !fuse_default_req_timeout)
>> > + return;
>> > +
>> > + if (!timeout)
>> > + timeout = fuse_default_req_timeout;
>> > +
>> > + if (fuse_max_req_timeout) {
>> > + if (timeout)
>> > + timeout = min(fuse_max_req_timeout, timeout);
>>
>> Sorry for this late comment (this is v12 already!), but I wonder if it
>> would be worth to use clamp() instead of min(). Limiting this value to
>> the range [FUSE_TIMEOUT_TIMER_FREQ, fuxe_max_req_timeout] would prevent
>> accidentally setting the timeout to a very low value.
>
> I don't think we need to clamp to FUSE_TIMEOUT_TIMER_FREQ. If the user
> sets a timeout value lower than FUSE_TIMEOUT_TIMER_FREQ (15 secs), imo
> that should be supported. For example, if the user sets the timeout to
> 10 seconds, then the connection should be aborted if the request takes
> 13 seconds. I don't see why we need to have the min bound be
> FUSE_TIMEOUT_TIMER_FREQ. imo, the user-specified timeout value and
> FUSE_TIMEOUT_TIMER_FREQ value are orthogonal. But I also don't feel
> strongly about this and am fine with whichever way we go.
Sure, my comment was just a suggestion too. I just thought it would be
easy to accidentally set the timeout to '1' instead of '10', and that this
very low value could cause troubles. On the other hand, I understand that
15 secondss is probably too high for using as a minimum. Anyway, this was
just my 2¢.
Cheers,
--
Luis
>
> Thanks,
> Joanne
>
>>
>> There are also some white-spaces/tabs issues with the other patch, which
>> are worth fixing before merging. Other than that, feel free to add my
>>
>> Reviewed-by: Luis Henriques <luis@igalia.com>
>>
>> Cheers,
>> --
>> Luís
>>
>> > + else
>> > + timeout = fuse_max_req_timeout;
>> > + }
>> > +
>> > + set_request_timeout(fc, timeout);
>> > +}
>> > +
>> > struct fuse_init_args {
>> > struct fuse_args args;
>> > struct fuse_init_in in;
>> > @@ -1286,6 +1307,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
>> > ok = false;
>> > else {
>> > unsigned long ra_pages;
>> > + unsigned int timeout = 0;
>> >
>> > process_init_limits(fc, arg);
>> >
>> > @@ -1404,14 +1426,16 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
>> > if (flags & FUSE_OVER_IO_URING && fuse_uring_enabled())
>> > fc->io_uring = 1;
>> >
>> > - if ((flags & FUSE_REQUEST_TIMEOUT) && arg->request_timeout)
>> > - set_request_timeout(fc, arg->request_timeout);
>> > + if (flags & FUSE_REQUEST_TIMEOUT)
>> > + timeout = arg->request_timeout;
>> > } else {
>> > ra_pages = fc->max_read / PAGE_SIZE;
>> > fc->no_lock = 1;
>> > fc->no_flock = 1;
>> > }
>> >
>> > + init_server_timeout(fc, timeout);
>> > +
>> > fm->sb->s_bdi->ra_pages =
>> > min(fm->sb->s_bdi->ra_pages, ra_pages);
>> > fc->minor = arg->minor;
>> > diff --git a/fs/fuse/sysctl.c b/fs/fuse/sysctl.c
>> > index b272bb333005..3d542ef9d889 100644
>> > --- a/fs/fuse/sysctl.c
>> > +++ b/fs/fuse/sysctl.c
>> > @@ -13,6 +13,12 @@ static struct ctl_table_header *fuse_table_header;
>> > /* Bound by fuse_init_out max_pages, which is a u16 */
>> > static unsigned int sysctl_fuse_max_pages_limit = 65535;
>> >
>> > +/*
>> > + * fuse_init_out request timeouts are u16.
>> > + * This goes up to ~18 hours, which is plenty for a timeout.
>> > + */
>> > +static unsigned int sysctl_fuse_req_timeout_limit = 65535;
>> > +
>> > static struct ctl_table fuse_sysctl_table[] = {
>> > {
>> > .procname = "max_pages_limit",
>> > @@ -23,6 +29,24 @@ static struct ctl_table fuse_sysctl_table[] = {
>> > .extra1 = SYSCTL_ONE,
>> > .extra2 = &sysctl_fuse_max_pages_limit,
>> > },
>> > + {
>> > + .procname = "default_request_timeout",
>> > + .data = &fuse_default_req_timeout,
>> > + .maxlen = sizeof(fuse_default_req_timeout),
>> > + .mode = 0644,
>> > + .proc_handler = proc_douintvec_minmax,
>> > + .extra1 = SYSCTL_ZERO,
>> > + .extra2 = &sysctl_fuse_req_timeout_limit,
>> > + },
>> > + {
>> > + .procname = "max_request_timeout",
>> > + .data = &fuse_max_req_timeout,
>> > + .maxlen = sizeof(fuse_max_req_timeout),
>> > + .mode = 0644,
>> > + .proc_handler = proc_douintvec_minmax,
>> > + .extra1 = SYSCTL_ZERO,
>> > + .extra2 = &sysctl_fuse_req_timeout_limit,
>> > + },
>> > };
>> >
>> > int fuse_sysctl_register(void)
>> > --
>> > 2.43.5
>> >
>>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls
2025-01-23 21:54 ` Luis Henriques
@ 2025-01-23 23:15 ` Joanne Koong
0 siblings, 0 replies; 24+ messages in thread
From: Joanne Koong @ 2025-01-23 23:15 UTC (permalink / raw)
To: Luis Henriques
Cc: miklos, linux-fsdevel, bernd.schubert, jefflexu, laoar.shao,
jlayton, senozhatsky, tfiga, bgeffon, etmartin4313, kernel-team,
Bernd Schubert, Josef Bacik
On Thu, Jan 23, 2025 at 1:53 PM Luis Henriques <luis@igalia.com> wrote:
>
> Hi Joanne,
>
> On Thu, Jan 23 2025, Joanne Koong wrote:
>
> > On Thu, Jan 23, 2025 at 1:21 AM Luis Henriques <luis@igalia.com> wrote:
> >>
> >> Hi Joanne,
> >>
> >> On Wed, Jan 22 2025, Joanne Koong wrote:
> >>
> >> > Introduce two new sysctls, "default_request_timeout" and
> >> > "max_request_timeout". These control how long (in seconds) a server can
> >> > take to reply to a request. If the server does not reply by the timeout,
> >> > then the connection will be aborted. The upper bound on these sysctl
> >> > values is 65535.
> >> >
> >> > "default_request_timeout" sets the default timeout if no timeout is
> >> > specified by the fuse server on mount. 0 (default) indicates no default
> >> > timeout should be enforced. If the server did specify a timeout, then
> >> > default_request_timeout will be ignored.
> >> >
> >> > "max_request_timeout" sets the max amount of time the server may take to
> >> > reply to a request. 0 (default) indicates no maximum timeout. If
> >> > max_request_timeout is set and the fuse server attempts to set a
> >> > timeout greater than max_request_timeout, the system will use
> >> > max_request_timeout as the timeout. Similarly, if default_request_timeout
> >> > is greater than max_request_timeout, the system will use
> >> > max_request_timeout as the timeout. If the server does not request a
> >> > timeout and default_request_timeout is set to 0 but max_request_timeout
> >> > is set, then the timeout will be max_request_timeout.
> >> >
> >> > Please note that these timeouts are not 100% precise. The request may
> >> > take roughly an extra FUSE_TIMEOUT_TIMER_FREQ seconds beyond the set max
> >> > timeout due to how it's internally implemented.
> >> >
> >> > $ sysctl -a | grep fuse.default_request_timeout
> >> > fs.fuse.default_request_timeout = 0
> >> >
> >> > $ echo 65536 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> >> > tee: /proc/sys/fs/fuse/default_request_timeout: Invalid argument
> >> >
> >> > $ echo 65535 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> >> > 65535
> >> >
> >> > $ sysctl -a | grep fuse.default_request_timeout
> >> > fs.fuse.default_request_timeout = 65535
> >> >
> >> > $ echo 0 | sudo tee /proc/sys/fs/fuse/default_request_timeout
> >> > 0
> >> >
> >> > $ sysctl -a | grep fuse.default_request_timeout
> >> > fs.fuse.default_request_timeout = 0
> >> >
> >> > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> >> > Reviewed-by: Bernd Schubert <bschubert@ddn.com>
> >> > Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> >> > Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> >> > ---
> >> > Documentation/admin-guide/sysctl/fs.rst | 25 ++++++++++++++++++++++
> >> > fs/fuse/fuse_i.h | 10 +++++++++
> >> > fs/fuse/inode.c | 28 +++++++++++++++++++++++--
> >> > fs/fuse/sysctl.c | 24 +++++++++++++++++++++
> >> > 4 files changed, 85 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/Documentation/admin-guide/sysctl/fs.rst b/Documentation/admin-guide/sysctl/fs.rst
> >> > index f5ec6c9312e1..35aeb30bed8b 100644
> >> > --- a/Documentation/admin-guide/sysctl/fs.rst
> >> > +++ b/Documentation/admin-guide/sysctl/fs.rst
> >> > @@ -347,3 +347,28 @@ filesystems:
> >> > ``/proc/sys/fs/fuse/max_pages_limit`` is a read/write file for
> >> > setting/getting the maximum number of pages that can be used for servicing
> >> > requests in FUSE.
> >> > +
> >> > +``/proc/sys/fs/fuse/default_request_timeout`` is a read/write file for
> >> > +setting/getting the default timeout (in seconds) for a fuse server to
> >> > +reply to a kernel-issued request in the event where the server did not
> >> > +specify a timeout at mount. If the server set a timeout,
> >> > +then default_request_timeout will be ignored. The default
> >> > +"default_request_timeout" is set to 0. 0 indicates no default timeout.
> >> > +The maximum value that can be set is 65535.
> >> > +
> >> > +``/proc/sys/fs/fuse/max_request_timeout`` is a read/write file for
> >> > +setting/getting the maximum timeout (in seconds) for a fuse server to
> >> > +reply to a kernel-issued request. A value greater than 0 automatically opts
> >> > +the server into a timeout that will be set to at most "max_request_timeout",
> >> > +even if the server did not specify a timeout and default_request_timeout is
> >> > +set to 0. If max_request_timeout is greater than 0 and the server set a timeout
> >> > +greater than max_request_timeout or default_request_timeout is set to a value
> >> > +greater than max_request_timeout, the system will use max_request_timeout as the
> >> > +timeout. 0 indicates no max request timeout. The maximum value that can be set
> >> > +is 65535.
> >> > +
> >> > +For timeouts, if the server does not respond to the request by the time
> >> > +the set timeout elapses, then the connection to the fuse server will be aborted.
> >> > +Please note that the timeouts are not 100% precise (eg you may set 60 seconds but
> >> > +the timeout may kick in after 70 seconds). The upper margin of error for the
> >> > +timeout is roughly FUSE_TIMEOUT_TIMER_FREQ seconds.
> >> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> >> > index 1321cc4ed2ab..e5114831798f 100644
> >> > --- a/fs/fuse/fuse_i.h
> >> > +++ b/fs/fuse/fuse_i.h
> >> > @@ -49,6 +49,16 @@ extern const unsigned long fuse_timeout_timer_freq;
> >> >
> >> > /** Maximum of max_pages received in init_out */
> >> > extern unsigned int fuse_max_pages_limit;
> >> > +/*
> >> > + * Default timeout (in seconds) for the server to reply to a request
> >> > + * before the connection is aborted, if no timeout was specified on mount.
> >> > + */
> >> > +extern unsigned int fuse_default_req_timeout;
> >> > +/*
> >> > + * Max timeout (in seconds) for the server to reply to a request before
> >> > + * the connection is aborted.
> >> > + */
> >> > +extern unsigned int fuse_max_req_timeout;
> >> >
> >> > /** List of active connections */
> >> > extern struct list_head fuse_conn_list;
> >> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> >> > index 79ebeb60015c..4e36d99fae52 100644
> >> > --- a/fs/fuse/inode.c
> >> > +++ b/fs/fuse/inode.c
> >> > @@ -37,6 +37,9 @@ DEFINE_MUTEX(fuse_mutex);
> >> > static int set_global_limit(const char *val, const struct kernel_param *kp);
> >> >
> >> > unsigned int fuse_max_pages_limit = 256;
> >> > +/* default is no timeout */
> >> > +unsigned int fuse_default_req_timeout = 0;
> >> > +unsigned int fuse_max_req_timeout = 0;
> >> >
> >> > unsigned max_user_bgreq;
> >> > module_param_call(max_user_bgreq, set_global_limit, param_get_uint,
> >> > @@ -1268,6 +1271,24 @@ static void set_request_timeout(struct fuse_conn *fc, unsigned int timeout)
> >> > fuse_timeout_timer_freq);
> >> > }
> >> >
> >> > +static void init_server_timeout(struct fuse_conn *fc, unsigned int timeout)
> >> > +{
> >> > + if (!timeout && !fuse_max_req_timeout && !fuse_default_req_timeout)
> >> > + return;
> >> > +
> >> > + if (!timeout)
> >> > + timeout = fuse_default_req_timeout;
> >> > +
> >> > + if (fuse_max_req_timeout) {
> >> > + if (timeout)
> >> > + timeout = min(fuse_max_req_timeout, timeout);
> >>
> >> Sorry for this late comment (this is v12 already!), but I wonder if it
> >> would be worth to use clamp() instead of min(). Limiting this value to
> >> the range [FUSE_TIMEOUT_TIMER_FREQ, fuxe_max_req_timeout] would prevent
> >> accidentally setting the timeout to a very low value.
> >
> > I don't think we need to clamp to FUSE_TIMEOUT_TIMER_FREQ. If the user
> > sets a timeout value lower than FUSE_TIMEOUT_TIMER_FREQ (15 secs), imo
> > that should be supported. For example, if the user sets the timeout to
> > 10 seconds, then the connection should be aborted if the request takes
> > 13 seconds. I don't see why we need to have the min bound be
> > FUSE_TIMEOUT_TIMER_FREQ. imo, the user-specified timeout value and
> > FUSE_TIMEOUT_TIMER_FREQ value are orthogonal. But I also don't feel
> > strongly about this and am fine with whichever way we go.
>
> Sure, my comment was just a suggestion too. I just thought it would be
> easy to accidentally set the timeout to '1' instead of '10', and that this
> very low value could cause troubles. On the other hand, I understand that
> 15 secondss is probably too high for using as a minimum. Anyway, this was
> just my 2¢.
>
Hi Luis,
i see that Miklos already merged the suggestion in. I don't feel
strongly either way so I'll just leave it as is then.
Thanks,
Joanne
> Cheers,
> --
> Luis
>
> >
> >>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 0/2] fuse: add kernel-enforced request timeout option
2025-01-22 21:55 [PATCH v12 0/2] fuse: add kernel-enforced request timeout option Joanne Koong
2025-01-22 21:55 ` [PATCH v12 1/2] fuse: add kernel-enforced timeout option for requests Joanne Koong
2025-01-22 21:55 ` [PATCH v12 2/2] fuse: add default_request_timeout and max_request_timeout sysctls Joanne Koong
@ 2025-01-22 22:53 ` Jeff Layton
2025-02-25 2:04 ` Sergey Senozhatsky
3 siblings, 0 replies; 24+ messages in thread
From: Jeff Layton @ 2025-01-22 22:53 UTC (permalink / raw)
To: Joanne Koong, miklos, linux-fsdevel
Cc: bernd.schubert, jefflexu, laoar.shao, senozhatsky, tfiga, bgeffon,
etmartin4313, kernel-team
On Wed, 2025-01-22 at 13:55 -0800, Joanne Koong wrote:
> There are situations where fuse servers can become unresponsive or
> stuck, for example if the server is in a deadlock. Currently, there's
> no good way to detect if a server is stuck and needs to be killed
> manually.
>
> This patchset adds a timeout option where if the server does not reply to a
> request by the time the timeout elapses, the connection will be aborted.
> This patchset also adds two dynamically configurable fuse sysctls
> "default_request_timeout" and "max_request_timeout" for controlling/enforcing
> timeout behavior system-wide.
>
> Existing systems running fuse servers will not be affected unless they
> explicitly opt into the timeout.
>
> v11:
> https://lore.kernel.org/linux-fsdevel/20241218222630.99920-1-joannelkoong@gmail.com/
> Changes from v11 -> v12:
> * Pass request timeout through init instead of mount option (Miklos)
> * Change sysctl upper bound to max u16 val
> * Rebase on top of for-next, need to incorporate io-uring timeouts
>
> v10:
> https://lore.kernel.org/linux-fsdevel/20241214022827.1773071-1-joannelkoong@gmail.com/
> Changes from v10 -> v11:
> * Refactor check for request expiration (Sergey)
> * Move workqueue cancellation to earlier in function (Jeff)
> * Check fc->num_waiting as a shortcut in workqueue job (Etienne)
>
> v9:
> https://lore.kernel.org/linux-fsdevel/20241114191332.669127-1-joannelkoong@gmail.com/
> Changes from v9 -> v10:
> * Use delayed workqueues instead of timers (Sergey and Jeff)
> * Change granularity to seconds instead of minutes (Sergey and Jeff)
> * Use time_after() api for checking jiffies expiration (Sergey)
> * Change timer check to run every 15 secs instead of every min
> * Update documentation wording to be more clear
>
> v8:
> https://lore.kernel.org/linux-fsdevel/20241011191320.91592-1-joannelkoong@gmail.com/
> Changes from v8 -> v9:
> * Fix comment for u16 fs_parse_result, ULONG_MAX instead of U32_MAX, fix
> spacing (Bernd)
>
> v7:
> https://lore.kernel.org/linux-fsdevel/20241007184258.2837492-1-joannelkoong@gmail.com/
> Changes from v7 -> v8:
> * Use existing lists for checking expirations (Miklos)
>
> v6:
> https://lore.kernel.org/linux-fsdevel/20240830162649.3849586-1-joannelkoong@gmail.com/
> Changes from v6 -> v7:
> - Make timer per-connection instead of per-request (Miklos)
> - Make default granularity of time minutes instead of seconds
> - Removed the reviewed-bys since the interface of this has changed (now
> minutes, instead of seconds)
>
> v5:
> https://lore.kernel.org/linux-fsdevel/20240826203234.4079338-1-joannelkoong@gmail.com/
> Changes from v5 -> v6:
> - Gate sysctl.o behind CONFIG_SYSCTL in makefile (kernel test robot)
> - Reword/clarify last sentence in cover letter (Miklos)
>
> v4:
> https://lore.kernel.org/linux-fsdevel/20240813232241.2369855-1-joannelkoong@gmail.com/
> Changes from v4 -> v5:
> - Change timeout behavior from aborting request to aborting connection (Miklos)
> - Clarify wording for sysctl documentation (Jingbo)
>
> v3:
> https://lore.kernel.org/linux-fsdevel/20240808190110.3188039-1-joannelkoong@gmail.com/
> Changes from v3 -> v4:
> - Fix wording on some comments to make it more clear
> - Use simpler logic for timer (eg remove extra if checks, use mod timer API) (Josef)
> - Sanity-check should be on FR_FINISHING not FR_FINISHED (Jingbo)
> - Fix comment for "processing queue", add req->fpq = NULL safeguard (Bernd)
>
> v2:
> https://lore.kernel.org/linux-fsdevel/20240730002348.3431931-1-joannelkoong@gmail.com/
> Changes from v2 -> v3:
> - Disarm / rearm timer in dev_do_read to handle race conditions (Bernrd)
> - Disarm timer in error handling for fatal interrupt (Yafang)
> - Clean up do_fuse_request_end (Jingbo)
> - Add timer for notify retrieve requests
> - Fix kernel test robot errors for #define no-op functions
>
> v1:
> https://lore.kernel.org/linux-fsdevel/20240717213458.1613347-1-joannelkoong@gmail.com/
> Changes from v1 -> v2:
> - Add timeout for background requests
> - Handle resend race condition
> - Add sysctls
>
>
> Joanne Koong (2):
> fuse: add kernel-enforced timeout option for requests
> fuse: add default_request_timeout and max_request_timeout sysctls
>
> Documentation/admin-guide/sysctl/fs.rst | 25 ++++++
> fs/fuse/dev.c | 101 ++++++++++++++++++++++++
> fs/fuse/dev_uring.c | 27 +++++++
> fs/fuse/dev_uring_i.h | 6 ++
> fs/fuse/fuse_dev_i.h | 3 +
> fs/fuse/fuse_i.h | 27 +++++++
> fs/fuse/inode.c | 41 +++++++++-
> fs/fuse/sysctl.c | 24 ++++++
> include/uapi/linux/fuse.h | 10 ++-
> 9 files changed, 261 insertions(+), 3 deletions(-)
>
Nice work, Joanne. You can add:
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 0/2] fuse: add kernel-enforced request timeout option
2025-01-22 21:55 [PATCH v12 0/2] fuse: add kernel-enforced request timeout option Joanne Koong
` (2 preceding siblings ...)
2025-01-22 22:53 ` [PATCH v12 0/2] fuse: add kernel-enforced request timeout option Jeff Layton
@ 2025-02-25 2:04 ` Sergey Senozhatsky
2025-02-25 17:35 ` Joanne Koong
3 siblings, 1 reply; 24+ messages in thread
From: Sergey Senozhatsky @ 2025-02-25 2:04 UTC (permalink / raw)
To: Joanne Koong
Cc: miklos, linux-fsdevel, bernd.schubert, jefflexu, laoar.shao,
jlayton, senozhatsky, tfiga, bgeffon, etmartin4313, kernel-team
On (25/01/22 13:55), Joanne Koong wrote:
> There are situations where fuse servers can become unresponsive or
> stuck, for example if the server is in a deadlock. Currently, there's
> no good way to detect if a server is stuck and needs to be killed
> manually.
>
> This patchset adds a timeout option where if the server does not reply to a
> request by the time the timeout elapses, the connection will be aborted.
> This patchset also adds two dynamically configurable fuse sysctls
> "default_request_timeout" and "max_request_timeout" for controlling/enforcing
> timeout behavior system-wide.
>
> Existing systems running fuse servers will not be affected unless they
> explicitly opt into the timeout.
Sorry folks, has this series stuck? I'm not sure I'm seeing it
in linux-next.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v12 0/2] fuse: add kernel-enforced request timeout option
2025-02-25 2:04 ` Sergey Senozhatsky
@ 2025-02-25 17:35 ` Joanne Koong
2025-03-03 11:38 ` Miklos Szeredi
0 siblings, 1 reply; 24+ messages in thread
From: Joanne Koong @ 2025-02-25 17:35 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: miklos, linux-fsdevel, bernd.schubert, jefflexu, laoar.shao,
jlayton, tfiga, bgeffon, etmartin4313, kernel-team
On Mon, Feb 24, 2025 at 6:04 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (25/01/22 13:55), Joanne Koong wrote:
> > There are situations where fuse servers can become unresponsive or
> > stuck, for example if the server is in a deadlock. Currently, there's
> > no good way to detect if a server is stuck and needs to be killed
> > manually.
> >
> > This patchset adds a timeout option where if the server does not reply to a
> > request by the time the timeout elapses, the connection will be aborted.
> > This patchset also adds two dynamically configurable fuse sysctls
> > "default_request_timeout" and "max_request_timeout" for controlling/enforcing
> > timeout behavior system-wide.
> >
> > Existing systems running fuse servers will not be affected unless they
> > explicitly opt into the timeout.
>
> Sorry folks, has this series stuck? I'm not sure I'm seeing it
> in linux-next.
I'm not seeing it in linux-next either.
These patches were merged last month into Miklos' for-next tree as
9afd7336f3ac (miklos/for-next) fuse: add default_request_timeout and
max_request_timeout sysctls
ac5eab212a58 fuse: add kernel-enforced timeout option for requests
but I no longer see these commits in his tree anymore.
Miklos, why were these patches taken out?
Thanks,
Joanne
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 0/2] fuse: add kernel-enforced request timeout option
2025-02-25 17:35 ` Joanne Koong
@ 2025-03-03 11:38 ` Miklos Szeredi
2025-03-03 22:43 ` Joanne Koong
0 siblings, 1 reply; 24+ messages in thread
From: Miklos Szeredi @ 2025-03-03 11:38 UTC (permalink / raw)
To: Joanne Koong
Cc: Sergey Senozhatsky, linux-fsdevel, bernd.schubert, jefflexu,
laoar.shao, jlayton, tfiga, bgeffon, etmartin4313, kernel-team
On Tue, 25 Feb 2025 at 18:35, Joanne Koong <joannelkoong@gmail.com> wrote:
> but I no longer see these commits in his tree anymore.
>
> Miklos, why were these patches taken out?
Sorry, forgot to re-apply with the io-uring interaction fixed.
Done now.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v12 0/2] fuse: add kernel-enforced request timeout option
2025-03-03 11:38 ` Miklos Szeredi
@ 2025-03-03 22:43 ` Joanne Koong
2025-03-04 9:37 ` Miklos Szeredi
0 siblings, 1 reply; 24+ messages in thread
From: Joanne Koong @ 2025-03-03 22:43 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Sergey Senozhatsky, linux-fsdevel, bernd.schubert, jefflexu,
laoar.shao, jlayton, tfiga, bgeffon, etmartin4313, kernel-team
On Mon, Mar 3, 2025 at 3:39 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, 25 Feb 2025 at 18:35, Joanne Koong <joannelkoong@gmail.com> wrote:
>
> > but I no longer see these commits in his tree anymore.
> >
> > Miklos, why were these patches taken out?
>
> Sorry, forgot to re-apply with the io-uring interaction fixed.
>
> Done now.
Hi Miklos,
Will the 2nd patch ("fuse: add default_request_timeout and
max_request_timeout sysctls") also be re-applied? I'm only seeing the
1st patch (" fuse: add kernel-enforced timeout option for requests")
in the for-next tree right now.
Thanks,
Joanne
>
> Thanks,
> Miklos
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v12 0/2] fuse: add kernel-enforced request timeout option
2025-03-03 22:43 ` Joanne Koong
@ 2025-03-04 9:37 ` Miklos Szeredi
0 siblings, 0 replies; 24+ messages in thread
From: Miklos Szeredi @ 2025-03-04 9:37 UTC (permalink / raw)
To: Joanne Koong
Cc: Sergey Senozhatsky, linux-fsdevel, bernd.schubert, jefflexu,
laoar.shao, jlayton, tfiga, bgeffon, etmartin4313, kernel-team
On Mon, 3 Mar 2025 at 23:43, Joanne Koong <joannelkoong@gmail.com> wrote:
> Will the 2nd patch ("fuse: add default_request_timeout and
> max_request_timeout sysctls") also be re-applied? I'm only seeing the
> 1st patch (" fuse: add kernel-enforced timeout option for requests")
> in the for-next tree right now.
I wasn't paying proper attention, sorry.
Re-applied the second (modified) patch.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 24+ messages in thread