Subject: Re: sigwait(2) implementation
To: Jason R Thorpe <thorpej@wasabisystems.com>
From: Jaromir Dolecek <jdolecek@netbsd.org>
List: tech-kern
Date: 01/25/2003 23:11:53
--ELM7400597-8362-0_
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII

Here is updated version.

It's implemented in terms of sigtimedwait(2). sigwaitinfo() and sigwait()
would be userland wrappers around that. In this patch, the timeout arg
for sigtimedwait() is ignored. I'll look on that part tomorrow.

> Do not attempt to put any multi-threaded awareness into the kernel's
> signal delivery mechanism -- that is all for single-threaded processes;
> the thread library deals with the complexity of signals for multi-threaded
> applications.

Okay, simplified assuming single-threading process. I also changed
psignal1() to clear ps_sigwait set when it would awake the sigwaiter,
to defeat my problem #2. I.e. a single signal is delivered to
signal waiter, and any further signal is sent normal way. It would
not linger on ps_sigwaited list any more. I also use wakeup_one()
in psignal1(), since now can have single waiter maximum.

> You need to handle the case where the sched_lock might be held, here:
> 
> 		if (dolock)
> 			wakeup(&p->p_sigctx.ps_sigwait);
> 		else
> 			sched_wakeup(&p->p_sigctx.ps_sigwait);

OK. Done.
 
> I think you want to find a more efficient way to do this.

Hopefully found :)
 
> I think you want to use PPAUSE|PCATCH; see sigsuspend1().

Done.
 
> Again, I think you simply want to sigemptyset the wait list.

Yeah. Done.

> A multi-threaded
> program is going to handle all of this in the thread library.

I'll experiment with this, and post patch for review later.
 
> It would also be nice if you wrote a regression test for this.

I will :) 

Jaromir
-- 
Jaromir Dolecek <jdolecek@NetBSD.org>            http://www.NetBSD.org/
-=- We should be mindful of the potential goal, but as the tantric    -=-
-=- Buddhist masters say, ``You may notice during meditation that you -=-
-=- sometimes levitate or glow.   Do not let this distract you.''     -=-

--ELM7400597-8362-0_
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=ISO-8859-2
Content-Disposition: attachment; filename=sigtimedwait.txt

Index: kern_sig.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_sig.c,v
retrieving revision 1.130
diff -u -p -r1.130 kern_sig.c
--- kern_sig.c	2003/01/18 10:06:29	1.130
+++ kern_sig.c	2003/01/25 22:11:20
@@ -356,6 +356,8 @@ siginit(struct proc *p)
 		SIGACTION_PS(ps, signum).sa_flags = SA_RESTART;
 	}
 	sigemptyset(&p->p_sigctx.ps_sigcatch);
+	sigemptyset(&p->p_sigctx.ps_sigwait);
+	sigemptyset(&p->p_sigctx.ps_sigwaited);
 	p->p_flag &= ~P_NOCLDSTOP;
 
 	/*
@@ -402,6 +404,8 @@ execsigs(struct proc *p)
 		SIGACTION_PS(ps, signum).sa_flags = SA_RESTART;
 	}
 	sigemptyset(&p->p_sigctx.ps_sigcatch);
+	sigemptyset(&p->p_sigctx.ps_sigwait);
+	sigemptyset(&p->p_sigctx.ps_sigwaited);
 	p->p_flag &= ~P_NOCLDSTOP;
 
 	/*
@@ -856,6 +860,26 @@ psignal1(struct proc *p, int signum,
 	p->p_sigctx.ps_sigcheck = 1;
 
 	/*
+	 * If the signal doesn't have SA_CANTMASK (no override for SIGKILL,
+	 * please!), check if anything waits on it. If yes, clear the
+	 * pending signal from siglist set, add it to sigwaited set,
+	 * clear sigwait list, and wakeup any sigwaiters.
+	 * The signal won't be processed further here.
+	 */
+	if ((prop & SA_CANTMASK) == 0
+	    && sigismember(&p->p_sigctx.ps_sigwait, signum)) {
+		sigdelset(&p->p_sigctx.ps_siglist, signum);
+		sigaddset(&p->p_sigctx.ps_sigwaited, signum);
+		sigemptyset(&p->p_sigctx.ps_sigwait);
+
+		if (dolock)
+			wakeup_one(&p->p_sigctx.ps_sigwait);
+		else
+			sched_wakeup(&p->p_sigctx.ps_sigwait);
+		return;
+	}
+
+	/*
 	 * Defer further processing for signals which are held,
 	 * except that stopped processes must be continued by SIGCONT.
 	 */
