mbox series

[v3,0/5] Automatic RCU read unlock

Message ID 20190913102538.24167-1-dgilbert@redhat.com (mailing list archive)
Headers show
Series Automatic RCU read unlock | expand

Message

Dr. David Alan Gilbert Sept. 13, 2019, 10:25 a.m. UTC
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

This patch uses glib's g_auto mechanism to automatically free
rcu_read_lock's at the end of the block.  Given that humans
have a habit of forgetting an error path somewhere it's
best to leave it to the compiler.

v3
  Add block-head version of macro
  Rename
  Add docs
  Convert more cases using the block-head version

Dr. David Alan Gilbert (5):
  rcu: Add automatically released rcu_read_lock variants
  migration: Fix missing rcu_read_unlock
  migration: Use automatic rcu_read unlock in ram.c
  migration: Use automatic rcu_read unlock in rdma.c
  rcu: Use automatic rc_read unlock in core memory/exec code

 docs/devel/rcu.txt      |  16 +++
 exec.c                  | 120 +++++++---------
 include/exec/ram_addr.h | 138 +++++++++----------
 include/qemu/rcu.h      |  25 ++++
 memory.c                |  15 +-
 migration/ram.c         | 295 +++++++++++++++++++---------------------
 migration/rdma.c        |  57 ++------
 7 files changed, 310 insertions(+), 356 deletions(-)

Comments

Dr. David Alan Gilbert Sept. 25, 2019, 10:32 a.m. UTC | #1
* Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> This patch uses glib's g_auto mechanism to automatically free
> rcu_read_lock's at the end of the block.  Given that humans
> have a habit of forgetting an error path somewhere it's
> best to leave it to the compiler.

Queued

> v3
>   Add block-head version of macro
>   Rename
>   Add docs
>   Convert more cases using the block-head version
> 
> Dr. David Alan Gilbert (5):
>   rcu: Add automatically released rcu_read_lock variants
>   migration: Fix missing rcu_read_unlock
>   migration: Use automatic rcu_read unlock in ram.c
>   migration: Use automatic rcu_read unlock in rdma.c
>   rcu: Use automatic rc_read unlock in core memory/exec code
> 
>  docs/devel/rcu.txt      |  16 +++
>  exec.c                  | 120 +++++++---------
>  include/exec/ram_addr.h | 138 +++++++++----------
>  include/qemu/rcu.h      |  25 ++++
>  memory.c                |  15 +-
>  migration/ram.c         | 295 +++++++++++++++++++---------------------
>  migration/rdma.c        |  57 ++------
>  7 files changed, 310 insertions(+), 356 deletions(-)
> 
> -- 
> 2.21.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Dr. David Alan Gilbert Sept. 25, 2019, 1:13 p.m. UTC | #2
* Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> This patch uses glib's g_auto mechanism to automatically free
> rcu_read_lock's at the end of the block.  Given that humans
> have a habit of forgetting an error path somewhere it's
> best to leave it to the compiler.

I've had to unqueue this - clang doesn't like the apparently unused
auto variable; we need to find a way to make that happy.

Dave

> v3
>   Add block-head version of macro
>   Rename
>   Add docs
>   Convert more cases using the block-head version
> 
> Dr. David Alan Gilbert (5):
>   rcu: Add automatically released rcu_read_lock variants
>   migration: Fix missing rcu_read_unlock
>   migration: Use automatic rcu_read unlock in ram.c
>   migration: Use automatic rcu_read unlock in rdma.c
>   rcu: Use automatic rc_read unlock in core memory/exec code
> 
>  docs/devel/rcu.txt      |  16 +++
>  exec.c                  | 120 +++++++---------
>  include/exec/ram_addr.h | 138 +++++++++----------
>  include/qemu/rcu.h      |  25 ++++
>  memory.c                |  15 +-
>  migration/ram.c         | 295 +++++++++++++++++++---------------------
>  migration/rdma.c        |  57 ++------
>  7 files changed, 310 insertions(+), 356 deletions(-)
> 
> -- 
> 2.21.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Paolo Bonzini Sept. 25, 2019, 1:21 p.m. UTC | #3
On 25/09/19 15:13, Dr. David Alan Gilbert wrote:
> * Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>>
>> This patch uses glib's g_auto mechanism to automatically free
>> rcu_read_lock's at the end of the block.  Given that humans
>> have a habit of forgetting an error path somewhere it's
>> best to leave it to the compiler.
> 
> I've had to unqueue this - clang doesn't like the apparently unused
> auto variable; we need to find a way to make that happy.

__attribute__((unused))?

Paolo

