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
_______________________________________________
ltt-dev mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
