Re: [Qemu-devel] [PATCH v3 1/5] rcu: Add automatically released rcu_read_lock variants

[Dr. David Alan Gilbert]

* Paolo Bonzini (address@hidden) wrote:
> On 13/09/19 12:25, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> > 
> > RCU_READ_LOCK_GUARD() takes the rcu_read_lock and then uses glib's
> > g_auto infrastructure (and thus whatever the compiler's hooks are) to
> > release it on all exits of the block.
> > 
> > WITH_RCU_READ_LOCK_GUARD() is similar but is used as a wrapper for the
> > lock, i.e.:
> > 
> >    WITH_RCU_READ_LOCK_GUARD() {
> >        stuff under lock
> >    }
> > 
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> >  docs/devel/rcu.txt | 16 ++++++++++++++++
> >  include/qemu/rcu.h | 25 +++++++++++++++++++++++++
> >  2 files changed, 41 insertions(+)
> > 
> > diff --git a/docs/devel/rcu.txt b/docs/devel/rcu.txt
> > index c84e7f42b2..d83fed2f79 100644
> > --- a/docs/devel/rcu.txt
> > +++ b/docs/devel/rcu.txt
> > @@ -187,6 +187,22 @@ The following APIs must be used before RCU is used in 
> > a thread:
> >  Note that these APIs are relatively heavyweight, and should _not_ be
> >  nested.
> >  
> > +Convenience macros
> > +==================
> > +
> > +Two macros are provided that automatically release the read lock at the
> > +end of the scope.
> > +
> > +      RCU_READ_LOCK_GUARD()
> > +
> > +         Takes the lock and will release it at the end of the block it's
> > +         used in.
> > +
> > +      WITH_RCU_READ_LOCK_GUARD()  { code }
> > +
> > +         Is used at the head of a block to protect the code within the 
> > block.
> > +
> > +Note that 'goto'ing out of the guarded block will also drop the lock.
> >  
> >  DIFFERENCES WITH LINUX
> >  ======================
> > diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h
> > index 22876d1428..3a8d4cf28b 100644
> > --- a/include/qemu/rcu.h
> > +++ b/include/qemu/rcu.h
> > @@ -154,6 +154,31 @@ extern void call_rcu1(struct rcu_head *head, RCUCBFunc 
> > *func);
> >        }),                                                                \
> >        (RCUCBFunc *)g_free);
> >  
> > +typedef void RCUReadAuto;
> > +static inline RCUReadAuto *rcu_read_auto_lock(void)
> > +{
> > +    rcu_read_lock();
> > +    /* Anything non-NULL causes the cleanup function to be called */
> > +    return (void *)(uintptr_t)0x1;
> > +}
> > +
> > +static inline void rcu_read_auto_unlock(RCUReadAuto *r)
> > +{
> > +    rcu_read_unlock();
> > +}
> > +
> > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(RCUReadAuto, rcu_read_auto_unlock)
> > +
> > +#define WITH_RCU_READ_LOCK_GUARD() \
> > +    WITH_RCU_READ_LOCK_GUARD_(_rcu_read_auto##__COUNTER__)
> > +
> > +#define WITH_RCU_READ_LOCK_GUARD_(var) \
> > +    for (g_autoptr(RCUReadAuto) var = rcu_read_auto_lock(); \
> > +        (var); rcu_read_auto_unlock(var), (var) = NULL)
> > +
> > +#define RCU_READ_LOCK_GUARD() \
> > +    g_autoptr(RCUReadAuto) _rcu_read_auto = rcu_read_auto_lock()
> > +
> >  #ifdef __cplusplus
> >  }
> >  #endif
> > 
> 
> Acked-by: Paolo Bonzini <address@hidden>

Can you look at the other bits of this series and tell me if they're OK?
If they are, do you want to take them (since it's RCU based)?

Dave

--
Dr. David Alan Gilbert / address@hidden / Manchester, UK