From owner-freebsd-threads@FreeBSD.ORG Wed Nov 26 22:12:38 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 622F02DE; Wed, 26 Nov 2014 22:12:38 +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 00E0AB55; Wed, 26 Nov 2014 22:12:37 +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=OXLNRk8KSA6NH8Lit0offkPZ6TWuhShwHkSAY+9TMJQ=; b=HvLrniC/5+eEHbyls7bkB45I7FG2SRtfOIzbF92tkcG9uvsewJN5vcSZQeJ8GTcSVzT24uZSS7MdOInuuZHjUeF9cu2p526fm0gNgmtSMzGOWYx24oP/a3bg6o2o28I6EwxONJXV2NX2coJTl5J+v0DYD8tHGgoYKKhMVllr5NpwiGZ7JqXWWTn1DcBm3/8i2yL6r3L+Hk0Pwj2j3yYej0Mm8cDJQjRwHaCm53G/Iov22AHxdyeJ4Azy62sYQFEjS13gkOcR6GCuht7rEL3jmGNgwSwXgG7hcwNGxxkPhES8lUylR8GjrjwFcZSdPXIbO7wv0dThp9jwwhOjY/BfG8hbPXNDGq5lD5zkpRPhhY4ZiOu4kAkjl2tqp/RvZPPz/DJr/dPrckiN80KDplRcPMWmFceGqAHcTuJBHCa0NnNAGE+Xp0BQEzVP5ErXLLpUcUmjMocx8y2+wx8P8cWmUh0Ob8vkD/G94me48x7fGHFrCRz4awuhhP00hIbIIOWj2ehowmRY9lXLupPADdRk86EplqLaqfV1nmw9nEbzebhaUMRTa+GCbsiznedcU0lNqrPSM710lU835BsIzYSN6InjNPIsW0LtvWBw+FjjvWZ0ohGpsdxzQDbEGDlf1CXlg2SfZGpSDRrMnBbl+HRht5j22AdxXjECWwp+HkeerWU=; 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 1Xtkp9-000C3m-2v; Thu, 27 Nov 2014 02:12:35 +0400 Date: Thu, 27 Nov 2014 01:12:31 +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> <20141121203227.GS17068@kib.kiev.ua> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="DocE+STaALJfprDB" Content-Disposition: inline In-Reply-To: <20141121203227.GS17068@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: Wed, 26 Nov 2014 22:12:38 -0000 --DocE+STaALJfprDB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Konstantin, good day. Fri, Nov 21, 2014 at 10:32:27PM +0200, Konstantin Belousov wrote: > On Fri, Nov 21, 2014 at 10:55:55PM +0300, Eygene Ryabinkin wrote: > > 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-process= ing-with-RESETHAND-v2.diff > >=20 > > 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. Since in original (unmodified) code returnmask is calculated before the call to kern_sigprocmask(), {{{ if (td->td_pflags & TDP_OLDMASK) { returnmask =3D td->td_oldsigmask; td->td_pflags &=3D ~TDP_OLDMASK; } else returnmask =3D td->td_sigmask; mask =3D ps->ps_catchmask[_SIG_IDX(sig)]; if (!SIGISMEMBER(ps->ps_signodefer, sig)) SIGADDSET(mask, 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 =3D=3D sig) { p->p_code =3D 0; p->p_sig =3D 0; } (*p->p_sysent->sv_sendsig)(action, &ksi, &returnmask); }}} and kern_sigprocmask() only touches td->td_sigmask, such change should be safe, because mach-dependent handlers do not use or manipulate td_sigmask. This looks logical, because we want to pass current original signal mask for the thread to sv_sendsig() to make it to be restored after signal handler's work. So returnmask is to be calculated before mangling and should not depend on it. > Can you test that signal mask after signal return is correct with your > patch? Tested with the following code: http://codelabs.ru/fbsd/patches/libthr/kern-sig-test-sigmask-restoration.c All tested masks are restored properly both for modified and unmodified kernels. > 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. >=20 > 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. Changed it to sendsig_adjust that should carry more sense. Also modified comment. > Second, function needs asserts that process lock and sigact mutex > (ps_mtx) are locked. Added them too. New patch is at http://codelabs.ru/fbsd/patches/libthr/11-CURRENT-fix-SIGINFO-processing-= with-RESETHAND-v3.diff 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 ] --DocE+STaALJfprDB Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (FreeBSD) iL4EABEKAGYFAlR2UE9fFIAAAAAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3Bl bnBncC5maWZ0aGhvcnNlbWFuLm5ldDgyRkUwNkJDRDQ5N0MwREU0OUVDNEZGMDE2 QUY5RUFFODE1MkVDRkIACgkQFq+eroFS7Pv3mgD/ewMG9CFOtosyKsUxdNk5ooZ1 UpypzuLV4ZBzyWT7ZQMA/1XU/oFd/jwkQYTSZnfBY4zXZeqwWyVhKuNNKOAX9GyZ =Dqzb -----END PGP SIGNATURE----- --DocE+STaALJfprDB--