[Qemu-devel] [PATCH 0/3] Automatic RCU read unlock

Dr. David Alan Gilbert (git) posted 3 patches 1 week ago
Test docker-clang@ubuntu failed
Test FreeBSD passed
Test checkpatch failed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190911164202.31136-1-dgilbert@redhat.com
Maintainers: Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>
include/qemu/rcu.h | 12 ++++++++++
migration/ram.c    | 25 ++++++++------------
migration/rdma.c   | 57 +++++++++-------------------------------------
3 files changed, 32 insertions(+), 62 deletions(-)

[Qemu-devel] [PATCH 0/3] Automatic RCU read unlock

Posted by Dr. David Alan Gilbert (git) 1 week ago
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.

In particular:
  https://bugzilla.redhat.com/show_bug.cgi?id=1746787
suggests we've forgotten an unlock case somewhere in the
rdma migration code.

Dave


Dr. David Alan Gilbert (3):
  rcu: Add automatically released rcu_read_lock variant
  migration: Use automatic rcu_read unlock in ram.c
  migration: Use automatic rcu_read unlock in rdma.c

 include/qemu/rcu.h | 12 ++++++++++
 migration/ram.c    | 25 ++++++++------------
 migration/rdma.c   | 57 +++++++++-------------------------------------
 3 files changed, 32 insertions(+), 62 deletions(-)

-- 
2.21.0


Re: [Qemu-devel] [PATCH 0/3] Automatic RCU read unlock

Posted by no-reply@patchew.org 1 week ago
Patchew URL: https://patchew.org/QEMU/20190911164202.31136-1-dgilbert@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH 0/3] Automatic RCU read unlock
Message-id: 20190911164202.31136-1-dgilbert@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
bf33be9 migration: Use automatic rcu_read unlock in rdma.c
c64f2f4 migration: Use automatic rcu_read unlock in ram.c
af6a608 rcu: Add automatically released rcu_read_lock variant

=== OUTPUT BEGIN ===
1/3 Checking commit af6a608b908d (rcu: Add automatically released rcu_read_lock variant)
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#33: FILE: include/qemu/rcu.h:165:
+#define RCU_READ_LOCK_AUTO g_auto(rcu_read_auto_t) \
+    _rcu_read_auto = 'x'; \
+    rcu_read_lock();

total: 1 errors, 0 warnings, 18 lines checked

Patch 1/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/3 Checking commit c64f2f4c923f (migration: Use automatic rcu_read unlock in ram.c)
3/3 Checking commit bf33be959c2b (migration: Use automatic rcu_read unlock in rdma.c)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190911164202.31136-1-dgilbert@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 0/3] Automatic RCU read unlock

Posted by Daniel P. Berrangé 1 week ago
On Wed, Sep 11, 2019 at 05:41:59PM +0100, Dr. David Alan Gilbert (git) 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.
> 
> In particular:
>   https://bugzilla.redhat.com/show_bug.cgi?id=1746787
> suggests we've forgotten an unlock case somewhere in the
> rdma migration code.

Probably worth mentioning this in the commit message of the 3rd patch
so someone reading history sees that the patch wasn't just a no-op
conversion, but instead actively fixing a bug.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/3] Automatic RCU read unlock

Posted by Dr. David Alan Gilbert 1 week ago
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Wed, Sep 11, 2019 at 05:41:59PM +0100, Dr. David Alan Gilbert (git) 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.
> > 
> > In particular:
> >   https://bugzilla.redhat.com/show_bug.cgi?id=1746787
> > suggests we've forgotten an unlock case somewhere in the
> > rdma migration code.
> 
> Probably worth mentioning this in the commit message of the 3rd patch
> so someone reading history sees that the patch wasn't just a no-op
> conversion, but instead actively fixing a bug.

Added.

Dave

> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK