Date: Fri, 28 Nov 2014 13:28:14 +0300 From: Eygene Ryabinkin <rea@freebsd.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: davidxu@FreeBSD.org, freebsd-threads@FreeBSD.org Subject: Re: [CFR][PATCH] Call proper signal handler from libthr for non-SA_SIGINFO case Message-ID: <tU72TMajcuUd7thnQviKJo5eKx0@1d%2BaJAniZP50FCDdGj54nd51%2Bks> In-Reply-To: <20141128101312.GT17068@kib.kiev.ua> References: <ngq2VsHPChtgNNL/z%2BlRaAXEeTI@1d%2BaJAniZP50FCDdGj54nd51%2Bks> <20141121165658.GP17068@kib.kiev.ua> <cxMeZKcIhV0OesPiyimV3oGOays@W98LlNNy5PXu%2BS3VxPCL6cVmtdM> <20141121203227.GS17068@kib.kiev.ua> <jFEd1ENuyp8%2BKOBFhFLA4ucYsq0@btiPEv4nTmhOyKpuIO1ud%2BMiSV0> <20141126221734.GM17068@kib.kiev.ua> <t6N4RkaeaRObVTAlzRpdhmmcDSg@yxQcZffeD9XmruHr2cPBouRqdXo> <20141127101405.GP17068@kib.kiev.ua> <Y9IeRbaEfX1V2/ZQ54l6PUaYmWw@1d%2BaJAniZP50FCDdGj54nd51%2Bks> <20141128101312.GT17068@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
--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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?tU72TMajcuUd7thnQviKJo5eKx0>