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

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck at linux.vnet.ibm.com) wrote:
> 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.

Paul, this is why I created urcu-futex.h. It offers the futex API, but with a
polling or pthread-cond-based fallback for non-Linux platforms. So please feel
free to implement this using futex_noasync() (this is the version using a
pthread_cond() fallback, which is not signal-safe, but does not require
polling).

Thanks,

Mathieu


-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com