From: Noah Goldstein <goldstein.w.n@gmail.com>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: libc-alpha@sourceware.org
Subject: Re: [PATCH] __check_pf: Add a cancellation cleanup handler [BZ #20975]
Date: Thu, 27 Apr 2023 17:03:27 -0500 [thread overview]
Message-ID: <CAFUsyf+frCBVbSrK-ivvsCvge1UTFg9dO3De2+HqsEBzgraNBA@mail.gmail.com> (raw)
In-Reply-To: <20230427200615.1496059-1-hjl.tools@gmail.com>
On Thu, Apr 27, 2023 at 3:06 PM H.J. Lu via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> There are reports for hang in __check_pf:
>
> https://github.com/JoeDog/siege/issues/4
>
> It is reproducible only under specific configurations:
>
> 1. Large number of cores (>= 64) and large number of threads (> 3X of
> the number of cores) with long lived socket connection.
> 2. Low power (frequency) mode.
> 3. Power management is enabled.
>
> While holding lock, __check_pf calls make_request which calls __sendto
> and __recvmsg. Since __sendto and __recvmsg are cancellation points,
> lock held by __check_pf won't be released and can cause deadlock when
> thread cancellation happens in __sendto or __recvmsg. Add a cancellation
> cleanup handler for __check_pf to unlock the lock when cancelled by
> another thread. This fixes BZ #20975 and the siege hang issue.
> ---
> sysdeps/unix/sysv/linux/Makefile | 2 ++
> sysdeps/unix/sysv/linux/check_pf.c | 15 +++++++++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index aec7a94785..0160be8790 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -529,6 +529,8 @@ sysdep_headers += \
> sysdep_routines += \
> netlink_assert_response \
> # sysdep_routines
> +
> +CFLAGS-check_pf.c += -fexceptions
> endif
>
> # Don't compile the ctype glue code, since there is no old non-GNU C library.
> diff --git a/sysdeps/unix/sysv/linux/check_pf.c b/sysdeps/unix/sysv/linux/check_pf.c
> index b157c5126c..2b0b8b6368 100644
> --- a/sysdeps/unix/sysv/linux/check_pf.c
> +++ b/sysdeps/unix/sysv/linux/check_pf.c
> @@ -292,6 +292,14 @@ make_request (int fd, pid_t pid)
> return NULL;
> }
>
> +#ifdef __EXCEPTIONS
> +static void
> +cancel_handler (void *arg __attribute__((unused)))
> +{
> + /* Release the lock. */
> + __libc_lock_unlock (lock);
> +}
> +#endif
>
> void
> attribute_hidden
> @@ -304,6 +312,10 @@ __check_pf (bool *seen_ipv4, bool *seen_ipv6,
> struct cached_data *olddata = NULL;
> struct cached_data *data = NULL;
>
> +#ifdef __EXCEPTIONS
> + /* Make sure that lock is released when the thread is cancelled. */
> + __libc_cleanup_push (cancel_handler, NULL);
> +#endif
Should we wait until we have successfully taken the lock to push this?
> __libc_lock_lock (lock);
>
> if (cache_valid_p ())
> @@ -338,6 +350,9 @@ __check_pf (bool *seen_ipv4, bool *seen_ipv6,
> }
> }
>
> +#ifdef __EXCEPTIONS
> + __libc_cleanup_pop (0);
> +#endif
> __libc_lock_unlock (lock);
>
> if (data != NULL)
> --
> 2.40.0
>
next prev parent reply other threads:[~2023-04-27 22:03 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-27 20:06 H.J. Lu
2023-04-27 20:42 ` Noah Goldstein
2023-04-27 20:59 ` H.J. Lu
2023-04-27 21:44 ` Noah Goldstein
2023-04-27 21:50 ` H.J. Lu
2023-04-27 22:03 ` Noah Goldstein [this message]
2023-04-27 22:10 ` H.J. Lu
2023-04-28 8:38 ` Florian Weimer
2023-04-28 15:33 ` H.J. Lu
2023-04-28 19:17 ` Noah Goldstein
2023-05-22 20:37 ` H.J. Lu
2023-05-23 8:58 ` Florian Weimer
2023-05-23 16:56 ` Noah Goldstein
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=CAFUsyf+frCBVbSrK-ivvsCvge1UTFg9dO3De2+HqsEBzgraNBA@mail.gmail.com \
--to=goldstein.w.n@gmail.com \
--cc=hjl.tools@gmail.com \
--cc=libc-alpha@sourceware.org \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).