From owner-freebsd-threads@FreeBSD.ORG Fri Nov 28 10:28:20 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 3DFF3E86; Fri, 28 Nov 2014 10:28:20 +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 BE477AA; Fri, 28 Nov 2014 10:28:19 +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=CexCYhGE/NmttifFPd1supGrNXBEkDoS0vR044nuYPk=; b=w6ii1//IgDtAOSc+OQSyvhzyIIcTUoVuqy9/o7LVCrM1pifiijfxA4vk9IBik99yCXewhVmdnIsZMdnbYp0ugADLPUjdPPmmKYQSsU7o4cZvhUfSBBxhOSbEHLhBiz1fFLbtqqzm/Prho9R+qbyh534tNyovc+Z6ktiYxxrGaJgdEgqbZQErHN8pRkG8mRF4eA6AptZY+yptLOb7MxZ+FakkTM9IfWrYA+++pjmrz52VsDrjF5XTM+cAoDoOJl1z+1UWJN3blc2fWhexSYJxK3PlIIpJepBERwXtqNpUjpKJqjsfIAztp0vpxF6E5JUIQGQaERpryUldVMcuKXh8grOGr+ADEUg0Rs9DUX5kMR2Ob4bwKS+dEG1YA0JxykGAgxeIQjFEQN2MrT2sdBhxjjUp0rExlsxkAvi8sXcxFrn0rJWt3i5mU1gg1m59gOu0hGBpA4yh9f1iZ1VGYiJ35JVeWyngsKTQ83CKZohWb0PLlm8w2+XSZYx0Ld/wuvmJBYpqZvm2pBasSD1TGiZT5j53sjz6qfuWf+T6TlQ8V1Q+Eo8MPuzz+wtdlLgcFLlRkccU+ZpZEMETJjwTF1vzt06Cj0SFtKOqs6DKjB3J6Ra7OB6WUk0W1u43YJlPLuNNNhaxkYpv0/RdP6YjR6ALrrXi8OAYkonOeVliBciTgRE=; 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 1XuIme-0001lL-Ey; Fri, 28 Nov 2014 14:28:16 +0400 Date: Fri, 28 Nov 2014 13:28: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: <20141121165658.GP17068@kib.kiev.ua> <20141121203227.GS17068@kib.kiev.ua> <20141126221734.GM17068@kib.kiev.ua> <20141127101405.GP17068@kib.kiev.ua> <20141128101312.GT17068@kib.kiev.ua> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="mL5prfgbrp62Xbpc" Content-Disposition: inline In-Reply-To: <20141128101312.GT17068@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, 28 Nov 2014 10:28:20 -0000 --mL5prfgbrp62Xbpc Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Fri, Nov 28, 2014 at 12:13:12PM +0200, Konstantin Belousov wrote: > On Thu, Nov 27, 2014 at 08:14:09PM +0300, Eygene Ryabinkin wrote: > > But sendsig_done() calls kern_sigprocmask with > > (SIGPROCMASK_PROC_LOCKED | SIGPROCMASK_PS_LOCKED) thus guaranteeing > > some set of locks, so its better to have some ways to support this > > assertion. I understand that the current implementation of > > kern_sigprocmask() will test such an assertion, but things may change > > with time. > > kern_sigprocmask (will) assert this on its own. (will =3D=3D after the pa= tch > is committed, which I am going to do right now). kern_sigprocmask() will. But calling something with such set of flags (that guarantee that both locks are taken), but not checking for one of the locks that it is really so it a bit inconsistent. > > These are good, but, probably, you can do something like > > {{{ > > PROC_LOCK_ASSERT(p, (flags & SIGPROCMASK_PROC_LOCKED) ? MA_OWNED : MA_N= OTOWNED) > > }}} > > inside kern_sigprocmask to have a bit stronger assertion that covers > > unlocked case. > This is not needed. The process lock is not recursive, so the mere call > to acquire the lock triggers the assertion on recursive acquire. Got it, thanks! > > And, being in the nit-picking mode, I'd apply the following patch > > {{{ > > Index: sys/kern/kern_sig.c > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > --- sys/kern/kern_sig.c (revision 275189) > > +++ sys/kern/kern_sig.c (working copy) > > @@ -1043,7 +1043,7 @@ > > * signal, possibly waking it up. > > */ > > if (p->p_numthreads !=3D 1) > > - reschedule_signals(p, new_block, flags); > > + reschedule_signals(p, new_block, flags & SIGPROCMASK_PS_LOCKED); > > } > > =20 > > out: > > }}} > > because reschedule_signals() is interested only in SIGPROCMASK_PS_LOCKE= D, > > so there is no reason to pass other bits, since it may lead to some > > confusion. >=20 > I disagree, I wrote reschedule_signals() to take the same set of > flags as kern_sigprocmask().=20 Since reschedule_signals() now unconditionally needs PROC_LOCK to be owned, it doesn't care for at least SIGPROCMASK_PROC_LOCKED bit in flags. And, for example, if you'll pass the flags to any routine called from reschedule_signals() that checks for SIGPROCMASK_PROC_LOCKED and acquires the lock when this bit is deasserted, it will loudly (as you explained above) fail trying to take non-recursive lock for the case when original flags passed to reschedule_signals() have no SIGPROCMASK_PROC_LOCKED bit enabled. --=20 Eygene Ryabinkin ,,,^..^,,, [ Life's unfair - but root password helps! | codelabs.ru ] [ 82FE 06BC D497 C0DE 49EC 4FF0 16AF 9EAE 8152 ECFB | freebsd.org ] --mL5prfgbrp62Xbpc Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iL4EABEKAGYFAlR4Tj5fFIAAAAAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3Bl bnBncC5maWZ0aGhvcnNlbWFuLm5ldDgyRkUwNkJDRDQ5N0MwREU0OUVDNEZGMDE2 QUY5RUFFODE1MkVDRkIACgkQFq+eroFS7PviaQD/eWt9cHqUA7w+i5hfeLlWijaT D7JBplMN2UIqUxosgSAA/1yu4BGZmHkGYpPsZJPOATdhu5SBche2MQbiEpfaUSaH =WvM8 -----END PGP SIGNATURE----- --mL5prfgbrp62Xbpc--