Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 8 Jan 2011 18:51:16 +0000 (UTC)
From:      Attilio Rao <attilio@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r217161 - head/sys/kern
Message-ID:  <201101081851.p08IpGvH001144@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: attilio
Date: Sat Jan  8 18:51:15 2011
New Revision: 217161
URL: http://svn.freebsd.org/changeset/base/217161

Log:
  Revert r216805.
  That revision is introducing a bug which is more visible than problems
  it is trying to fix.
  
  As long as my time is very limited in this period I am going to
  commit back this patch just once it is fully fixed.
  
  Reported by:	dim, Nicholas Esborn

Modified:
  head/sys/kern/kern_timeout.c

Modified: head/sys/kern/kern_timeout.c
==============================================================================
--- head/sys/kern/kern_timeout.c	Sat Jan  8 18:41:19 2011	(r217160)
+++ head/sys/kern/kern_timeout.c	Sat Jan  8 18:51:15 2011	(r217161)
@@ -56,10 +56,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/sysctl.h>
 #include <sys/smp.h>
 
-#ifdef SMP
-#include <machine/cpu.h>
-#endif
-
 SDT_PROVIDER_DEFINE(callout_execute);
 SDT_PROBE_DEFINE(callout_execute, kernel, , callout_start, callout-start);
 SDT_PROBE_ARGTYPE(callout_execute, kernel, , callout_start, 0,
@@ -111,15 +107,11 @@ struct callout_cpu {
 	struct callout		*cc_next;
 	struct callout		*cc_curr;
 	void			*cc_cookie;
-	void			(*cc_migration_func)(void *);
-	void			*cc_migration_arg;
 	int 			cc_ticks;
 	int 			cc_softticks;
 	int			cc_cancel;
 	int			cc_waiting;
 	int 			cc_firsttick;
-	int			cc_migration_cpu;
-	int			cc_migration_ticks;
 };
 
 #ifdef SMP
@@ -131,10 +123,8 @@ struct callout_cpu cc_cpu;
 #define	CC_CPU(cpu)	&cc_cpu
 #define	CC_SELF()	&cc_cpu
 #endif
-#define	CPUBLOCK	MAXCPU
 #define	CC_LOCK(cc)	mtx_lock_spin(&(cc)->cc_lock)
 #define	CC_UNLOCK(cc)	mtx_unlock_spin(&(cc)->cc_lock)
-#define	CC_LOCK_ASSERT(cc)	mtx_assert(&(cc)->cc_lock, MA_OWNED)
 
 static int timeout_cpu;
 void (*callout_new_inserted)(int cpu, int ticks) = NULL;
@@ -198,7 +188,6 @@ callout_cpu_init(struct callout_cpu *cc)
 	for (i = 0; i < callwheelsize; i++) {
 		TAILQ_INIT(&cc->cc_callwheel[i]);
 	}
-	cc->cc_migration_cpu = CPUBLOCK;
 	if (cc->cc_callout == NULL)
 		return;
 	for (i = 0; i < ncallout; i++) {
@@ -322,13 +311,6 @@ callout_lock(struct callout *c)
 
 	for (;;) {
 		cpu = c->c_cpu;
-#ifdef SMP
-		if (cpu == CPUBLOCK) {
-			while (c->c_cpu == CPUBLOCK)
-				cpu_spinwait();
-			continue;
-		}
-#endif
 		cc = CC_CPU(cpu);
 		CC_LOCK(cc);
 		if (cpu == c->c_cpu)
@@ -338,29 +320,6 @@ callout_lock(struct callout *c)
 	return (cc);
 }
 
-static void
-callout_cc_add(struct callout *c, struct callout_cpu *cc, int to_ticks,
-    void (*func)(void *), void *arg, int cpu)
-{
-
-	CC_LOCK_ASSERT(cc);
-
-	if (to_ticks <= 0)
-		to_ticks = 1;
-	c->c_arg = arg;
-	c->c_flags |= (CALLOUT_ACTIVE | CALLOUT_PENDING);
-	c->c_func = func;
-	c->c_time = ticks + to_ticks;
-	TAILQ_INSERT_TAIL(&cc->cc_callwheel[c->c_time & callwheelmask], 
-	    c, c_links.tqe);
-	if ((c->c_time - cc->cc_firsttick) < 0 &&
-	    callout_new_inserted != NULL) {
-		cc->cc_firsttick = c->c_time;
-		(*callout_new_inserted)(cpu,
-		    to_ticks + (ticks - cc->cc_ticks));
-	}
-}
-
 /*
  * The callout mechanism is based on the work of Adam M. Costello and 
  * George Varghese, published in a technical report entitled "Redesigning
@@ -431,14 +390,11 @@ softclock(void *arg)
 					steps = 0;
 				}
 			} else {
-				struct callout_cpu *new_cc;
 				void (*c_func)(void *);
-				void (*new_func)(void *);
-				void *c_arg, *new_arg;
+				void *c_arg;
 				struct lock_class *class;
 				struct lock_object *c_lock;
 				int c_flags, sharedlock;
-				int new_cpu, new_ticks;
 
 				cc->cc_next = TAILQ_NEXT(c, c_links.tqe);
 				TAILQ_REMOVE(bucket, c, c_links.tqe);
@@ -540,40 +496,7 @@ softclock(void *arg)
 					    c_links.sle);
 				}
 				cc->cc_curr = NULL;
-
-				/*
-				 * If the callout was scheduled for
-				 * migration just perform it now.
-				 */
-				if (cc->cc_migration_cpu != CPUBLOCK) {
-
-					/*
-					 * There must not be any waiting
-					 * thread now because the callout
-					 * has a blocked CPU.
-					 * Also, the callout must not be
-					 * freed, but that is not easy to
-					 * assert.
-					 */
-					MPASS(cc->cc_waiting == 0);
-					new_cpu = cc->cc_migration_cpu;
-					new_ticks = cc->cc_migration_ticks;
-					new_func = cc->cc_migration_func;
-					new_arg = cc->cc_migration_arg;
-					cc->cc_migration_cpu = CPUBLOCK;
-					cc->cc_migration_ticks = 0;
-					cc->cc_migration_func = NULL;
-					cc->cc_migration_arg = NULL;
-					CC_UNLOCK(cc);
-					new_cc = CC_CPU(new_cpu);
-					CC_LOCK(new_cc);
-					MPASS(c->c_cpu == CPUBLOCK);
-					c->c_cpu = new_cpu;
-					callout_cc_add(c, new_cc, new_ticks,
-					    new_func, new_arg, new_cpu);
-					CC_UNLOCK(new_cc);
-					CC_LOCK(cc);
-				} else if (cc->cc_waiting) {
+				if (cc->cc_waiting) {
 					/*
 					 * There is someone waiting
 					 * for the callout to complete.
@@ -694,6 +617,7 @@ callout_reset_on(struct callout *c, int 
 	 */
 	if (c->c_flags & CALLOUT_LOCAL_ALLOC)
 		cpu = c->c_cpu;
+retry:
 	cc = callout_lock(c);
 	if (cc->cc_curr == c) {
 		/*
@@ -725,34 +649,31 @@ callout_reset_on(struct callout *c, int 
 		cancelled = 1;
 		c->c_flags &= ~(CALLOUT_ACTIVE | CALLOUT_PENDING);
 	}
-#ifdef SMP
 	/*
-	 * If the lock must migrate we have to block the callout locking
-	 * until migration is completed.
-	 * If the callout is currently running, just defer the migration
-	 * to a more appropriate moment.
+	 * If the lock must migrate we have to check the state again as
+	 * we can't hold both the new and old locks simultaneously.
 	 */
 	if (c->c_cpu != cpu) {
-		c->c_cpu = CPUBLOCK;
-		if (cc->cc_curr == c) {
-			cc->cc_migration_cpu = cpu;
-			cc->cc_migration_ticks = to_ticks;
-			cc->cc_migration_func = ftn;
-			cc->cc_migration_arg = arg;
-			CTR5(KTR_CALLOUT,
-		    "migration of %p func %p arg %p in %d to %u deferred",
-			    c, c->c_func, c->c_arg, to_ticks, cpu);
-			CC_UNLOCK(cc);
-			return (cancelled);
-		}
-		CC_UNLOCK(cc);
-		cc = CC_CPU(cpu);
-		CC_LOCK(cc);
 		c->c_cpu = cpu;
+		CC_UNLOCK(cc);
+		goto retry;
 	}
-#endif
 
-	callout_cc_add(c, cc, to_ticks, ftn, arg, cpu);
+	if (to_ticks <= 0)
+		to_ticks = 1;
+
+	c->c_arg = arg;
+	c->c_flags |= (CALLOUT_ACTIVE | CALLOUT_PENDING);
+	c->c_func = ftn;
+	c->c_time = ticks + to_ticks;
+	TAILQ_INSERT_TAIL(&cc->cc_callwheel[c->c_time & callwheelmask], 
+			  c, c_links.tqe);
+	if ((c->c_time - cc->cc_firsttick) < 0 &&
+	    callout_new_inserted != NULL) {
+		cc->cc_firsttick = c->c_time;
+		(*callout_new_inserted)(cpu,
+		    to_ticks + (ticks - cc->cc_ticks));
+	}
 	CTR5(KTR_CALLOUT, "%sscheduled %p func %p arg %p in %d",
 	    cancelled ? "re" : "", c, c->c_func, c->c_arg, to_ticks);
 	CC_UNLOCK(cc);
@@ -780,7 +701,7 @@ _callout_stop_safe(c, safe)
 	struct	callout *c;
 	int	safe;
 {
-	struct callout_cpu *cc, *old_cc;
+	struct callout_cpu *cc;
 	struct lock_class *class;
 	int use_lock, sq_locked;
 
@@ -800,23 +721,8 @@ _callout_stop_safe(c, safe)
 		use_lock = 0;
 
 	sq_locked = 0;
-	old_cc = NULL;
 again:
 	cc = callout_lock(c);
-
-	/*
-	 * If the callout was migrating while the callout cpu lock was
-	 * dropped,  just drop the sleepqueue lock and check the states
-	 * again.
-	 */
-	if (sq_locked != 0 && cc != old_cc) {
-		CC_UNLOCK(cc);
-		sleepq_release(&old_cc->cc_waiting);
-		sq_locked = 0;
-		old_cc = NULL;
-		goto again;
-	}
-
 	/*
 	 * If the callout isn't pending, it's not on the queue, so
 	 * don't attempt to remove it from the queue.  We can try to
@@ -868,7 +774,6 @@ again:
 					CC_UNLOCK(cc);
 					sleepq_lock(&cc->cc_waiting);
 					sq_locked = 1;
-					old_cc = cc;
 					goto again;
 				}
 				cc->cc_waiting = 1;
@@ -879,7 +784,6 @@ again:
 				    SLEEPQ_SLEEP, 0);
 				sleepq_wait(&cc->cc_waiting, 0);
 				sq_locked = 0;
-				old_cc = NULL;
 
 				/* Reacquire locks previously released. */
 				PICKUP_GIANT();



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201101081851.p08IpGvH001144>