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>