request for review, callout fix.

--6lXr1rPCNTf1w0X8
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi guys, attached is a fix for the timeout/untimeout race with
Giant locked code.

Basically the old code would make the callout inaccessable
right before calling it inside of softclock.

However only the callout lock is held, so when switching to
the callout's associated mutex (in this case Giant) there's
a race where a "local" (timeout/untimeout) callout would be
fired even if stopped.

This patch fixes that.  We've run several hours of regression
testing on a version of this for 6.x.

People internal to Juniper and iedowse@ helped with this.

Please review/comment.

thank you,
-- 
- Alfred Perlstein

--6lXr1rPCNTf1w0X8
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="kern_timeout.diff"

cvs diff: Diffing .
Index: kern_timeout.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_timeout.c,v
retrieving revision 1.110
diff -u -r1.110 kern_timeout.c
--- kern_timeout.c	12 Mar 2008 06:31:06 -0000	1.110
+++ kern_timeout.c	21 Mar 2008 04:28:35 -0000
@@ -230,11 +230,8 @@
 				c_arg = c->c_arg;
 				c_flags = c->c_flags;
 				if (c->c_flags & CALLOUT_LOCAL_ALLOC) {
-					c->c_func = NULL;
 					c->c_flags = CALLOUT_LOCAL_ALLOC;
-					SLIST_INSERT_HEAD(&callfree, c,
-							  c_links.sle);
-					curr_callout = NULL;
+					curr_callout = c;
 				} else {
 					c->c_flags =
 					    (c->c_flags & ~CALLOUT_PENDING);
@@ -299,6 +296,24 @@
 					class->lc_unlock(c_lock);
 			skip:
 				mtx_lock_spin(&callout_lock);
+				/*
+				 * If the current callout is locally
+				 * allocated (timeout(9)) and if it has not
+				 * been killed by untimeout(9)
+				 * then put it on the freelist.
+				 *
+				 * Note: we need to check the cached
+				 * copy of c_flags because if it was not
+				 * local, then it's not safe to deref the
+				 * callout pointer.
+				 */
+				if (c_flags & CALLOUT_LOCAL_ALLOC &&
+				    !(c->c_flags &
+					(CALLOUT_PENDING | CALLOUT_ACTIVE))) {
+					c->c_func = NULL;
+					SLIST_INSERT_HEAD(&callfree, c,
+					    c_links.sle);
+				}
 				curr_callout = NULL;
 				if (callout_wait) {
 					/*

--6lXr1rPCNTf1w0X8
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
[e-mail] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-smp
To unsubscribe, send any mail to "[e-mail]"
--6lXr1rPCNTf1w0X8--