[SSI] openssi/kernel/cluster/ssi/vproc dvp_move.c,1.20,1.21

Roger Tsang <[email protected]> Wed, 15 Dec 2010 06:46:58 +0000
Newsgroups gmane.linux.cluster.ssic.cvs
Message-ID <[email protected]>
Update of /cvsroot/ssic-linux/openssi/kernel/cluster/ssi/vproc
In directory sfp-cvsdas-3.v30.ch3.sourceforge.com:/tmp/cvs-serv10439/cluster/ssi/vproc

Modified Files:
      Tag: OPENSSI-FC
	dvp_move.c 
Log Message:
- use assert_spin_locked() where appropriate.
- setup_pid_move: no need to hold mh_lock while reading m_seen. (#ifdef VPROC_RCU_MOVEMENT_LIST)
- setup_pid_move: reduce unnecessary work when there are no threads in group.
- setup_pid_move: no longer hold VPROC LOCK for the duration of move_data setup. Reduce latency in VPROC context. We rely on alloc_lock to prevent race with exiting process in clear_task_execnode().
- move_start_thread: no need to set migrate_args.optype twice.
- ssi_quiesce_movement: revert, don't skip test for PM_CONTINUE since this state is reached when an error is encountered on migrate server.
- check_move_list: optimize away mh_lock if we saw new processes. (#ifdef VPROC_RCU_MOVEMENT_LIST)
- add_thread_group: skip threads that are being destroyed, optimize away setup_pid_move() calls. (#ifdef VPROC_RCU_MOVEMENT_LIST)
- setup_execnode_move: search and update movement_list early so we can optimize away setup_pid_move() and cancel_move_out() if we lost the race.
- move_out_daemon: skip zeroing proc_list and data_list. not necessary.
- clear_task_execnode: do panic if encountered bad value in execnode field in task_struct since this condition likely indicates system is corrupted. we used to print a warning and continue.
- clear_task_execnode: no need to obtain temporary reference to move_data since we have that from execnode field in task_struct.
- move_data_clear_execnode: convert to macro. reduce function call overhead.
- ssi_quiesce_movement: fix race over mh_dstnode when more than one thread in group sees CLUSTERNODE_BEST. The race could result in threads in the group migrated to different OpenSSI nodes when the whole group should migrate to the same node. Auto load-balancing uses CLUSTERNODE_BEST by default.
- move_data_wait_for_state_change: use wait_event_interruptible(). Wait queue exclusive flag is not needed considering everybody calls wake_up_all() on m_wqh field in move_data structure.


Index: dvp_move.c
===================================================================
RCS file: /cvsroot/ssic-linux/openssi/kernel/cluster/ssi/vproc/dvp_move.c,v
retrieving revision 1.20
retrieving revision 1.21
diff -u -d -r1.20 -r1.21
--- dvp_move.c	9 Nov 2010 02:08:51 -0000	1.20
+++ dvp_move.c	15 Dec 2010 06:46:56 -0000	1.21
@@ -158,6 +158,8 @@
 	int error = -EINVAL;
 	enum move_state state = h->mh_state;
 
+	assert_spin_locked(&h->mh_lock);
+
 	if (state == expected_state) {
 		error = 0;
 		set_mb(h->mh_state, new_state);
@@ -188,6 +190,8 @@
 	enum move_state state = d->m_state;
 	int error = -EINVAL;
 
+	assert_spin_locked(&h->mh_lock);
+
 	if (state == expected_state) {
 		set_mb(d->m_state, new_state);
 		h->mh_done += incr;
@@ -219,6 +223,8 @@
 	struct move_header *h = d->m_mhp;
 	int error;
 
+	assert_spin_locked(&h->mh_lock);
+
 	error = __set_move_data_state(d, expected_dstate, new_state, 1,
 				      function, line);
 	if (error > 0)
@@ -236,8 +242,6 @@
 static inline struct move_header *
 __get_move_header(struct move_header *h)
 {
-	if (atomic_read(&h->mh_abort))
-		return NULL;
 #ifdef DEBUG
 	spin_lock(&h->mh_lock);
 	if (!h->mh_magic) {
@@ -258,8 +262,12 @@
 static inline struct move_header *
 get_move_header(struct move_header *h)
 {
-	if (h)
+	if (h) {
+#ifdef DEBUG
+		BUG_ON(atomic_read(&h->mh_refcnt) < 1);
+#endif
 		atomic_inc(&h->mh_refcnt);
+	}
 	return h;
 }
 
@@ -273,8 +281,8 @@
 		      " %p\n",
 		      __FUNCTION__, h, (void *)h->mh_magic);
 	}
-#endif
 	BUG_ON(atomic_read(&h->mh_refcnt) < 1);
+#endif
 	if (!atomic_dec_and_lock(&h->mh_refcnt, &movement_list_lock))
 		return;
 #ifdef VPROC_RCU_MOVEMENT_LIST
@@ -290,11 +298,11 @@
 #endif /* DEBUG */
 	list_del_rcu(&h->mh_list);
 	spin_unlock(&movement_list_lock);
-
+#ifdef DEBUG
 	if (unlikely(h->mh_total))
 		panic("%s:move_header %p, mh_datalist not empty\n",
 		      __FUNCTION__, h);
-
+#endif
 	call_rcu(&h->mh_rcu, free_move_header);
 #else
 	list_del(&h->mh_list);
@@ -303,6 +311,44 @@
 #endif /* !VPROC_RCU_MOVEMENT_LIST */
 }
 
+static struct move_data *
+alloc_move_data(struct move_header *h, int pid, enum move_state state)
+{
+	struct move_data *d;
+
+#ifdef VPROC_MOVE_DATA_KMEM_CACHE
+	d = kmem_cache_alloc(move_data_cachep, SLAB_KERNEL);
+#else
+	d = kmalloc(sizeof(*d), GFP_USER);
+#endif
+	if (!d)
+		return NULL;
+
+	INIT_LIST_HEAD(&d->m_list);
+#ifdef DEBUG
+	d->m_magic = ((u_long)&d->m_magic) + 1;
+#endif
+	atomic_set(&d->m_refcnt, 1);
+	d->m_pid = pid;
+	d->m_leader = 0;
+	INIT_LIST_HEAD(&d->m_threadlist);
+	d->m_migargs = NULL;
+	d->m_state = state;
+	d->m_seen = 0;
+	init_waitqueue_head(&d->m_wqh);
+	d->m_mhp = get_move_header(h);
+#ifdef VPROC_RCU_MOVEMENT_LIST
+	SSI_ASSERT(d->m_mhp);
+#endif
+	d->m_taskp = NULL;
+#ifdef VPROC_RCU_MOVEMENT_LIST
+	atomic_set(&d->m_hashed, 0);
+	INIT_RCU_HEAD(&d->m_rcu);
+#endif
+
+	return d;
+}
+
 /* Caller must ensure reference to move_data exists
  * otherwise use __get_move_data()
  */
@@ -312,6 +358,7 @@
 	if (d) {
 #ifdef DEBUG
 		BUG_ON(d->m_magic != ((u_long)&d->m_magic) + 1);
+		BUG_ON(atomic_read(&d->m_refcnt) < 1);
 #endif
 		atomic_inc(&d->m_refcnt);
 	}
@@ -362,23 +409,21 @@
 		panic("%s:move_data %p has invalid magic number %p\n",
 		      __FUNCTION__, d, (void *)d->m_magic);
 	}
-#endif
 	BUG_ON(atomic_read(&d->m_refcnt) < 1);
+#endif
 #ifdef VPROC_RCU_MOVEMENT_LIST
 	if (!atomic_dec_and_test(&d->m_refcnt))
 		return;
 #ifdef DEBUG
 	d->m_magic = 0;
-#endif
-	/* NB: already removed from mh_datalist in drop_move_data() */
-	put_move_header(d->m_mhp);
-
 	if (unlikely(d->m_taskp))
 		panic("%s:move_data %p freed with m_taskp %p set\n",
 		      __FUNCTION__, d, d->m_taskp);
+	/* NB: removed from mh_datalist in drop_move_data() */
 	if (unlikely(atomic_read(&d->m_hashed)))
 		panic("%s:move_data %p still hashed\n", __FUNCTION__, d);
-
+#endif /* DEBUG */
+	put_move_header(d->m_mhp);
 	call_rcu(&d->m_rcu, free_move_data);
 #else /* VPROC_RCU_MOVEMENT_LIST */
 	if (!atomic_dec_and_lock(&d->m_refcnt, &movement_list_lock))
@@ -403,6 +448,7 @@
 {
 	struct move_header *h;
 
+	/* Test whether we are in mh_datalist */
 	if (atomic_xchg(&d->m_hashed, 0) == 0)
 		return;
 
@@ -431,7 +477,9 @@
 static int
 __drop_move_data(struct move_data *d)
 {
-	struct move_header *h;
+	struct move_header *h = d->m_mhp;
+
+	assert_spin_locked(&h->mh_lock);
 
 	if (list_empty(&d->m_list))
 		return 0;
@@ -472,9 +520,10 @@
 }
 #endif /* !VPROC_RCU_MOVEMENT_LIST */
 
+#define move_data_clear_execnode(_d) clear_task_execnode(NULL, _d)
 
 /* Might acquire movement_list_lock, mh_lock, task->alloc_lock */
-static inline void
+void
 clear_task_execnode(struct task_struct *p, struct move_data *d)
 {
 	struct move_header *h;
@@ -486,10 +535,9 @@
 		spin_lock(&p->alloc_lock);
 		if (!d && !(d = p->execnode)) {
 			spin_unlock(&p->alloc_lock);
-			goto out;
+			return;
 		}
 	}
-	get_move_data(d);
 	h = d->m_mhp;
 	spin_lock(&h->mh_lock);
 
@@ -510,18 +558,14 @@
 			spin_unlock(&h->mh_lock);
 			spin_unlock(&p->alloc_lock);
 			put_task_struct(p);
-			p = NULL;
 			goto out;
 		}
 		put_task_struct(p);
 	}
 
 	if (unlikely(p->execnode != d)) {
-		printk(KERN_WARNING "%s: task %p execnode %p != %p\n",
+		panic("%s: task %p execnode %p != %p\n",
 		      __FUNCTION__, p, p->execnode, d);
-		spin_unlock(&h->mh_lock);
-		spin_unlock(&p->alloc_lock);
-		goto out;
 	}
 	p->execnode = NULL;
 	spin_unlock(&p->alloc_lock);
@@ -544,7 +588,6 @@
 #endif /* !VPROC_RCU_MOVEMENT_LIST */
 
 	put_task_struct(p); /* for d->m_taskp */
-	put_move_data(d); /* for p->execnode */
 
 	switch (state) {
 	case PM_NOTIFIED:
@@ -560,24 +603,11 @@
 	}
  out:
 	if (d) {
-		WARN_ON(d->m_taskp != NULL);
 		drop_move_data(d);
-		put_move_data(d);
+		put_move_data(d); /* for p->execnode */
 	}
 }
 
-void
-ssi_clear_task_execnode(struct task_struct *p)
-{
-	clear_task_execnode(p, NULL);
-}
-
-static void
-move_data_clear_execnode(struct move_data *d)
-{
-	clear_task_execnode(NULL, d);
-}
-
 #ifdef VPROC_RCU_MOVEMENT_LIST
 static struct move_header *
 find_move_header(int id, int type)
@@ -728,6 +758,8 @@
 {
 	struct move_data *d;
 
+	assert_spin_locked(&h->mh_lock);
+
 	h->mh_state = state;
 	list_for_each_entry(d, &h->mh_datalist, m_list) {
 		d->m_state = state;
@@ -756,9 +788,6 @@
 	h->mh_magic = ((u_long)&h->mh_magic) + 1;
 #endif
 	atomic_set(&h->mh_refcnt, 1);
-#ifdef VPROC_RCU_MOVEMENT_LIST
-	atomic_set(&h->mh_abort, 0);
-#endif
 	h->mh_type = type;
 	h->mh_id = id;
 	h->mh_ctlnode = ctlnode;
@@ -770,6 +799,7 @@
 	h->mh_state = state;
 	h->mh_error = 0;
 	init_MUTEX_LOCKED(&h->mh_sync_done);
+	init_MUTEX(&h->mh_select_node);
 	h->mh_timeout = 0;
 	h->mh_aborting = 0;
 	init_waitqueue_head(&h->mh_wqh);
@@ -857,9 +887,8 @@
 	struct task_struct *p = current;
 	struct move_data *d;
 	struct move_header *h;
-	int select_node = 0;
 	clusternode_t node;
-	int error;
+	int error, select_node = 0;
 	enum move_state state;
 
 	spin_lock(&p->alloc_lock);
@@ -879,29 +908,50 @@
 	}
 
 	error = set_move_data_state(d, PM_NOTIFIED, PM_QUIESCED, 1);
-	if (error > 0) {
+
+	if (unlikely(d->m_state == PM_CONTINUE)) {
+		spin_unlock(&h->mh_lock);
+		if (error)
+			error = 0;
+		goto out_clear;
+	} else
 		node = h->mh_dstnode;
+
+	if (error > 0) {
 		if (node == CLUSTERNODE_BEST) {
 			spin_unlock(&h->mh_lock);
 
-			(void)VPOP_SELECT_NODE(p->p_vproc, NSC_COMM(p), &node);
-			select_node = 1;
-			if (node == this_node || node == CLUSTERNODE_INVAL) {
-				cleanup_load(node);
-				error = cancel_move_out(h, d, 0);
-				goto out_clear;
+			/* Ensure all in mh_datalist migrate to the same node */
+			down(&h->mh_select_node);
+
+			if (d->m_state == PM_ABORT) {
+				/* cancel_move_out called */
+				up(&h->mh_select_node);
+				error = -EINVAL;
+				goto out;
+			} else if (h->mh_dstnode != CLUSTERNODE_BEST) {
+				node = h->mh_dstnode;
+				up(&h->mh_select_node);
+			} else {
+				(void)VPOP_SELECT_NODE(p->p_vproc,
+							NSC_COMM(p), &node);
+				if (node == this_node ||
+				    node == CLUSTERNODE_INVAL) {
+					cleanup_load(node);
+					error = cancel_move_out(h, d, 0);
+					up(&h->mh_select_node);
+					goto out_clear;
+				}
+				select_node = 1;
 			}
 
 			spin_lock(&h->mh_lock);
-			h->mh_dstnode = node;
+			if (select_node) {
+				h->mh_dstnode = node;
+				up(&h->mh_select_node);
+			}
 		}
-
 		error = set_move_header_state(h, PM_NOTIFIED, PM_QUIESCED);
-	} else if (d->m_state == PM_CONTINUE) {
-		spin_unlock(&h->mh_lock);
-		if (error)
-			error = 0;
-		goto out_clear;
 	}
 	spin_unlock(&h->mh_lock);
 	if (error < 0)
@@ -909,20 +959,14 @@
 
 	/* Wait for daemon to tell us to start the migrate */
 	state = move_data_wait_for_state_change(d, PM_QUIESCED);
-#ifdef VPROC_MIGRATE_THREAD_GROUP_FIX_XXX
-	if (state == PM_CONTINUE /* SSI_XXX: unseen */ ||
-	    check_move_data_state(d, state, PM_MIGRATE_OUT_START)) {
-#else
-	if (check_move_data_state(d, state, PM_MIGRATE_OUT_START)) {
-#endif
+	if (unlikely(state == PM_CONTINUE ||
+	    	     check_move_data_state(d, state, PM_MIGRATE_OUT_START))) {
 		if (select_node)
 			cleanup_load(node);
 		error = -EINVAL;
 		goto out;
 	}
 
-	node = h->mh_dstnode;
-
 #ifdef CONFIG_LDLVL
 	/* borrowing pvp_pin field for node info for logging */
 	if (h->mh_type & MOV_BESTNODE)
@@ -971,44 +1015,6 @@
 	return error;
 }
 
-static struct move_data *
-alloc_move_data(struct move_header *h, int pid, enum move_state state)
-{
-	struct move_data *d;
-
-#ifdef VPROC_MOVE_DATA_KMEM_CACHE
-	d = kmem_cache_alloc(move_data_cachep, SLAB_KERNEL);
-#else
-	d = kmalloc(sizeof(*d), GFP_USER);
-#endif
-	if (!d)
-		return NULL;
-
-	INIT_LIST_HEAD(&d->m_list);
-#ifdef DEBUG
-	d->m_magic = ((u_long)&d->m_magic) + 1;
-#endif
-	atomic_set(&d->m_refcnt, 1);
-	d->m_pid = pid;
-	d->m_leader = 0;
-	INIT_LIST_HEAD(&d->m_threadlist);
-	d->m_migargs = NULL;
-	d->m_state = state;
-	d->m_seen = 0;
-	init_waitqueue_head(&d->m_wqh);
-	d->m_mhp = get_move_header(h);
-#ifdef VPROC_RCU_MOVEMENT_LIST
-	SSI_ASSERT(d->m_mhp);
-#endif
-	d->m_taskp = NULL;
-#ifdef VPROC_RCU_MOVEMENT_LIST
-	atomic_set(&d->m_hashed, 0);
-	INIT_RCU_HEAD(&d->m_rcu);
-#endif
-
-	return d;
-}
-
 #ifdef SSI_NOTUSED
 static inline int
 __proc_is_shared(task_t *p, task_t *t)
@@ -1081,6 +1087,8 @@
 #ifdef VPROC_MIGRATE_THREAD_GROUP_FIX
 		t = p;
 		while_each_thread(p, t) {
+			if (t->flags & PF_EXITING)
+				continue;
 #else
 		for_each_process(t) {
 			if (t == p)
@@ -1216,15 +1224,16 @@
 /* Might acquire movement_list_lock, mh_lock */
 /* Might acquire task_lock */
 /* Might acquire read tasklist_lock */
+/* Returns number of move_data (aka. processes) added to mh_datalist */
 static int
 setup_pid_move(struct move_header *h, pid_t pid, pid_t leader)
 {
-	int retval = 0, was_seen = 0, tmpret;
-	struct move_data *d;
-	struct task_struct *p;
-	unsigned long flags;
 	sigset_t fillset;
+	struct task_struct *p = NULL;
 	struct vproc *v;
+	struct move_data *d;
+	unsigned long flags;
+	int count, tmpret, was_seen;
 
 	v = LOCATE_VPROC_PID_ORIGIN(pid, "setup_pid_move");
 	if (!v)
@@ -1236,30 +1245,35 @@
 	VPROC_LOCK_EXCL(v, "setup_pid_move");
 #endif
 #ifdef VPROC_HOLD_ZERO_GET_TASK
-	if ((PVP(v)->pvp_flag & PV_EXITING) || !PV_IS_ALIVE(PVP(v))) {
+	if (PV_IS_ALIVE(PVP(v))) {
 #else
-	if ((PVP(v)->pvp_flag & PV_EXITING) || !PVP(v)->pvp_pproc) {
+	if (!(PVP(v)->pvp_flag & PV_EXITING) && PVP(v)->pvp_pproc) {
 #endif /* !VPROC_HOLD_ZERO_GET_TASK */
+		p = PVP(v)->pvp_pproc;
+		get_task_struct(p);
+	}
 #ifdef VPROC_RW_LOCK
-		VPROC_UNLOCK_SHARED(v, "setup_pid_move");
+	VPROC_UNLOCK_SHARED(v, "setup_pid_move");
 #else
-		VPROC_UNLOCK_EXCL(v, "setup_pid_move");
+	VPROC_UNLOCK_EXCL(v, "setup_pid_move");
 #endif
-		VPROC_RELE(v, "setup_pid_move");
+	VPROC_RELE(v, "setup_pid_move");
+	if (p == NULL)
 		return -ESRCH;
-	}
-	p = PVP(v)->pvp_pproc;
 
 #ifdef VPROC_RCU_MOVEMENT_LIST
 	d = find_move_data(h, pid);
 	if (d) {
-		spin_lock(&h->mh_lock);
+		/* NB: skipped mh_lock for m_seen since the lock
+		 * does not prevent race with check_move_list().
+		 * Caller must prevent race with check_move_list()
+		 */
 		was_seen = d->m_seen;
 		d->m_seen = 1;
-		spin_unlock(&h->mh_lock);
+		count = 0;
 		goto already_done;
 	}
-#else
+#else /* VPROC_RCU_MOVEMENT_LIST */
 	spin_lock(&h->mh_lock);
 	d = __find_move_data(h, pid);
 	if (d) {
@@ -1269,39 +1283,35 @@
 		goto already_done;
 	}
 	spin_unlock(&h->mh_lock);
-#endif
+#endif /* !VPROC_RCU_MOVEMENT_LIST */
 
 	d = alloc_move_data(h, pid, PM_NOTIFIED);
-	if (!d) {
-#ifdef VPROC_RW_LOCK
-		VPROC_UNLOCK_SHARED(v, "setup_pid_move");
-#else
-		VPROC_UNLOCK_EXCL(v, "setup_pid_move");
-#endif
-		VPROC_RELE(v, "setup_pid_move");
+	if (!d)
 		return -ENOMEM;
-	}
 	d->m_seen = 1;
-	retval = 1;
 
 	spin_lock(&p->alloc_lock);
-	if (p->execnode) {
+
+	if (p->execnode ||
+	    (p->flags & PF_EXITING)) {
+		/* lost the race */
 		spin_unlock(&p->alloc_lock);
-#ifdef VPROC_RW_LOCK
-		VPROC_UNLOCK_SHARED(v, "setup_pid_move");
-#else
-		VPROC_UNLOCK_EXCL(v, "setup_pid_move");
-#endif
-		VPROC_RELE(v, "setup_pid_move");
 		put_move_data(d);
+
+		/* We might have lost the race with
+		 * ssi_clear_task_execnode()
+		 */
+		if (p->flags & PF_EXITING)
+			return -ESRCH;
+
 		return -EAGAIN;
 	}
 	p->execnode = get_move_data(d);
-	spin_unlock(&p->alloc_lock);
 
 	get_task_struct(p);
 	d->m_taskp = p;
 
+	get_move_data(d); /* for mh_datalist; active */
 #ifdef VPROC_RCU_MOVEMENT_LIST
 	atomic_inc(&d->m_hashed);
 	spin_lock(&h->mh_lock);
@@ -1316,6 +1326,12 @@
 		h->mh_state = PM_NOTIFIED;
 	spin_unlock(&h->mh_lock);
 
+	/* To prevent race with clear_task_execnode()
+	 * we must hold alloc_lock until we are finished setting up
+	 * move_data structure and mh_state.
+	 */
+	spin_unlock(&p->alloc_lock);
+
 	if ((h->mh_type & MOV_SIGMIG) || p != current) {
 #ifdef CFS_INTR
 		/* Avoid interrupted system call on non-chard mounts */
@@ -1335,55 +1351,30 @@
 		spin_unlock_irqrestore(&p->sighand->siglock, flags);
 	}
 
-	get_move_data(d);
- already_done:
-#ifdef VPROC_RW_LOCK
-	VPROC_UNLOCK_SHARED(v, "setup_pid_move");
-#else
-	VPROC_UNLOCK_EXCL(v, "setup_pid_move");
-#endif
-	get_task_struct(p);
-	VPROC_RELE(v, "setup_pid_move");
+	count = 1;
+	was_seen = 0;
 
+already_done:
 	if (!was_seen) {
 		d->m_leader = 0;
 		if (!leader) {
 			if (!thread_group_empty(p)) {
 				if (d->m_pid != p->tgid)
 					d->m_leader = p->tgid;
+
 				tmpret = add_thread_group(h, p);
-			} else {
-				tmpret = 0;
-				task_lock(p);
-				if ((p->mm &&
-				     atomic_read(&p->mm->mm_users) > 1) ||
-				    (p->fs &&
-				     atomic_read(&p->fs->count) > 1) ||
-				    (p->files &&
-				     atomic_read(&p->files->count) > 1))
-				    tmpret = 1;
-				task_unlock(p);
-				read_lock(&tasklist_lock);
-				if ((p->signal &&
-				     atomic_read(&p->signal->count) > 1) ||
-				    (p->sighand &&
-				     atomic_read(&p->sighand->count) > 1))
-					tmpret = 1;
-				read_unlock(&tasklist_lock);
-				if (tmpret)
-					tmpret = add_thread_group(h, p);
+				if (tmpret < 0)
+					count = tmpret;
+				else
+					count += tmpret;
 			}
-			if (tmpret < 0)
-				retval = tmpret;
-			else
-				retval += tmpret;
 		} else if (d->m_pid != leader && d->m_pid != -leader)
 			d->m_leader = leader;
 	}
 	put_move_data(d);
 	put_task_struct(p);
 
-	return retval;
+	return count;
 }
 
 static int
@@ -1430,36 +1421,32 @@
 
 /* Acquires mh_lock */
 /* Might acquire movement_list_lock */
+/* Returns 0 only if all in mh_datalist are quiesced */
 static int
 check_move_list(struct move_header *h)
 {
-#ifdef VPROC_RCU_MOVEMENT_LIST
-	struct move_data *d;
 	int retval;
+	struct move_data *d;
 
+#ifdef VPROC_RCU_MOVEMENT_LIST
+	/* No race over m_seen.
+	 * Caller ensures only one setup_pid_move() / setup_pgrp_move()
+	 * thread per move_header struct.
+	 */
 	rcu_read_lock();
 	list_for_each_entry_rcu(d, &h->mh_datalist, m_list) {
 		d->m_seen = 0;
 	}
 	rcu_read_unlock();
-#else
-	int retval, do_drop;
-	struct move_data *d, *next;
-	LIST_HEAD(unseen_list);
-
-	spin_lock(&h->mh_lock);
-	list_for_each_entry(d, &h->mh_datalist, m_list) {
-		d->m_seen = 0;
-	}
-	spin_unlock(&h->mh_lock);
-#endif /* !VPROC_RCU_MOVEMENT_LIST */
 
 	if (h->mh_type & MOV_PGRP)
 		retval = setup_pgrp_move(h);
 	else
 		retval = setup_pid_move(h, h->mh_id, 0);
 
-#ifdef VPROC_RCU_MOVEMENT_LIST
+	/* Skip tasks that have detached from the group
+	 * before they could be quiesced.
+	 */
 	rcu_read_lock();
 	list_for_each_entry_rcu(d, &h->mh_datalist, m_list) {
 		if (d->m_seen)
@@ -1473,8 +1460,30 @@
 	}
 	rcu_read_unlock();
 
+	if (retval)
+		return retval;
 	spin_lock(&h->mh_lock);
+	if (h->mh_state == PM_NOTIFIED && h->mh_done == h->mh_wanted)
+		retval = set_move_header_state(h, PM_NOTIFIED, PM_QUIESCED);
+	spin_unlock(&h->mh_lock);
+
+	return retval;
 #else /* VPROC_RCU_MOVEMENT_LIST */
+	int do_drop;
+	struct move_data *next;
+	LIST_HEAD(unseen_list);
+
+	spin_lock(&h->mh_lock);
+	list_for_each_entry(d, &h->mh_datalist, m_list) {
+		d->m_seen = 0;
+	}
+	spin_unlock(&h->mh_lock);
+
+	if (h->mh_type & MOV_PGRP)
+		retval = setup_pgrp_move(h);
+	else
+		retval = setup_pid_move(h, h->mh_id, 0);
+
 	spin_lock(&h->mh_lock);
 
 	list_for_each_entry_safe(d, next, &h->mh_datalist, m_list) {
@@ -1495,8 +1504,6 @@
 		put_move_data(d); /* temp */
 		spin_lock(&h->mh_lock);
 	}
-#endif /* !VPROC_RCU_MOVEMENT_LIST */
-
 	if (!retval && h->mh_state == PM_NOTIFIED &&
 	    h->mh_done == h->mh_wanted)
 		retval = set_move_header_state(h, PM_NOTIFIED, PM_QUIESCED);
@@ -1504,6 +1511,7 @@
 	spin_unlock(&h->mh_lock);
 
 	return retval;
+#endif /* !VPROC_RCU_MOVEMENT_LIST */
 }
 
 static enum move_state
@@ -1517,7 +1525,7 @@
 
 	ssi_block_signals(current, &saved_blocked);
 	wait_event_interruptible(h->mh_wqh,
-			(state = h->mh_state) != expected_state || h->mh_timeout);
+		(state = h->mh_state) != expected_state || h->mh_timeout);
 	ssi_unblock_signals(current, &saved_blocked);
 	return state;
 }
@@ -1532,8 +1540,8 @@
 	might_sleep();
 
 	ssi_block_signals(current, &saved_blocked);
-	wait_event_interruptible_exclusive(d->m_wqh,
-				(state = d->m_state) != expected_state);
+	wait_event_interruptible(d->m_wqh,
+			(state = d->m_state) != expected_state);
 	ssi_unblock_signals(current, &saved_blocked);
 	return state;
 }
@@ -1615,6 +1623,7 @@
 			error = -EINVAL;
 			goto out;
 		}
+
 		error = check_move_list(h);
 		if (error < 0)
 			goto out;
@@ -1622,21 +1631,22 @@
 			break;
 	}
 
-	spin_lock(&h->mh_lock);
+	/* NB: No need to hold mh_lock for mh_total,
+	 * above for-loop should have dropped stale entries from mh_datalist.
+	 */
 	total = h->mh_total;
-	spin_unlock(&h->mh_lock);
 	if (total == 0)
 		goto out;
 
 	/* XXX objects */
 
-	proc_list = kzmalloc(sizeof(*proc_list) * total, GFP_KERNEL);
+	proc_list = kmalloc(sizeof(*proc_list) * total, GFP_KERNEL);
 	if (!proc_list) {
 		error = -ENOMEM;
 		goto out;
 	}
 
-	data_list = kzmalloc(sizeof(*data_list) * total, GFP_KERNEL);
+	data_list = kmalloc(sizeof(*data_list) * total, GFP_KERNEL);
 	if (!data_list) {
 		kfree(proc_list);
 		error = -ENOMEM;
@@ -1730,7 +1740,7 @@
 	if (error < 0)
 		(void) cancel_move_out(h, NULL, error);
 
-	put_move_header(h);
+	put_move_header(h); /* move_out_daemon */
 	exit_daemon_proc();
 }
 
@@ -1747,6 +1757,7 @@
 		       __FUNCTION__, type);
 		return -EINVAL;
 	}
+
 	if (node < 0) {
 		type |= MOV_PGRP;
 		id = process_group(p);
@@ -1755,6 +1766,7 @@
 		type |= MOV_PID;
 		id = p->pid;
 	}
+
 	if (node == CLUSTERNODE_BEST)
 		type |= MOV_BESTNODE;
 	else if (node == this_node || node == CLUSTERNODE_THIS)
@@ -1769,26 +1781,15 @@
 	if (!h)
 		return -ENOMEM;
 
-	if (type & MOV_PGRP)
-		error = setup_pgrp_move(h);
-	else
-		error = setup_pid_move(h, id, 0);
-	if (error <= 0) {
-		error = cancel_move_out(h, NULL, error);
-		put_move_header(h);
-		return error;
-	}
-
 	/* Check if entry already exists */
 #ifdef VPROC_RCU_MOVEMENT_LIST
 	down(&movement_list_sem);
 	h_old = find_move_header(id, MOV_OUT);
 	if (h_old) {
 		up(&movement_list_sem);
-		error = cancel_move_out(h, NULL, -EAGAIN);
-		put_move_header(h);
 		put_move_header(h_old);
-		return error;
+		put_move_header(h);
+		return -EAGAIN;
 	}
 	spin_lock(&movement_list_lock);
 	list_add_rcu(&h->mh_list, &movement_list);
@@ -1800,14 +1801,23 @@
 	if (h_old) {
 		spin_unlock(&movement_list_lock);
 		put_move_header(h_old);
-		error = cancel_move_out(h, NULL, -EAGAIN);
 		put_move_header(h);
-		return error;
+		return -EAGAIN;
 	}
 	list_add(&h->mh_list, &movement_list);
 	spin_unlock(&movement_list_lock);
 #endif /* !VPROC_RCU_MOVEMENT_LIST */
 
+	if (type & MOV_PGRP)
+		error = setup_pgrp_move(h);
+	else
+		error = setup_pid_move(h, id, 0);
+	if (error <= 0) {
+		error = cancel_move_out(h, NULL, error);
+		put_move_header(h);
+		return error;
+	}
+
 	get_move_header(h); /* for move_out_daemon */
 	error = spawn_daemon_proc("move out daemon", move_out_daemon, h);
 	if (error < 0) {
@@ -1823,8 +1833,8 @@
 		}
 
 		error = cancel_move_out(h, NULL, error);
+		put_move_header(h); /* move_out_daemon */
 		put_move_header(h);
-		put_move_header(h); /* final */
 		return error;
 	}
 
@@ -1942,8 +1952,8 @@
 		goto out;
 
 	args->migrate_args.data = get_move_data(d);
-	args->migrate_args.optype = RVP_MIGRATE;
 	if (!leader || leader == args->childpid) {
+		args->migrate_args.optype = RVP_MIGRATE;
 		retval = rproc_create_proc(migrate_server_setup, args, 0, 0);
 		goto out;
 	}
@@ -2173,24 +2183,25 @@
 
 	for(i = 0 ; i < pidcnt; i++) {
 		d = alloc_move_data(h, pidlist[i], PM_QUIESCED);
-		if (!d)
+		if (!d) {
+			h->mh_wanted = h->mh_total;
 			goto out_err;
+		}
+		/* Skipped mh_lock, no race; we're not in movement_list */
 #ifdef VPROC_RCU_MOVEMENT_LIST
-		atomic_inc(&d->m_hashed);
-		spin_lock(&h->mh_lock);
+		atomic_set(&d->m_hashed, 1);
 		list_add_tail_rcu(&d->m_list, &h->mh_datalist);
 #else
-		spin_lock(&h->mh_lock);
 		list_add(&d->m_list, &h->mh_datalist);
 #endif
-		h->mh_wanted = ++(h->mh_total);
-		spin_unlock(&h->mh_lock);
+		h->mh_total++;
 	}
+	h->mh_wanted = h->mh_total;
 
 #ifdef VPROC_RCU_MOVEMENT_LIST
 	down(&movement_list_sem);
 	h_old = find_move_header(id, type);
-	if (h_old) {
+	if (unlikely(h_old)) {
 		up(&movement_list_sem);
 		put_move_header(h_old);
 		*rval = -EEXIST;
@@ -2252,16 +2263,14 @@
 	spin_unlock(&movement_list_lock);
 #else
 	h = find_move_header(id, MOV_IN);
-	if (!h) {
+	if (unlikely(!h)) {
 		printk(KERN_WARNING
 		       "%s:Failed to find move in header for %d\n",
 		       __FUNCTION__, id);
 		*rval = -ENOENT;
 		return 0;
 	}
-
-	/* Hide from find_move_header() */
-	atomic_inc(&h->mh_abort);
+	/* NB: no longer hide entry from find_move_header(), side-effects? */
 #endif /* VPROC_RCU_MOVEMENT_LIST */
 
 	/* enable all local objects */
@@ -2304,7 +2313,6 @@
 		*rval = -ENOENT;
 		return 0;
 	}
-
 	BUG_ON(d->m_mhp != h);
 
 	spin_lock(&h->mh_lock);


------------------------------------------------------------------------------
Lotusphere 2011
Register now for Lotusphere 2011 and learn how
to connect the dots, take your collaborative environment
to the next level, and enter the era of Social Business.
http://p.sf.net/sfu/lotusphere-d2d