[Qemu-devel] [PATCH v2] monitor: let cur_mon be per-thread

Peter Xu posted 1 patch 6 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180412060358.9118-1-peterx@redhat.com
Test checkpatch failed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test s390x passed
There is a newer version of this series
include/monitor/monitor.h | 2 +-
monitor.c                 | 2 +-
stubs/monitor.c           | 2 +-
tests/test-util-sockets.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
[Qemu-devel] [PATCH v2] monitor: let cur_mon be per-thread
Posted by Peter Xu 6 years ago
In the future the monitor iothread may be accessing the cur_mon as
well (via monitor_qmp_dispatch_one()).  Before we introduce a real
Out-Of-Band command, let's convert the cur_mon variable to be a
per-thread variable to make sure there won't be a race between threads.

Note that thread variables are not initialized to a valid value when new
thread is created.  However for our case we don't need to set it up,
since the cur_mon variable is only used in such a pattern:

  old_mon = cur_mon;
  cur_mon = xxx;
  (do something, read cur_mon if necessary in the stack)
  cur_mon = old_mon;

It plays a role as stack variable, so no need to be initialized at all.
We only need to make sure the variable won't be changed unexpectedly by
other threads.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
v2:
- drop qemu-thread changes
---
 include/monitor/monitor.h | 2 +-
 monitor.c                 | 2 +-
 stubs/monitor.c           | 2 +-
 tests/test-util-sockets.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index d6ab70cae2..2ef5e04b37 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -6,7 +6,7 @@
 #include "qapi/qapi-types-misc.h"
 #include "qemu/readline.h"
 
-extern Monitor *cur_mon;
+extern __thread Monitor *cur_mon;
 
 /* flags for monitor_init */
 /* 0x01 unused */
diff --git a/monitor.c b/monitor.c
index 39f8ee17ba..c90c31b6b3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -266,7 +266,7 @@ static mon_cmd_t info_cmds[];
 
 QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
 
-Monitor *cur_mon;
+__thread Monitor *cur_mon;
 
 static QEMUClockType event_clock_type = QEMU_CLOCK_REALTIME;
 
diff --git a/stubs/monitor.c b/stubs/monitor.c
index e018c8f594..9cdf6f3e37 100644
--- a/stubs/monitor.c
+++ b/stubs/monitor.c
@@ -3,7 +3,7 @@
 #include "qemu-common.h"
 #include "monitor/monitor.h"
 
-Monitor *cur_mon = NULL;
+__thread Monitor *cur_mon = NULL;
 
 int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
 {
diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
index acadd85e8f..6195a3ac36 100644
--- a/tests/test-util-sockets.c
+++ b/tests/test-util-sockets.c
@@ -69,7 +69,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
  * stubs/monitor.c is defined, to make sure monitor.o is discarded
  * otherwise we get duplicate syms at link time.
  */
-Monitor *cur_mon;
+__thread Monitor *cur_mon;
 void monitor_init(Chardev *chr, int flags) {}
 
 
-- 
2.14.3


Re: [Qemu-devel] [PATCH v2] monitor: let cur_mon be per-thread
Posted by no-reply@patchew.org 6 years ago
Hi,

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

Type: series
Message-id: 20180412060358.9118-1-peterx@redhat.com
Subject: [Qemu-devel] [PATCH v2] monitor: let cur_mon be per-thread

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20180412060358.9118-1-peterx@redhat.com -> patchew/20180412060358.9118-1-peterx@redhat.com
Switched to a new branch 'test'
e898fa8de3 monitor: let cur_mon be per-thread

=== OUTPUT BEGIN ===
Checking PATCH 1/1: monitor: let cur_mon be per-thread...
ERROR: do not initialise globals to 0 or NULL
#62: FILE: stubs/monitor.c:6:
+__thread Monitor *cur_mon = NULL;

total: 1 errors, 0 warnings, 32 lines checked

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

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com