Re: cvs commit: src/sys/dev/sound/pci/hda hdac.c

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

: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 
					<dillon@backplane.com>

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