From owner-svn-src-all@FreeBSD.ORG Thu Sep 2 09:03:15 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B4595106570C; Thu, 2 Sep 2010 09:03:15 +0000 (UTC) (envelope-from davidxu@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 85A4F8FC1C; Thu, 2 Sep 2010 09:03:15 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o8293D7j001669; Thu, 2 Sep 2010 09:03:14 GMT (envelope-from davidxu@freebsd.org) Message-ID: <4C7FD8D1.1000702@freebsd.org> Date: Thu, 02 Sep 2010 17:03:13 +0000 From: David Xu User-Agent: Thunderbird 2.0.0.24 (X11/20100630) MIME-Version: 1.0 To: Kostik Belousov References: <201009010218.o812IX5G048257@svn.freebsd.org> <20100902081136.GG2396@deviant.kiev.zoral.com.ua> <4C7FD232.9080100@freebsd.org> <20100902084116.GJ2396@deviant.kiev.zoral.com.ua> In-Reply-To: <20100902084116.GJ2396@deviant.kiev.zoral.com.ua> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r212076 - head/lib/libthr/thread X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Sep 2010 09:03:15 -0000 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 expensive. >>>> The signal handler wrapper works in this way: >>>> 1) libthr installs its signal handler if user code invokes sigaction >>>> 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 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 temporarily >>>> disabled, after user signal handler is returned, cancellation 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) != 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 :(. 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. In RELENG_4, I found sys/i386/i386/machdep.c has following code in sendsig(): sf.sf_signum = sig; sf.sf_ucontext = (register_t)&sfp->sf_uc; if (SIGISMEMBER(p->p_sigacts->ps_siginfo, sig)) { /* Signal handler installed with SA_SIGINFO. */ sf.sf_siginfo = (register_t)&sfp->sf_si; sf.sf_ahu.sf_action = (__siginfohandler_t *)catcher; /* fill siginfo structure */ sf.sf_si.si_signo = sig; sf.sf_si.si_code = code; sf.sf_si.si_addr = (void*)regs->tf_err; } else { /* Old FreeBSD-style arguments. */ sf.sf_siginfo = code; sf.sf_addr = regs->tf_err; <------- sf.sf_ahu.sf_handler = catcher; } the sf.sf_addr is assigned by tf_err here. In later branch ( I don't know which ), it seems it uses ksi.ksi_addr, /* Old FreeBSD-style arguments. */ sf.sf_arg2 = ksi->ksi_code; sf.sf_addr = (register_t)ksi->ksi_addr; sf.sf_ahu.sf_handler = catcher; the tf_err may not be equal to ksi_addr! This may need to be fixed.