OpenVZ / Virtuozzo kernel development (devel@openvz.org)
 help / color / mirror / Atom feed
* [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch
@ 2026-09-21 11:28 Liu Kui
  2026-09-21 12:13 ` Alexey Kuznetsov
  2026-09-21 20:52 ` Vladimir Riabchun
  0 siblings, 2 replies; 11+ messages in thread
From: Liu Kui @ 2026-09-21 11:28 UTC (permalink / raw)
  To: Konstantin Khorenko; +Cc: azaitsev, devel, Liu Kui

pcs_rpc_connect() is synchronous: it performs a full connect+auth
cycle while holding ep->mutex. rpc_queue_work() splices the entire
ep->input_queue onto an on-stack list and used to trigger the connect
from pcs_rpc_send() on the first staged message. When the connect
fails, pcs_rpc_reset() purges ep->input_queue and ep->state_queue,
but the messages still sitting on the on-stack list are invisible to
the purge - each of them re-triggers another blocking connect attempt
under the same mutex hold. With a large backlog and an unreachable
peer this keeps ep->mutex held for O(messages * connect timeout):
in one crashdump the rescuer of pcs_wq ground through such a batch
for days, starving calendar_work/rpc_close_work on the same ep and
wedging umount in flush_workqueue(pcs_wq).

Stage all messages from the batch on ep->state_queue first and start
the connect once afterwards. A failed connect now aborts the whole
batch in a single rpc_abort(), bounding the mutex hold to one connect
attempt. This also restores the meaning of state_queue, which
otherwise never held more than one message.

https://virtuozzo.atlassian.net/browse/VSTOR-135626

Signed-off-by: Liu Kui <kui.liu@virtuozzo.com>
---
Fix a pathological case seen in ACSSP-872 (and ASUP-3562) crashdumps:
a single rpc_queue_work() invocation could hold ep->mutex for days,
re-running a blocking connect+auth once per queued message to an
unreachable peer, wedging the pcs_wq rescuer, calendar/close works
and umount in flush_workqueue().

https://virtuozzo.atlassian.net/browse/ACSSP-872
---
 fs/fuse/kio/pcs/pcs_rpc.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/fuse/kio/pcs/pcs_rpc.c b/fs/fuse/kio/pcs/pcs_rpc.c
index 540225006726..c50e31c08a31 100644
--- a/fs/fuse/kio/pcs/pcs_rpc.c
+++ b/fs/fuse/kio/pcs/pcs_rpc.c
@@ -702,9 +702,6 @@ static void pcs_rpc_send(struct pcs_rpc * ep, struct pcs_msg * msg, bool requeue
 
 	list_add_tail(&msg->list, &ep->state_queue);
 	msg->stage = PCS_MSG_STAGE_UNSENT;
-
-	if (ep->state == PCS_RPC_UNCONN)
-		pcs_rpc_connect(ep);
 }
 
 void pcs_rpc_kick_queue(struct pcs_rpc * ep)
@@ -930,6 +927,21 @@ static void rpc_queue_work(struct work_struct *w)
 		list_del_init(&msg->list);
 		pcs_rpc_send(ep, msg, 0);
 	}
