Hello,

1. Following gimpy's suggestion I converted the code to use <queue.h>
   macros for list manipulation ; this required to add code to
   initialize the head of the queue in a couple of places.  I chose
   circleq instead of stailq, because to remove an element stailq
   needs to re-traverse the list. It would be nice to have a special
   iterator that keeps track of the previous element, and can remove
   the current element without re-scanning.

2. I added a lock to guard the queue.

3. Following enami's suggestion I made get remove the element from the
   list, and eliminated other unnecessary functions.

4. in kern_proc.c, I initialized p_lwplock just for consistency, although
   it is not strictly needed for proc0, and fork() takes care of the rest
   of the cases.

Thanks everyone for their feedback, and keep it coming.

christos
   
Index: siginfo.h
===================================================================
RCS file: /cvsroot/src/sys/sys/siginfo.h,v
retrieving revision 1.3
diff -u -u -r1.3 siginfo.h
--- siginfo.h	2003/09/14 07:00:45	1.3
+++ siginfo.h	2003/09/15 14:31:34
@@ -40,6 +40,7 @@
 #define	_SYS_SIGINFO_H_
 
 #include <machine/signal.h>	/* XXX: __HAVE_SIGINFO */
+#include <sys/queue.h>
 
 typedef union sigval {
 	int	sival_int;
@@ -79,8 +80,7 @@
 			int	_fd;
 		} _poll;
 	} _reason;
