:On Tue, Jun 26, 2007 at 03:00:02PM +0300, Hasso Tepper wrote:
:> Joerg Sonnenberger wrote:
:> > You run the callout_stop twice?
:>
:> I asked in intial submit mail whether it's correct just to use
:> callout_stop() instead of callout_drain() - we don't have the latter. No
:> feedback except "go, commit".
:
:callout_stop is interlocking itself against the processing, so no
:callout_drain is needed.
:
:Joerg
I looked at the code more closely. The callout code is all
cpu-localized but it isn't 100% SMP safe. It has SMP code to
handle cross-cpu removal but the CALLOUT_PENDING check is just
not safe.
It is a very small window never likely to occur during a detach, but
I think I need to move up the globaldata pointer test. What do you
think about this?
-Matt
Matthew Dillon
<[EMAIL PROTECTED]>
Index: kern_timeout.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_timeout.c,v
retrieving revision 1.25
diff -u -p -r1.25 kern_timeout.c
--- kern_timeout.c 30 Apr 2007 07:18:54 -0000 1.25
+++ kern_timeout.c 26 Jun 2007 16:51:36 -0000
@@ -482,14 +482,6 @@ }
#endif
crit_enter_gd(gd);
- /*
- * Don't attempt to delete a callout that's not on the queue.
- */
- if ((c->c_flags & CALLOUT_PENDING) == 0) {
- c->c_flags &= ~CALLOUT_ACTIVE;
- crit_exit_gd(gd);
- return (0);
- }
#ifdef SMP
if ((tgd = c->c_gd) != gd) {
/*
@@ -503,7 +495,14 @@ seq = lwkt_send_ipiq(tgd, (void *)call
lwkt_wait_ipiq(tgd, seq);
} else
#endif
- {
+ if ((c->c_flags & CALLOUT_PENDING) == 0) {
+ /*
+ * Don't attempt to delete a callout that's not on the queue.
+ */
+ c->c_flags &= ~CALLOUT_ACTIVE;
+ crit_exit_gd(gd);
+ return (0);
+ } else {
/*
* If the callout is owned by the same CPU we can
* process it directly, but if we are racing our helper
