* [ltt-dev] [PATCH RFC] call_rcu() interface for userspace-rcu
[not found] ` <4CCBDE0B.8060102@redhat.com>
@ 2010-11-01 20:04 ` Paul E. McKenney
2010-11-01 20:25 ` Mathieu Desnoyers
0 siblings, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2010-11-01 20:04 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ltt-dev] [PATCH RFC] call_rcu() interface for userspace-rcu
2010-11-01 20:04 ` [ltt-dev] [PATCH RFC] call_rcu() interface for userspace-rcu Paul E. McKenney
@ 2010-11-01 20:25 ` Mathieu Desnoyers
2010-11-02 12:12 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Mathieu Desnoyers @ 2010-11-01 20:25 UTC (permalink / raw)
* 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ltt-dev] [PATCH RFC] call_rcu() interface for userspace-rcu
2010-11-01 20:25 ` Mathieu Desnoyers
@ 2010-11-02 12:12 ` Paul E. McKenney
2010-11-11 8:44 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2010-11-02 12:12 UTC (permalink / raw)
On Mon, Nov 01, 2010 at 04:25:30PM -0400, Mathieu Desnoyers wrote:
> * 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).
OK, once I become more confident that I don't have bugs elsewhere, I will
try out this approach.
Thanx, Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ltt-dev] [PATCH RFC] call_rcu() interface for userspace-rcu
2010-11-02 12:12 ` Paul E. McKenney
@ 2010-11-11 8:44 ` Paolo Bonzini
2010-11-12 0:24 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2010-11-11 8:44 UTC (permalink / raw)
On 11/02/2010 01:12 PM, Paul E. McKenney wrote:
> OK, once I become more confident that I don't have bugs elsewhere, I will
> try out this approach.
I can also play with it myself since now your patch is in urcu.git.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ltt-dev] [PATCH RFC] call_rcu() interface for userspace-rcu
2010-11-11 8:44 ` Paolo Bonzini
@ 2010-11-12 0:24 ` Paolo Bonzini
2010-11-16 0:16 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2010-11-12 0:24 UTC (permalink / raw)
On 11/11/2010 09:44 AM, Paolo Bonzini wrote:
> I can also play with it myself since now your patch is in urcu.git.
Ahem, it's not, but that doesn't mean I cannot do the futex stuff. :)
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ltt-dev] [PATCH RFC] call_rcu() interface for userspace-rcu
2010-11-12 0:24 ` Paolo Bonzini
@ 2010-11-16 0:16 ` Paul E. McKenney
0 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2010-11-16 0:16 UTC (permalink / raw)
On Fri, Nov 12, 2010 at 01:24:03AM +0100, Paolo Bonzini wrote:
> On 11/11/2010 09:44 AM, Paolo Bonzini wrote:
> >I can also play with it myself since now your patch is in urcu.git.
>
> Ahem, it's not, but that doesn't mean I cannot do the futex stuff. :)
I would very much welcome that, by the way! ;-)
Thanx, Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-11-16 0:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20101029162534.GA11742@linux.vnet.ibm.com>
[not found] ` <4CCBDE0B.8060102@redhat.com>
2010-11-01 20:04 ` [ltt-dev] [PATCH RFC] call_rcu() interface for userspace-rcu Paul E. McKenney
2010-11-01 20:25 ` Mathieu Desnoyers
2010-11-02 12:12 ` Paul E. McKenney
2010-11-11 8:44 ` Paolo Bonzini
2010-11-12 0:24 ` Paolo Bonzini
2010-11-16 0:16 ` Paul E. McKenney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox