2023-11-20 04:17:07

by Jia Zhu

[permalink] [raw]
Subject: [PATCH V6 RESEND 0/5] cachefiles: Introduce failover mechanism for on-demand mode

Changes since v5:
In cachefiles_daemon_poll(), replace xa_for_each_marked with xas_for_each_marked.

[Background]
============
In the on-demand read mode, if user daemon unexpectedly closes an on-demand fd
(for example, due to daemon crashing), subsequent read operations and inflight
requests relying on these fd will result in a return value of -EIO, indicating
an I/O error.
While this situation might be tolerable for individual personal users, it
becomes a significant concern when it occurs in a real public cloud service
production environment (like us). Such I/O errors will be propagated to cloud
service users, potentially impacting the execution of their jobs and
compromising the overall stability of the cloud service. Besides, we have no
way to recover this.

[Design]
========
The main concept behind daemon failover is to reopen the inflight request-related
objects so that the newly started daemon can process the requests as usual.
To achieve this, certain requirements need to be met:
1. Storing inflight requests during a daemon crash:
It is necessary to have a mechanism in place to store the inflight
requests while the daemon is offline or during a crash. This ensures
that the requests are not lost and can be processed once the daemon
is up and running again.
2. Holding the handle of /dev/cachefiles:
The handle of /dev/cachefiles should be retained, either by the container
snapshotter or systemd, to facilitate the failover process. This allows
the newly started daemon to access the necessary resources and continue
processing the requests seamlessly.

It's important to note that if the user chooses not to keep the /dev/cachefiles
fd, the failover feature will not be enabled. In this case, inflight requests
will return error, which will be passed on to the container, maintaining the same
behavior as the current setup.

By implementing these mechanisms, the failover system ensures that inflight requests
are not lost during a daemon crash and that the newly started daemon can resume
its operations smoothly, providing a more robust and reliable service for users.

[Flow Path]
===========
This patchset introduce three states for ondemand object:
CLOSE: This state represents an object that has either just been allocated or
closed by the user daemon.
OPEN: This state indicates that the object is open and ready for processing.
It signifies that the related OPEN request has been successfully handled
and the object is available for read operations or other interactions.
REOPENING: This state is assigned to an object that has been previously closed
but is now being driven to reopen due to a read request. The REOPENING state
indicates that the object is in the process of being reopened, preparing
for subsequent read operations.

1. The daemon utilizes Unix Domain Sockets (UDS) to send and receive fd in order to
maintain and pass the reference to "/dev/cachefiles".

2. In the event of a user daemon crash, the daemon is restarted and the reference
to the file descriptor for "/dev/cachefiles" is recovered.

3. The user daemon writes "restore" to the device, triggering the following actions:
3.1. The object's state is reset from CLOSE to REOPENING, indicating that it
is in the process of reopening.
3.2. A work unit is initialized, which reinitializes the object and adds it to
the work queue. This allows the daemon to handle the open request,
transitioning from kernel space to user space.

4. As a result of these recovery mechanisms, the user of the upper filesystem
remains unaware of the daemon crash. The inflight I/O operations are restored
and correctly handled, ensuring that the system operates seamlessly without
any noticeable disruptions.

By implementing these steps, the system achieves fault tolerance by recovering and
restoring the necessary references and states, ensuring the smooth functioning of
the user daemon and providing a seamless experience to the users of the upper filesystem.

[GitWeb]
========
https://github.com/userzj/linux/tree/fscache-failover-v6

RFC: https://lore.kernel.org/all/[email protected]/
V1: https://lore.kernel.org/all/[email protected]/
V2: https://lore.kernel.org/all/[email protected]/
V3: https://lore.kernel.org/all/[email protected]/
V4: https://lore.kernel.org/all/[email protected]/
V5: https://lore.kernel.org/all/[email protected]/

[Test]
======
There are testcases for above mentioned scenario.
A user process read the file by fscache on-demand reading.
At the same time, we kill the daemon constantly.
The expected result is that the file read by user is consistent with
original, and the user doesn't notice that daemon has ever been killed.

https://github.com/userzj/demand-read-cachefilesd/commits/failover-test

