<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Yeah, the first issue is valid and serious, especially when working with the older user-space where there is still connect timeout.</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
I've made the changes to the patch, please take the version 2. </div>
<div id="appendonsend"></div>
<div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<hr style="display: inline-block; width: 98%;">
<div id="divRplyFwdMsg">
<div style="direction: ltr; font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
<b>From:</b> Konstantin Khorenko <khorenko@virtuozzo.com><br>
<b>Sent:</b> 02 September 2026 04:59<br>
<b>To:</b> Kui Liu <kui.liu@virtuozzo.com><br>
<b>Cc:</b> Andrey Zaitsev <azaitsev@virtuozzo.com>; Alexey Kuznetsov <kuznet@virtuozzo.com>; devel@openvz.org <devel@openvz.org><br>
<b>Subject:</b> Re: [Devel] [PATCH VZ10] fs/fuse kio: track pending kRPC connect via state machine only</div>
<div style="direction: ltr;"> </div>
</div>
<div style="font-size: 11pt;">> Rework the previous fix ("fs/fuse kio: fix kRPC connect issues") to not<br>
> require the new struct pcs_krpc member "connect_req", so the fix can be<br>
> shipped as a livepatch.<br>
><br>
> Both things connect_req was tracking are already derivable from the<br>
> existing state machine once PCS_KRPC_STATE_CONNECT is made to mean<br>
> exactly "a connect req is in flight":<br>
><br>
> - krpc_connect_done() settles a failed connect back to UNCONN instead<br>
> of leaving the state in CONNECT forever;<br>
><br>
> - pcs_krpc_abort() no longer resets CONNECT to UNCONN: the req is<br>
> still in flight, and only its completion settles the state;<br>
><br>
> - pcs_krpc_connect() proceeds only from UNCONN or ABORTED, refusing<br>
> new connects (-EPERM) while a req is in flight - at most one connect<br>
> req exists at a time, same as with the connect_req check;<br>
><br>
> - pcs_krpc_poll() reports EPOLLERR on UNCONN: poll bails out earlier<br>
> unless ctx->gen == krpc->gen, and the current session can only be in<br>
> UNCONN if its connect failed or was aborted, which is what the<br>
> (CONNECT && !connect_req) test used to detect.<br>
><br>
> gen only advances in pcs_krpc_connect(), which is blocked during<br>
> CONNECT, so within that state the in-flight req always carries the<br>
> current gen and krpc_connect_done()'s existing staleness check is<br>
> sufficient.<br>
><br>
> Related to:<br>
> <a href="https://virtuozzo.atlassian.net/browse/VSTOR-135626" data-auth="NotApplicable" id="OWA2eb79a82-ff9f-8b21-2276-b403ca3406ce" class="OWAAutoLink">
https://virtuozzo.atlassian.net/browse/VSTOR-135626</a><br>
> Fixes: d0d6034c36010 ("fs/fuse kio: fix kRPC connect issues")<br>
> Feature: fuse: kRPC - single RPC for kernel and userspace<br>
><br>
> Signed-off-by: Liu Kui <kui.liu@virtuozzo.com><br>
><br>
> diff --git a/fs/fuse/kio/pcs/pcs_krpc.c b/fs/fuse/kio/pcs/pcs_krpc.c<br>
> index 0930fb4adf125..f9fb6b3699062 100644<br>
> --- a/fs/fuse/kio/pcs/pcs_krpc.c<br>
> +++ b/fs/fuse/kio/pcs/pcs_krpc.c<br>
> @@ -787,8 +787,12 @@ static int pcs_krpc_abort(struct pcs_krpc *krpc)<br>
> spin_lock(&krpc->lock);<br>
><br>
> if (krpc->state != PCS_KRPC_STATE_CONNECTED) {<br>
> - if (krpc->state == PCS_KRPC_STATE_CONNECT)<br>
> - krpc->state = PCS_KRPC_STATE_UNCONN;<br>
> + /*<br>
> + * A pending connect stays in CONNECT state: its connect req<br>
> + * is still in flight and krpc_connect_done() will settle the<br>
> + * state to UNCONN when it completes. Until then new connects<br>
> + * are refused, so at most one connect req exists at a time.<br>
> + */<br>
> spin_unlock(&krpc->lock);<br>
> return 0;<br>
> }<br>
<br>
[Severity: High]<br>
Can this change leave a krpc stuck in the connected state with no<br>
session fd attached to it?<br>
<br>
Consider a connect req in flight (state is CONNECT) whose fd is closed<br>
by userspace after its connect timeout expires - the scenario the<br>
original fix was written for:<br>
<br>
pcs_krpc_release()<br>
if (ctx->gen == krpc->gen)<br>
pcs_krpc_abort(krpc); /* state is CONNECT: does nothing now */<br>
<br>
gen cannot advance while the state stays CONNECT, because<br>
pcs_krpc_connect() refuses new connects with -EPERM in that state. So<br>
when the in-flight req later completes successfully (the peer became<br>
reachable again), krpc_connect_done() passes its staleness check and<br>
commits the dead session:<br>
<br>
if (req->gen != krpc->gen || krpc->state != PCS_KRPC_STATE_CONNECT) {<br>
spin_unlock(&krpc->lock);<br>
goto out;<br>
}<br>
<br>
if (!pcs_if_error(&msg->error)) {<br>
krpc->state = PCS_KRPC_STATE_CONNECTED;<br>
<br>
Now the krpc is CONNECTED while its gen still names a session whose fd<br>
is gone. Every following PCS_IOC_KRPC_CONNECT returns -EPERM, there is<br>
no fd left on which userspace could issue PCS_KRPC_IOC_ABORT, and no<br>
kernel path resets the state, so the node stays unconnectable until the<br>
krpc is destroyed.<br>
<br>
The connect_req based code handled this case: pcs_krpc_abort() moved<br>
CONNECT to UNCONN, a successful krpc_connect_done() then took the stale<br>
path without transitioning to CONNECTED, and the next connect was<br>
allowed as soon as the old req completed.<br>
<br>
Does the abort/release path need to invalidate the pending connect, so<br>
that a late successful completion settles the state to UNCONN instead<br>
of resurrecting the closed session?<br>
<br>
> @@ -956,8 +960,13 @@ static __poll_t pcs_krpc_poll(struct file *file, poll_table *wait)<br>
><br>
> spin_lock(&krpc->lock);<br>
><br>
> + /*<br>
> + * ctx->gen == krpc->gen (checked above) means this is the current<br>
> + * session, so UNCONN here can only mean its connect attempt has<br>
> + * failed (see krpc_connect_done()) or the session was aborted.<br>
> + */<br>
> if (krpc->state == PCS_KRPC_STATE_ABORTED ||<br>
> - (krpc->state == PCS_KRPC_STATE_CONNECT && !krpc->connect_req)) {<br>
> + krpc->state == PCS_KRPC_STATE_UNCONN) {<br>
> pollflags |= EPOLLERR;<br>
> } else if (krpc->state == PCS_KRPC_STATE_CONNECTED) {<br>
> pollflags |= EPOLLOUT;<br>
<br>
[Severity: Low]<br>
This isn't a bug, but after this change pcs_krpc_abort() never sets<br>
UNCONN, and an aborted session is left in ABORTED, which the first<br>
check already handles.<br>
<br>
For a session with ctx->gen == krpc->gen, can UNCONN still be reached<br>
through an abort as the comment says, or only through a failed<br>
connect? The commit message carries the same wording ("the current<br>
session can only be in UNCONN if its connect failed or was aborted").<br>
<br>
[ ... ]<br>
<br>
--<br>
Konstantin Khorenko <khorenko@virtuozzo.com></div>
</body>
</html>