@@ -1032,7 +1056,8 @@ psignal1(struct proc *p, int signum,
 			goto out;
 		} else {
 			/* Else what? */
-			panic("psignal: Invalid process state.");
+			panic("psignal: Invalid process state %d.",
+				p->p_stat);
 		}
 	}
 	/*NOTREACHED*/
@@ -1818,6 +1843,99 @@ sys_setcontext(struct lwp *l, void *v, r
 	return (EJUSTRETURN);
 }
 
+/*
+ * sigtimedwait(2) system call, used also for implementation
+ * of sigwaitinfo() and sigwait().
+ *
+ * Note this system call is only used by single-threaded programs.
+ * libpthread handles signal waiting for threaded ones. Due to
+ * this, we don't need to care about concurrent waits or access. 
+ *
+ * XXX We currently don't actually support queued signals, so we fill
+ * only si_signo of siginfo_t, and always set si_code to SI_USER.
+ */
+int
+sys_sigtimedwait(struct lwp *l, void *v, register_t *retval)
+{
+	struct sys_sigtimedwait_args /* {
+		syscallarg(const sigset_t *) set;
+		syscallarg(siginfo_t *) info;
+		syscallarg(const struct timespec *) timeout;
+	} */ *uap = v;
+	sigset_t waitset, twaitset;
+	struct proc *p = l->l_proc;
+	int error;
+	int signum;
+
+	if ((error = copyin(SCARG(uap, set), &waitset, sizeof(waitset))))
+		return (error);
+
+	/* Silently ignore SA_CANTMASK signals. */
+	sigminusset(&sigcantmask, &waitset);
+
+	/*
+	 * First scan siglist and check if there already is signal from
+	 * our waitset pending.
+	 */
+	twaitset = waitset;
+	__sigandset(&p->p_sigctx.ps_siglist, &twaitset);
+	if ((signum = firstsig(&twaitset))) {
+		/* found pending signal */
+		sigdelset(&p->p_sigctx.ps_siglist, signum);
+		goto out;
+	}
+
+	/*
+	 * Add to ps_sigwait list according to our needs.
+	 */
+	sigplusset(&waitset, &p->p_sigctx.ps_sigwait);
+		
+	/*
+	 * Loop until we'd either:
+	 * 1. receive the signal in wait set
+	 * 2. be interrupted by signal
+	 */
+	for(; error == 0;) {
+		/*
+		 * Check if a signal from our wait set has arrived.
+		 */
+		twaitset = waitset;
+		__sigandset(&p->p_sigctx.ps_sigwaited, &twaitset);
+		if ((signum = firstsig(&twaitset))) {
+			/* signal from our wait set has been posted */
+			sigdelset(&p->p_sigctx.ps_sigwaited, signum);
+			break;
+		}
+
+		/* wake us in better times */
+		error = tsleep(&p->p_sigctx.ps_sigwait, PPAUSE|PCATCH,
+			"sigwait", 0);
+	}
+
+	/*
+	 * On error, clear the wait set. Otherwise, psignal1() did
+	 * it already.
+	 */
+	if (error)
+		sigemptyset(&p->p_sigctx.ps_sigwait);
+
+    out:
+	/*
+	 * If a signal from our wait set arrived, copy it to userland.
+	 * XXX no support for queued signals
+	 */
+	if (signum > 0) {
+		siginfo_t si;
+
+		memset(&si, 0, sizeof(si));
+		si.si_signo = signum;
+		si.si_code = SI_USER;
+
+		error = copyout(&si, SCARG(uap, info), sizeof(si));
+	}
+
+	return (error);
+}
 
 /*
  * Returns true if signal is ignored or masked for passed process.

--ELM7400597-8362-0_--