In addition, this patchset has also been merged in our downstream kernel
for almost one year as out-of-tree patches for real production use.
Therefore, we hope it could be landed upstream too.

Jia Zhu (5):
cachefiles: introduce object ondemand state
cachefiles: extract ondemand info field from cachefiles_object
cachefiles: resend an open request if the read request's object is
closed
cachefiles: narrow the scope of triggering EPOLLIN events in ondemand
mode
cachefiles: add restore command to recover inflight ondemand read
requests

fs/cachefiles/daemon.c | 15 +++-
fs/cachefiles/interface.c | 7 +-
fs/cachefiles/internal.h | 59 +++++++++++++-
fs/cachefiles/ondemand.c | 166 ++++++++++++++++++++++++++++----------
4 files changed, 201 insertions(+), 46 deletions(-)

--
2.20.1


2023-11-20 04:17:17

by Jia Zhu

[permalink] [raw]
Subject: [PATCH V6 RESEND 5/5] cachefiles: add restore command to recover inflight ondemand read requests

Previously, in ondemand read scenario, if the anonymous fd was closed by
user daemon, inflight and subsequent read requests would return EIO.
As long as the device connection is not released, user daemon can hold
and restore inflight requests by setting the request flag to
CACHEFILES_REQ_NEW.

Suggested-by: Gao Xiang <[email protected]>
Signed-off-by: Jia Zhu <[email protected]>
Signed-off-by: Xin Yin <[email protected]>
Reviewed-by: Jingbo Xu <[email protected]>
---
fs/cachefiles/daemon.c | 1 +
fs/cachefiles/internal.h | 3 +++
fs/cachefiles/ondemand.c | 23 +++++++++++++++++++++++
3 files changed, 27 insertions(+)

diff --git a/fs/cachefiles/daemon.c b/fs/cachefiles/daemon.c
index 70caa1946207..3f24905f4066 100644
--- a/fs/cachefiles/daemon.c
+++ b/fs/cachefiles/daemon.c
@@ -77,6 +77,7 @@ static const struct cachefiles_daemon_cmd cachefiles_daemon_cmds[] = {
{ "tag", cachefiles_daemon_tag },
#ifdef CONFIG_CACHEFILES_ONDEMAND
{ "copen", cachefiles_ondemand_copen },
+ { "restore", cachefiles_ondemand_restore },
#endif
{ "", NULL }
};
diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h
index 26e5f8f123ef..4a87c9d714a9 100644
--- a/fs/cachefiles/internal.h
+++ b/fs/cachefiles/internal.h
@@ -303,6 +303,9 @@ extern ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
extern int cachefiles_ondemand_copen(struct cachefiles_cache *cache,
char *args);

+extern int cachefiles_ondemand_restore(struct cachefiles_cache *cache,
+ char *args);
+
extern int cachefiles_ondemand_init_object(struct cachefiles_object *object);
extern void cachefiles_ondemand_clean_object(struct cachefiles_object *object);

diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c
index 8e130de952f7..b8fbbb1961bb 100644
--- a/fs/cachefiles/ondemand.c
+++ b/fs/cachefiles/ondemand.c
@@ -182,6 +182,29 @@ int cachefiles_ondemand_copen(struct cachefiles_cache *cache, char *args)
return ret;
}

