Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 2 Sep 2010 12:26:33 +0300
From:      Kostik Belousov <kostikbel@gmail.com>
To:        David Xu <davidxu@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r212076 - head/lib/libthr/thread
Message-ID:  <20100902092633.GM2396@deviant.kiev.zoral.com.ua>
In-Reply-To: <4C7FD8D1.1000702@freebsd.org>
References:  <201009010218.o812IX5G048257@svn.freebsd.org> <20100902081136.GG2396@deviant.kiev.zoral.com.ua> <4C7FD232.9080100@freebsd.org> <20100902084116.GJ2396@deviant.kiev.zoral.com.ua> <4C7FD8D1.1000702@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--EEu/qcNrcbWqPwHS
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Sep 02, 2010 at 05:03:13PM +0000, David Xu wrote:
> Kostik Belousov wrote:
> >On Thu, Sep 02, 2010 at 04:34:58PM +0000, David Xu wrote:
> >>Kostik Belousov wrote:
> >>>On Wed, Sep 01, 2010 at 02:18:33AM +0000, David Xu wrote:
> >>>>Author: davidxu
> >>>>Date: Wed Sep  1 02:18:33 2010
> >>>>New Revision: 212076
> >>>>URL: http://svn.freebsd.org/changeset/base/212076
> >>>>
> >>>>Log:
> >>>> Add signal handler wrapper, the reason to add it becauses there are
> >>>> some cases we want to improve:
> >>>>   1) if a thread signal got a signal while in cancellation point,
> >>>>      it is possible the TDP_WAKEUP may be eaten by signal handler
> >>>>      if the handler called some interruptibly system calls.
> >>>>   2) In signal handler, we want to disable cancellation.
> >>>>   3) When thread holding some low level locks, it is better to
> >>>>      disable signal, those code need not to worry reentrancy,
> >>>>      sigprocmask system call is avoided because it is a bit expensiv=
e.
> >>>> The signal handler wrapper works in this way:
> >>>>   1) libthr installs its signal handler if user code invokes sigacti=
on
> >>>>      to install its handler, the user handler is recorded in internal
> >>>>      array.
> >>>>   2) when a signal is delivered, libthr's signal handler is invoke,
> >>>>      libthr checks if thread holds some low level lock or is in=20
> >>>>      critical
> >>>>      region, if it is true, the signal is buffered, and all signals =
are
> >>>>      masked, once the thread leaves critical region, correct signal
> >>>>      mask is restored and buffered signal is processed.
> >>>>   3) before user signal handler is invoked, cancellation is temporar=
ily
> >>>>      disabled, after user signal handler is returned, cancellation=
=20
> >>>>      state
> >>>>      is restored, and pending cancellation is rescheduled.
> >>>>+static void
> >>>>+thr_sighandler(int sig, siginfo_t *info, void *_ucp)
> >>>>+{
> >>>>+	if ((actp->sa_flags & SA_SIGINFO) !=3D 0)
> >>>>+		(*(sigfunc))(sig, info, ucp);
> >>>>+	else {
> >>>>+		((ohandler)(*sigfunc))(
> >>>>+			sig, info->si_code, (struct sigcontext *)ucp,
> >>>>+			info->si_addr, (__sighandler_t *)sigfunc);
> >>>>+	}
> >>>I do not think this is very important, but freebsd old-style signal
> >>>handler fourth argument is usually the faulted %eip value. This is
> >>>most likely irrelevant for any source that is linked with libthr.so
> >>>new enough to contain this change.
> >>Isn't the si_addr in siginfo a fault address ? I remembered I saved
> >>the fault address in ksiginfo_t which is converted to userland
> >>siginfo, and fault address should be there. what's wrong here ?
> >>
> >Oops, sorry, I miscalculated the position of the arguments :(.
>=20
> Sorry, I think I also misunderstood you too. :(
> I think kernel still has some compatible problems. I just skimmed
> it again, and I found a problem.
>=20
> In RELENG_4, I found sys/i386/i386/machdep.c has following code in
> sendsig():
>=20
>=20
> 	sf.sf_signum =3D sig;
> 	sf.sf_ucontext =3D (register_t)&sfp->sf_uc;
> 	if (SIGISMEMBER(p->p_sigacts->ps_siginfo, sig)) {
> 		/* Signal handler installed with SA_SIGINFO. */
> 		sf.sf_siginfo =3D (register_t)&sfp->sf_si;
> 		sf.sf_ahu.sf_action =3D (__siginfohandler_t *)catcher;
>=20
> 		/* fill siginfo structure */
> 		sf.sf_si.si_signo =3D sig;
> 		sf.sf_si.si_code =3D code;
> 		sf.sf_si.si_addr =3D (void*)regs->tf_err;
> 	}
> 	else {
> 		/* Old FreeBSD-style arguments. */
> 		sf.sf_siginfo =3D code;
> 		sf.sf_addr =3D regs->tf_err;             <-------
> 		sf.sf_ahu.sf_handler =3D catcher;
> 	}
>=20
> the sf.sf_addr is assigned by tf_err here.
>=20
> In later branch ( I don't know which ), it seems it uses ksi.ksi_addr,
>=20
>       /* Old FreeBSD-style arguments. */
>                 sf.sf_arg2 =3D ksi->ksi_code;
>                 sf.sf_addr =3D (register_t)ksi->ksi_addr;
>                 sf.sf_ahu.sf_handler =3D catcher;
>=20
>=20
>=20
> the tf_err may not be equal to ksi_addr! This may need to be fixed.
>=20
The change was introduced by
r151316 | davidxu | 2005-10-14
      /* Old FreeBSD-style arguments. */
-     sf.sf_siginfo =3D code;
-     sf.sf_addr =3D regs->tf_err;
+     sf.sf_siginfo =3D ksi->ksi_code;
+     sf.sf_addr =3D (register_t)ksi->ksi_addr;
      sf.sf_ahu.sf_handler =3D catcher;


--EEu/qcNrcbWqPwHS
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (FreeBSD)

iEYEARECAAYFAkx/bckACgkQC3+MBN1Mb4jDewCg9oK3MITarwsG1hpppvsS5V/y
ZAQAoITzHIw/F10pF2XYQM9pRzPRjsve
=BWAc
-----END PGP SIGNATURE-----

--EEu/qcNrcbWqPwHS--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100902092633.GM2396>