[ltt-dev] [PATCH RFC] call_rcu() interface for userspace-rcu

[Paul E. McKenney]

On Sat, Oct 30, 2010 at 10:57:47AM +0200, Paolo Bonzini wrote:
> >+static void *call_rcu_thread(void *arg)
> >+{
> >+		[...]
> >+		else {
> >+			call_rcu_lock(&crdp->mtx);
> >+			crdp->flags &= ~URCU_CALL_RCU_RUNNING;
> >+			if (&cbs->next != cbs_tail&&
> >+			    pthread_cond_wait(&crdp->cond,&crdp->mtx) != 0) {
> >+				perror("pthread_cond_wait");
> >+				exit(-1);
> >+			} else
> >+				poll(NULL, 0, 10);
> >+			crdp->flags |= URCU_CALL_RCU_RUNNING;
> >+			call_rcu_unlock(&crdp->mtx);
> >+		}
> >+	}
> >+	return NULL;  /* NOTREACHED */
> >+}
> 
> Given the way you handle URCU_CALL_RCU_RUNNING above, the flag will
> be reset in call_rcu iff call_rcu sees contention on the lock.

Doesn't pthread_cond_wait() release the mutex for the duration of the
wait?  Ah, are you worried about the poll() under the lock?  I am
moving this out from under the lock.

> >+		call_rcu_lock(&crdp->mtx);
> >+		if (!(crdp->flags&  URCU_CALL_RCU_RUNNING)) {
> >+			if (pthread_cond_signal(&crdp->cond) != 0) {
> >+				perror("pthread_cond_signal");
> >+				exit(-1);
> >+			}
> >+		}
> >+		call_rcu_unlock(&crdp->mtx);
> >+	}
> >+}
> 
> So, the mutex is basically unnecessary if some futex magic replaces
> the condition variable.  For example, in the thread:
> 
> 	else {
> 	   retry:
> 		flags = crdp->flags;
> 		if ((flags & URCU_CALL_RCU_REQUESTED))
> 			continue;
> 		if (cmpxchg (&crdp->flags, flags,
> 		             flags & ~URCU_CALL_RCU_RUNNING) != flags)
> 			goto retry;
> 
> 		futex_wait (&crdp->flags,
> 			    flags & ~URCU_CALL_RCU_RUNNING);
> 	}
> 
> and in call_rcu:
> 
> 	mb ();
> 	/* If the thread is not blocked, it will see our request.  */
> 	do {
> 		flags = crdp->flags;
> 		/* If there's already a request pending, no need to
> 		   wake up the process.  If the thread is running, no
> 		   need to do anything, it'll pick up our request.  */
> 		if (flags &
> 		    (URCU_CALL_RCU_REQUESTED | URCU_CALL_RCU_RUNNING))
> 			return;
> 	} while (cmpxchg (&crdp->flags, flags,
> 			  flags | URCU_CALL_RCU_REQUESTED) != flags);
> 	futex_wake (&crdp->flags, 1);

OK, sounds like a nice optimization, though a bit Linux-specific.
I will stick with the POSIX stuff for the moment, and once I am
convinced that it really is working, I might consider doing futexes
if running on Linux.

							Thanx, Paul