OpenVZ / Virtuozzo kernel development (devel@openvz.org)
 help / color / mirror / Atom feed
From: Alexey Kuznetsov <kuznet@virtuozzo.com>
To: Liu Kui <kui.liu@virtuozzo.com>
Cc: azaitsev@virtuozzo.com, devel@openvz.org
Subject: Re: [Devel] [PATCH hci-8.1] fs/fuse kio: start connect once per rpc_queue_work batch
Date: Mon, 21 Sep 2026 20:13:53 +0800	[thread overview]
Message-ID: <CAMrnHh5Nu+eF5G-1PAwKQTqMzZ0z1ibQPm6vTToprXg=7WgkkA@mail.gmail.com> (raw)
In-Reply-To: <20260921-acssp872-batch-connect-v1-1-ded37d934f28@virtuozzo.com>

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

  reply	other threads:[~2026-09-21 12:15 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-21 11:28 Liu Kui
2026-09-21 12:13 ` Alexey Kuznetsov [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMrnHh5Nu+eF5G-1PAwKQTqMzZ0z1ibQPm6vTToprXg=7WgkkA@mail.gmail.com' \
    --to=kuznet@virtuozzo.com \
    --cc=azaitsev@virtuozzo.com \
    --cc=devel@openvz.org \
    --cc=kui.liu@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox