| Message ID | 20190911164202.31136-1-dgilbert@redhat.com |
|---|---|
| Headers | show |
| Series | Automatic RCU read unlock | expand |
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
* 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
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
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(-)