Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 27 Oct 2009 10:42:24 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r198506 - in head/sys: compat/freebsd32 kern sys
Message-ID:  <200910271042.n9RAgOcT028750@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kib
Date: Tue Oct 27 10:42:24 2009
New Revision: 198506
URL: http://svn.freebsd.org/changeset/base/198506

Log:
  In kern_sigsuspend(), better manipulate thread signal mask using
  kern_sigprocmask() to properly notify other possible candidate threads
  for signal delivery.
  
  Since sigsuspend() shall only return to usermode after a signal was
  delivered, do cursig/postsig loop immediately after waiting for
  signal, repeating the wait if wakeup was spurious due to race with
  other thread fetching signal from the process queue before us. Add
  thread_suspend_check() call to allow the thread to be stopped or killed
  while in loop.
  
  Modify last argument of kern_sigprocmask() from boolean to flags,
  allowing the function to be called with locked proc. Convertion of the
  callers that supplied 1 to the old argument will be done in the next
  commit, and due to SIGPROCMASK_OLD value equial to 1, code is formally
  correct in between.
  
  Reviewed by:	davidxu
  Tested by:	pho
  MFC after:	1 month

Modified:
  head/sys/compat/freebsd32/freebsd32_misc.c
  head/sys/kern/kern_sig.c
  head/sys/sys/signalvar.h
  head/sys/sys/syscallsubr.h

