[Devel] [PATCH VZ10 v2] fs/fuse kio: track pending kRPC connect via state machine, not a pointer
Konstantin Khorenko
khorenko at virtuozzo.com
Fri Sep 4 12:59:46 MSK 2026
Hi Lui,
the patch itself looks correct to me for a kernel that boots with it:
CONNECT is entered only by pcs_krpc_connect() with a req queued, and
pcs_rpc_send() guarantees msg->done() for every queued msg (ABORT/DESTROY
fail it at once, WORK completes it at once, otherwise it sits in
state_queue until WORK or a fatal rpc_abort()), so "CONNECT means a req is
in flight" holds from the first connect on.
The problem is the livepatch case this rework is made for. On the running
kernel (without 7ae23fa1c145e and this patch) the invariant does not hold:
- old krpc_connect_done() leaves state == CONNECT when the connect
fails, it only wakes poll with EPOLLHUP;
- old pcs_krpc_poll() returns 0 for CONNECT, so userspace sits in poll
until its own connect timeout, then closes the fd;
- only then old pcs_krpc_abort() moves CONNECT -> UNCONN.
So at the moment the livepatch is applied, every krpc whose CS is currently
unreachable is periodically in "CONNECT, no req in flight", and stays there
for a whole userspace connect timeout on each retry. With the new code
such a krpc never recovers:
- new pcs_krpc_poll(): state CONNECT, gen matches -> 0, userspace waits
for its timeout as before and closes the fd;
- new pcs_krpc_release() -> pcs_krpc_abort(): state CONNECT -> gen++
only, state stays CONNECT;
- there is no req whose completion would settle the state to UNCONN;
- every later pcs_krpc_connect() for this CS returns -EPERM, until the
krpc is destroyed (i.e. until the mount is recreated).
Note this can not be fixed in the patched code by looking at the krpc
alone: the new code has no way to tell "req in flight" from "stale CONNECT
inherited from the old kernel", that is exactly the information
connect_req used to carry.
i'm not sure if we need take care of this, may be we can consider the
probability of that very low.
But in case we deside to handle it, options I see:
1. Livepatch post-patch callback: walk fuse_conn_list -> kio ctx ->
krpcset, and for every krpc in CONNECT check whether its rpc has a msg
with done == krpc_connect_done in input_queue / state_queue; if none,
reset state to UNCONN. Correct but needs care with locking and
reaching into fuse internals from the patch.
2. Livepatch-only self-healing: in the -EPERM branch of
pcs_krpc_connect() (and/or in pcs_krpc_abort() for CONNECT) do the same
queue check under ep->mutex and treat "CONNECT without a connect msg
queued" as UNCONN. Cheap, only runs on the rare refusal path. There
is a small window while rpc_queue_work() has the msg moved to its local
list; a false "no req" there just leads to two reqs in flight, which
the new code settles on its own (stale one -> UNCONN, live one dropped,
userspace retries).
3. Operational: apply the livepatch while all CSes are reachable, or
restart vstorage-mount after applying it, so all krpcs are recreated by
the patched code.
May be just to notify support about that so they can quicly restart
vstorage-mount in case of a problem.
Nothing needs to change in the mainline vz kernel version of the patch.
--
Best regards,
Konstantin Khorenko,
Virtuozzo Linux Kernel Team
On 9/4/26 03:57, Liu Kui wrote:
> Rework the previous fix ("fs/fuse kio: fix kRPC connect issues") to not
> require the new struct pcs_krpc member "connect_req", so the fix can be
> shipped as a livepatch: struct pcs_krpc objects are long-lived, and a
> patched kernel would dereference the new member on objects allocated
> before the livepatch was loaded, reading unallocated slab space.
>
> Both things connect_req was tracking are already derivable from the
> existing state machine once PCS_KRPC_STATE_CONNECT is made to mean
> exactly "a connect req is in flight":
>
> - krpc_connect_done() settles a failed connect back to UNCONN instead
> of leaving the state in CONNECT forever;
>
> - pcs_krpc_abort() no longer resets CONNECT to UNCONN: the req is
> still in flight, and only its completion settles the state. It
> advances gen instead, disowning the pending req: when the req
> completes, krpc_connect_done() settles the state to UNCONN without
> committing the dead session, even if the late connect succeeded;
>
> - pcs_krpc_connect() proceeds only from UNCONN or ABORTED, refusing
> new connects (-EPERM) while a req is in flight - at most one connect
> req exists at a time, same as with the connect_req check;
>
> - pcs_krpc_poll() reports EPOLLERR on UNCONN: poll bails out earlier
> unless ctx->gen == krpc->gen, and the current session can only be in
> UNCONN if its connect failed, which is what the (CONNECT && !connect_req)
> test used to detect.
>
> Within CONNECT the pending req carries the current gen unless the
> session was aborted, so a gen mismatch in krpc_connect_done() reliably
> identifies a disowned req.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-135626
>
> Signed-off-by: Liu Kui <kui.liu at virtuozzo.com>
> ---
> fs/fuse/kio/pcs/pcs_krpc.c | 63 ++++++++++++++++++++++++++++----------
> fs/fuse/kio/pcs/pcs_krpc.h | 2 --
> 2 files changed, 46 insertions(+), 19 deletions(-)
>
> diff --git a/fs/fuse/kio/pcs/pcs_krpc.c b/fs/fuse/kio/pcs/pcs_krpc.c
> index 0930fb4adf12..9d534b037cd1 100644
> --- a/fs/fuse/kio/pcs/pcs_krpc.c
> +++ b/fs/fuse/kio/pcs/pcs_krpc.c
> @@ -787,8 +787,15 @@ static int pcs_krpc_abort(struct pcs_krpc *krpc)
> spin_lock(&krpc->lock);
>
> if (krpc->state != PCS_KRPC_STATE_CONNECTED) {
> + /*
> + * A pending connect req stays in flight and the state stays
> + * CONNECT, refusing new connects until the req completes.
> + * Advancing gen disowns the req, so that a late completion
> + * settles the state to UNCONN in krpc_connect_done() instead
> + * of committing this dead session on success.
> + */
> if (krpc->state == PCS_KRPC_STATE_CONNECT)
> - krpc->state = PCS_KRPC_STATE_UNCONN;
> + krpc->gen++;
> spin_unlock(&krpc->lock);
> return 0;
> }
> @@ -949,15 +956,15 @@ static __poll_t pcs_krpc_poll(struct file *file, poll_table *wait)
>
> poll_wait(file, &krpc->poll_wait, wait);
>
> - if (unlikely(ctx->gen != krpc->gen)) {
> - pollflags |= EPOLLERR;
> - return pollflags;
> - }
> -
> spin_lock(&krpc->lock);
>
> - if (krpc->state == PCS_KRPC_STATE_ABORTED ||
> - (krpc->state == PCS_KRPC_STATE_CONNECT && !krpc->connect_req)) {
> + /*
> + * when ctx->gen == krpc->gen, UNCONN here can only mean its
> + * connect attempt has failed (see krpc_connect_done()).
> + */
> + if (ctx->gen != krpc->gen ||
> + krpc->state == PCS_KRPC_STATE_ABORTED ||
> + krpc->state == PCS_KRPC_STATE_UNCONN) {
> pollflags |= EPOLLERR;
> } else if (krpc->state == PCS_KRPC_STATE_CONNECTED) {
> pollflags |= EPOLLOUT;
> @@ -1047,7 +1054,6 @@ int pcs_krpc_create(struct pcs_krpc_set *krpcs, PCS_NODE_ID_T *id,
> krpc->gen = 0;
> krpc->state = PCS_KRPC_STATE_UNCONN;
> krpc->cs = NULL;
> - krpc->connect_req = NULL;
>
> krpc->rpc = pcs_rpc_clnt_create(&cc_from_krpcset(krpcs)->eng, id, addr, cs_flags);
> if (!krpc->rpc) {
> @@ -1099,10 +1105,20 @@ static void krpc_connect_done(struct pcs_msg *msg)
> }
>
> spin_lock(&krpc->lock);
> - if (krpc->connect_req == req)
> - krpc->connect_req = NULL;
> - /* from a stale session, do nothing */
> - if (req->gen != krpc->gen || krpc->state != PCS_KRPC_STATE_CONNECT) {
> + /* the session was aborted or destroyed, nothing to settle */
> + if (krpc->state != PCS_KRPC_STATE_CONNECT) {
> + spin_unlock(&krpc->lock);
> + goto out;
> + }
> +
> + if (req->gen != krpc->gen) {
> + /*
> + * The session that started this connect was aborted while the
> + * req was in flight (pcs_krpc_abort() advanced gen): settle
> + * the state so a new connect is allowed, but never commit the
> + * dead session, even on success.
> + */
> + krpc->state = PCS_KRPC_STATE_UNCONN;
> spin_unlock(&krpc->lock);
> goto out;
> }
> @@ -1110,6 +1126,14 @@ static void krpc_connect_done(struct pcs_msg *msg)
> if (!pcs_if_error(&msg->error)) {
> krpc->state = PCS_KRPC_STATE_CONNECTED;
> pollflags = EPOLLOUT;
> + } else {
> + /*
> + * Connect failed: settle back to UNCONN so that a new connect
> + * is allowed again, and report the failure to poll(). Since
> + * gen is unchanged, the current session's poll sees UNCONN
> + * and returns EPOLLERR.
> + */
> + krpc->state = PCS_KRPC_STATE_UNCONN;
> }
> spin_unlock(&krpc->lock);
>
> @@ -1167,9 +1191,15 @@ int pcs_krpc_connect(struct pcs_krpc_set *krpcs, PCS_NODE_ID_T *id)
> }
>
> spin_lock(&krpc->lock);
> - if (krpc->state == PCS_KRPC_STATE_CONNECTED ||
> - krpc->state == PCS_KRPC_STATE_DESTROYED ||
> - krpc->connect_req) {
> + /*
> + * A connect is allowed only when there is neither an established
> + * session nor a connect req in flight (CONNECT state, see
> + * krpc_connect_done()). This limits connect reqs to one at a time:
> + * if userspace gave up on a connect and retries, the new connect
> + * fails immediately until the old req completes.
> + */
> + if (krpc->state != PCS_KRPC_STATE_UNCONN &&
> + krpc->state != PCS_KRPC_STATE_ABORTED) {
> spin_unlock(&krpc->lock);
> err = -EPERM;
> /* fput() drops ctx and its krpc reference via pcs_krpc_release() */
> @@ -1181,7 +1211,6 @@ int pcs_krpc_connect(struct pcs_krpc_set *krpcs, PCS_NODE_ID_T *id)
> connect_req->gen = krpc->gen;
> connect_req->krpc = pcs_krpc_get(krpc);
> krpc->state = PCS_KRPC_STATE_CONNECT;
> - krpc->connect_req = connect_req;
> spin_unlock(&krpc->lock);
>
> /* publish the fd only after the connect is committed */
> diff --git a/fs/fuse/kio/pcs/pcs_krpc.h b/fs/fuse/kio/pcs/pcs_krpc.h
> index 803376895cc5..96b4815abf86 100644
> --- a/fs/fuse/kio/pcs/pcs_krpc.h
> +++ b/fs/fuse/kio/pcs/pcs_krpc.h
> @@ -84,8 +84,6 @@ struct pcs_krpc {
> /** Wait queue head for poll */
> wait_queue_head_t poll_wait;
> struct pcs_cs *cs;
> -
> - struct krpc_connect_req *connect_req;
> };
>
> struct pcs_krpc_context {
More information about the Devel
mailing list