+int cachefiles_ondemand_restore(struct cachefiles_cache *cache, char *args)
+{
+ struct cachefiles_req *req;
+
+ XA_STATE(xas, &cache->reqs, 0);
+
+ if (!test_bit(CACHEFILES_ONDEMAND_MODE, &cache->flags))
+ return -EOPNOTSUPP;
+
+ /*
+ * Reset the requests to CACHEFILES_REQ_NEW state, so that the
+ * requests have been processed halfway before the crash of the
+ * user daemon could be reprocessed after the recovery.
+ */
+ xas_lock(&xas);
+ xas_for_each(&xas, req, ULONG_MAX)
+ xas_set_mark(&xas, CACHEFILES_REQ_NEW);
+ xas_unlock(&xas);
+
+ wake_up_all(&cache->daemon_pollwq);
+ return 0;
+}
+
static int cachefiles_ondemand_get_fd(struct cachefiles_req *req)
{
struct cachefiles_object *object;
--
2.20.1

2023-11-20 04:17:17

by Jia Zhu

[permalink] [raw]
Subject: [PATCH V6 RESEND 3/5] cachefiles: resend an open request if the read request's object is closed

When an anonymous fd is closed by user daemon, if there is a new read
request for this file comes up, the anonymous fd should be re-opened
to handle that read request rather than fail it directly.

1. Introduce reopening state for objects that are closed but have
inflight/subsequent read requests.
2. No longer flush READ requests but only CLOSE requests when anonymous
fd is closed.
3. Enqueue the reopen work to workqueue, thus user daemon could get rid
of daemon_read context and handle that request smoothly. Otherwise,
the user daemon will send a reopen request and wait for itself to
process the request.

Signed-off-by: Jia Zhu <[email protected]>
Reviewed-by: Jingbo Xu <[email protected]>
---
fs/cachefiles/internal.h | 3 ++
fs/cachefiles/ondemand.c | 98 ++++++++++++++++++++++++++++------------
2 files changed, 72 insertions(+), 29 deletions(-)

diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h
index b0fe76964bc0..b9a90f1a0c01 100644
--- a/fs/cachefiles/internal.h
+++ b/fs/cachefiles/internal.h
@@ -47,9 +47,11 @@ struct cachefiles_volume {
enum cachefiles_object_state {
CACHEFILES_ONDEMAND_OBJSTATE_CLOSE, /* Anonymous fd closed by daemon or initial state */
CACHEFILES_ONDEMAND_OBJSTATE_OPEN, /* Anonymous fd associated with object is available */
+ CACHEFILES_ONDEMAND_OBJSTATE_REOPENING, /* Object that was closed and is being reopened. */
};

struct cachefiles_ondemand_info {
+ struct work_struct ondemand_work;
int ondemand_id;
enum cachefiles_object_state state;
struct cachefiles_object *object;
@@ -326,6 +328,7 @@ cachefiles_ondemand_set_object_##_state(struct cachefiles_object *object) \

CACHEFILES_OBJECT_STATE_FUNCS(open, OPEN);
CACHEFILES_OBJECT_STATE_FUNCS(close, CLOSE);
+CACHEFILES_OBJECT_STATE_FUNCS(reopening, REOPENING);
#else
static inline ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
char __user *_buffer, size_t buflen)
diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c
index deb7e3007aa1..8e130de952f7 100644
--- a/fs/cachefiles/ondemand.c
+++ b/fs/cachefiles/ondemand.c
@@ -18,14 +18,10 @@ static int cachefiles_ondemand_fd_release(struct inode *inode,
info->ondemand_id = CACHEFILES_ONDEMAND_ID_CLOSED;
cachefiles_ondemand_set_object_close(object);

- /*
- * Flush all pending READ requests since their completion depends on
- * anon_fd.
- */
- xas_for_each(&xas, req, ULONG_MAX) {
+ /* Only flush CACHEFILES_REQ_NEW marked req to avoid race with daemon_read */
+ xas_for_each_marked(&xas, req, ULONG_MAX, CACHEFILES_REQ_NEW) {
if (req->msg.object_id == object_id &&
- req->msg.opcode == CACHEFILES_OP_READ) {
- req->error = -EIO;
+ req->msg.opcode == CACHEFILES_OP_CLOSE) {
complete(&req->done);
xas_store(&xas, NULL);
}
@@ -179,6 +175,7 @@ int cachefiles_ondemand_copen(struct cachefiles_cache *cache, char *args)
trace_cachefiles_ondemand_copen(req->object, id, size);

cachefiles_ondemand_set_object_open(req->object);
+ wake_up_all(&cache->daemon_pollwq);

out:
complete(&req->done);
@@ -222,7 +219,6 @@ static int cachefiles_ondemand_get_fd(struct cachefiles_req *req)

load = (void *)req->msg.data;
load->fd = fd;
- req->msg.object_id = object_id;
object->ondemand->ondemand_id = object_id;

cachefiles_get_unbind_pincount(cache);
@@ -238,6 +234,43 @@ static int cachefiles_ondemand_get_fd(struct cachefiles_req *req)
return ret;
}

+static void ondemand_object_worker(struct work_struct *work)
+{
+ struct cachefiles_ondemand_info *info =
+ container_of(work, struct cachefiles_ondemand_info, ondemand_work);
+
+ cachefiles_ondemand_init_object(info->object);
+}
+
+/*
+ * If there are any inflight or subsequent READ requests on the
+ * closed object, reopen it.
+ * Skip read requests whose related object is reopening.
+ */
+static struct cachefiles_req *cachefiles_ondemand_select_req(struct xa_state *xas,
+ unsigned long xa_max)
+{
+ struct cachefiles_req *req;
+ struct cachefiles_object *object;
+ struct cachefiles_ondemand_info *info;
+
+ xas_for_each_marked(xas, req, xa_max, CACHEFILES_REQ_NEW) {
+ if (req->msg.opcode != CACHEFILES_OP_READ)
+ return req;
+ object = req->object;
+ info = object->ondemand;
+ if (cachefiles_ondemand_object_is_close(object)) {
+ cachefiles_ondemand_set_object_reopening(object);
+ queue_work(fscache_wq, &info->ondemand_work);
+ continue;
+ }
+ if (cachefiles_ondemand_object_is_reopening(object))
+ continue;
+ return req;
+ }
+ return NULL;
+}
+
ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
char __user *_buffer, size_t buflen)
{
@@ -248,16 +281,16 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
int ret = 0;
XA_STATE(xas, &cache->reqs, cache->req_id_next);

+ xa_lock(&cache->reqs);
/*
* Cyclically search for a request that has not ever been processed,
* to prevent requests from being processed repeatedly, and make
* request distribution fair.
*/
- xa_lock(&cache->reqs);
- req = xas_find_marked(&xas, UINT_MAX, CACHEFILES_REQ_NEW);
+ req = cachefiles_ondemand_select_req(&xas, ULONG_MAX);
if (!req && cache->req_id_next > 0) {
xas_set(&xas, 0);
- req = xas_find_marked(&xas, cache->req_id_next - 1, CACHEFILES_REQ_NEW);
+ req = cachefiles_ondemand_select_req(&xas, cache->req_id_next - 1);
}
if (!req) {
xa_unlock(&cache->reqs);
@@ -277,14 +310,18 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
xa_unlock(&cache->reqs);

id = xas.xa_index;
- msg->msg_id = id;

if (msg->opcode == CACHEFILES_OP_OPEN) {
ret = cachefiles_ondemand_get_fd(req);
- if (ret)
+ if (ret) {
+ cachefiles_ondemand_set_object_close(req->object);
goto error;
+ }
}

+ msg->msg_id = id;
+ msg->object_id = req->object->ondemand->ondemand_id;
+
if (copy_to_user(_buffer, msg, n) != 0) {
ret = -EFAULT;
goto err_put_fd;
@@ -317,19 +354,23 @@ static int cachefiles_ondemand_send_req(struct cachefiles_object *object,
void *private)
{
struct cachefiles_cache *cache = object->volume->cache;
- struct cachefiles_req *req;
+ struct cachefiles_req *req = NULL;
XA_STATE(xas, &cache->reqs, 0);
int ret;

if (!test_bit(CACHEFILES_ONDEMAND_MODE, &cache->flags))
return 0;

- if (test_bit(CACHEFILES_DEAD, &cache->flags))
- return -EIO;
+ if (test_bit(CACHEFILES_DEAD, &cache->flags)) {
+ ret = -EIO;
+ goto out;
+ }

req = kzalloc(sizeof(*req) + data_len, GFP_KERNEL);
- if (!req)
- return -ENOMEM;
+ if (!req) {
+ ret = -ENOMEM;
+ goto out;
+ }

req->object = object;
init_completion(&req->done);
@@ -367,7 +408,7 @@ static int cachefiles_ondemand_send_req(struct cachefiles_object *object,
/* coupled with the barrier in cachefiles_flush_reqs() */
smp_mb();

- if (opcode != CACHEFILES_OP_OPEN &&
+ if (opcode == CACHEFILES_OP_CLOSE &&
!cachefiles_ondemand_object_is_open(object)) {
WARN_ON_ONCE(object->ondemand->ondemand_id == 0);
xas_unlock(&xas);
@@ -392,7 +433,15 @@ static int cachefiles_ondemand_send_req(struct cachefiles_object *object,
wake_up_all(&cache->daemon_pollwq);
wait_for_completion(&req->done);
ret = req->error;
+ kfree(req);
+ return ret;
out:
+ /* Reset the object to close state in error handling path.
+ * If error occurs after creating the anonymous fd,
+ * cachefiles_ondemand_fd_release() will set object to close.
+ */
+ if (opcode == CACHEFILES_OP_OPEN)
+ cachefiles_ondemand_set_object_close(object);
kfree(req);
return ret;
}
@@ -439,7 +488,6 @@ static int cachefiles_ondemand_init_close_req(struct cachefiles_req *req,
if (!cachefiles_ondemand_object_is_open(object))
return -ENOENT;

- req->msg.object_id = object->ondemand->ondemand_id;
trace_cachefiles_ondemand_close(object, &req->msg);
return 0;
}
@@ -455,16 +503,7 @@ static int cachefiles_ondemand_init_read_req(struct cachefiles_req *req,
struct cachefiles_object *object = req->object;
struct cachefiles_read *load = (void *)req->msg.data;
struct cachefiles_read_ctx *read_ctx = private;
- int object_id = object->ondemand->ondemand_id;
-
- /* Stop enqueuing requests when daemon has closed anon_fd. */
- if (!cachefiles_ondemand_object_is_open(object)) {
- WARN_ON_ONCE(object_id == 0);
- pr_info_once("READ: anonymous fd closed prematurely.\n");
- return -EIO;
- }

- req->msg.object_id = object_id;
load->off = read_ctx->off;
load->len = read_ctx->len;
trace_cachefiles_ondemand_read(object, &req->msg, load);
@@ -513,6 +552,7 @@ int cachefiles_ondemand_init_obj_info(struct cachefiles_object *object,
return -ENOMEM;

object->ondemand->object = object;
+ INIT_WORK(&object->ondemand->ondemand_work, ondemand_object_worker);
return 0;
}

--
2.20.1

2023-11-25 15:07:50

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH V6 RESEND 0/5] cachefiles: Introduce failover mechanism for on-demand mode

On Mon, 20 Nov 2023 12:14:17 +0800, Jia Zhu wrote:
> Changes since v5:
> In cachefiles_daemon_poll(), replace xa_for_each_marked with xas_for_each_marked.
>
> [Background]
> ============
> In the on-demand read mode, if user daemon unexpectedly closes an on-demand fd
> (for example, due to daemon crashing), subsequent read operations and inflight
> requests relying on these fd will result in a return value of -EIO, indicating
> an I/O error.
> While this situation might be tolerable for individual personal users, it
> becomes a significant concern when it occurs in a real public cloud service
> production environment (like us). Such I/O errors will be propagated to cloud
> service users, potentially impacting the execution of their jobs and
> compromising the overall stability of the cloud service. Besides, we have no
> way to recover this.
>
> [...]

Applied to the vfs.fscache branch of the vfs/vfs.git tree.
Patches in the vfs.fscache branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fscache

[1/5] cachefiles: introduce object ondemand state
https://git.kernel.org/vfs/vfs/c/357a18d03314
[2/5] cachefiles: extract ondemand info field from cachefiles_object
https://git.kernel.org/vfs/vfs/c/3c5ecfe16e76
[3/5] cachefiles: resend an open request if the read request's object is closed
https://git.kernel.org/vfs/vfs/c/0a7e54c1959c
[4/5] cachefiles: narrow the scope of triggering EPOLLIN events in ondemand mode
https://git.kernel.org/vfs/vfs/c/b817e22b2e91
[5/5] cachefiles: add restore command to recover inflight ondemand read requests
https://git.kernel.org/vfs/vfs/c/e73fa11a356c