Subject: SA Patch
To: None <tech-kern@netbsd.org>
From: Stephan Uphoff <ups@stups.com>
List: tech-kern
Date: 08/10/2003 19:43:36
Hi,

The patch below (hopefully) improves some signaling problems
found by Nathan.

It also contains some cleanup of the sa_upcall_userret() function
removing any sleep calls using PCATCH.

Unblocked threads now only use an upcall stack after they
acquire the virtual CPU.
This prevents unblocked threads from stealing all available
upcall stacks.


	Stephan
 

Index: kern_sa.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_sa.c,v
retrieving revision 1.19
diff -u -r1.19 kern_sa.c
--- kern_sa.c	2003/07/21 19:21:12	1.19
+++ kern_sa.c	2003/07/23 23:00:02
@@ -209,8 +209,13 @@
 	    sizeof(stack_t) * count);
 	if (error)
 		return (error);
+
+	if ((sa->sa_nstacks == 0) && (sa->sa_vp_wait_count != 0))
+	{
+		l->l_flag |= L_SA_UPCALL; 
+	}
+
 	sa->sa_nstacks += count;
-	wakeup((caddr_t)&sa->sa_nstacks);
 	DPRINTFN(9, ("sa_stacks(%d.%d) nstacks + %d = %2d\n",
 	    l->l_proc->p_pid, l->l_lid, count, sa->sa_nstacks));
 
@@ -701,15 +706,10 @@
 
 	/*
 	 * Okay, now we've been woken up. This means that it's time
-	 * for a SA_UNBLOCKED upcall when we get back to userlevel, provided
-	 * that the SA_BLOCKED upcall happened.
+	 * for a SA_UNBLOCKED upcall when we get back to userlevel
 	 */
-	if (l->l_flag & L_SA_BLOCKING)
-		l->l_flag |= L_SA_UPCALL;
-#if 0
-	else
-		sa_vp_repossess(l);
-#endif
+	l->l_flag |= L_SA_UPCALL;
+
 }
 
 void
@@ -885,66 +885,19 @@
 		sau = sadata_upcall_alloc(1);
 		sau->sau_arg = NULL;
 
-		while (sa->sa_nstacks == 0) {
-			int status;
-
-			/*
-			 * This should be a transient condition, so we'll just
-			 * sleep until some stacks come in; presumably, some
-			 * userland LWP is busy and hasn't gotten to return
-			 * stacks yet.
-			 *
-			 * XXX There is a slight protocol breach here, in
-			 * that control is not handed back directly to the
-			 * LWP that was running before we were woken up from
-			 * the sleep that invoked the UNBLOCKED upcall.
-			 * If it was a running one, it is on the runqueue
-			 * and it will run again when the system gets to it,
-			 * just not this second. If it's idle, it will
-			 * remain idle, because we don't touch it.
-			 *
-			 * Ideally, tsleep() would have a variant that took
-			 * a LWP to switch to.
-			 */
-
-			if (p->p_flag & P_WEXIT)
-			{
-				sadata_upcall_free(sau);
-				lwp_exit(l);
-			}
-
-			DPRINTFN(7, ("sa_upcall_userret(%d.%d) sleeping"
-			    " for stacks\n", l->l_proc->p_pid, l->l_lid));
-			status = tsleep((caddr_t) &sa->sa_nstacks, PWAIT|PCATCH, 
-			    "sastacks", 0);
-			if(status)
-			{
-				if (p->p_flag & P_WEXIT)
-				{
-					sadata_upcall_free(sau);
-					lwp_exit(l);
-				}
-				/* Signal pending - can't sleep */
-				/* Wait a while .. things might get better */  
-				 tsleep((caddr_t) &lbolt, PWAIT, "lbolt: sastacks", 0);
-			}	
-
-			/* XXXUPSXXX NEED TO STOP THE LWP HERE ON REQUEST */
-
-		
-		}
-
 		if (p->p_flag & P_WEXIT) {
 			sadata_upcall_free(sau);
 			lwp_exit(l);
 		}
-
-		KDASSERT(sa->sa_nstacks > 0);
-		st = sa->sa_stacks[--sa->sa_nstacks];
 
+	
 		SCHED_ASSERT_UNLOCKED();
 
 		l2 = sa_vp_repossess(l);
+
+		KDASSERT(sa->sa_nstacks > 0);
+		st = sa->sa_stacks[--sa->sa_nstacks];
+
 		
 		SCHED_ASSERT_UNLOCKED();
 			
@@ -1242,7 +1195,12 @@
 
 	SCHED_ASSERT_UNLOCKED();
 
+	/* Nobody wants the vp */
 	if (sa->sa_vp_wait_count == 0)
+		return;
+
+	/* No stack for an unblock call */
+	if (sa->sa_nstacks == 0)
 		return;
 
 	LIST_FOREACH(l2, &p->p_lwps, l_sibling) {
Index: kern_sig.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_sig.c,v
retrieving revision 1.145
diff -u -r1.145 kern_sig.c
--- kern_sig.c	2003/07/21 22:57:46	1.145
+++ kern_sig.c	2003/07/23 23:00:07
@@ -892,7 +892,8 @@
 		SCHED_LOCK(s);
 
 	/* XXXUPSXXX LWPs might go to sleep without passing signal handling */ 
-	if (p->p_nrlwps > 0 && (p->p_stat != SSTOP)) {
+	if (p->p_nrlwps > 0 && (p->p_stat != SSTOP) 
+	    && !((p->p_flag & P_SA) && (p->p_sa->sa_idle != NULL))) {
 		/*
 		 * At least one LWP is running or on a run queue. 
 		 * The signal will be noticed when one of them returns 
@@ -935,8 +936,7 @@
 			}
 		}
 		if (p->p_stat == SACTIVE) {
-			/* All LWPs must be sleeping */
-			KDASSERT(((p->p_flag & P_SA) == 0) || (l != NULL));
+		
 
 			if (l != NULL && (p->p_flag & P_TRACED))
 				goto run;