From owner-svn-src-all@FreeBSD.ORG Fri Oct 30 04:12:22 2009 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 93F7C106568D; Fri, 30 Oct 2009 04:12:22 +0000 (UTC) (envelope-from keramida@freebsd.org) Received: from poseidon.ceid.upatras.gr (poseidon.ceid.upatras.gr [150.140.141.169]) by mx1.freebsd.org (Postfix) with ESMTP id E62068FC1C; Fri, 30 Oct 2009 04:12:21 +0000 (UTC) Received: from mail.ceid.upatras.gr (unknown [10.1.0.143]) by poseidon.ceid.upatras.gr (Postfix) with ESMTP id D36C1EB4721; Fri, 30 Oct 2009 06:12:20 +0200 (EET) Received: from localhost (europa.ceid.upatras.gr [127.0.0.1]) by mail.ceid.upatras.gr (Postfix) with ESMTP id AB12D451B2; Fri, 30 Oct 2009 06:12:20 +0200 (EET) X-Virus-Scanned: amavisd-new at ceid.upatras.gr Received: from mail.ceid.upatras.gr ([127.0.0.1]) by localhost (europa.ceid.upatras.gr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7GzRfE5vEW2i; Fri, 30 Oct 2009 06:12:20 +0200 (EET) Received: from kobe.laptop (ppp-94-64-196-111.home.otenet.gr [94.64.196.111]) by mail.ceid.upatras.gr (Postfix) with ESMTP id 4AC2A45152; Fri, 30 Oct 2009 06:12:20 +0200 (EET) Received: from kobe.laptop (kobe.laptop [127.0.0.1]) by kobe.laptop (8.14.3/8.14.3) with ESMTP id n9U4CJ55011875 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 30 Oct 2009 06:12:19 +0200 (EET) (envelope-from keramida@freebsd.org) Received: (from keramida@localhost) by kobe.laptop (8.14.3/8.14.3/Submit) id n9U4CI50011874; Fri, 30 Oct 2009 06:12:18 +0200 (EET) (envelope-from keramida@freebsd.org) From: Giorgos Keramidas To: Kostik Belousov References: <200910291434.n9TEYOVJ099388@svn.freebsd.org> <871vklbxyf.fsf@kobe.laptop> <20091030032240.GD2147@deviant.kiev.zoral.com.ua> Date: Fri, 30 Oct 2009 06:12:11 +0200 In-Reply-To: <20091030032240.GD2147@deviant.kiev.zoral.com.ua> (Kostik Belousov's message of "Fri, 30 Oct 2009 05:22:40 +0200") Message-ID: <87d445a5b8.fsf@kobe.laptop> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1.50 (berkeley-unix) MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r198590 - head/sys/kern 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: Fri, 30 Oct 2009 04:12:22 -0000 --=-=-= > No, this was caused by the r198507 fragment of postsig(), as well as a > fragment from the trapsignal(), that added the call to kern_sigprocmask() > instead of direct manipulation of thread signal mask. > >> AFAICT, postsig() is called with proc->p_sigacts->ps_mtx locked, so when >> we are recursing when reschedule_signals() tries to lock it once more. > > Yes, the same statement holds for trapsignal(). > >> Since we are holding the proc lock in kern_sigprocmask(), is it safe to >> assert that we own ps_mtx, drop it and re-acquire it immediately after >> calling reschedule_signals()? > > No. Most callers of kern_sigprocmask() do not hold curproc->ps_mtx. > Only postsig() and trapsig() call kern_sigprocmask() with ps_mtx locked, > so I have to special-case them. > > Could you, please, test the following patch ? What application did > exposed the issue ? Logging out of GNOME consistently triggers this. The thread that is active in kgdb is gnome-session: (kgdb) info threads ... 138 Thread 100142 (PID=2314: gnome-session) doadump () at pcpu.h:246 137 Thread 100100 (PID=2314: gnome-session/initial thread) sched_switch (td=0xc6a86230, newtd=0xc6564230, flags=1538) at /usr/src/sys/kern/sched_ule.c:1864 ... I'll try the patch in a few hours and report back. Thanks! :) > diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c > index 7f5cfa3..e174df1 100644 > --- a/sys/kern/kern_sig.c > +++ b/sys/kern/kern_sig.c > @@ -220,7 +220,7 @@ static int sigproptbl[NSIG] = { > SA_KILL|SA_PROC, /* SIGUSR2 */ > }; > > -static void reschedule_signals(struct proc *p, sigset_t block); > +static void reschedule_signals(struct proc *p, sigset_t block, int flags); > > static void > sigqueue_start(void) > @@ -1024,7 +1024,7 @@ kern_sigprocmask(struct thread *td, int how, sigset_t *set, sigset_t *oset, > * possibly waking it up. > */ > if (p->p_numthreads != 1) > - reschedule_signals(p, new_block); > + reschedule_signals(p, new_block, flags); > > if (!(flags & SIGPROCMASK_PROC_LOCKED)) > PROC_UNLOCK(p); > @@ -1859,13 +1859,11 @@ trapsignal(struct thread *td, ksiginfo_t *ksi) > #endif > (*p->p_sysent->sv_sendsig)(ps->ps_sigact[_SIG_IDX(sig)], > ksi, &td->td_sigmask); > - SIGSETOR(td->td_sigmask, ps->ps_catchmask[_SIG_IDX(sig)]); > - if (!SIGISMEMBER(ps->ps_signodefer, sig)) { > - SIGEMPTYSET(mask); > + mask = ps->ps_catchmask[_SIG_IDX(sig)]; > + if (!SIGISMEMBER(ps->ps_signodefer, sig)) > SIGADDSET(mask, sig); > - kern_sigprocmask(td, SIG_BLOCK, &mask, NULL, > - SIGPROCMASK_PROC_LOCKED); > - } > + kern_sigprocmask(td, SIG_BLOCK, &mask, NULL, > + SIGPROCMASK_PROC_LOCKED | SIGPROCMASK_PS_LOCKED); > if (SIGISMEMBER(ps->ps_sigreset, sig)) { > /* > * See kern_sigaction() for origin of this code. > @@ -2401,7 +2399,7 @@ stopme: > } > > static void > -reschedule_signals(struct proc *p, sigset_t block) > +reschedule_signals(struct proc *p, sigset_t block, int flags) > { > struct sigacts *ps; > struct thread *td; > @@ -2419,12 +2417,14 @@ reschedule_signals(struct proc *p, sigset_t block) > > td = sigtd(p, i, 0); > signotify(td); > - mtx_lock(&ps->ps_mtx); > + if (!(flags & SIGPROCMASK_PS_LOCKED)) > + mtx_lock(&ps->ps_mtx); > if (p->p_flag & P_TRACED || SIGISMEMBER(ps->ps_sigcatch, i)) > tdsigwakeup(td, i, SIG_CATCH, > (SIGISMEMBER(ps->ps_sigintr, i) ? EINTR : > ERESTART)); > - mtx_unlock(&ps->ps_mtx); > + if (!(flags & SIGPROCMASK_PS_LOCKED)) > + mtx_unlock(&ps->ps_mtx); > } > } > > @@ -2452,7 +2452,7 @@ tdsigcleanup(struct thread *td) > SIGFILLSET(unblocked); > SIGSETNAND(unblocked, td->td_sigmask); > SIGFILLSET(td->td_sigmask); > - reschedule_signals(p, unblocked); > + reschedule_signals(p, unblocked, 0); > > } > > @@ -2734,15 +2734,11 @@ postsig(sig) > } else > returnmask = td->td_sigmask; > > - kern_sigprocmask(td, SIG_BLOCK, > - &ps->ps_catchmask[_SIG_IDX(sig)], NULL, > - SIGPROCMASK_PROC_LOCKED); > - if (!SIGISMEMBER(ps->ps_signodefer, sig)) { > - SIGEMPTYSET(mask); > + mask = ps->ps_catchmask[_SIG_IDX(sig)]; > + if (!SIGISMEMBER(ps->ps_signodefer, sig)) > SIGADDSET(mask, sig); > - kern_sigprocmask(td, SIG_BLOCK, &mask, NULL, > - SIGPROCMASK_PROC_LOCKED); > - } > + kern_sigprocmask(td, SIG_BLOCK, &mask, NULL, > + SIGPROCMASK_PROC_LOCKED | SIGPROCMASK_PS_LOCKED); > > if (SIGISMEMBER(ps->ps_sigreset, sig)) { > /* > diff --git a/sys/sys/signalvar.h b/sys/sys/signalvar.h > index b9a54f0..c27a128 100644 > --- a/sys/sys/signalvar.h > +++ b/sys/sys/signalvar.h > @@ -319,6 +319,7 @@ extern int kern_logsigexit; /* Sysctl variable kern.logsigexit */ > /* flags for kern_sigprocmask */ > #define SIGPROCMASK_OLD 0x0001 > #define SIGPROCMASK_PROC_LOCKED 0x0002 > +#define SIGPROCMASK_PS_LOCKED 0x0004 > > /* > * Machine-independent functions: --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.11 (FreeBSD) iEYEARECAAYFAkrqZ6IACgkQ1g+UGjGGA7Yt8ACfRNfjWFWhjV4Ntic7dAXOZh5D 7CgAn2r+bLuCSbWm/sOugWfUcZ4DzST2 =gTB4 -----END PGP SIGNATURE----- --=-=-=--