Modified: head/sys/compat/freebsd32/freebsd32_misc.c
==============================================================================
--- head/sys/compat/freebsd32/freebsd32_misc.c	Tue Oct 27 10:15:58 2009	(r198505)
+++ head/sys/compat/freebsd32/freebsd32_misc.c	Tue Oct 27 10:42:24 2009	(r198506)
@@ -2579,21 +2579,10 @@ int
 ofreebsd32_sigsuspend(struct thread *td,
 			      struct ofreebsd32_sigsuspend_args *uap)
 {
-	struct proc *p = td->td_proc;
 	sigset_t mask;
 
-	PROC_LOCK(p);
-	td->td_oldsigmask = td->td_sigmask;
-	td->td_pflags |= TDP_OLDMASK;
 	OSIG2SIG(uap->mask, mask);
-	SIG_CANTMASK(mask);
-	SIGSETLO(td->td_sigmask, mask);
-	signotify(td);
-	while (msleep(&p->p_sigacts, &p->p_mtx, PPAUSE|PCATCH, "opause", 0) == 0)
-		/* void */;
-	PROC_UNLOCK(p);
-	/* always return EINTR rather than ERESTART... */
-	return (EINTR);
+	return (kern_sigsuspend(td, mask));
 }
 
 struct sigstack32 {

Modified: head/sys/kern/kern_sig.c
==============================================================================
--- head/sys/kern/kern_sig.c	Tue Oct 27 10:15:58 2009	(r198505)
+++ head/sys/kern/kern_sig.c	Tue Oct 27 10:42:24 2009	(r198506)
@@ -970,14 +970,15 @@ execsigs(struct proc *p)
  */
 int
 kern_sigprocmask(struct thread *td, int how, sigset_t *set, sigset_t *oset,
-    int old)
+    int flags)
 {
 	sigset_t new_block, oset1;
 	struct proc *p;
 	int error;
 
 	p = td->td_proc;
-	PROC_LOCK(p);
+	if (!(flags & SIGPROCMASK_PROC_LOCKED))
+		PROC_LOCK(p);
 	if (oset != NULL)
 		*oset = td->td_sigmask;
 
@@ -999,7 +1000,7 @@ kern_sigprocmask(struct thread *td, int 
 		case SIG_SETMASK:
 			SIG_CANTMASK(*set);
 			oset1 = td->td_sigmask;
-			if (old)
+			if (flags & SIGPROCMASK_OLD)
 				SIGSETLO(td->td_sigmask, *set);
 			else
 				td->td_sigmask = *set;
@@ -1025,7 +1026,8 @@ kern_sigprocmask(struct thread *td, int 
 	if (p->p_numthreads != 1)
 		reschedule_signals(p, new_block);
 
-	PROC_UNLOCK(p);
+	if (!(flags & SIGPROCMASK_PROC_LOCKED))
+		PROC_UNLOCK(p);
 	return (error);
 }
 
@@ -1458,6 +1460,7 @@ int
 kern_sigsuspend(struct thread *td, sigset_t mask)
 {
 	struct proc *p = td->td_proc;
+	int has_sig, sig;
 
 	/*
 	 * When returning from sigsuspend, we want
@@ -1467,13 +1470,28 @@ kern_sigsuspend(struct thread *td, sigse
 	 * to indicate this.
 	 */
 	PROC_LOCK(p);
-	td->td_oldsigmask = td->td_sigmask;
+	kern_sigprocmask(td, SIG_SETMASK, &mask, &td->td_oldsigmask,
+	    SIGPROCMASK_PROC_LOCKED);
 	td->td_pflags |= TDP_OLDMASK;
-	SIG_CANTMASK(mask);
-	td->td_sigmask = mask;
-	signotify(td);
-	while (msleep(&p->p_sigacts, &p->p_mtx, PPAUSE|PCATCH, "pause", 0) == 0)
-		/* void */;
+
+	/*
+	 * Process signals now. Otherwise, we can get spurious wakeup
+	 * due to signal entered process queue, but delivered to other
+	 * thread. But sigsuspend should return only on signal
+	 * delivery.
+	 */
+	for (has_sig = 0; !has_sig;) {
+		while (msleep(&p->p_sigacts, &p->p_mtx, PPAUSE|PCATCH, "pause",
+			0) == 0)
+			/* void */;
+		thread_suspend_check(0);
+		mtx_lock(&p->p_sigacts->ps_mtx);
+		while ((sig = cursig(td, SIG_STOP_ALLOWED)) != 0) {
+			postsig(sig);
+			has_sig = 1;
+		}
+		mtx_unlock(&p->p_sigacts->ps_mtx);
+	}
 	PROC_UNLOCK(p);
 	/* always return EINTR rather than ERESTART... */
 	return (EINTR);
@@ -1495,21 +1513,10 @@ osigsuspend(td, uap)
 	struct thread *td;
 	struct osigsuspend_args *uap;
 {
-	struct proc *p = td->td_proc;
 	sigset_t mask;
 
-	PROC_LOCK(p);
-	td->td_oldsigmask = td->td_sigmask;
-	td->td_pflags |= TDP_OLDMASK;
 	OSIG2SIG(uap->mask, mask);
-	SIG_CANTMASK(mask);
-	SIGSETLO(td->td_sigmask, mask);
-	signotify(td);
-	while (msleep(&p->p_sigacts, &p->p_mtx, PPAUSE|PCATCH, "opause", 0) == 0)
-		/* void */;
-	PROC_UNLOCK(p);
-	/* always return EINTR rather than ERESTART... */
-	return (EINTR);
+	return (kern_sigsuspend(td, mask));
 }
 #endif /* COMPAT_43 */
 

Modified: head/sys/sys/signalvar.h
==============================================================================
--- head/sys/sys/signalvar.h	Tue Oct 27 10:15:58 2009	(r198505)
+++ head/sys/sys/signalvar.h	Tue Oct 27 10:42:24 2009	(r198506)
@@ -316,6 +316,10 @@ extern int kern_logsigexit;	/* Sysctl va
 #define	SIG_STOP_ALLOWED	100
 #define	SIG_STOP_NOT_ALLOWED	101
 
+/* flags for kern_sigprocmask */
+#define	SIGPROCMASK_OLD		0x0001
+#define	SIGPROCMASK_PROC_LOCKED	0x0002
+
 /*
  * Machine-independent functions:
  */
@@ -359,7 +363,8 @@ void	sigqueue_delete_stopmask_proc(struc
 void	sigqueue_take(ksiginfo_t *ksi);
 int	kern_sigtimedwait(struct thread *, sigset_t,
 		ksiginfo_t *, struct timespec *);
-
+int	kern_sigprocmask(struct thread *td, int how,
+	    sigset_t *set, sigset_t *oset, int flags);
 /*
  * Machine-dependent functions:
  */

Modified: head/sys/sys/syscallsubr.h
==============================================================================
--- head/sys/sys/syscallsubr.h	Tue Oct 27 10:15:58 2009	(r198505)
+++ head/sys/sys/syscallsubr.h	Tue Oct 27 10:42:24 2009	(r198506)
@@ -190,8 +190,6 @@ int	kern_shmctl(struct thread *td, int s
 int	kern_sigaction(struct thread *td, int sig, struct sigaction *act,
 	    struct sigaction *oact, int flags);
 int	kern_sigaltstack(struct thread *td, stack_t *ss, stack_t *oss);
-int	kern_sigprocmask(struct thread *td, int how,
-	    sigset_t *set, sigset_t *oset, int old);
 int	kern_sigsuspend(struct thread *td, sigset_t mask);
 int	kern_stat(struct thread *td, char *path, enum uio_seg pathseg,
 	    struct stat *sbp);



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