From owner-freebsd-threads@FreeBSD.ORG Mon Nov 17 10:30:04 2014 Return-Path: Delivered-To: freebsd-threads@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id B6B84447 for ; Mon, 17 Nov 2014 10:30:04 +0000 (UTC) Received: from kenobi.freebsd.org (kenobi.freebsd.org [IPv6:2001:1900:2254:206a::16:76]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 9E924ED0 for ; Mon, 17 Nov 2014 10:30:04 +0000 (UTC) Received: from bugs.freebsd.org ([127.0.1.118]) by kenobi.freebsd.org (8.14.9/8.14.9) with ESMTP id sAHAU4B0098312 for ; Mon, 17 Nov 2014 10:30:04 GMT (envelope-from bugzilla-noreply@freebsd.org) From: bugzilla-noreply@freebsd.org To: freebsd-threads@FreeBSD.org Subject: [Bug 195098] New: pthread lock performance degradation Date: Mon, 17 Nov 2014 10:30:04 +0000 X-Bugzilla-Reason: AssignedTo X-Bugzilla-Type: new X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: Base System X-Bugzilla-Component: threads X-Bugzilla-Version: 10.0-RELEASE X-Bugzilla-Keywords: X-Bugzilla-Severity: Affects Some People X-Bugzilla-Who: ueno@gnu.org X-Bugzilla-Status: Needs Triage X-Bugzilla-Priority: --- X-Bugzilla-Assigned-To: freebsd-threads@FreeBSD.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: bug_id short_desc product version rep_platform op_sys bug_status bug_severity priority component assigned_to reporter attachments.created Message-ID: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Bugzilla-URL: https://bugs.freebsd.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Nov 2014 10:30:04 -0000 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=195098 Bug ID: 195098 Summary: pthread lock performance degradation Product: Base System Version: 10.0-RELEASE Hardware: amd64 OS: Any Status: Needs Triage Severity: Affects Some People Priority: --- Component: threads Assignee: freebsd-threads@FreeBSD.org Reporter: ueno@gnu.org Created attachment 149508 --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=149508&action=edit test case It is known that Gnulib's test for its 'lock' module sometimes takes long time to complete. As the module is a thin wrapper around pthreads, I've rewritten the test with pthreads functions to isolate the problem, and reproduced the same problem. The test basically does: - for 50000 rounds: - initialize a variable PERFORMED to 0 - create 10 threads - in each thread, try to increase PERFORMED by one, through pthread_once - check PERFORMED is 1 The symptoms I observed are: - when the test completes in time, ~1000-3000 rounds are performed per second - when the test takes long time, only ~10 rounds are performed per second I'm attaching the test program, which also prints timestamp for each iteration. See also: https://lists.gnu.org/archive/html/bug-gnulib/2014-09/msg00025.html for the original bug report. -- You are receiving this mail because: You are the assignee for the bug. From owner-freebsd-threads@FreeBSD.ORG Thu Nov 20 18:32:30 2014 Return-Path: Delivered-To: freebsd-threads@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2474B787; Thu, 20 Nov 2014 18:32:30 +0000 (UTC) Received: from 0.mx.codelabs.ru (0.mx.codelabs.ru [144.206.233.71]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id B9D4C610; Thu, 20 Nov 2014 18:32:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=codelabs.ru; s=three; h=Sender:Content-Type:MIME-Version:Message-ID:Subject:Cc:To:From:Date; bh=fW37QGGusbSFQlVdny3KC/GAHL7OBS1Gilalbyw0w+E=; b=fz2EFk0XpodUAYE7gJoHfY30nOmnhKPVXI7ras8ToX8eRiCux3y+zLkRJz5AOqJcGNq7HYb8LsuOf7uBRPuEDgQaE4Nrx8xFlkw9pBMHfn67mSqCNNSUxi8sM9KVaFcXnPKiZ8gYF3WmchoxjTj46Uj5S7UoUeATb4UEfn0fmp9SuGPCtT1/VxrKnTNp2C3b0pFC5JkqcTg5jBcJZvsn19nc3LyarttSMvxB1GwDzpKj9Kn+Z9/aCna8mh9n34uFi3QAOVjIzv2wiqpSyZS360ijxtHYOM8RKdGBuWaUCIRv+Frf/dtK+Sp7qp9Bx9TDe0sTVYrMvkBcmWI5bTNiU+1HfuF2n+iHS5wooO0r0FzMxE8LjGrq79cfIvBqmO6eUTG5Iuo+61JwRd55GyaJFwAKrSIb5DedCmSale+44D0xzQXN+mBvJp4CsH8p9gGjme9vjHl3bHEHna0c0k0ge4yfMBW1nXu4YsHHbtcJDo1xAGgEZ62zS+jl4ckReD/goBO4vVH9twUXPmcplGXTKzQeuXmY4l32Vx5VCyFpTtjoUSngltttvI5PRAuIv5d2E6L+6nZWUvMM+kabGk4xZ5GAf8xIHyCNUU7/wrUkUfB31LQ3rV7nZbnjjru43xbziunRnN0OQCMcW1MWgEpT992v/J3WZSxs4pyFVYR2160=; Received: from void.codelabs.ru (void.codelabs.ru [144.206.233.66]) by 0.mx.codelabs.ru with esmtpsa (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) id 1XrWWj-000JUT-IR; Thu, 20 Nov 2014 22:32:21 +0400 Date: Thu, 20 Nov 2014 21:32:19 +0300 From: Eygene Ryabinkin To: freebsd-threads@FreeBSD.org Subject: [CFR][PATCH] Call proper signal handler from libthr for non-SA_SIGINFO case Message-ID: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="3D7yMlnunRPwJqC7" Content-Disposition: inline Sender: rea@codelabs.ru Cc: kib@FreeBSD.org, davidxu@FreeBSD.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Nov 2014 18:32:30 -0000 --3D7yMlnunRPwJqC7 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Good day. Was hacking on Squid that used to dump core when signal 15 was delivered to it via 'squid -k shutdown' and found out that the reason for core dumps is that thr_sighandler() is _sometimes_ passed 0x10001 as the value of "info" argument despite that _sigaction() always arms flags for __sys_sigaction() with SA_SIGINFO. But looking at handle_signal() I had discovered that if libthr client wants no SA_SIGINFO, then actp->sa_sigaction() will be called, though specs, http://pubs.opengroup.org/onlinepubs/009695399/functions/sigaction.html say that in this case sa_handler should be used. And this is consistent with the non-threaded case. Moreover, the called handler is passed 4 extra arguments, {{{ info->si_code, (struct sigcontext *)ucp, info->si_addr, (__sighandler_t *= )sigfunc); }}} and for my poor Squid this is the root of the problem, because 0x10001 gets dereferenced (as info->SOMETHING), so SIGSEGV gets delivered immediately. By the way, the issue with seamonkey (but not Python) at https://lists.freebsd.org/pipermail/freebsd-current/2013-June/042199.html https://lists.freebsd.org/pipermail/freebsd-current/2013-June/042210.html has the same pattern: inside one invocation of signal handler we see SIGSEGV that can be triggered by this very problem: frame #5 handles initial signal, while frame #4 is in-kernel stuff for SIGSEGV and #3 is its userspace counterpart that takes us into libthr once again, but this time for the different signal, I guess. And this one goes with proper "info", so no more harm is inflicted by libthr's code. The origin of 0x10001 as info isn't yet clear to me, though I have a feeling that it is SI_USER that is slipping somewhere to the wrong place. Will dig further. Anyway, it seems like that the currend code inside handle_signal() should be modified with the patch like http://codelabs.ru/fbsd/patches/libthr/11-CURRENT-fix-sighandler-invocati= on.patch Could anyone, please, review it and, probably, come up with the idea of why the current code is such as it is. Revision when it first appeared is 212076, https://svnweb.freebsd.org/base/head/lib/libthr/thread/thr_sig.c?r1=3D211= 737&r2=3D212076&view=3Dpatch Thanks. PS: I am not subscribed to the list, so, please, keep me CC'ed. --=20 Eygene Ryabinkin ,,,^..^,,, [ Life's unfair - but root password helps! | codelabs.ru ] [ 82FE 06BC D497 C0DE 49EC 4FF0 16AF 9EAE 8152 ECFB | freebsd.org ] --3D7yMlnunRPwJqC7 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iL4EABEKAGYFAlRuM7NfFIAAAAAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3Bl bnBncC5maWZ0aGhvcnNlbWFuLm5ldDgyRkUwNkJDRDQ5N0MwREU0OUVDNEZGMDE2 QUY5RUFFODE1MkVDRkIACgkQFq+eroFS7PvMKwD+M1RGSFvorQD2H+GGSPSVr7Fo 4A3B8LzBRiEVcCv83AEBAIKsbLuoEPwoAjFNLu9DB+4TW+DC3o2S0sjsdiZhRbPg =85DT -----END PGP SIGNATURE----- --3D7yMlnunRPwJqC7-- From owner-freebsd-threads@FreeBSD.ORG Thu Nov 20 19:49:36 2014 Return-Path: Delivered-To: freebsd-threads@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 5A63A75A; Thu, 20 Nov 2014 19:49:36 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id D7F6F1EE; Thu, 20 Nov 2014 19:49:35 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id sAKJnPCS038672 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 20 Nov 2014 21:49:25 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua sAKJnPCS038672 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id sAKJnPNG038671; Thu, 20 Nov 2014 21:49:25 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 20 Nov 2014 21:49:25 +0200 From: Konstantin Belousov To: Eygene Ryabinkin Subject: Re: [CFR][PATCH] Call proper signal handler from libthr for non-SA_SIGINFO case Message-ID: <20141120194925.GK17068@kib.kiev.ua> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: davidxu@FreeBSD.org, freebsd-threads@FreeBSD.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Nov 2014 19:49:36 -0000 On Thu, Nov 20, 2014 at 09:32:19PM +0300, Eygene Ryabinkin wrote: > Good day. > > Was hacking on Squid that used to dump core when signal 15 was > delivered to it via 'squid -k shutdown' and found out that the reason > for core dumps is that thr_sighandler() is _sometimes_ passed 0x10001 > as the value of "info" argument despite that _sigaction() always arms > flags for __sys_sigaction() with SA_SIGINFO. > > But looking at handle_signal() I had discovered that if libthr client > wants no SA_SIGINFO, then actp->sa_sigaction() will be called, though > specs, > http://pubs.opengroup.org/onlinepubs/009695399/functions/sigaction.html > say that in this case sa_handler should be used. And this is consistent > with the non-threaded case. The sa_sigaction and sa_handler live in the same bits, look at the union in the definition of the struct sigaction in sys/signal.h. It is the same pointer, cast to different functions signature depending on SA_SIGSINFO. > > Moreover, the called handler is passed 4 extra arguments, > {{{ > info->si_code, (struct sigcontext *)ucp, info->si_addr, (__sighandler_t *)sigfunc); > }}} The additional arguments for sa_handler is traditional BSD behaviour. All ABIs allow the undeclared arguments after the required, so it works. > and for my poor Squid this is the root of the problem, because 0x10001 > gets dereferenced (as info->SOMETHING), so SIGSEGV gets delivered > immediately. By the way, the issue with seamonkey (but not Python) at > https://lists.freebsd.org/pipermail/freebsd-current/2013-June/042199.html > https://lists.freebsd.org/pipermail/freebsd-current/2013-June/042210.html > has the same pattern: inside one invocation of signal handler we see > SIGSEGV that can be triggered by this very problem: frame #5 handles > initial signal, while frame #4 is in-kernel stuff for SIGSEGV and > #3 is its userspace counterpart that takes us into libthr once again, > but this time for the different signal, I guess. And this one goes > with proper "info", so no more harm is inflicted by libthr's code. Except the same pattern, which really boils down to the fact that fault happens in the signal handler code, I do not see anything similar. > > > The origin of 0x10001 as info isn't yet clear to me, though I have > a feeling that it is SI_USER that is slipping somewhere to the wrong > place. Will dig further. It is not clear to me what your problem is, at all. Compile the faulting program with the debugging information, possibly compile libc/libthr/ld-elf.so with debugging as well, and obtain backtrace from the coredump. I briefly looked at the squid 3.4.9 source code, and one outstanding thing there is the use of streams in the signal handler. Since signals are async, and there is no masking of signals around << calls in the code, I would be not much surprised that the cause is the interruption of another <<. > > > Anyway, it seems like that the currend code inside handle_signal() > should be modified with the patch like > http://codelabs.ru/fbsd/patches/libthr/11-CURRENT-fix-sighandler-invocation.patch > I do not see how the patch could have any effect on the issue mentioned, and it definitely introduces ABI incompatibility (bug) by removing the traditional arguments. > Could anyone, please, review it and, probably, come up with the idea of > why the current code is such as it is. Revision when it first appeared > is 212076, > https://svnweb.freebsd.org/base/head/lib/libthr/thread/thr_sig.c?r1=211737&r2=212076&view=patch > > Thanks. > > > PS: I am not subscribed to the list, so, please, keep me CC'ed. > -- > Eygene Ryabinkin ,,,^..^,,, > [ Life's unfair - but root password helps! | codelabs.ru ] > [ 82FE 06BC D497 C0DE 49EC 4FF0 16AF 9EAE 8152 ECFB | freebsd.org ] From owner-freebsd-threads@FreeBSD.ORG Fri Nov 21 16:07:21 2014 Return-Path: Delivered-To: freebsd-threads@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 3672BF66; Fri, 21 Nov 2014 16:07:21 +0000 (UTC) Received: from 0.mx.codelabs.ru (0.mx.codelabs.ru [144.206.233.71]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C6AA0334; Fri, 21 Nov 2014 16:07:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=codelabs.ru; s=three; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=0Naqu0nVafz1FtfCYPX2VdrqJUecWdH1tErmzG3pKug=; b=mkgakdqGz8+v9kYlh9n9YtwGh917mlID7bsLbSr3qXf9b3AYa/bRIQbm2aWuTyW7seH515/OoNTn5Gz2xjC82/wgl9MXMs9W5erEV2YvKs9EIqUzCOikqndKX4NHL8raM0kvhVqNRL7OphxH+Cd13dk/nL6sKhNYMOM002YaQj6fHJ92+DSRZTID9UYjgWZto4hTi7VNqk3DKrV2YD3X7C62ZF78PwbEmuen+5dfS/IeyjTa3KKjIZvmD9sKI6+aIVkzToleK7cWoRoTRF83ieAnQ6TXs4w/+QjFHKZn0ToPdinBOJE/xAGZfiu0MAzO4UWSAF5zdnKV5nj31uGoLgvYJuvg8WNBPgSEmyg1NWbyRnsJOqB+rHDJc+3ndcduM9/Fw0On68traMwOUIDRN9aE5aTOvaaojFBGPqEbl6+YT20mhIzI/YmZ6as531KJo0kZGqr0UwXMbjZa9iT3PTZWe5s8J6BdvkYbLW3hDC1/izQ9BgkpoC75g4JclifqO9Vfm/wYaQ2RF3aEGuNefYJqUO7NqMFI6+4/gGEsxz83nhiM0n3bI0BNcu6/2Sar01782fX8rJRv2cGSHOyK+TFwd+S4YWNzfuWgNMF870bWsr3hnnByMpl7AiJaZgnqv3P6kelLpS8E/WiJJ07+z1DvCpnBAtk2Zq1IM6qqyTY=; Received: from void.codelabs.ru (void.codelabs.ru [144.206.233.66]) by 0.mx.codelabs.ru with esmtpsa (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) id 1Xrqjs-000Ina-US; Fri, 21 Nov 2014 20:07:17 +0400 Date: Fri, 21 Nov 2014 19:07:14 +0300 From: Eygene Ryabinkin To: Konstantin Belousov Subject: Re: [CFR][PATCH] Call proper signal handler from libthr for non-SA_SIGINFO case Message-ID: References: <20141120194925.GK17068@kib.kiev.ua> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="LXqH7sWsIplhrbjF" Content-Disposition: inline In-Reply-To: <20141120194925.GK17068@kib.kiev.ua> Sender: rea@codelabs.ru X-Content-Filtered-By: Mailman/MimeDel 2.1.18-1 Cc: davidxu@FreeBSD.org, freebsd-threads@FreeBSD.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Nov 2014 16:07:21 -0000 --LXqH7sWsIplhrbjF Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Konstantin, good day. Thu, Nov 20, 2014 at 09:49:25PM +0200, Konstantin Belousov wrote: > On Thu, Nov 20, 2014 at 09:32:19PM +0300, Eygene Ryabinkin wrote: > > Was hacking on Squid that used to dump core when signal 15 was > > delivered to it via 'squid -k shutdown' and found out that the reason > > for core dumps is that thr_sighandler() is _sometimes_ passed 0x10001 > > as the value of "info" argument despite that _sigaction() always arms > > flags for __sys_sigaction() with SA_SIGINFO. > >=20 > > But looking at handle_signal() I had discovered that if libthr client > > wants no SA_SIGINFO, then actp->sa_sigaction() will be called, though > > specs, > > http://pubs.opengroup.org/onlinepubs/009695399/functions/sigaction.ht= ml > > say that in this case sa_handler should be used. And this is consistent > > with the non-threaded case. > > The sa_sigaction and sa_handler live in the same bits, look at the > union in the definition of the struct sigaction in sys/signal.h. It is > the same pointer, cast to different functions signature depending on > SA_SIGSINFO. Sorry, should have been studied sigaction(2) more carefully: everything turned out to be written there. So, libthr is fine here. > > The origin of 0x10001 as info isn't yet clear to me, though I have > > a feeling that it is SI_USER that is slipping somewhere to the wrong > > place. Will dig further. > > It is not clear to me what your problem is, at all. The problem is that libthr's _sigaction() proxies sa_flags, so if SA_RESETHAND was specified by the caller, it will also be passed to the kernel when thr_sighandler() is installed. And since sa_flags are equipped with SA_SIGINFO inside _sigaction(), thr_sighandler() and handle_signal() always expect to be called with POSIX SA_SIGINFO semantics. This isn't the case for SA_RESETHAND, because sigdflt() from kern/kern_sig.c will be called before mach-dependent sv_sendsig vector, so {{{ SIGDELSET(ps->ps_siginfo, sig) }}} will be issued. Thus, any code inside mach-dependent handler won't see ps_siginfo, so will always use traditional semantics. This explains why I see 0x10001 as "siginfo_t *info": it is just "int code" for the traditional case and the signal in question is really produced by userland, so it is SI_USER. So, it looks like a kernel bug (since if we request SA_SIGINFO, we should get the proper handler to be called even for the SA_RESETHAND case). I see two possibilities: - invoke SA_RESETHAND processing inside mach-dependent code; that's a kind of ugly and makes mach-specific code to deal with the generic signal handling logics; - pass information about SA_SIGINFO "out-of-band" (not in ps->ps_siginfo). We can't postpone sigdflt() to be called after signal being delivered, since spec requires it to be done before calling user-space handler. Had created a patch that adds 4th argument to sv_sendsig and fixes this problem: http://codelabs.ru/fbsd/patches/libthr/11-CURRENT-fix-SIGINFO-processing-= with-RESETHAND.diff This changes internal KABI, but hopefully sv_sendsig is an internal kernel interface that isn't used by anything else outside kern_sig.c. It works fine with virgin libthr and solves Squid restart problem. Will try to install it to more test machines and see if it will work as expected. Seems like a kernel regression test like the attached one will also be handy. --=20 Eygene Ryabinkin ,,,^..^,,, [ Life's unfair - but root password helps! | codelabs.ru ] [ 82FE 06BC D497 C0DE 49EC 4FF0 16AF 9EAE 8152 ECFB | freebsd.org ] --LXqH7sWsIplhrbjF Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iL4EABEKAGYFAlRvYzJfFIAAAAAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3Bl bnBncC5maWZ0aGhvcnNlbWFuLm5ldDgyRkUwNkJDRDQ5N0MwREU0OUVDNEZGMDE2 QUY5RUFFODE1MkVDRkIACgkQFq+eroFS7PuBvAD/XrhsUYpvMRgUJXW6W22OGN9R EKZYqQhKw8IyFYl1NLkA/0FOEjikQJWMA1SH6BB3h+3osS7e0AQkS2cwOAulpUM7 =RnhA -----END PGP SIGNATURE----- --LXqH7sWsIplhrbjF-- From owner-freebsd-threads@FreeBSD.ORG Fri Nov 21 16:57:04 2014 Return-Path: Delivered-To: freebsd-threads@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 9B7BB9E4; Fri, 21 Nov 2014 16:57:04 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 255A8B7A; Fri, 21 Nov 2014 16:57:03 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id sALGuwCq020767 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 21 Nov 2014 18:56:58 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua sALGuwCq020767 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id sALGuwN5020766; Fri, 21 Nov 2014 18:56:58 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 21 Nov 2014 18:56:58 +0200 From: Konstantin Belousov To: Eygene Ryabinkin Subject: Re: [CFR][PATCH] Call proper signal handler from libthr for non-SA_SIGINFO case Message-ID: <20141121165658.GP17068@kib.kiev.ua> References: <20141120194925.GK17068@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: davidxu@FreeBSD.org, freebsd-threads@FreeBSD.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Nov 2014 16:57:04 -0000 On Fri, Nov 21, 2014 at 07:07:14PM +0300, Eygene Ryabinkin wrote: > The problem is that libthr's _sigaction() proxies sa_flags, so if > SA_RESETHAND was specified by the caller, it will also be passed to > the kernel when thr_sighandler() is installed. And since sa_flags are > equipped with SA_SIGINFO inside _sigaction(), thr_sighandler() and > handle_signal() always expect to be called with POSIX SA_SIGINFO > semantics. > > This isn't the case for SA_RESETHAND, because sigdflt() from > kern/kern_sig.c will be called before mach-dependent sv_sendsig > vector, so > {{{ > SIGDELSET(ps->ps_siginfo, sig) > }}} > will be issued. Thus, any code inside mach-dependent handler won't > see ps_siginfo, so will always use traditional semantics. This > explains why I see 0x10001 as "siginfo_t *info": it is just "int code" > for the traditional case and the signal in question is really produced > by userland, so it is SI_USER. Great, thank you for tracking this down. > > So, it looks like a kernel bug (since if we request SA_SIGINFO, we > should get the proper handler to be called even for the SA_RESETHAND > case). I see two possibilities: > > - invoke SA_RESETHAND processing inside mach-dependent code; that's > a kind of ugly and makes mach-specific code to deal with the > generic signal handling logics; > > - pass information about SA_SIGINFO "out-of-band" (not in > ps->ps_siginfo). > > We can't postpone sigdflt() to be called after signal being delivered, > since spec requires it to be done before calling user-space handler. No, userspace is not called from sv_sendsig(), it simply cannot work because nested signals would cause nesting on the kernel stack frame. sv_sendsig() only prepares the usermode signal stack frame and arranges the saved thread CPU state so that on return from kernel to usermode, the signal handler is executed. And, amusingly, the only thing which sv_sendsig() methods need and which is also touched by sigdflt(), is ps_siginfo. Simply rearranging the order of calls should be enough. I put the patch at the end of message, it worked for me. > > Had created a patch that adds 4th argument to sv_sendsig and fixes > this problem: > http://codelabs.ru/fbsd/patches/libthr/11-CURRENT-fix-SIGINFO-processing-with-RESETHAND.diff > This changes internal KABI, but hopefully sv_sendsig is an internal > kernel interface that isn't used by anything else outside kern_sig.c. > > It works fine with virgin libthr and solves Squid restart problem. > Will try to install it to more test machines and see if it will work > as expected. Seems like a kernel regression test like the attached > one will also be handy. There are people who develop the regression suite(s) for FreeBSD. Might be, they would note your test, may be, you should somehow contact them and propose the inclusion of the test into suite. I believe it is freebsd-testing@f.o. The test would require adoption to the framework. diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index 5cdc2ce..2c3cc80 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -2863,14 +2863,14 @@ postsig(sig) kern_sigprocmask(td, SIG_BLOCK, &mask, NULL, SIGPROCMASK_PROC_LOCKED | SIGPROCMASK_PS_LOCKED); - if (SIGISMEMBER(ps->ps_sigreset, sig)) - sigdflt(ps, sig); td->td_ru.ru_nsignals++; if (p->p_sig == sig) { p->p_code = 0; p->p_sig = 0; } (*p->p_sysent->sv_sendsig)(action, &ksi, &returnmask); + if (SIGISMEMBER(ps->ps_sigreset, sig)) + sigdflt(ps, sig); } return (1); } From owner-freebsd-threads@FreeBSD.ORG Fri Nov 21 19:56:21 2014 Return-Path: Delivered-To: freebsd-threads@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id BAB6E6DB; Fri, 21 Nov 2014 19:56:21 +0000 (UTC) Received: from 0.mx.codelabs.ru (0.mx.codelabs.ru [144.206.233.71]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 5A6C31AD; Fri, 21 Nov 2014 19:56:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=codelabs.ru; s=three; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=8LnyTYpL/8fg4RYqxwFKgZcmgC4dzt6srtq/243EkDc=; b=JzB4UJ257C7bNtoRDuvj2wa2MgnTn/gzRDUh34J4TbH7YkYrVNvHTCKHSsxdsJt4NiM/GSzDbrY4Y1nl4DCCwV3mxIsO+sE9Ar1gdsX3ngHkS+MxyMCVpBTgQPk3WxTCBVkgvkkgRuFO4VgOPBAn4ujPtEAg1gexOYApT2PY6IC2/n11UEazxEmxQmDdrzKUX6PqM5c+wm9QPw48eDEYIc+h2iuspXpz/mGT8Kn/S3CHbR2i+ytvVnSq/uIrnREtKOWE/ENb//C7K+D6OghxFduPDQ1Esescmxno/QD+HawIf+njODyhJOPHP0G5VSc2Pm1y8Qf5Qh5reUoQkw4/j9pK6AEzgIZRXpOoO1fSs+koAoK5cEgmH7rEEDFTN9zR/OcePI7mSRWU5O23A1sfFYkq7H35WgyLUUxtiHtOI/FXCk6Xp6i5hBbhRJl8BOHkcMMkHCzjYYpU9jf/BMXSrlBqsFh1tQQ3+X7/+xghM1GO9pz/Kls49JtINpszeK59NekqltW2ld9qTqowDeomlx0z87yYhSkLirc9rJ2eL3n0Z++4KjUaQfsblDyhvM3XzMpKxtU63gQHRBH4ILImb0AgrxfKtk8nGeFD87uKwLycmOLYSDQx8HbU6e5A3kbCiEpRYtf7JIAFMrkml/ZRlsvgusEchdRnX9hyYBIIz5U=; Received: from light.codelabs.ru (v-light.codelabs.ru [144.206.233.83]) by 0.mx.codelabs.ru with esmtpsa (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) id 1XruJW-000Nkm-SW; Fri, 21 Nov 2014 23:56:19 +0400 Date: Fri, 21 Nov 2014 22:55:55 +0300 From: Eygene Ryabinkin To: Konstantin Belousov Subject: Re: [CFR][PATCH] Call proper signal handler from libthr for non-SA_SIGINFO case Message-ID: References: <20141120194925.GK17068@kib.kiev.ua> <20141121165658.GP17068@kib.kiev.ua> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="tsOsTdHNUZQcU9Ye" Content-Disposition: inline In-Reply-To: <20141121165658.GP17068@kib.kiev.ua> Sender: rea@codelabs.ru Cc: davidxu@FreeBSD.org, freebsd-threads@FreeBSD.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Nov 2014 19:56:21 -0000 --tsOsTdHNUZQcU9Ye Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Fri, Nov 21, 2014 at 06:56:58PM +0200, Konstantin Belousov wrote: > On Fri, Nov 21, 2014 at 07:07:14PM +0300, Eygene Ryabinkin wrote: > > So, it looks like a kernel bug (since if we request SA_SIGINFO, we > > should get the proper handler to be called even for the SA_RESETHAND > > case). I see two possibilities: > >=20 > > - invoke SA_RESETHAND processing inside mach-dependent code; that's > > a kind of ugly and makes mach-specific code to deal with the > > generic signal handling logics; > >=20 > > - pass information about SA_SIGINFO "out-of-band" (not in > > ps->ps_siginfo). > >=20 > > We can't postpone sigdflt() to be called after signal being delivered, > > since spec requires it to be done before calling user-space handler. > > No, userspace is not called from sv_sendsig(), it simply cannot work > because nested signals would cause nesting on the kernel stack frame. > sv_sendsig() only prepares the usermode signal stack frame and arranges > the saved thread CPU state so that on return from kernel to usermode, > the signal handler is executed. So, if we rearrange calls to sv_sendsig() and sigdflt() there can't be cases when the process being signalled will pick the signal before sigdflt() will be called? Just because XSI wants that handler reset and clearing of SIGINFO happen before handler execution, http://pubs.opengroup.org/onlinepubs/009695399/functions/sigaction.html So, I went into more trouble of not touching the order. > And, amusingly, the only thing which sv_sendsig() methods need and which > is also touched by sigdflt(), is ps_siginfo. Simply rearranging the > order of calls should be enough. I put the patch at the end of message, > it worked for me. If there is no change in behaviour that will arise from rearranging the order of calls to mach-dependent and mach-independent code, I'd go a bit firther and unify some repeated code, http://codelabs.ru/fbsd/patches/libthr/11-CURRENT-fix-SIGINFO-processing-= with-RESETHAND-v2.diff Works for me too, just tested with the same Squid installation. > > It works fine with virgin libthr and solves Squid restart problem. > > Will try to install it to more test machines and see if it will work > > as expected. Seems like a kernel regression test like the attached > > one will also be handy. >=20 > There are people who develop the regression suite(s) for FreeBSD. > Might be, they would note your test, may be, you should somehow contact > them and propose the inclusion of the test into suite. I believe > it is freebsd-testing@f.o. The test would require adoption to the > framework. Yep, will try to catch this train. Thanks! --=20 Eygene Ryabinkin ,,,^..^,,, [ Life's unfair - but root password helps! | codelabs.ru ] [ 82FE 06BC D497 C0DE 49EC 4FF0 16AF 9EAE 8152 ECFB | freebsd.org ] --tsOsTdHNUZQcU9Ye Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (FreeBSD) iL4EABEKAGYFAlRvmMtfFIAAAAAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3Bl bnBncC5maWZ0aGhvcnNlbWFuLm5ldDgyRkUwNkJDRDQ5N0MwREU0OUVDNEZGMDE2 QUY5RUFFODE1MkVDRkIACgkQFq+eroFS7PsUjwD9E4An7To8hTMjMj/AB0U9CaIU zMOtGJIujMTyXOWOFwUA/29vGNznci41I8RI2hFoZENcJtxjPr0nvABZyevYs7Yx =SAqx -----END PGP SIGNATURE----- --tsOsTdHNUZQcU9Ye-- From owner-freebsd-threads@FreeBSD.ORG Fri Nov 21 20:32:34 2014 Return-Path: Delivered-To: freebsd-threads@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 889C02F2; Fri, 21 Nov 2014 20:32:34 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 1047F874; Fri, 21 Nov 2014 20:32:33 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id sALKWSZj068737 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 21 Nov 2014 22:32:28 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua sALKWSZj068737 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id sALKWSDh068736; Fri, 21 Nov 2014 22:32:28 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 21 Nov 2014 22:32:27 +0200 From: Konstantin Belousov To: Eygene Ryabinkin Subject: Re: [CFR][PATCH] Call proper signal handler from libthr for non-SA_SIGINFO case Message-ID: <20141121203227.GS17068@kib.kiev.ua> References: <20141120194925.GK17068@kib.kiev.ua> <20141121165658.GP17068@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: davidxu@FreeBSD.org, freebsd-threads@FreeBSD.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Nov 2014 20:32:34 -0000 On Fri, Nov 21, 2014 at 10:55:55PM +0300, Eygene Ryabinkin wrote: > Fri, Nov 21, 2014 at 06:56:58PM +0200, Konstantin Belousov wrote: > > On Fri, Nov 21, 2014 at 07:07:14PM +0300, Eygene Ryabinkin wrote: > > > So, it looks like a kernel bug (since if we request SA_SIGINFO, we > > > should get the proper handler to be called even for the SA_RESETHAND > > > case). I see two possibilities: > > > > > > - invoke SA_RESETHAND processing inside mach-dependent code; that's > > > a kind of ugly and makes mach-specific code to deal with the > > > generic signal handling logics; > > > > > > - pass information about SA_SIGINFO "out-of-band" (not in > > > ps->ps_siginfo). > > > > > > We can't postpone sigdflt() to be called after signal being delivered, > > > since spec requires it to be done before calling user-space handler. > > > > No, userspace is not called from sv_sendsig(), it simply cannot work > > because nested signals would cause nesting on the kernel stack frame. > > sv_sendsig() only prepares the usermode signal stack frame and arranges > > the saved thread CPU state so that on return from kernel to usermode, > > the signal handler is executed. > > So, if we rearrange calls to sv_sendsig() and sigdflt() there can't be > cases when the process being signalled will pick the signal before > sigdflt() will be called? Just because XSI wants that handler reset > and clearing of SIGINFO happen before handler execution, > http://pubs.opengroup.org/onlinepubs/009695399/functions/sigaction.html > So, I went into more trouble of not touching the order. We do not return into usermode in random places of the kernel. As I explained, signal delivery is done by arranging normal return to usermode to effectively return to new signal frame. > > > And, amusingly, the only thing which sv_sendsig() methods need and which > > is also touched by sigdflt(), is ps_siginfo. Simply rearranging the > > order of calls should be enough. I put the patch at the end of message, > > it worked for me. > > If there is no change in behaviour that will arise from rearranging > the order of calls to mach-dependent and mach-independent code, > I'd go a bit firther and unify some repeated code, > http://codelabs.ru/fbsd/patches/libthr/11-CURRENT-fix-SIGINFO-processing-with-RESETHAND-v2.diff > > Works for me too, just tested with the same Squid installation. This looks correct, but is much more delicate change. In particular, the signal mask copied to usermode as part of ucontext, for restoration at sigreturn(2), seems to be correct in both cases, but in the postsig() case, where sendsig_common() is put after sv_sendsig() call, it depends on the returnmask calculation. Can you test that signal mask after signal return is correct with your patch ? Two other notes about your patch, which should be changed before it is committable. First, the name sendsig_common() is not telling anything. Might be, rename the function to postsig_sigprop() or like. In the same vein, comment is too ambitious for small helper. This is not The common code, just a helper to handle thread signal mask and several other details of delivery. Just specifying that this is the thing to call after sysent->sv_sendsig() to arrange for proper bookkeeping, is enough. Second, function needs asserts that process lock and sigact mutex (ps_mtx) are locked.