-	struct ksiginfo *ksi_next;
-	struct ksiginfo *ksi_prev;
+	CIRCLEQ_ENTRY(ksiginfo)	_list;
 };
 
 typedef union siginfo {
@@ -125,6 +125,8 @@
 
 #define	ksi_band	_reason._poll._band
 #define	ksi_fd		_reason._poll._fd
+
+#define	ksi_list	_list
 #endif
 
 /** si_code */
Index: signalvar.h
===================================================================
RCS file: /cvsroot/src/sys/sys/signalvar.h,v
retrieving revision 1.43
diff -u -u -r1.43 signalvar.h
--- signalvar.h	2003/09/14 07:00:46	1.43
+++ signalvar.h	2003/09/15 14:31:35
@@ -34,6 +34,8 @@
 #ifndef	_SYS_SIGNALVAR_H_		/* tmp for user.h */
 #define	_SYS_SIGNALVAR_H_
 
+#include <sys/lock.h>
+#include <sys/queue.h>
 /*
  * Kernel signal definitions and data structures,
  * not exported to user programs.
@@ -61,7 +63,8 @@
 	char	ps_sigcheck;		/* May have deliverable signals. */
 	int	ps_sigwaited;		/* Delivered signal from wait set */
 	sigset_t ps_sigwait;		/* Signals being waited for */
-	struct ksiginfo *ps_siginfo;	/* for SA_SIGINFO */
+	struct simplelock ps_silock;	/* Lock for ps_siginfo */
+	CIRCLEQ_HEAD(, ksiginfo) ps_siginfo;/* for SA_SIGINFO */
 
 	/* This should be copied on fork */
 #define	ps_startcopy	ps_sigstk
Index: kern_fork.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_fork.c,v
retrieving revision 1.110
diff -u -u -r1.110 kern_fork.c
--- kern_fork.c	2003/08/07 16:31:45	1.110
+++ kern_fork.c	2003/09/15 14:34:02
@@ -274,6 +274,8 @@
 	memcpy(&p2->p_startcopy, &p1->p_startcopy,
 	    (unsigned) ((caddr_t)&p2->p_endcopy - (caddr_t)&p2->p_startcopy));
 
+	simple_lock_init(&p2->p_sigctx.ps_silock);
+	CIRCLEQ_INIT(&p2->p_sigctx.ps_siginfo);
 	simple_lock_init(&p2->p_lwplock);
 	LIST_INIT(&p2->p_lwps);
 
Index: kern_proc.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_proc.c,v
retrieving revision 1.65
diff -u -u -r1.65 kern_proc.c
--- kern_proc.c	2003/08/07 16:31:47	1.65
+++ kern_proc.c	2003/09/15 14:34:03
@@ -464,9 +464,12 @@
 {
 	int s;
 
+	simple_lock_init(&p->p_lwplock);
 	LIST_INIT(&p->p_lwps);
 	LIST_INSERT_HEAD(&p->p_lwps, l, l_sibling);
 	p->p_nlwps = 1;
+	simple_lock_init(&p->p_sigctx.ps_silock);
+	CIRCLEQ_INIT(&p->p_sigctx.ps_siginfo);
 
 	s = proclist_lock_write();
 
Index: kern_sig.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_sig.c,v
retrieving revision 1.154
diff -u -u -r1.154 kern_sig.c
--- kern_sig.c	2003/09/14 23:45:53	1.154
+++ kern_sig.c	2003/09/15 14:34:04
@@ -83,8 +83,6 @@
 static void	proc_stop(struct proc *);
 static int	build_corename(struct proc *, char [MAXPATHLEN]);
 static void	ksiginfo_exithook(struct proc *, void *);
-static void	ksiginfo_save(struct proc *, ksiginfo_t *);
-static void	ksiginfo_del(struct proc *, ksiginfo_t *);
 static void	ksiginfo_put(struct proc *, ksiginfo_t *);
 static ksiginfo_t *ksiginfo_get(struct proc *, int);
 
@@ -106,72 +104,61 @@
 	    ((signum) == SIGCONT && (q)->p_session == (p)->p_session))
 
 /*
- * return the first ksiginfo struct from our hash table
+ * Remove and return the first ksiginfo element that matches our requested
+ * signal, or return NULL if one not found.
  */
 static ksiginfo_t *
 ksiginfo_get(struct proc *p, int signo)
 {
-	ksiginfo_t *ksi, *hp = p->p_sigctx.ps_siginfo;
+	ksiginfo_t *ksi;
 
-	if ((ksi = hp) == NULL)
-		return NULL;
-
-	for (;;) {
-		if (ksi->ksi_signo == signo)
+	simple_lock(&p->p_sigctx.ps_silock);
+	CIRCLEQ_FOREACH(ksi, &p->p_sigctx.ps_siginfo, ksi_list) {
+		if (ksi->ksi_signo == signo) {
+			CIRCLEQ_REMOVE(&p->p_sigctx.ps_siginfo, ksi, ksi_list);
+			simple_unlock(&p->p_sigctx.ps_silock);
 			return ksi;
-		if ((ksi = ksi->ksi_next) == hp)
-			return NULL;
+		}
 	}
+	simple_unlock(&p->p_sigctx.ps_silock);
+	return NULL;
 }
 
+/*
+ * Append a new ksiginfo element to the list of pending ksiginfo's, if
+ * we need to (SA_SIGINFO was requested). We replace non RT signals if
+ * they already existed in the queue and we add new entries for RT signals,
+ * or for non RT signals with non-existing entries.
+ */
 static void
 ksiginfo_put(struct proc *p, ksiginfo_t *ksi)
 {
-	ksiginfo_t *hp = p->p_sigctx.ps_siginfo;
-
-	if (hp == NULL)
-		p->p_sigctx.ps_siginfo = ksi->ksi_next = ksi->ksi_prev = ksi;
-	else {
-		ksi->ksi_prev = hp->ksi_prev;
-		hp->ksi_prev->ksi_next = ksi;
-		hp->ksi_prev = ksi;
-		ksi->ksi_next = hp;
-	}
-}
-
-static void
-ksiginfo_del(struct proc *p, ksiginfo_t *ksi)
-{
-	if (ksi->ksi_next == ksi)
-		p->p_sigctx.ps_siginfo = NULL;
-	else {
-		ksi->ksi_prev->ksi_next = ksi->ksi_next;
-		ksi->ksi_next->ksi_prev = ksi->ksi_prev;
-	}
-}
+	ksiginfo_t *kp;
+	struct sigaction *sa = &SIGACTION_PS(p->p_sigacts, ksi->ksi_signo);
 
-static void
-ksiginfo_save(struct proc *p, ksiginfo_t *ksi)
-{
-	ksiginfo_t *kpool = NULL;
-	if ((SIGACTION_PS(p->p_sigacts, ksi->ksi_signo).sa_flags & SA_SIGINFO)
-	    == 0)
+	if ((sa->sa_flags & SA_SIGINFO) == 0)
 		return;
-	if (
+
+	simple_lock(&p->p_sigctx.ps_silock);
 #ifdef notyet	/* XXX: QUEUING */
-	    ksi->ksi_signo >= SIGRTMIN ||
+	if (ksi->ksi_signo < SIGRTMIN)
 #endif
-	    (kpool = ksiginfo_get(p, ksi->ksi_signo)) == NULL) {
-		if ((kpool = pool_get(&ksiginfo_pool, PR_NOWAIT)) == NULL)
-			return;
-		*kpool = *ksi;
-		ksiginfo_put(p, kpool);
-	} else {
-		ksiginfo_t *next = ksi->ksi_next, *prev = ksi->ksi_prev;
-		*kpool = *ksi;
-		kpool->ksi_next = next;
-		kpool->ksi_prev = prev;
+	{
+		CIRCLEQ_FOREACH(kp, &p->p_sigctx.ps_siginfo, ksi_list) {
+			if (kp->ksi_signo == ksi->ksi_signo) {
+				CIRCLEQ_ENTRY(ksiginfo) sv;
+				(void)memcpy(&sv, &kp->ksi_list, sizeof(sv));
+				*kp = *ksi;
+				(void)memcpy(&kp->ksi_list, &sv, sizeof(sv));
+				simple_unlock(&p->p_sigctx.ps_silock);
+				return;
+			}
+		}
 	}
+	kp = pool_get(&ksiginfo_pool, PR_NOWAIT);
+	*kp = *ksi;
+	CIRCLEQ_INSERT_TAIL(&p->p_sigctx.ps_siginfo, kp, ksi_list);
+	simple_unlock(&p->p_sigctx.ps_silock);
 }
 
 /*
@@ -180,15 +167,14 @@
 static void
 ksiginfo_exithook(struct proc *p, void *v)
 {
-	ksiginfo_t *ksi, *hp = p->p_sigctx.ps_siginfo;
 
-	if ((ksi = hp) == NULL)
-		return;
-	for (;;) {
+	simple_lock(&p->p_sigctx.ps_silock);
+	while (!CIRCLEQ_EMPTY(&p->p_sigctx.ps_siginfo)) {
+		ksiginfo_t *ksi = CIRCLEQ_FIRST(&p->p_sigctx.ps_siginfo);
+		CIRCLEQ_REMOVE(&p->p_sigctx.ps_siginfo, ksi, ksi_list);
 		pool_put(&ksiginfo_pool, ksi);
-		if ((ksi = ksi->ksi_next) == hp)
-			break;
 	}
+	simple_unlock(&p->p_sigctx.ps_silock);
 }
 
 /*
@@ -1060,7 +1046,7 @@
 	    && sigismember(&p->p_sigctx.ps_sigwait, signum)
 	    && p->p_stat != SSTOP) {
 		if (action == SIG_CATCH)
-			ksiginfo_save(p, ksi);
+			ksiginfo_put(p, ksi);
 		sigdelset(&p->p_sigctx.ps_siglist, signum);
 		p->p_sigctx.ps_sigwaited = signum;
 		sigemptyset(&p->p_sigctx.ps_sigwait);
@@ -1077,7 +1063,7 @@
 	 */
 	if (action == SIG_HOLD &&
 	    ((prop & SA_CONT) == 0 || p->p_stat != SSTOP)) {
-		ksiginfo_save(p, ksi);
+		ksiginfo_put(p, ksi);
 		return;
 	}
 	/* XXXSMP: works, but icky */
@@ -1261,7 +1247,7 @@
 
  runfast:
 	if (action == SIG_CATCH) {
-		ksiginfo_save(p, ksi);
+		ksiginfo_put(p, ksi);
 		action = SIG_HOLD;
 	}
 	/*
@@ -1271,14 +1257,14 @@
 		l->l_priority = PUSER;
  run:
 	if (action == SIG_CATCH) {
-		ksiginfo_save(p, ksi);
+		ksiginfo_put(p, ksi);
 		action = SIG_HOLD;
 	}
 	
 	setrunnable(l);		/* XXXSMP: recurse? */
  out:
 	if (action == SIG_CATCH)
-		ksiginfo_save(p, ksi);
+		ksiginfo_put(p, ksi);
  done:
 	/* XXXSMP: works, but icky */
 	if (dolock)
@@ -1757,7 +1743,6 @@
 			kpsendsig(l, &ksi1, returnmask);
 		} else {
 			kpsendsig(l, ksi, returnmask);
-			ksiginfo_del(p, ksi);
 			pool_put(&ksiginfo_pool, ksi);
 		}
 		p->p_sigctx.ps_lwp = 0;