+
+	/* pcs_rpc_connect() is synchronous: it can block for a full
+	 * connect+auth cycle while holding ep->mutex. Start it once, after
+	 * every message from input_q has been staged on ep->state_queue,
+	 * instead of from pcs_rpc_send() on the first message. This way a
+	 * failed connect aborts the whole batch at once: rpc_abort() purges
+	 * ep->state_queue, whereas messages still sitting on the on-stack
+	 * input_q are invisible to the purge and would each trigger another
+	 * blocking connect attempt under the same mutex hold. It also
+	 * restores the meaning of state_queue, which otherwise never holds
+	 * more than one message.
+	 */
+	if (ep->state == PCS_RPC_UNCONN && !list_empty(&ep->state_queue))
+		pcs_rpc_connect(ep);
+
 	list_splice_tail_init(&ep->state_queue, &state_q);
 	while (!list_empty(&state_q)) {
 		struct pcs_msg * msg = list_first_entry(&state_q, struct pcs_msg, list);

---
base-commit: 3851feefb321d7dee9e57cbe7926410aa6dd043f
change-id: 20260921-acssp872-batch-connect-bec05e9db04d

Best regards,
--  
Liu Kui <kui.liu@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch
  2026-09-21 11:28 [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch Liu Kui
@ 2026-09-21 12:13 ` Alexey Kuznetsov
  2026-09-21 20:52 ` Vladimir Riabchun
  1 sibling, 0 replies; 11+ messages in thread
From: Alexey Kuznetsov @ 2026-09-21 12:13 UTC (permalink / raw)
  To: Liu Kui; +Cc: azaitsev, devel

Ack.

IMHO this patch looks very good. It can go to all active branches
and must be safe for RK. If pending pcs_rpc_connect blocks RK,
we have to invent some trick to cheat that (imho, invalid as whole) protection
mechanism.

On Mon, Sep 21, 2026 at 7:41 PM Liu Kui <kui.liu@virtuozzo.com> wrote:
>
> pcs_rpc_connect() is synchronous: it performs a full connect+auth
> cycle while holding ep->mutex. rpc_queue_work() splices the entire
> ep->input_queue onto an on-stack list and used to trigger the connect
> from pcs_rpc_send() on the first staged message. When the connect
> fails, pcs_rpc_reset() purges ep->input_queue and ep->state_queue,
> but the messages still sitting on the on-stack list are invisible to
> the purge - each of them re-triggers another blocking connect attempt
> under the same mutex hold. With a large backlog and an unreachable
> peer this keeps ep->mutex held for O(messages * connect timeout):
> in one crashdump the rescuer of pcs_wq ground through such a batch
> for days, starving calendar_work/rpc_close_work on the same ep and
> wedging umount in flush_workqueue(pcs_wq).
>
> Stage all messages from the batch on ep->state_queue first and start
> the connect once afterwards. A failed connect now aborts the whole
> batch in a single rpc_abort(), bounding the mutex hold to one connect
> attempt. This also restores the meaning of state_queue, which
> otherwise never held more than one message.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-135626
>
> Signed-off-by: Liu Kui <kui.liu@virtuozzo.com>
> ---
> Fix a pathological case seen in ACSSP-872 (and ASUP-3562) crashdumps:
> a single rpc_queue_work() invocation could hold ep->mutex for days,
> re-running a blocking connect+auth once per queued message to an
> unreachable peer, wedging the pcs_wq rescuer, calendar/close works
> and umount in flush_workqueue().
>
> https://virtuozzo.atlassian.net/browse/ACSSP-872
> ---
>  fs/fuse/kio/pcs/pcs_rpc.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/fs/fuse/kio/pcs/pcs_rpc.c b/fs/fuse/kio/pcs/pcs_rpc.c
> index 540225006726..c50e31c08a31 100644
> --- a/fs/fuse/kio/pcs/pcs_rpc.c
> +++ b/fs/fuse/kio/pcs/pcs_rpc.c
> @@ -702,9 +702,6 @@ static void pcs_rpc_send(struct pcs_rpc * ep, struct pcs_msg * msg, bool requeue
>
>         list_add_tail(&msg->list, &ep->state_queue);
>         msg->stage = PCS_MSG_STAGE_UNSENT;
> -
> -       if (ep->state == PCS_RPC_UNCONN)
> -               pcs_rpc_connect(ep);
>  }
>
>  void pcs_rpc_kick_queue(struct pcs_rpc * ep)
> @@ -930,6 +927,21 @@ static void rpc_queue_work(struct work_struct *w)
>                 list_del_init(&msg->list);
>                 pcs_rpc_send(ep, msg, 0);
>         }
> +
> +       /* pcs_rpc_connect() is synchronous: it can block for a full
> +        * connect+auth cycle while holding ep->mutex. Start it once, after
> +        * every message from input_q has been staged on ep->state_queue,
> +        * instead of from pcs_rpc_send() on the first message. This way a
> +        * failed connect aborts the whole batch at once: rpc_abort() purges
> +        * ep->state_queue, whereas messages still sitting on the on-stack
> +        * input_q are invisible to the purge and would each trigger another
> +        * blocking connect attempt under the same mutex hold. It also
> +        * restores the meaning of state_queue, which otherwise never holds
> +        * more than one message.
> +        */
> +       if (ep->state == PCS_RPC_UNCONN && !list_empty(&ep->state_queue))
> +               pcs_rpc_connect(ep);
> +
>         list_splice_tail_init(&ep->state_queue, &state_q);
>         while (!list_empty(&state_q)) {
>                 struct pcs_msg * msg = list_first_entry(&state_q, struct pcs_msg, list);
>
> ---
> base-commit: 3851feefb321d7dee9e57cbe7926410aa6dd043f
> change-id: 20260921-acssp872-batch-connect-bec05e9db04d
>
> Best regards,
> --
> Liu Kui <kui.liu@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch
  2026-09-21 11:28 [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch Liu Kui
  2026-09-21 12:13 ` Alexey Kuznetsov
@ 2026-09-21 20:52 ` Vladimir Riabchun
  2026-09-21 21:17   ` Alexey Kuznetsov
  1 sibling, 1 reply; 11+ messages in thread
From: Vladimir Riabchun @ 2026-09-21 20:52 UTC (permalink / raw)
  To: Liu Kui; +Cc: devel, azaitsev


On 9/21/26 13:28, Liu Kui wrote:
> pcs_rpc_connect() is synchronous: it performs a full connect+auth
> cycle while holding ep->mutex. rpc_queue_work() splices the entire
> ep->input_queue onto an on-stack list and used to trigger the connect
> from pcs_rpc_send() on the first staged message. When the connect
> fails, pcs_rpc_reset() purges ep->input_queue and ep->state_queue,
> but the messages still sitting on the on-stack list are invisible to
> the purge - each of them re-triggers another blocking connect attempt
> under the same mutex hold. With a large backlog and an unreachable
> peer this keeps ep->mutex held for O(messages * connect timeout):
> in one crashdump the rescuer of pcs_wq ground through such a batch
> for days, starving calendar_work/rpc_close_work on the same ep and
> wedging umount in flush_workqueue(pcs_wq).
> 
> Stage all messages from the batch on ep->state_queue first and start
> the connect once afterwards. A failed connect now aborts the whole
> batch in a single rpc_abort(), bounding the mutex hold to one connect
> attempt. This also restores the meaning of state_queue, which
> otherwise never held more than one message.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135626
> 
> Signed-off-by: Liu Kui <kui.liu@virtuozzo.com>> ---
> Fix a pathological case seen in ACSSP-872 (and ASUP-3562) crashdumps:
> a single rpc_queue_work() invocation could hold ep->mutex for days,
> re-running a blocking connect+auth once per queued message to an
> unreachable peer, wedging the pcs_wq rescuer, calendar/close works
> and umount in flush_workqueue().
> 
> https://virtuozzo.atlassian.net/browse/ACSSP-872
> ---
>   fs/fuse/kio/pcs/pcs_rpc.c | 18 +++++++++++++++---
>   1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fuse/kio/pcs/pcs_rpc.c b/fs/fuse/kio/pcs/pcs_rpc.c
> index 540225006726..c50e31c08a31 100644
> --- a/fs/fuse/kio/pcs/pcs_rpc.c
> +++ b/fs/fuse/kio/pcs/pcs_rpc.c
> @@ -702,9 +702,6 @@ static void pcs_rpc_send(struct pcs_rpc * ep, struct pcs_msg * msg, bool requeue
>   
>   	list_add_tail(&msg->list, &ep->state_queue);
>   	msg->stage = PCS_MSG_STAGE_UNSENT;
> -
> -	if (ep->state == PCS_RPC_UNCONN)
> -		pcs_rpc_connect(ep);
>   }
>   
>   void pcs_rpc_kick_queue(struct pcs_rpc * ep)
> @@ -930,6 +927,21 @@ static void rpc_queue_work(struct work_struct *w)
>   		list_del_init(&msg->list);
>   		pcs_rpc_send(ep, msg, 0);
>   	}
> +
> +	/* pcs_rpc_connect() is synchronous: it can block for a full
> +	 * connect+auth cycle while holding ep->mutex. Start it once, after
> +	 * every message from input_q has been staged on ep->state_queue,
> +	 * instead of from pcs_rpc_send() on the first message. This way a
> +	 * failed connect aborts the whole batch at once: rpc_abort() purges
> +	 * ep->state_queue, whereas messages still sitting on the on-stack
> +	 * input_q are invisible to the purge and would each trigger another
> +	 * blocking connect attempt under the same mutex hold. It also
> +	 * restores the meaning of state_queue, which otherwise never holds
> +	 * more than one message.
> +	 */
> +	if (ep->state == PCS_RPC_UNCONN && !list_empty(&ep->state_queue))
> +		pcs_rpc_connect(ep);
> +

Now I seem to get the problem we are fixing. This will work fine when
no problems pre-existed, but if the thread already collected a bunch of
messages in its private list, it will prevent live patching.

Ref: https://docs.kernel.org/livepatch/livepatch.html#consistency-model

rpc_queue_work task has patched function on its stack (itself), so
approach 1 is not for us. We can't poke kernel thread task with any signals
and waking it up in klp_send_signals will not help as well.

The doc I referenced above describes /sys/kernel/livepatch/<patch>/force
mechanism, that can force patch transition for all tasks.
We may make use of it to fixup the thread.

I checked disassembly and pcs_rpc_send is _not_ inlined in rpc_queue_work,
so after forcing livepatch, new versions of pcs_rpc_send will be executed.
However, rpc_queue_work will be old one - jump to new function version
happens in ftrace header, we missed it and never come there for a running
kthread.

So with this forced scenario we can clear local input_q list, but nothing
here (I don't know about other places) will then call pcs_rpc_connect
and flush ep->state_queue.
After moving all input_q to ep->state_queue, without pcs_rpc_connect
this state_queue will move to local state_q, which will then do
patched pcs_rpc_send with requeue = 1, again without pcs_rpc_connect,
and put these messages back to ep->state_queue.

My question is:
   Will anything blow up in this case if we just move these messages
   from one queue to another without calling connect?
   For me it seems that it will be a fast journey, all messages will
   go to ep->state_queue and we will leave the kthread (repeat = 0).
   Am I right and just forcing livepatch will work, or we need something
   smarter to fix stuck threads?

>   	list_splice_tail_init(&ep->state_queue, &state_q);
>   	while (!list_empty(&state_q)) {
>   		struct pcs_msg * msg = list_first_entry(&state_q, struct pcs_msg, list);
> 
> ---
> base-commit: 3851feefb321d7dee9e57cbe7926410aa6dd043f
> change-id: 20260921-acssp872-batch-connect-bec05e9db04d
> 
> Best regards,
> --
> Liu Kui <kui.liu@virtuozzo.com>
> 
> _______________________________________________
> Devel mailing list
> Devel@openvz.org
> https://lists.openvz.org/mailman/listinfo/devel

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch
  2026-09-21 20:52 ` Vladimir Riabchun
@ 2026-09-21 21:17   ` Alexey Kuznetsov
  2026-09-22  9:38     ` Vladimir Riabchun
  0 siblings, 1 reply; 11+ messages in thread
From: Alexey Kuznetsov @ 2026-09-21 21:17 UTC (permalink / raw)
  To: Vladimir Riabchun; +Cc: devel, Liu Kui, azaitsev

IMHO if we are inside unpatched rpc_queue_work() it can continue to loop,
it is not a big deal, it will finish the work eventually, hours or even days.
We just prevent future works queued after it from slow execution.

Normally, cluster does not even sense this background job. It is
single rescuer thread
which is not used for normal work. It starts to be
sensed when it is required to flush all pending requests like it
happens on unmount.
Then it can be fixed manually, it is simple with some iptables ... -j REJECT.

Main thing is that RK did not fail, then the problem will disappear
after the first run
is finally finished. But if customer has already got huge request
queues (like with the last case it was 15 rpcs 200000-300000 reqs each),
and needs urgent unmount, he has to wait till current rpc_queue_work() finsihes,
or support can add a temporary iptables rule.

On Tue, Sep 22, 2026 at 4:56 AM Vladimir Riabchun
<vladimir.riabchun@virtuozzo.com> wrote:
>
>
> On 9/21/26 13:28, Liu Kui wrote:
> > pcs_rpc_connect() is synchronous: it performs a full connect+auth
> > cycle while holding ep->mutex. rpc_queue_work() splices the entire
> > ep->input_queue onto an on-stack list and used to trigger the connect
> > from pcs_rpc_send() on the first staged message. When the connect
> > fails, pcs_rpc_reset() purges ep->input_queue and ep->state_queue,
> > but the messages still sitting on the on-stack list are invisible to
> > the purge - each of them re-triggers another blocking connect attempt
> > under the same mutex hold. With a large backlog and an unreachable
> > peer this keeps ep->mutex held for O(messages * connect timeout):
> > in one crashdump the rescuer of pcs_wq ground through such a batch
> > for days, starving calendar_work/rpc_close_work on the same ep and
> > wedging umount in flush_workqueue(pcs_wq).
> >
> > Stage all messages from the batch on ep->state_queue first and start
> > the connect once afterwards. A failed connect now aborts the whole
> > batch in a single rpc_abort(), bounding the mutex hold to one connect
> > attempt. This also restores the meaning of state_queue, which
> > otherwise never held more than one message.
> >
> > https://virtuozzo.atlassian.net/browse/VSTOR-135626
> >
> > Signed-off-by: Liu Kui <kui.liu@virtuozzo.com>> ---
> > Fix a pathological case seen in ACSSP-872 (and ASUP-3562) crashdumps:
> > a single rpc_queue_work() invocation could hold ep->mutex for days,
> > re-running a blocking connect+auth once per queued message to an
> > unreachable peer, wedging the pcs_wq rescuer, calendar/close works
> > and umount in flush_workqueue().
> >
> > https://virtuozzo.atlassian.net/browse/ACSSP-872
> > ---
> >   fs/fuse/kio/pcs/pcs_rpc.c | 18 +++++++++++++++---
> >   1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/fuse/kio/pcs/pcs_rpc.c b/fs/fuse/kio/pcs/pcs_rpc.c
> > index 540225006726..c50e31c08a31 100644
> > --- a/fs/fuse/kio/pcs/pcs_rpc.c
> > +++ b/fs/fuse/kio/pcs/pcs_rpc.c
> > @@ -702,9 +702,6 @@ static void pcs_rpc_send(struct pcs_rpc * ep, struct pcs_msg * msg, bool requeue
> >
> >       list_add_tail(&msg->list, &ep->state_queue);
> >       msg->stage = PCS_MSG_STAGE_UNSENT;
> > -
> > -     if (ep->state == PCS_RPC_UNCONN)
> > -             pcs_rpc_connect(ep);
> >   }
> >
> >   void pcs_rpc_kick_queue(struct pcs_rpc * ep)
> > @@ -930,6 +927,21 @@ static void rpc_queue_work(struct work_struct *w)
> >               list_del_init(&msg->list);
> >               pcs_rpc_send(ep, msg, 0);
> >       }
> > +
> > +     /* pcs_rpc_connect() is synchronous: it can block for a full
> > +      * connect+auth cycle while holding ep->mutex. Start it once, after
> > +      * every message from input_q has been staged on ep->state_queue,
> > +      * instead of from pcs_rpc_send() on the first message. This way a
> > +      * failed connect aborts the whole batch at once: rpc_abort() purges
> > +      * ep->state_queue, whereas messages still sitting on the on-stack
> > +      * input_q are invisible to the purge and would each trigger another
> > +      * blocking connect attempt under the same mutex hold. It also
> > +      * restores the meaning of state_queue, which otherwise never holds
> > +      * more than one message.
> > +      */
> > +     if (ep->state == PCS_RPC_UNCONN && !list_empty(&ep->state_queue))
> > +             pcs_rpc_connect(ep);
> > +
>
> Now I seem to get the problem we are fixing. This will work fine when
> no problems pre-existed, but if the thread already collected a bunch of
> messages in its private list, it will prevent live patching.
>
> Ref: https://docs.kernel.org/livepatch/livepatch.html#consistency-model
>
> rpc_queue_work task has patched function on its stack (itself), so
> approach 1 is not for us. We can't poke kernel thread task with any signals
> and waking it up in klp_send_signals will not help as well.
>
> The doc I referenced above describes /sys/kernel/livepatch/<patch>/force
> mechanism, that can force patch transition for all tasks.
> We may make use of it to fixup the thread.
>
> I checked disassembly and pcs_rpc_send is _not_ inlined in rpc_queue_work,
> so after forcing livepatch, new versions of pcs_rpc_send will be executed.
> However, rpc_queue_work will be old one - jump to new function version
> happens in ftrace header, we missed it and never come there for a running
> kthread.
>
> So with this forced scenario we can clear local input_q list, but nothing
> here (I don't know about other places) will then call pcs_rpc_connect
> and flush ep->state_queue.
> After moving all input_q to ep->state_queue, without pcs_rpc_connect
> this state_queue will move to local state_q, which will then do
> patched pcs_rpc_send with requeue = 1, again without pcs_rpc_connect,
> and put these messages back to ep->state_queue.
>
> My question is:
>    Will anything blow up in this case if we just move these messages
>    from one queue to another without calling connect?
>    For me it seems that it will be a fast journey, all messages will
>    go to ep->state_queue and we will leave the kthread (repeat = 0).
>    Am I right and just forcing livepatch will work, or we need something
>    smarter to fix stuck threads?
>
> >       list_splice_tail_init(&ep->state_queue, &state_q);
> >       while (!list_empty(&state_q)) {
> >               struct pcs_msg * msg = list_first_entry(&state_q, struct pcs_msg, list);
> >
> > ---
> > base-commit: 3851feefb321d7dee9e57cbe7926410aa6dd043f
> > change-id: 20260921-acssp872-batch-connect-bec05e9db04d
> >
> > Best regards,
> > --
> > Liu Kui <kui.liu@virtuozzo.com>
> >
> > _______________________________________________
> > Devel mailing list
> > Devel@openvz.org
> > https://lists.openvz.org/mailman/listinfo/devel
>
> --
> Best regards, Riabchun Vladimir
> Linux Kernel Developer, Virtuozzo

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch
  2026-09-21 21:17   ` Alexey Kuznetsov
@ 2026-09-22  9:38     ` Vladimir Riabchun
  2026-09-22 10:46       ` Alexey Kuznetsov
  0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Riabchun @ 2026-09-22  9:38 UTC (permalink / raw)
  To: Alexey Kuznetsov; +Cc: devel, Liu Kui, azaitsev



On 9/21/26 23:17, Alexey Kuznetsov wrote:
> IMHO if we are inside unpatched rpc_queue_work() it can continue to loop,
> it is not a big deal, it will finish the work eventually, hours or even days.
> We just prevent future works queued after it from slow execution.

With already present queue the patch will be able to proceed only with
force option. And even in this case we will run partially modified
version of rpc_queue_work - it will never call pcs_rpc_connect.

I now think it won't work: we take messages from ep->input_queue,
move them to ep->state_queue via pcs_rpc_send, but also while doing
this we set msg->rpc = ep. When we reach second pcs_rpc_send with
this message, it will trigger BUG_ON(msg->rpc != (requeue ? ep: NULL)).

> 
> Normally, cluster does not even sense this background job. It is
> single rescuer thread
> which is not used for normal work. It starts to be
> sensed when it is required to flush all pending requests like it
> happens on unmount.
> Then it can be fixed manually, it is simple with some iptables ... -j REJECT.
> 
> Main thing is that RK did not fail, then the problem will disappear
> after the first run
> is finally finished. But if customer has already got huge request
> queues (like with the last case it was 15 rpcs 200000-300000 reqs each),
> and needs urgent unmount, he has to wait till current rpc_queue_work() finsihes,
> or support can add a temporary iptables rule.

So you think we should keep the RK patch simple and not bother with
already stuck nodes and force options?

> 
> On Tue, Sep 22, 2026 at 4:56 AM Vladimir Riabchun
> <vladimir.riabchun@virtuozzo.com> wrote:
>>
>>
>> On 9/21/26 13:28, Liu Kui wrote:
>>> pcs_rpc_connect() is synchronous: it performs a full connect+auth
>>> cycle while holding ep->mutex. rpc_queue_work() splices the entire
>>> ep->input_queue onto an on-stack list and used to trigger the connect
>>> from pcs_rpc_send() on the first staged message. When the connect
>>> fails, pcs_rpc_reset() purges ep->input_queue and ep->state_queue,
>>> but the messages still sitting on the on-stack list are invisible to
>>> the purge - each of them re-triggers another blocking connect attempt
>>> under the same mutex hold. With a large backlog and an unreachable
>>> peer this keeps ep->mutex held for O(messages * connect timeout):
>>> in one crashdump the rescuer of pcs_wq ground through such a batch
>>> for days, starving calendar_work/rpc_close_work on the same ep and
>>> wedging umount in flush_workqueue(pcs_wq).
>>>
>>> Stage all messages from the batch on ep->state_queue first and start
>>> the connect once afterwards. A failed connect now aborts the whole
>>> batch in a single rpc_abort(), bounding the mutex hold to one connect
>>> attempt. This also restores the meaning of state_queue, which
>>> otherwise never held more than one message.
>>>
>>> https://virtuozzo.atlassian.net/browse/VSTOR-135626
>>>
>>> Signed-off-by: Liu Kui <kui.liu@virtuozzo.com>> ---
>>> Fix a pathological case seen in ACSSP-872 (and ASUP-3562) crashdumps:
>>> a single rpc_queue_work() invocation could hold ep->mutex for days,
>>> re-running a blocking connect+auth once per queued message to an
>>> unreachable peer, wedging the pcs_wq rescuer, calendar/close works
>>> and umount in flush_workqueue().
>>>
>>> https://virtuozzo.atlassian.net/browse/ACSSP-872
>>> ---
>>>    fs/fuse/kio/pcs/pcs_rpc.c | 18 +++++++++++++++---
>>>    1 file changed, 15 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fs/fuse/kio/pcs/pcs_rpc.c b/fs/fuse/kio/pcs/pcs_rpc.c
>>> index 540225006726..c50e31c08a31 100644
>>> --- a/fs/fuse/kio/pcs/pcs_rpc.c
>>> +++ b/fs/fuse/kio/pcs/pcs_rpc.c
>>> @@ -702,9 +702,6 @@ static void pcs_rpc_send(struct pcs_rpc * ep, struct pcs_msg * msg, bool requeue
>>>
>>>        list_add_tail(&msg->list, &ep->state_queue);
>>>        msg->stage = PCS_MSG_STAGE_UNSENT;
>>> -
>>> -     if (ep->state == PCS_RPC_UNCONN)
>>> -             pcs_rpc_connect(ep);
>>>    }
>>>
>>>    void pcs_rpc_kick_queue(struct pcs_rpc * ep)
>>> @@ -930,6 +927,21 @@ static void rpc_queue_work(struct work_struct *w)
>>>                list_del_init(&msg->list);
>>>                pcs_rpc_send(ep, msg, 0);
>>>        }
>>> +
>>> +     /* pcs_rpc_connect() is synchronous: it can block for a full
>>> +      * connect+auth cycle while holding ep->mutex. Start it once, after
>>> +      * every message from input_q has been staged on ep->state_queue,
>>> +      * instead of from pcs_rpc_send() on the first message. This way a
>>> +      * failed connect aborts the whole batch at once: rpc_abort() purges
>>> +      * ep->state_queue, whereas messages still sitting on the on-stack
>>> +      * input_q are invisible to the purge and would each trigger another
>>> +      * blocking connect attempt under the same mutex hold. It also
>>> +      * restores the meaning of state_queue, which otherwise never holds
>>> +      * more than one message.
>>> +      */
>>> +     if (ep->state == PCS_RPC_UNCONN && !list_empty(&ep->state_queue))
>>> +             pcs_rpc_connect(ep);
>>> +
>>
>> Now I seem to get the problem we are fixing. This will work fine when
>> no problems pre-existed, but if the thread already collected a bunch of
>> messages in its private list, it will prevent live patching.
>>
>> Ref: https://docs.kernel.org/livepatch/livepatch.html#consistency-model
>>
>> rpc_queue_work task has patched function on its stack (itself), so
>> approach 1 is not for us. We can't poke kernel thread task with any signals
>> and waking it up in klp_send_signals will not help as well.
>>
>> The doc I referenced above describes /sys/kernel/livepatch/<patch>/force
>> mechanism, that can force patch transition for all tasks.
>> We may make use of it to fixup the thread.
>>
>> I checked disassembly and pcs_rpc_send is _not_ inlined in rpc_queue_work,
>> so after forcing livepatch, new versions of pcs_rpc_send will be executed.
>> However, rpc_queue_work will be old one - jump to new function version
>> happens in ftrace header, we missed it and never come there for a running
>> kthread.
>>
>> So with this forced scenario we can clear local input_q list, but nothing
>> here (I don't know about other places) will then call pcs_rpc_connect
>> and flush ep->state_queue.
>> After moving all input_q to ep->state_queue, without pcs_rpc_connect
>> this state_queue will move to local state_q, which will then do
>> patched pcs_rpc_send with requeue = 1, again without pcs_rpc_connect,
>> and put these messages back to ep->state_queue.
>>
>> My question is:
>>     Will anything blow up in this case if we just move these messages
>>     from one queue to another without calling connect?
>>     For me it seems that it will be a fast journey, all messages will
>>     go to ep->state_queue and we will leave the kthread (repeat = 0).
>>     Am I right and just forcing livepatch will work, or we need something
>>     smarter to fix stuck threads?
>>
>>>        list_splice_tail_init(&ep->state_queue, &state_q);
>>>        while (!list_empty(&state_q)) {
>>>                struct pcs_msg * msg = list_first_entry(&state_q, struct pcs_msg, list);
>>>
>>> ---
>>> base-commit: 3851feefb321d7dee9e57cbe7926410aa6dd043f
>>> change-id: 20260921-acssp872-batch-connect-bec05e9db04d
>>>
>>> Best regards,
>>> --
>>> Liu Kui <kui.liu@virtuozzo.com>
>>>
>>> _______________________________________________
>>> Devel mailing list
>>> Devel@openvz.org
>>> https://lists.openvz.org/mailman/listinfo/devel
>>
>> --
>> Best regards, Riabchun Vladimir
>> Linux Kernel Developer, Virtuozzo

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch
  2026-09-22  9:38     ` Vladimir Riabchun
@ 2026-09-22 10:46       ` Alexey Kuznetsov
  2026-09-22 11:36         ` Vladimir Riabchun
  0 siblings, 1 reply; 11+ messages in thread
From: Alexey Kuznetsov @ 2026-09-22 10:46 UTC (permalink / raw)
  To: Vladimir Riabchun; +Cc: devel, Liu Kui, azaitsev

On Tue, Sep 22, 2026 at 5:41 PM Vladimir Riabchun
<vladimir.riabchun@virtuozzo.com> wrote:
> I now think it won't work: we take messages from ep->input_queue,
> move them to ep->state_queue via pcs_rpc_send, but also while doing
> this we set msg->rpc = ep. When we reach second pcs_rpc_send with
> this message, it will trigger BUG_ON(msg->rpc != (requeue ? ep: NULL)).

Please, elaborate.

Yes, the code is kind of strange, I daresay. Apparently, original author did
some wrong assumptions and left mud, and when we cleaned it up later, instead of
just cleanup we left old paths, added BUG_ONs and abundant comments,
apparently being afraid that we miss something. :-) Must just be
deleted altogether.

Yet the intent is clear. msg->rpc is NULL while message is in
input_queue. It grabs ref
and assigns it the first time it leaves this queue. And after this
it stays == ep. It does not even correlate with the patch.

> So you think we should keep the RK patch simple and not bother with
> already stuck nodes

This is yes.

>... and force options?

And this is up to you. IMHO the whole digging of stack in livepatch is
entirely useless,
protects from nothing. At least I do not understand this wisdom and
leave paranoia to persons
who do understand.

As I see the only real danger is when after patch old code which is
still running (old rpc_queue_work())
calls pcs_rpc_send, which after the patch is _new_. This means that
rpc connect can
be missed because it is removed from pcs_rpc_send, and old rpc_queue_work does
not have it. Well, it is not a big deal, it will timeout and redone later.

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch
  2026-09-22 10:46       ` Alexey Kuznetsov
@ 2026-09-22 11:36         ` Vladimir Riabchun
  2026-09-22 13:12           ` Alexey Kuznetsov
  0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Riabchun @ 2026-09-22 11:36 UTC (permalink / raw)
  To: Alexey Kuznetsov; +Cc: devel, Liu Kui, azaitsev



On 9/22/26 12:46, Alexey Kuznetsov wrote:
> On Tue, Sep 22, 2026 at 5:41 PM Vladimir Riabchun
> <vladimir.riabchun@virtuozzo.com> wrote:
>> I now think it won't work: we take messages from ep->input_queue,
>> move them to ep->state_queue via pcs_rpc_send, but also while doing
>> this we set msg->rpc = ep. When we reach second pcs_rpc_send with
>> this message, it will trigger BUG_ON(msg->rpc != (requeue ? ep: NULL)).
> 
> Please, elaborate.
> 
> Yes, the code is kind of strange, I daresay. Apparently, original author did
> some wrong assumptions and left mud, and when we cleaned it up later, instead of
> just cleanup we left old paths, added BUG_ONs and abundant comments,
> apparently being afraid that we miss something. :-) Must just be
> deleted altogether.
> 
> Yet the intent is clear. msg->rpc is NULL while message is in
> input_queue. It grabs ref
> and assigns it the first time it leaves this queue. And after this
> it stays == ep. It does not even correlate with the patch.

These were just my thoughts if we can heal the stuck thread with
low effort. Seems like no - at least we need to drop these
assertions.

> 
>> So you think we should keep the RK patch simple and not bother with
>> already stuck nodes
> 
> This is yes.
> 
>> ... and force options?
> 
> And this is up to you. IMHO the whole digging of stack in livepatch is
> entirely useless,
> protects from nothing. At least I do not understand this wisdom and
> leave paranoia to persons
> who do understand.

To prevent cases like the one below - inconsistent code state.

> 
> As I see the only real danger is when after patch old code which is
> still running (old rpc_queue_work())
> calls pcs_rpc_send, which after the patch is _new_. This means that
> rpc connect can
> be missed because it is removed from pcs_rpc_send, and old rpc_queue_work does
> not have it. Well, it is not a big deal, it will timeout and redone later.

Making some smart livepatch tricks is too dangerous here without proper testing,
and from what I read in VSTOR-135626 comments, we can't do this easily.

I'll just take this patch as-is, and if guys need to fix stuck thread, they
should do some extra work.

Btw, I'm just curious, how iptables rule will help? It will speed up pcs_rpc_connect?

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch
  2026-09-22 11:36         ` Vladimir Riabchun
@ 2026-09-22 13:12           ` Alexey Kuznetsov
  2026-09-22 14:10             ` Vladimir Riabchun
  0 siblings, 1 reply; 11+ messages in thread
From: Alexey Kuznetsov @ 2026-09-22 13:12 UTC (permalink / raw)
  To: Vladimir Riabchun; +Cc: devel, Liu Kui, azaitsev

Hello!

On Tue, Sep 22, 2026 at 7:41 PM Vladimir Riabchun
<vladimir.riabchun@virtuozzo.com> wrote:
> These were just my thoughts if we can heal the stuck thread with
> low effort. Seems like no - at least we need to drop these
> assertions.

I do not see any problems there. Well, except for too scary code.
If you see, elaborate, I could miss something.

> To prevent cases like the one below - inconsistent code state.

IMHO it prevents nothing. It just minimizes patch success. :-)
Is it correct that all these kludges are done to avoid stop machine?
Or is it still used? Sleeping states must be responsibility of patch maker,
protections based on traces are  just useless, kernel simply cannot do it
by many reasons f.e. because of tail calls.

> Btw, I'm just curious, how iptables rule will help? It will tesspeed up pcs_rpc_connect?

Yes, almost infinitely. In the last case we had 16 dead rpcs, each has
queued 200000-300000
requests. Each connect waits for ~2sec. So, counting: it would take
~75 days to finish. :-)
With -j DROP they were processed faster than I started drgn to measure
the rate, some
seconds.

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch
  2026-09-22 13:12           ` Alexey Kuznetsov
@ 2026-09-22 14:10             ` Vladimir Riabchun
  2026-09-22 15:01               ` Alexey Kuznetsov
  0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Riabchun @ 2026-09-22 14:10 UTC (permalink / raw)
  To: Alexey Kuznetsov; +Cc: devel, Liu Kui, azaitsev



On 9/22/26 15:12, Alexey Kuznetsov wrote:
> Hello!
> 
> On Tue, Sep 22, 2026 at 7:41 PM Vladimir Riabchun
> <vladimir.riabchun@virtuozzo.com> wrote:
>> These were just my thoughts if we can heal the stuck thread with
>> low effort. Seems like no - at least we need to drop these
>> assertions.
> 
> I do not see any problems there. Well, except for too scary code.
> If you see, elaborate, I could miss something.

You are right, sorry. There is no BUG_ON problem.
Condition in BUG_ON(msg->rpc != (requeue ? ep: NULL)) was too hard
for me to process correctly in my head, while writing reply I decoded
it.
> 
>> To prevent cases like the one below - inconsistent code state.
> 
> IMHO it prevents nothing. It just minimizes patch success. :-)
> Is it correct that all these kludges are done to avoid stop machine?

Probably no. stop_machine-based livepatch I worked with does the same
stack checks and even more. But there it is 100% needed because it could
patch any place of code(not ftrace-based patching), so it is possible
to corrupt return site.

Guys just doing their best to avoid half-patches code from being run.

> Or is it still used? Sleeping states must be responsibility of patch maker,
> protections based on traces are  just useless, kernel simply cannot do it
> by many reasons f.e. because of tail calls.

What's wrong with tail calls? If it is inlined, we will just patch
the parent and stack checks would be performed for parent.

> 
>> Btw, I'm just curious, how iptables rule will help? It will tesspeed up pcs_rpc_connect?
> 
> Yes, almost infinitely. In the last case we had 16 dead rpcs, each has
> queued 200000-300000
> requests. Each connect waits for ~2sec. So, counting: it would take
> ~75 days to finish. :-)
> With -j DROP they were processed faster than I started drgn to measure
> the rate, some
> seconds.

So, this is the cure if livepatch doesn't apply.

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch
  2026-09-22 14:10             ` Vladimir Riabchun
@ 2026-09-22 15:01               ` Alexey Kuznetsov
  2026-09-22 15:19                 ` Vladimir Riabchun
  0 siblings, 1 reply; 11+ messages in thread
From: Alexey Kuznetsov @ 2026-09-22 15:01 UTC (permalink / raw)
  To: Vladimir Riabchun; +Cc: devel, Liu Kui, azaitsev

Hello!

On Tue, Sep 22, 2026 at 10:11 PM Vladimir Riabchun
<vladimir.riabchun@virtuozzo.com> wrote:
> Condition in BUG_ON(msg->rpc != (requeue ? ep: NULL)) was too hard
> for me to process correctly in my head, while writing reply I decoded
> it.

Exactly. As I said the code is too scary.

> What's wrong with tail calls? If it is inlined, we will just patch
> the parent and stack checks would be performed for parent.

tail called functions are not on stack. at least they used to,
probably with ORCA they are visible, I still did not learn this.

Return should not be an issue, old code remains executable and not corrupted,
it can live forever if someobe sleeps in it.
Or it does not?? I thought it uses __fentry__ placeholder to redirect.

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch
  2026-09-22 15:01               ` Alexey Kuznetsov
@ 2026-09-22 15:19                 ` Vladimir Riabchun
  0 siblings, 0 replies; 11+ messages in thread
From: Vladimir Riabchun @ 2026-09-22 15:19 UTC (permalink / raw)
  To: Alexey Kuznetsov; +Cc: devel, Liu Kui, azaitsev



On 9/22/26 17:01, Alexey Kuznetsov wrote:
> Hello!
> 
> On Tue, Sep 22, 2026 at 10:11 PM Vladimir Riabchun
> <vladimir.riabchun@virtuozzo.com> wrote:
>> Condition in BUG_ON(msg->rpc != (requeue ? ep: NULL)) was too hard
>> for me to process correctly in my head, while writing reply I decoded
>> it.
> 
> Exactly. As I said the code is too scary.
> 
>> What's wrong with tail calls? If it is inlined, we will just patch
>> the parent and stack checks would be performed for parent.
> 
> tail called functions are not on stack. at least they used to,
> probably with ORCA they are visible, I still did not learn this.
> 
> Return should not be an issue, old code remains executable and not corrupted,
> it can live forever if someobe sleeps in it.
> Or it does not?? I thought it uses __fentry__ placeholder to redirect.

I mentioned return sites to show that these stack trace checks might be
useful even if patching does stop_machine. But it is not the case for
ftrace-based livepatches.

There I think guys just want some additional guarantees on executed code
state, to think less about such problems.

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-09-22 15:21 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-21 11:28 [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch Liu Kui
2026-09-21 12:13 ` Alexey Kuznetsov
2026-09-21 20:52 ` Vladimir Riabchun
2026-09-21 21:17   ` Alexey Kuznetsov
2026-09-22  9:38     ` Vladimir Riabchun
2026-09-22 10:46       ` Alexey Kuznetsov
2026-09-22 11:36         ` Vladimir Riabchun
2026-09-22 13:12           ` Alexey Kuznetsov
2026-09-22 14:10             ` Vladimir Riabchun
2026-09-22 15:01               ` Alexey Kuznetsov
2026-09-22 15:19                 ` Vladimir Riabchun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox