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
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) {
/*
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-smp
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