> Dave
> 
>> v3
>>   Add block-head version of macro
>>   Rename
>>   Add docs
>>   Convert more cases using the block-head version
>>
>> Dr. David Alan Gilbert (5):
>>   rcu: Add automatically released rcu_read_lock variants
>>   migration: Fix missing rcu_read_unlock
>>   migration: Use automatic rcu_read unlock in ram.c
>>   migration: Use automatic rcu_read unlock in rdma.c
>>   rcu: Use automatic rc_read unlock in core memory/exec code
>>
>>  docs/devel/rcu.txt      |  16 +++
>>  exec.c                  | 120 +++++++---------
>>  include/exec/ram_addr.h | 138 +++++++++----------
>>  include/qemu/rcu.h      |  25 ++++
>>  memory.c                |  15 +-
>>  migration/ram.c         | 295 +++++++++++++++++++---------------------
>>  migration/rdma.c        |  57 ++------
>>  7 files changed, 310 insertions(+), 356 deletions(-)
>>
>> -- 
>> 2.21.0
>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
Dr. David Alan Gilbert Sept. 25, 2019, 3:28 p.m. UTC | #4
* Paolo Bonzini (pbonzini@redhat.com) wrote:
> On 25/09/19 15:13, Dr. David Alan Gilbert wrote:
> > * Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >>
> >> This patch uses glib's g_auto mechanism to automatically free
> >> rcu_read_lock's at the end of the block.  Given that humans
> >> have a habit of forgetting an error path somewhere it's
> >> best to leave it to the compiler.
> > 
> > I've had to unqueue this - clang doesn't like the apparently unused
> > auto variable; we need to find a way to make that happy.
> 
> __attribute__((unused))?

I worry that if I do that, then it'll optimise it out.

Dave

> Paolo
> 
> > Dave
> > 
> >> v3
> >>   Add block-head version of macro
> >>   Rename
> >>   Add docs
> >>   Convert more cases using the block-head version
> >>
> >> Dr. David Alan Gilbert (5):
> >>   rcu: Add automatically released rcu_read_lock variants
> >>   migration: Fix missing rcu_read_unlock
> >>   migration: Use automatic rcu_read unlock in ram.c
> >>   migration: Use automatic rcu_read unlock in rdma.c
> >>   rcu: Use automatic rc_read unlock in core memory/exec code
> >>
> >>  docs/devel/rcu.txt      |  16 +++
> >>  exec.c                  | 120 +++++++---------
> >>  include/exec/ram_addr.h | 138 +++++++++----------
> >>  include/qemu/rcu.h      |  25 ++++
> >>  memory.c                |  15 +-
> >>  migration/ram.c         | 295 +++++++++++++++++++---------------------
> >>  migration/rdma.c        |  57 ++------
> >>  7 files changed, 310 insertions(+), 356 deletions(-)
> >>
> >> -- 
> >> 2.21.0
> >>
> >>
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Daniel P. Berrangé Sept. 25, 2019, 3:49 p.m. UTC | #5
On Wed, Sep 25, 2019 at 04:28:58PM +0100, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonzini@redhat.com) wrote:
> > On 25/09/19 15:13, Dr. David Alan Gilbert wrote:
> > > * Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
> > >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > >>
> > >> This patch uses glib's g_auto mechanism to automatically free
> > >> rcu_read_lock's at the end of the block.  Given that humans
> > >> have a habit of forgetting an error path somewhere it's
> > >> best to leave it to the compiler.
> > > 
> > > I've had to unqueue this - clang doesn't like the apparently unused
> > > auto variable; we need to find a way to make that happy.
> > 
> > __attribute__((unused))?
> 
> I worry that if I do that, then it'll optimise it out.

Can you just insert a dummy use of the variable. eg libvirt does
this by just taking the address of the unused variable and casting
the result to void.


Regards,
Daniel
Paolo Bonzini Sept. 25, 2019, 4:47 p.m. UTC | #6
On 25/09/19 17:28, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>> On 25/09/19 15:13, Dr. David Alan Gilbert wrote:
>>> * Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
>>>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>>>>
>>>> This patch uses glib's g_auto mechanism to automatically free
>>>> rcu_read_lock's at the end of the block.  Given that humans
>>>> have a habit of forgetting an error path somewhere it's
>>>> best to leave it to the compiler.
>>>
>>> I've had to unqueue this - clang doesn't like the apparently unused
>>> auto variable; we need to find a way to make that happy.
>>
>> __attribute__((unused))?
> 
> I worry that if I do that, then it'll optimise it out.

It cannot, since the function passed to the cleanup attribute can have
side effects.

Paolo

> 
> Dave
> 
>> Paolo
>>
>>> Dave
>>>
>>>> v3
>>>>   Add block-head version of macro
>>>>   Rename
>>>>   Add docs
>>>>   Convert more cases using the block-head version
>>>>
>>>> Dr. David Alan Gilbert (5):
>>>>   rcu: Add automatically released rcu_read_lock variants
>>>>   migration: Fix missing rcu_read_unlock
>>>>   migration: Use automatic rcu_read unlock in ram.c
>>>>   migration: Use automatic rcu_read unlock in rdma.c
>>>>   rcu: Use automatic rc_read unlock in core memory/exec code
>>>>
>>>>  docs/devel/rcu.txt      |  16 +++
>>>>  exec.c                  | 120 +++++++---------
>>>>  include/exec/ram_addr.h | 138 +++++++++----------
>>>>  include/qemu/rcu.h      |  25 ++++
>>>>  memory.c                |  15 +-
>>>>  migration/ram.c         | 295 +++++++++++++++++++---------------------
>>>>  migration/rdma.c        |  57 ++------
>>>>  7 files changed, 310 insertions(+), 356 deletions(-)
>>>>
>>>> -- 
>>>> 2.21.0
>>>>
>>>>
>>> --
>>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>