From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw1-x112c.google.com (mail-yw1-x112c.google.com [IPv6:2607:f8b0:4864:20::112c]) by sourceware.org (Postfix) with ESMTPS id B348D3858C50 for ; Thu, 27 Apr 2023 22:10:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B348D3858C50 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-yw1-x112c.google.com with SMTP id 00721157ae682-54f99770f86so107717477b3.1 for ; Thu, 27 Apr 2023 15:10:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1682633447; x=1685225447; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=BRM+DBZ8pzm1khbgbeHD2KuNWXkaXwdVk4L5+Noclyc=; b=sKrIaSM6CRQmjbENS/MN3cPTJS/YTUkXOHDwuIL5d2bsucqydh1WsMA6sMsLRoDz0q T8TiE0BbFX11dff4URDj2rnPxdvGhnKt7Y45EtRLu8jjgefL+Z3Am69VC3pNlsvioMSF GgGzHi4tuIJbU8TMg+ClPH4mZbQvwBHmHqKxZfXmpjy1FQ0uJeonTqT4Yhhg98aKNW6z /Vr2zZ7Hqbs1kUkyl05gNYoWa02XxkFXMmCx9C6ogGmNGSXX2f+D/h5v4eIx+54JX7M9 z5NHHkC/geCkzIkITmMpIFfv6IMIl+weWPZx8aZ4B5vaXgLzJ7TH3EClG7vpclrYAPPL GfMQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682633447; x=1685225447; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=BRM+DBZ8pzm1khbgbeHD2KuNWXkaXwdVk4L5+Noclyc=; b=PL/D4CkoBvxhL7xIN8d4UcGn9R5iNW0kuUcEvormbG7d9m8L5CO2Ee6F0dTzTqM8iW 5PhQgJ2PMN5vuR5rCHodOxhcxYff4PBw2oVEHaLjBL/HX1fgCh00zR29BMwwEGYyWODE NH8WqyB/KEO7ccszet06m0gTF6zdHos3j3PMtJn+8J1EEm3koScoV+ipWB3FQkeLrZlV TIkB1+ZVqz7fKJYPpae8cdNc/fi08lMy6VyipzSazEGV4LiDOEGkQVhDVMBUtQGphMhA 3+7unrkc6TEuyGnuNLmniSlFRvaSrLysM+mbUkrpyMN7qCzrRylFBi1SiKozd1GUVu6f +gew== X-Gm-Message-State: AC+VfDwd+t7m9KPvvitQsVLQTXG+7KtBSd4rFz+2n7B8EdrkMXc03Jp3 zvY+JK6hoWQXhsmmRIYjIPsR1E0SKblpGYfnj+dWcf+ToC4= X-Google-Smtp-Source: ACHHUZ58fNk2zVLJfioDOx4E0jCba+9yvmUmmC54Qe6Ei3lSlpMuqzE0kgBiYovKGnWtMvCyaWLwPpjzK7u+59drPLE= X-Received: by 2002:a0d:dbd6:0:b0:555:cce2:8a16 with SMTP id d205-20020a0ddbd6000000b00555cce28a16mr2574173ywe.22.1682633447013; Thu, 27 Apr 2023 15:10:47 -0700 (PDT) MIME-Version: 1.0 References: <20230427200615.1496059-1-hjl.tools@gmail.com> In-Reply-To: From: "H.J. Lu" Date: Thu, 27 Apr 2023 15:10:11 -0700 Message-ID: Subject: Re: [PATCH] __check_pf: Add a cancellation cleanup handler [BZ #20975] To: Noah Goldstein Cc: libc-alpha@sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-3021.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, Apr 27, 2023 at 3:03=E2=80=AFPM Noah Goldstein wrote: > > On Thu, Apr 27, 2023 at 3:06=E2=80=AFPM H.J. Lu via Libc-alpha > 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 (>=3D 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 cancellati= on > > 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 +=3D \ > > sysdep_routines +=3D \ > > netlink_assert_response \ > > # sysdep_routines > > + > > +CFLAGS-check_pf.c +=3D -fexceptions > > endif > > > > # Don't compile the ctype glue code, since there is no old non-GNU C l= ibrary. > > diff --git a/sysdeps/unix/sysv/linux/check_pf.c b/sysdeps/unix/sysv/lin= ux/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 =3D NULL; > > struct cached_data *data =3D 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? The cleanup handler is called only when thread cancellation happens in __sendto or __recvmsg called from make_request after lock has been taken. The order here isn't critical. > > > __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 !=3D NULL) > > -- > > 2.40.0 > > --=20 H.J.