On 9/11/19 11:56 AM, Daniel P. Berrangé wrote:
> On Wed, Sep 11, 2019 at 05:42:00PM +0100, Dr. David Alan Gilbert (git) wrote:
>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>>
>> RCU_READ_LOCK_AUTO takes the rcu_read_lock  and then uses glib's

spurious double space

>> g_auto infrastrcture (and thus whatever the compilers hooks are) to

infrastructure
compiler's

>> release it on all exits of the block.
>>
>> Note this macro has a variable declaration in, and hence is not in
>> a while loop.
>>

>> +#define RCU_READ_LOCK_AUTO g_auto(rcu_read_auto_t) \
>> +    _rcu_read_auto = 'x'; \
>> +    rcu_read_lock();
>> +
> 
> Functionally this works, but my gut feeling would be to follow
> the design of GMutexLocker as-is:
> 
>   https://developer.gnome.org/glib/stable/glib-Threads.html#g-mutex-locker-new
> 
> so you get a use pattern of
> 
>   g_autoptr(rcu_read_locker) locker = rcu_read_locker_new();

Another pattern to consider: nbdkit uses:

#define ACQUIRE_LOCK_FOR_CURRENT_SCOPE(mutex) \
  CLEANUP_UNLOCK pthread_mutex_t *_lock = mutex; \
  do { \
    int _r = pthread_mutex_lock (_lock); \
    assert (!_r); \
  } while (0)

with later code calling:

  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);

> 
> This makes it explicit that the code is creating a variable here, which
> in turns means it is clear to force unlock early with
> 
>   g_clear_pointer(&locker, rcu_read_locker_free)

Yes, this aspect of glib is nicer than the corresponding nbdkit usage
pattern.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org