* [PATCH 1/7] docs/devel/blkdebug: Convert to rST format
2024-08-16 13:22 [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST Peter Maydell
@ 2024-08-16 13:22 ` Peter Maydell
2024-10-10 18:25 ` Thomas Huth
2024-08-16 13:22 ` [PATCH 2/7] docs/devel/blkverify: " Peter Maydell
` (6 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2024-08-16 13:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
Convert blkdebug.txt to rST format. We put it into index-build.rst
because it falls under the "test" part of "QEMU Build and Test
System".
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
At least, index-build seemed the most plausible home to me...
---
MAINTAINERS | 1 +
docs/devel/blkdebug.rst | 177 +++++++++++++++++++++++++++++++++++++
docs/devel/blkdebug.txt | 162 ---------------------------------
docs/devel/index-build.rst | 1 +
4 files changed, 179 insertions(+), 162 deletions(-)
create mode 100644 docs/devel/blkdebug.rst
delete mode 100644 docs/devel/blkdebug.txt
diff --git a/MAINTAINERS b/MAINTAINERS
index 3584d6a6c6d..ca0a5c731f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4015,6 +4015,7 @@ M: Hanna Reitz <hreitz@redhat.com>
L: qemu-block@nongnu.org
S: Supported
F: block/blkdebug.c
+F: docs/devel/blkdebug.rst
vpc
M: Kevin Wolf <kwolf@redhat.com>
diff --git a/docs/devel/blkdebug.rst b/docs/devel/blkdebug.rst
new file mode 100644
index 00000000000..63887c9aa9c
--- /dev/null
+++ b/docs/devel/blkdebug.rst
@@ -0,0 +1,177 @@
+Block I/O error injection using ``blkdebug``
+============================================
+
+..
+ Copyright (C) 2014-2015 Red Hat Inc
+
+ This work is licensed under the terms of the GNU GPL, version 2 or later. See
+ the COPYING file in the top-level directory.
+
+The ``blkdebug`` block driver is a rule-based error injection engine. It can be
+used to exercise error code paths in block drivers including ``ENOSPC`` (out of
+space) and ``EIO``.
+
+This document gives an overview of the features available in ``blkdebug``.
+
+Background
+----------
+Block drivers have many error code paths that handle I/O errors. Image formats
+are especially complex since metadata I/O errors during cluster allocation or
+while updating tables happen halfway through request processing and require
+discipline to keep image files consistent.
+
+Error injection allows test cases to trigger I/O errors at specific points.
+This way, all error paths can be tested to make sure they are correct.
+
+Rules
+-----
+The ``blkdebug`` block driver takes a list of "rules" that tell the error injection
+engine when to fail an I/O request.
+
+Each I/O request is evaluated against the rules. If a rule matches the request
+then its "action" is executed.
+
+Rules can be placed in a configuration file; the configuration file
+follows the same .ini-like format used by QEMU's ``-readconfig`` option, and
+each section of the file represents a rule.
+
+The following configuration file defines a single rule::
+
+ $ cat blkdebug.conf
+ [inject-error]
+ event = "read_aio"
+ errno = "28"
+
+This rule fails all aio read requests with ``ENOSPC`` (28). Note that the errno
+value depends on the host. On Linux, see
+``/usr/include/asm-generic/errno-base.h`` for errno values.
+
+Invoke QEMU as follows::
+
+ $ qemu-system-x86_64
+ -drive if=none,cache=none,file=blkdebug:blkdebug.conf:test.img,id=drive0 \
+ -device virtio-blk-pci,drive=drive0,id=virtio-blk-pci0
+
+Rules support the following attributes:
+
+``event``
+ which type of operation to match (e.g. ``read_aio``, ``write_aio``,
+ ``flush_to_os``, ``flush_to_disk``). See `Events`_ for
+ information on events.
+
+``state``
+ (optional) the engine must be in this state number in order for this
+ rule to match. See `State transitions`_ for information
+ on states.
+
+``errno``
+ the numeric errno value to return when a request matches this rule.
+ The errno values depend on the host since the numeric values are not
+ standardized in the POSIX specification.
+
+``sector``
+ (optional) a sector number that the request must overlap in order to
+ match this rule
+
+``once``
+ (optional, default ``off``) only execute this action on the first
+ matching request
+
+``immediately``
+ (optional, default ``off``) return a NULL ``BlockAIOCB``
+ pointer and fail without an errno instead. This
+ exercises the code path where ``BlockAIOCB`` fails and the
+ caller's ``BlockCompletionFunc`` is not invoked.
+
+Events
+------
+Block drivers provide information about the type of I/O request they are about
+to make so rules can match specific types of requests. For example, the ``qcow2``
+block driver tells ``blkdebug`` when it accesses the L1 table so rules can match
+only L1 table accesses and not other metadata or guest data requests.
+
+The core events are:
+
+``read_aio``
+ guest data read
+
+``write_aio``
+ guest data write
+
+``flush_to_os``
+ write out unwritten block driver state (e.g. cached metadata)
+
+``flush_to_disk``
+ flush the host block device's disk cache
+
+See ``qapi/block-core.json:BlkdebugEvent`` for the full list of events.
+You may need to grep block driver source code to understand the
+meaning of specific events.
+
+State transitions
+-----------------
+There are cases where more power is needed to match a particular I/O request in
+a longer sequence of requests. For example::
+
+ write_aio
+ flush_to_disk
+ write_aio
+
+How do we match the 2nd ``write_aio`` but not the first? This is where state
+transitions come in.
+
+The error injection engine has an integer called the "state" that always starts
+initialized to 1. The state integer is internal to ``blkdebug`` and cannot be
+observed from outside but rules can interact with it for powerful matching
+behavior.
+
+Rules can be conditional on the current state and they can transition to a new
+state.
+
+When a rule's "state" attribute is non-zero then the current state must equal
+the attribute in order for the rule to match.
+
+For example, to match the 2nd write_aio::
+
+ [set-state]
+ event = "write_aio"
+ state = "1"
+ new_state = "2"
+
+ [inject-error]
+ event = "write_aio"
+ state = "2"
+ errno = "5"
+
+The first ``write_aio`` request matches the ``set-state`` rule and transitions from
+state 1 to state 2. Once state 2 has been entered, the ``set-state`` rule no
+longer matches since it requires state 1. But the ``inject-error`` rule now
+matches the next ``write_aio`` request and injects ``EIO`` (5).
+
+State transition rules support the following attributes:
+
+``event``
+ which type of operation to match (e.g. ``read_aio``, ``write_aio``,
+ ``flush_to_os`, ``flush_to_disk``). See `Events`_ for
+ information on events.
+
+``state``
+ (optional) the engine must be in this state number in order for this
+ rule to match
+
+``new_state``
+ transition to this state number
+
+Suspend and resume
+------------------
+Exercising code paths in block drivers may require specific ordering amongst
+concurrent requests. The "breakpoint" feature allows requests to be halted on
+a ``blkdebug`` event and resumed later. This makes it possible to achieve
+deterministic ordering when multiple requests are in flight.
+
+Breakpoints on ``blkdebug`` events are associated with a user-defined ``tag`` string.
+This tag serves as an identifier by which the request can be resumed at a later
+point.
+
+See the ``qemu-io(1)`` ``break``, ``resume``, ``remove_break``, and ``wait_break``
+commands for details.
diff --git a/docs/devel/blkdebug.txt b/docs/devel/blkdebug.txt
deleted file mode 100644
index 0b0c128d356..00000000000
--- a/docs/devel/blkdebug.txt
+++ /dev/null
@@ -1,162 +0,0 @@
-Block I/O error injection using blkdebug
-----------------------------------------
-Copyright (C) 2014-2015 Red Hat Inc
-
-This work is licensed under the terms of the GNU GPL, version 2 or later. See
-the COPYING file in the top-level directory.
-
-The blkdebug block driver is a rule-based error injection engine. It can be
-used to exercise error code paths in block drivers including ENOSPC (out of
-space) and EIO.
-
-This document gives an overview of the features available in blkdebug.
-
-Background
-----------
-Block drivers have many error code paths that handle I/O errors. Image formats
-are especially complex since metadata I/O errors during cluster allocation or
-while updating tables happen halfway through request processing and require
-discipline to keep image files consistent.
-
-Error injection allows test cases to trigger I/O errors at specific points.
-This way, all error paths can be tested to make sure they are correct.
-
-Rules
------
-The blkdebug block driver takes a list of "rules" that tell the error injection
-engine when to fail an I/O request.
-
-Each I/O request is evaluated against the rules. If a rule matches the request
-then its "action" is executed.
-
-Rules can be placed in a configuration file; the configuration file
-follows the same .ini-like format used by QEMU's -readconfig option, and
-each section of the file represents a rule.
-
-The following configuration file defines a single rule:
-
- $ cat blkdebug.conf
- [inject-error]
- event = "read_aio"
- errno = "28"
-
-This rule fails all aio read requests with ENOSPC (28). Note that the errno
-value depends on the host. On Linux, see
-/usr/include/asm-generic/errno-base.h for errno values.
-
-Invoke QEMU as follows:
-
- $ qemu-system-x86_64
- -drive if=none,cache=none,file=blkdebug:blkdebug.conf:test.img,id=drive0 \
- -device virtio-blk-pci,drive=drive0,id=virtio-blk-pci0
-
-Rules support the following attributes:
-
- event - which type of operation to match (e.g. read_aio, write_aio,
- flush_to_os, flush_to_disk). See the "Events" section for
- information on events.
-
- state - (optional) the engine must be in this state number in order for this
- rule to match. See the "State transitions" section for information
- on states.
-
- errno - the numeric errno value to return when a request matches this rule.
- The errno values depend on the host since the numeric values are not
- standardized in the POSIX specification.
-
- sector - (optional) a sector number that the request must overlap in order to
- match this rule
-
- once - (optional, default "off") only execute this action on the first
- matching request
-
- immediately - (optional, default "off") return a NULL BlockAIOCB
- pointer and fail without an errno instead. This
- exercises the code path where BlockAIOCB fails and the
- caller's BlockCompletionFunc is not invoked.
-
-Events
-------
-Block drivers provide information about the type of I/O request they are about
-to make so rules can match specific types of requests. For example, the qcow2
-block driver tells blkdebug when it accesses the L1 table so rules can match
-only L1 table accesses and not other metadata or guest data requests.
-
-The core events are:
-
- read_aio - guest data read
-
- write_aio - guest data write
-
- flush_to_os - write out unwritten block driver state (e.g. cached metadata)
-
- flush_to_disk - flush the host block device's disk cache
-
-See qapi/block-core.json:BlkdebugEvent for the full list of events.
-You may need to grep block driver source code to understand the
-meaning of specific events.
-
-State transitions
------------------
-There are cases where more power is needed to match a particular I/O request in
-a longer sequence of requests. For example:
-
- write_aio
- flush_to_disk
- write_aio
-
-How do we match the 2nd write_aio but not the first? This is where state
-transitions come in.
-
-The error injection engine has an integer called the "state" that always starts
-initialized to 1. The state integer is internal to blkdebug and cannot be
-observed from outside but rules can interact with it for powerful matching
-behavior.
-
-Rules can be conditional on the current state and they can transition to a new
-state.
-
-When a rule's "state" attribute is non-zero then the current state must equal
-the attribute in order for the rule to match.
-
-For example, to match the 2nd write_aio:
-
- [set-state]
- event = "write_aio"
- state = "1"
- new_state = "2"
-
- [inject-error]
- event = "write_aio"
- state = "2"
- errno = "5"
-
-The first write_aio request matches the set-state rule and transitions from
-state 1 to state 2. Once state 2 has been entered, the set-state rule no
-longer matches since it requires state 1. But the inject-error rule now
-matches the next write_aio request and injects EIO (5).
-
-State transition rules support the following attributes:
-
- event - which type of operation to match (e.g. read_aio, write_aio,
- flush_to_os, flush_to_disk). See the "Events" section for
- information on events.
-
- state - (optional) the engine must be in this state number in order for this
- rule to match
-
- new_state - transition to this state number
-
-Suspend and resume
-------------------
-Exercising code paths in block drivers may require specific ordering amongst
-concurrent requests. The "breakpoint" feature allows requests to be halted on
-a blkdebug event and resumed later. This makes it possible to achieve
-deterministic ordering when multiple requests are in flight.
-
-Breakpoints on blkdebug events are associated with a user-defined "tag" string.
-This tag serves as an identifier by which the request can be resumed at a later
-point.
-
-See the qemu-io(1) break, resume, remove_break, and wait_break commands for
-details.
diff --git a/docs/devel/index-build.rst b/docs/devel/index-build.rst
index 90b406ca0ed..3a912aefcfa 100644
--- a/docs/devel/index-build.rst
+++ b/docs/devel/index-build.rst
@@ -18,3 +18,4 @@ the basics if you are adding new files and targets to the build.
qapi-code-gen
fuzzing
control-flow-integrity
+ blkdebug
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 1/7] docs/devel/blkdebug: Convert to rST format
2024-08-16 13:22 ` [PATCH 1/7] docs/devel/blkdebug: Convert to rST format Peter Maydell
@ 2024-10-10 18:25 ` Thomas Huth
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Huth @ 2024-10-10 18:25 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
On 16/08/2024 15.22, Peter Maydell wrote:
> Convert blkdebug.txt to rST format. We put it into index-build.rst
> because it falls under the "test" part of "QEMU Build and Test
> System".
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> At least, index-build seemed the most plausible home to me...
I recently split the testing docs into a separate folder, maybe it could
also go there instead?
Anyway:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/7] docs/devel/blkverify: Convert to rST format
2024-08-16 13:22 [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST Peter Maydell
2024-08-16 13:22 ` [PATCH 1/7] docs/devel/blkdebug: Convert to rST format Peter Maydell
@ 2024-08-16 13:22 ` Peter Maydell
2024-10-10 18:38 ` Thomas Huth
2024-08-16 13:22 ` [PATCH 3/7] docs/devel/lockcnt: " Peter Maydell
` (5 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2024-08-16 13:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
Convert blkverify.txt to rST format.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
MAINTAINERS | 1 +
docs/devel/{blkverify.txt => blkverify.rst} | 30 ++++++++++++---------
docs/devel/index-build.rst | 1 +
3 files changed, 19 insertions(+), 13 deletions(-)
rename docs/devel/{blkverify.txt => blkverify.rst} (77%)
diff --git a/MAINTAINERS b/MAINTAINERS
index ca0a5c731f5..e9dd1180077 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3938,6 +3938,7 @@ M: Stefan Hajnoczi <stefanha@redhat.com>
L: qemu-block@nongnu.org
S: Supported
F: block/blkverify.c
+F: docs/devel/blkverify.rst
bochs
M: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/docs/devel/blkverify.txt b/docs/devel/blkverify.rst
similarity index 77%
rename from docs/devel/blkverify.txt
rename to docs/devel/blkverify.rst
index aca826c51cc..2a71778b5e3 100644
--- a/docs/devel/blkverify.txt
+++ b/docs/devel/blkverify.rst
@@ -1,8 +1,10 @@
-= Block driver correctness testing with blkverify =
+Block driver correctness testing with ``blkverify``
+===================================================
-== Introduction ==
+Introduction
+------------
-This document describes how to use the blkverify protocol to test that a block
+This document describes how to use the ``blkverify`` protocol to test that a block
driver is operating correctly.
It is difficult to test and debug block drivers against real guests. Often
@@ -11,12 +13,13 @@ of the executable. Other times obscure errors are raised by a program inside
the guest. These issues are extremely hard to trace back to bugs in the block
driver.
-Blkverify solves this problem by catching data corruption inside QEMU the first
+``blkverify`` solves this problem by catching data corruption inside QEMU the first
time bad data is read and reporting the disk sector that is corrupted.
-== How it works ==
+How it works
+------------
-The blkverify protocol has two child block devices, the "test" device and the
+The ``blkverify`` protocol has two child block devices, the "test" device and the
"raw" device. Read/write operations are mirrored to both devices so their
state should always be in sync.
@@ -25,13 +28,14 @@ contents to the "test" image. The idea is that the "raw" device will handle
read/write operations correctly and not corrupt data. It can be used as a
reference for comparison against the "test" device.
-After a mirrored read operation completes, blkverify will compare the data and
+After a mirrored read operation completes, ``blkverify`` will compare the data and
raise an error if it is not identical. This makes it possible to catch the
first instance where corrupt data is read.
-== Example ==
+Example
+-------
-Imagine raw.img has 0xcd repeated throughout its first sector:
+Imagine raw.img has 0xcd repeated throughout its first sector::
$ ./qemu-io -c 'read -v 0 512' raw.img
00000000: cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd ................
@@ -42,7 +46,7 @@ Imagine raw.img has 0xcd repeated throughout its first sector:
read 512/512 bytes at offset 0
512.000000 bytes, 1 ops; 0.0000 sec (97.656 MiB/sec and 200000.0000 ops/sec)
-And test.img is corrupt, its first sector is zeroed when it shouldn't be:
+And test.img is corrupt, its first sector is zeroed when it shouldn't be::
$ ./qemu-io -c 'read -v 0 512' test.img
00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
@@ -53,17 +57,17 @@ And test.img is corrupt, its first sector is zeroed when it shouldn't be:
read 512/512 bytes at offset 0
512.000000 bytes, 1 ops; 0.0000 sec (81.380 MiB/sec and 166666.6667 ops/sec)
-This error is caught by blkverify:
+This error is caught by ``blkverify``::
$ ./qemu-io -c 'read 0 512' blkverify:a.img:b.img
blkverify: read sector_num=0 nb_sectors=4 contents mismatch in sector 0
-A more realistic scenario is verifying the installation of a guest OS:
+A more realistic scenario is verifying the installation of a guest OS::
$ ./qemu-img create raw.img 16G
$ ./qemu-img create -f qcow2 test.qcow2 16G
$ ./qemu-system-x86_64 -cdrom debian.iso \
-drive file=blkverify:raw.img:test.qcow2
-If the installation is aborted when blkverify detects corruption, use qemu-io
+If the installation is aborted when ``blkverify`` detects corruption, use ``qemu-io``
to explore the contents of the disk image at the sector in question.
diff --git a/docs/devel/index-build.rst b/docs/devel/index-build.rst
index 3a912aefcfa..a8f7c5cdebc 100644
--- a/docs/devel/index-build.rst
+++ b/docs/devel/index-build.rst
@@ -19,3 +19,4 @@ the basics if you are adding new files and targets to the build.
fuzzing
control-flow-integrity
blkdebug
+ blkverify
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/7] docs/devel/blkverify: Convert to rST format
2024-08-16 13:22 ` [PATCH 2/7] docs/devel/blkverify: " Peter Maydell
@ 2024-10-10 18:38 ` Thomas Huth
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Huth @ 2024-10-10 18:38 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
On 16/08/2024 15.22, Peter Maydell wrote:
> Convert blkverify.txt to rST format.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> MAINTAINERS | 1 +
> docs/devel/{blkverify.txt => blkverify.rst} | 30 ++++++++++++---------
> docs/devel/index-build.rst | 1 +
Maybe also rather add it to docs/devel/testing now instead?
Apart from that:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/7] docs/devel/lockcnt: Convert to rST format
2024-08-16 13:22 [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST Peter Maydell
2024-08-16 13:22 ` [PATCH 1/7] docs/devel/blkdebug: Convert to rST format Peter Maydell
2024-08-16 13:22 ` [PATCH 2/7] docs/devel/blkverify: " Peter Maydell
@ 2024-08-16 13:22 ` Peter Maydell
2024-08-16 13:22 ` [PATCH 4/7] docs/devel/multiple-iothreads: " Peter Maydell
` (4 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2024-08-16 13:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
Convert docs/devel/lockcnt.txt to rST format.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
MAINTAINERS | 2 +-
docs/devel/index-api.rst | 1 +
docs/devel/{lockcnt.txt => lockcnt.rst} | 89 +++++++++++++------------
3 files changed, 47 insertions(+), 45 deletions(-)
rename docs/devel/{lockcnt.txt => lockcnt.rst} (74%)
diff --git a/MAINTAINERS b/MAINTAINERS
index e9dd1180077..9e091a4e214 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3112,7 +3112,7 @@ F: qapi/run-state.json
Read, Copy, Update (RCU)
M: Paolo Bonzini <pbonzini@redhat.com>
S: Maintained
-F: docs/devel/lockcnt.txt
+F: docs/devel/lockcnt.rst
F: docs/devel/rcu.txt
F: include/qemu/rcu*.h
F: tests/unit/rcutorture.c
diff --git a/docs/devel/index-api.rst b/docs/devel/index-api.rst
index fe01b2b488d..1c487c152ab 100644
--- a/docs/devel/index-api.rst
+++ b/docs/devel/index-api.rst
@@ -9,6 +9,7 @@ generated from in-code annotations to function prototypes.
bitops
loads-stores
+ lockcnt
memory
modules
pci
diff --git a/docs/devel/lockcnt.txt b/docs/devel/lockcnt.rst
similarity index 74%
rename from docs/devel/lockcnt.txt
rename to docs/devel/lockcnt.rst
index a3fb3bc5d8d..994aeb57151 100644
--- a/docs/devel/lockcnt.txt
+++ b/docs/devel/lockcnt.rst
@@ -1,9 +1,9 @@
-DOCUMENTATION FOR LOCKED COUNTERS (aka QemuLockCnt)
-===================================================
+Locked Counters (aka ``QemuLockCnt``)
+=====================================
QEMU often uses reference counts to track data structures that are being
accessed and should not be freed. For example, a loop that invoke
-callbacks like this is not safe:
+callbacks like this is not safe::
QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) {
if (ioh->revents & G_IO_OUT) {
@@ -11,11 +11,11 @@ callbacks like this is not safe:
}
}
-QLIST_FOREACH_SAFE protects against deletion of the current node (ioh)
-by stashing away its "next" pointer. However, ioh->fd_write could
+``QLIST_FOREACH_SAFE`` protects against deletion of the current node (``ioh``)
+by stashing away its ``next`` pointer. However, ``ioh->fd_write`` could
actually delete the next node from the list. The simplest way to
avoid this is to mark the node as deleted, and remove it from the
-list in the above loop:
+list in the above loop::
QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) {
if (ioh->deleted) {
@@ -29,7 +29,7 @@ list in the above loop:
}
If however this loop must also be reentrant, i.e. it is possible that
-ioh->fd_write invokes the loop again, some kind of counting is needed:
+``ioh->fd_write`` invokes the loop again, some kind of counting is needed::
walking_handlers++;
QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) {
@@ -46,8 +46,8 @@ ioh->fd_write invokes the loop again, some kind of counting is needed:
}
walking_handlers--;
-One may think of using the RCU primitives, rcu_read_lock() and
-rcu_read_unlock(); effectively, the RCU nesting count would take
+One may think of using the RCU primitives, ``rcu_read_lock()`` and
+``rcu_read_unlock()``; effectively, the RCU nesting count would take
the place of the walking_handlers global variable. Indeed,
reference counting and RCU have similar purposes, but their usage in
general is complementary:
@@ -70,14 +70,14 @@ general is complementary:
this can improve performance, but also delay reclamation undesirably.
With reference counting, reclamation is deterministic.
-This file documents QemuLockCnt, an abstraction for using reference
+This file documents ``QemuLockCnt``, an abstraction for using reference
counting in code that has to be both thread-safe and reentrant.
-QemuLockCnt concepts
---------------------
+``QemuLockCnt`` concepts
+------------------------
-A QemuLockCnt comprises both a counter and a mutex; it has primitives
+A ``QemuLockCnt`` comprises both a counter and a mutex; it has primitives
to increment and decrement the counter, and to take and release the
mutex. The counter notes how many visits to the data structures are
taking place (the visits could be from different threads, or there could
@@ -95,13 +95,14 @@ not just frees, though there could be cases where this is not necessary.
Reads, instead, can be done without taking the mutex, as long as the
readers and writers use the same macros that are used for RCU, for
-example qatomic_rcu_read, qatomic_rcu_set, QLIST_FOREACH_RCU, etc. This is
-because the reads are done outside a lock and a set or QLIST_INSERT_HEAD
+example ``qatomic_rcu_read``, ``qatomic_rcu_set``, ``QLIST_FOREACH_RCU``,
+etc. This is because the reads are done outside a lock and a set
+or ``QLIST_INSERT_HEAD``
can happen concurrently with the read. The RCU API ensures that the
processor and the compiler see all required memory barriers.
This could be implemented simply by protecting the counter with the
-mutex, for example:
+mutex, for example::
// (1)
qemu_mutex_lock(&walking_handlers_mutex);
@@ -125,33 +126,33 @@ mutex, for example:
Here, no frees can happen in the code represented by the ellipsis.
If another thread is executing critical section (2), that part of
the code cannot be entered, because the thread will not be able
-to increment the walking_handlers variable. And of course
+to increment the ``walking_handlers`` variable. And of course
during the visit any other thread will see a nonzero value for
-walking_handlers, as in the single-threaded code.
+``walking_handlers``, as in the single-threaded code.
Note that it is possible for multiple concurrent accesses to delay
-the cleanup arbitrarily; in other words, for the walking_handlers
+the cleanup arbitrarily; in other words, for the ``walking_handlers``
counter to never become zero. For this reason, this technique is
more easily applicable if concurrent access to the structure is rare.
However, critical sections are easy to forget since you have to do
-them for each modification of the counter. QemuLockCnt ensures that
+them for each modification of the counter. ``QemuLockCnt`` ensures that
all modifications of the counter take the lock appropriately, and it
can also be more efficient in two ways:
- it avoids taking the lock for many operations (for example
incrementing the counter while it is non-zero);
-- on some platforms, one can implement QemuLockCnt to hold the lock
+- on some platforms, one can implement ``QemuLockCnt`` to hold the lock
and the mutex in a single word, making the fast path no more expensive
than simply managing a counter using atomic operations (see
- docs/devel/atomics.rst). This can be very helpful if concurrent access to
+ :doc:`atomics`). This can be very helpful if concurrent access to
the data structure is expected to be rare.
Using the same mutex for frees and writes can still incur some small
inefficiencies; for example, a visit can never start if the counter is
-zero and the mutex is taken---even if the mutex is taken by a write,
+zero and the mutex is taken -- even if the mutex is taken by a write,
which in principle need not block a visit of the data structure.
However, these are usually not a problem if any of the following
assumptions are valid:
@@ -163,27 +164,27 @@ assumptions are valid:
- writes are frequent, but this kind of write (e.g. appending to a
list) has a very small critical section.
-For example, QEMU uses QemuLockCnt to manage an AioContext's list of
+For example, QEMU uses ``QemuLockCnt`` to manage an ``AioContext``'s list of
bottom halves and file descriptor handlers. Modifications to the list
of file descriptor handlers are rare. Creation of a new bottom half is
frequent and can happen on a fast path; however: 1) it is almost never
concurrent with a visit to the list of bottom halves; 2) it only has
-three instructions in the critical path, two assignments and a smp_wmb().
+three instructions in the critical path, two assignments and a ``smp_wmb()``.
-QemuLockCnt API
----------------
+``QemuLockCnt`` API
+-------------------
-The QemuLockCnt API is described in include/qemu/thread.h.
+The ``QemuLockCnt`` API is described in ``include/qemu/thread.h``.
-QemuLockCnt usage
------------------
+``QemuLockCnt`` usage
+---------------------
-This section explains the typical usage patterns for QemuLockCnt functions.
+This section explains the typical usage patterns for ``QemuLockCnt`` functions.
Setting a variable to a non-NULL value can be done between
-qemu_lockcnt_lock and qemu_lockcnt_unlock:
+``qemu_lockcnt_lock`` and ``qemu_lockcnt_unlock``::
qemu_lockcnt_lock(&xyz_lockcnt);
if (!xyz) {
@@ -193,8 +194,8 @@ qemu_lockcnt_lock and qemu_lockcnt_unlock:
}
qemu_lockcnt_unlock(&xyz_lockcnt);
-Accessing the value can be done between qemu_lockcnt_inc and
-qemu_lockcnt_dec:
+Accessing the value can be done between ``qemu_lockcnt_inc`` and
+``qemu_lockcnt_dec``::
qemu_lockcnt_inc(&xyz_lockcnt);
if (xyz) {
@@ -204,11 +205,11 @@ qemu_lockcnt_dec:
}
qemu_lockcnt_dec(&xyz_lockcnt);
-Freeing the object can similarly use qemu_lockcnt_lock and
-qemu_lockcnt_unlock, but you also need to ensure that the count
-is zero (i.e. there is no concurrent visit). Because qemu_lockcnt_inc
-takes the QemuLockCnt's lock, the count cannot become non-zero while
-the object is being freed. Freeing an object looks like this:
+Freeing the object can similarly use ``qemu_lockcnt_lock`` and
+``qemu_lockcnt_unlock``, but you also need to ensure that the count
+is zero (i.e. there is no concurrent visit). Because ``qemu_lockcnt_inc``
+takes the ``QemuLockCnt``'s lock, the count cannot become non-zero while
+the object is being freed. Freeing an object looks like this::
qemu_lockcnt_lock(&xyz_lockcnt);
if (!qemu_lockcnt_count(&xyz_lockcnt)) {
@@ -218,7 +219,7 @@ the object is being freed. Freeing an object looks like this:
qemu_lockcnt_unlock(&xyz_lockcnt);
If an object has to be freed right after a visit, you can combine
-the decrement, the locking and the check on count as follows:
+the decrement, the locking and the check on count as follows::
qemu_lockcnt_inc(&xyz_lockcnt);
if (xyz) {
@@ -232,7 +233,7 @@ the decrement, the locking and the check on count as follows:
qemu_lockcnt_unlock(&xyz_lockcnt);
}
-QemuLockCnt can also be used to access a list as follows:
+``QemuLockCnt`` can also be used to access a list as follows::
qemu_lockcnt_inc(&io_handlers_lockcnt);
QLIST_FOREACH_RCU(ioh, &io_handlers, pioh) {
@@ -252,10 +253,10 @@ QemuLockCnt can also be used to access a list as follows:
}
Again, the RCU primitives are used because new items can be added to the
-list during the walk. QLIST_FOREACH_RCU ensures that the processor and
+list during the walk. ``QLIST_FOREACH_RCU`` ensures that the processor and
the compiler see the appropriate memory barriers.
-An alternative pattern uses qemu_lockcnt_dec_if_lock:
+An alternative pattern uses ``qemu_lockcnt_dec_if_lock``::
qemu_lockcnt_inc(&io_handlers_lockcnt);
QLIST_FOREACH_SAFE_RCU(ioh, &io_handlers, next, pioh) {
@@ -273,5 +274,5 @@ An alternative pattern uses qemu_lockcnt_dec_if_lock:
}
qemu_lockcnt_dec(&io_handlers_lockcnt);
-Here you can use qemu_lockcnt_dec instead of qemu_lockcnt_dec_and_lock,
+Here you can use ``qemu_lockcnt_dec`` instead of ``qemu_lockcnt_dec_and_lock``,
because there is no special task to do if the count goes from 1 to 0.
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/7] docs/devel/multiple-iothreads: Convert to rST format
2024-08-16 13:22 [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST Peter Maydell
` (2 preceding siblings ...)
2024-08-16 13:22 ` [PATCH 3/7] docs/devel/lockcnt: " Peter Maydell
@ 2024-08-16 13:22 ` Peter Maydell
2024-08-16 13:22 ` [PATCH 5/7] docs/devel/rcu: " Peter Maydell
` (3 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2024-08-16 13:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
Convert docs/devel/multiple-iothreads.txt to rST format.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
docs/devel/index-internals.rst | 1 +
docs/devel/multiple-iothreads.rst | 139 ++++++++++++++++++++++++++++++
docs/devel/multiple-iothreads.txt | 130 ----------------------------
3 files changed, 140 insertions(+), 130 deletions(-)
create mode 100644 docs/devel/multiple-iothreads.rst
delete mode 100644 docs/devel/multiple-iothreads.txt
diff --git a/docs/devel/index-internals.rst b/docs/devel/index-internals.rst
index 4ac7725d728..88fa0e9450d 100644
--- a/docs/devel/index-internals.rst
+++ b/docs/devel/index-internals.rst
@@ -21,3 +21,4 @@ Details about QEMU's various subsystems including how to add features to them.
writing-monitor-commands
virtio-backends
crypto
+ multiple-iothreads
diff --git a/docs/devel/multiple-iothreads.rst b/docs/devel/multiple-iothreads.rst
new file mode 100644
index 00000000000..d1f3fc4510a
--- /dev/null
+++ b/docs/devel/multiple-iothreads.rst
@@ -0,0 +1,139 @@
+Using Multiple ``IOThread``\ s
+==============================
+
+..
+ Copyright (c) 2014-2017 Red Hat Inc.
+
+ This work is licensed under the terms of the GNU GPL, version 2 or later. See
+ the COPYING file in the top-level directory.
+
+
+This document explains the ``IOThread`` feature and how to write code that runs
+outside the BQL.
+
+The main loop and ``IOThread``\ s
+---------------------------------
+QEMU is an event-driven program that can do several things at once using an
+event loop. The VNC server and the QMP monitor are both processed from the
+same event loop, which monitors their file descriptors until they become
+readable and then invokes a callback.
+
+The default event loop is called the main loop (see ``main-loop.c``). It is
+possible to create additional event loop threads using
+``-object iothread,id=my-iothread``.
+
+Side note: The main loop and ``IOThread`` are both event loops but their code is
+not shared completely. Sometimes it is useful to remember that although they
+are conceptually similar they are currently not interchangeable.
+
+Why ``IOThread``\ s are useful
+------------------------------
+``IOThread``\ s allow the user to control the placement of work. The main loop is a
+scalability bottleneck on hosts with many CPUs. Work can be spread across
+several ``IOThread``\ s instead of just one main loop. When set up correctly this
+can improve I/O latency and reduce jitter seen by the guest.
+
+The main loop is also deeply associated with the BQL, which is a
+scalability bottleneck in itself. vCPU threads and the main loop use the BQL
+to serialize execution of QEMU code. This mutex is necessary because a lot of
+QEMU's code historically was not thread-safe.
+
+The fact that all I/O processing is done in a single main loop and that the
+BQL is contended by all vCPU threads and the main loop explain
+why it is desirable to place work into ``IOThread``\ s.
+
+The experimental ``virtio-blk`` data-plane implementation has been benchmarked and
+shows these effects:
+ftp://public.dhe.ibm.com/linux/pdfs/KVM_Virtualized_IO_Performance_Paper.pdf
+
+.. _how-to-program:
+
+How to program for ``IOThread``\ s
+----------------------------------
+The main difference between legacy code and new code that can run in an
+``IOThread`` is dealing explicitly with the event loop object, ``AioContext``
+(see ``include/block/aio.h``). Code that only works in the main loop
+implicitly uses the main loop's ``AioContext``. Code that supports running
+in ``IOThread``\ s must be aware of its ``AioContext``.
+
+AioContext supports the following services:
+ * File descriptor monitoring (read/write/error on POSIX hosts)
+ * Event notifiers (inter-thread signalling)
+ * Timers
+ * Bottom Halves (BH) deferred callbacks
+
+There are several old APIs that use the main loop AioContext:
+ * LEGACY ``qemu_aio_set_fd_handler()`` - monitor a file descriptor
+ * LEGACY ``qemu_aio_set_event_notifier()`` - monitor an event notifier
+ * LEGACY ``timer_new_ms()`` - create a timer
+ * LEGACY ``qemu_bh_new()`` - create a BH
+ * LEGACY ``qemu_bh_new_guarded()`` - create a BH with a device re-entrancy guard
+ * LEGACY ``qemu_aio_wait()`` - run an event loop iteration
+
+Since they implicitly work on the main loop they cannot be used in code that
+runs in an ``IOThread``. They might cause a crash or deadlock if called from an
+``IOThread`` since the BQL is not held.
+
+Instead, use the ``AioContext`` functions directly (see ``include/block/aio.h``):
+ * ``aio_set_fd_handler()`` - monitor a file descriptor
+ * ``aio_set_event_notifier()`` - monitor an event notifier
+ * ``aio_timer_new()`` - create a timer
+ * ``aio_bh_new()`` - create a BH
+ * ``aio_bh_new_guarded()`` - create a BH with a device re-entrancy guard
+ * ``aio_poll()`` - run an event loop iteration
+
+The ``qemu_bh_new_guarded``/``aio_bh_new_guarded`` APIs accept a
+``MemReentrancyGuard``
+argument, which is used to check for and prevent re-entrancy problems. For
+BHs associated with devices, the reentrancy-guard is contained in the
+corresponding ``DeviceState`` and named ``mem_reentrancy_guard``.
+
+The ``AioContext`` can be obtained from the ``IOThread`` using
+``iothread_get_aio_context()`` or for the main loop using
+``qemu_get_aio_context()``. Code that takes an ``AioContext`` argument
+works both in ``IOThread``\ s or the main loop, depending on which ``AioContext``
+instance the caller passes in.
+
+How to synchronize with an ``IOThread``
+---------------------------------------
+Variables that can be accessed by multiple threads require some form of
+synchronization such as ``qemu_mutex_lock()``, ``rcu_read_lock()``, etc.
+
+``AioContext`` functions like ``aio_set_fd_handler()``,
+``aio_set_event_notifier()``, ``aio_bh_new()``, and ``aio_timer_new()``
+are thread-safe. They can be used to trigger activity in an ``IOThread``.
+
+Side note: the best way to schedule a function call across threads is to call
+``aio_bh_schedule_oneshot()``.
+
+The main loop thread can wait synchronously for a condition using
+``AIO_WAIT_WHILE()``.
+
+``AioContext`` and the block layer
+----------------------------------
+The ``AioContext`` originates from the QEMU block layer, even though nowadays
+``AioContext`` is a generic event loop that can be used by any QEMU subsystem.
+
+The block layer has support for ``AioContext`` integrated. Each
+``BlockDriverState`` is associated with an ``AioContext`` using
+``bdrv_try_change_aio_context()`` and ``bdrv_get_aio_context()``.
+This allows block layer code to process I/O inside the
+right ``AioContext``. Other subsystems may wish to follow a similar approach.
+
+Block layer code must therefore expect to run in an ``IOThread`` and avoid using
+old APIs that implicitly use the main loop. See
+`How to program for IOThreads`_ for information on how to do that.
+
+Code running in the monitor typically needs to ensure that past
+requests from the guest are completed. When a block device is running
+in an ``IOThread``, the ``IOThread`` can also process requests from the guest
+(via ioeventfd). To achieve both objects, wrap the code between
+``bdrv_drained_begin()`` and ``bdrv_drained_end()``, thus creating a "drained
+section".
+
+Long-running jobs (usually in the form of coroutines) are often scheduled in
+the ``BlockDriverState``'s ``AioContext``. The functions
+``bdrv_add``/``remove_aio_context_notifier``, or alternatively
+``blk_add``/``remove_aio_context_notifier`` if you use ``BlockBackends``,
+can be used to get a notification whenever ``bdrv_try_change_aio_context()``
+moves a ``BlockDriverState`` to a different ``AioContext``.
diff --git a/docs/devel/multiple-iothreads.txt b/docs/devel/multiple-iothreads.txt
deleted file mode 100644
index de85767b124..00000000000
--- a/docs/devel/multiple-iothreads.txt
+++ /dev/null
@@ -1,130 +0,0 @@
-Copyright (c) 2014-2017 Red Hat Inc.
-
-This work is licensed under the terms of the GNU GPL, version 2 or later. See
-the COPYING file in the top-level directory.
-
-
-This document explains the IOThread feature and how to write code that runs
-outside the BQL.
-
-The main loop and IOThreads
----------------------------
-QEMU is an event-driven program that can do several things at once using an
-event loop. The VNC server and the QMP monitor are both processed from the
-same event loop, which monitors their file descriptors until they become
-readable and then invokes a callback.
-
-The default event loop is called the main loop (see main-loop.c). It is
-possible to create additional event loop threads using -object
-iothread,id=my-iothread.
-
-Side note: The main loop and IOThread are both event loops but their code is
-not shared completely. Sometimes it is useful to remember that although they
-are conceptually similar they are currently not interchangeable.
-
-Why IOThreads are useful
-------------------------
-IOThreads allow the user to control the placement of work. The main loop is a
-scalability bottleneck on hosts with many CPUs. Work can be spread across
-several IOThreads instead of just one main loop. When set up correctly this
-can improve I/O latency and reduce jitter seen by the guest.
-
-The main loop is also deeply associated with the BQL, which is a
-scalability bottleneck in itself. vCPU threads and the main loop use the BQL
-to serialize execution of QEMU code. This mutex is necessary because a lot of
-QEMU's code historically was not thread-safe.
-
-The fact that all I/O processing is done in a single main loop and that the
-BQL is contended by all vCPU threads and the main loop explain
-why it is desirable to place work into IOThreads.
-
-The experimental virtio-blk data-plane implementation has been benchmarked and
-shows these effects:
-ftp://public.dhe.ibm.com/linux/pdfs/KVM_Virtualized_IO_Performance_Paper.pdf
-
-How to program for IOThreads
-----------------------------
-The main difference between legacy code and new code that can run in an
-IOThread is dealing explicitly with the event loop object, AioContext
-(see include/block/aio.h). Code that only works in the main loop
-implicitly uses the main loop's AioContext. Code that supports running
-in IOThreads must be aware of its AioContext.
-
-AioContext supports the following services:
- * File descriptor monitoring (read/write/error on POSIX hosts)
- * Event notifiers (inter-thread signalling)
- * Timers
- * Bottom Halves (BH) deferred callbacks
-
-There are several old APIs that use the main loop AioContext:
- * LEGACY qemu_aio_set_fd_handler() - monitor a file descriptor
- * LEGACY qemu_aio_set_event_notifier() - monitor an event notifier
- * LEGACY timer_new_ms() - create a timer
- * LEGACY qemu_bh_new() - create a BH
- * LEGACY qemu_bh_new_guarded() - create a BH with a device re-entrancy guard
- * LEGACY qemu_aio_wait() - run an event loop iteration
-
-Since they implicitly work on the main loop they cannot be used in code that
-runs in an IOThread. They might cause a crash or deadlock if called from an
-IOThread since the BQL is not held.
-
-Instead, use the AioContext functions directly (see include/block/aio.h):
- * aio_set_fd_handler() - monitor a file descriptor
- * aio_set_event_notifier() - monitor an event notifier
- * aio_timer_new() - create a timer
- * aio_bh_new() - create a BH
- * aio_bh_new_guarded() - create a BH with a device re-entrancy guard
- * aio_poll() - run an event loop iteration
-
-The qemu_bh_new_guarded/aio_bh_new_guarded APIs accept a "MemReentrancyGuard"
-argument, which is used to check for and prevent re-entrancy problems. For
-BHs associated with devices, the reentrancy-guard is contained in the
-corresponding DeviceState and named "mem_reentrancy_guard".
-
-The AioContext can be obtained from the IOThread using
-iothread_get_aio_context() or for the main loop using qemu_get_aio_context().
-Code that takes an AioContext argument works both in IOThreads or the main
-loop, depending on which AioContext instance the caller passes in.
-
-How to synchronize with an IOThread
------------------------------------
-Variables that can be accessed by multiple threads require some form of
-synchronization such as qemu_mutex_lock(), rcu_read_lock(), etc.
-
-AioContext functions like aio_set_fd_handler(), aio_set_event_notifier(),
-aio_bh_new(), and aio_timer_new() are thread-safe. They can be used to trigger
-activity in an IOThread.
-
-Side note: the best way to schedule a function call across threads is to call
-aio_bh_schedule_oneshot().
-
-The main loop thread can wait synchronously for a condition using
-AIO_WAIT_WHILE().
-
-AioContext and the block layer
-------------------------------
-The AioContext originates from the QEMU block layer, even though nowadays
-AioContext is a generic event loop that can be used by any QEMU subsystem.
-
-The block layer has support for AioContext integrated. Each BlockDriverState
-is associated with an AioContext using bdrv_try_change_aio_context() and
-bdrv_get_aio_context(). This allows block layer code to process I/O inside the
-right AioContext. Other subsystems may wish to follow a similar approach.
-
-Block layer code must therefore expect to run in an IOThread and avoid using
-old APIs that implicitly use the main loop. See the "How to program for
-IOThreads" above for information on how to do that.
-
-Code running in the monitor typically needs to ensure that past
-requests from the guest are completed. When a block device is running
-in an IOThread, the IOThread can also process requests from the guest
-(via ioeventfd). To achieve both objects, wrap the code between
-bdrv_drained_begin() and bdrv_drained_end(), thus creating a "drained
-section".
-
-Long-running jobs (usually in the form of coroutines) are often scheduled in
-the BlockDriverState's AioContext. The functions
-bdrv_add/remove_aio_context_notifier, or alternatively
-blk_add/remove_aio_context_notifier if you use BlockBackends, can be used to
-get a notification whenever bdrv_try_change_aio_context() moves a
-BlockDriverState to a different AioContext.
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 5/7] docs/devel/rcu: Convert to rST format
2024-08-16 13:22 [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST Peter Maydell
` (3 preceding siblings ...)
2024-08-16 13:22 ` [PATCH 4/7] docs/devel/multiple-iothreads: " Peter Maydell
@ 2024-08-16 13:22 ` Peter Maydell
2024-08-16 13:22 ` [PATCH 6/7] include: Move QemuLockCnt APIs to their own header Peter Maydell
` (2 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2024-08-16 13:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
Convert docs/devel/rcu.txt to rST format.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
MAINTAINERS | 2 +-
docs/devel/index-internals.rst | 1 +
docs/devel/{rcu.txt => rcu.rst} | 172 +++++++++++++++-----------------
3 files changed, 82 insertions(+), 93 deletions(-)
rename docs/devel/{rcu.txt => rcu.rst} (73%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9e091a4e214..f8f4df44460 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3113,7 +3113,7 @@ Read, Copy, Update (RCU)
M: Paolo Bonzini <pbonzini@redhat.com>
S: Maintained
F: docs/devel/lockcnt.rst
-F: docs/devel/rcu.txt
+F: docs/devel/rcu.rst
F: include/qemu/rcu*.h
F: tests/unit/rcutorture.c
F: tests/unit/test-rcu-*.c
diff --git a/docs/devel/index-internals.rst b/docs/devel/index-internals.rst
index 88fa0e9450d..ab9fbc44826 100644
--- a/docs/devel/index-internals.rst
+++ b/docs/devel/index-internals.rst
@@ -8,6 +8,7 @@ Details about QEMU's various subsystems including how to add features to them.
qom
atomics
+ rcu
block-coroutine-wrapper
clocks
ebpf_rss
diff --git a/docs/devel/rcu.txt b/docs/devel/rcu.rst
similarity index 73%
rename from docs/devel/rcu.txt
rename to docs/devel/rcu.rst
index 2e6cc607a17..dd07c1d9195 100644
--- a/docs/devel/rcu.txt
+++ b/docs/devel/rcu.rst
@@ -20,7 +20,7 @@ for the execution of all *currently running* critical sections before
proceeding, or before asynchronously executing a callback.
The key point here is that only the currently running critical sections
-are waited for; critical sections that are started _after_ the beginning
+are waited for; critical sections that are started **after** the beginning
of the wait do not extend the wait, despite running concurrently with
the updater. This is the reason why RCU is more scalable than,
for example, reader-writer locks. It is so much more scalable that
@@ -37,7 +37,7 @@ do not matter; as soon as all previous critical sections have finished,
there cannot be any readers who hold references to the data structure,
and these can now be safely reclaimed (e.g., freed or unref'ed).
-Here is a picture:
+Here is a picture::
thread 1 thread 2 thread 3
------------------- ------------------------ -------------------
@@ -58,43 +58,38 @@ that critical section.
RCU API
-=======
+-------
The core RCU API is small:
- void rcu_read_lock(void);
-
+``void rcu_read_lock(void);``
Used by a reader to inform the reclaimer that the reader is
entering an RCU read-side critical section.
- void rcu_read_unlock(void);
-
+``void rcu_read_unlock(void);``
Used by a reader to inform the reclaimer that the reader is
exiting an RCU read-side critical section. Note that RCU
read-side critical sections may be nested and/or overlapping.
- void synchronize_rcu(void);
-
+``void synchronize_rcu(void);``
Blocks until all pre-existing RCU read-side critical sections
on all threads have completed. This marks the end of the removal
phase and the beginning of reclamation phase.
Note that it would be valid for another update to come while
- synchronize_rcu is running. Because of this, it is better that
+ ``synchronize_rcu`` is running. Because of this, it is better that
the updater releases any locks it may hold before calling
- synchronize_rcu. If this is not possible (for example, because
- the updater is protected by the BQL), you can use call_rcu.
+ ``synchronize_rcu``. If this is not possible (for example, because
+ the updater is protected by the BQL), you can use ``call_rcu``.
- void call_rcu1(struct rcu_head * head,
- void (*func)(struct rcu_head *head));
-
- This function invokes func(head) after all pre-existing RCU
+``void call_rcu1(struct rcu_head * head, void (*func)(struct rcu_head *head));``
+ This function invokes ``func(head)`` after all pre-existing RCU
read-side critical sections on all threads have completed. This
marks the end of the removal phase, with func taking care
asynchronously of the reclamation phase.
- The foo struct needs to have an rcu_head structure added,
- perhaps as follows:
+ The ``foo`` struct needs to have an ``rcu_head`` structure added,
+ perhaps as follows::
struct foo {
struct rcu_head rcu;
@@ -103,8 +98,8 @@ The core RCU API is small:
long c;
};
- so that the reclaimer function can fetch the struct foo address
- and free it:
+ so that the reclaimer function can fetch the ``struct foo`` address
+ and free it::
call_rcu1(&foo.rcu, foo_reclaim);
@@ -114,29 +109,27 @@ The core RCU API is small:
g_free(fp);
}
- For the common case where the rcu_head member is the first of the
- struct, you can use the following macro.
+ ``call_rcu1`` is typically used via either the ``call_rcu`` or
+ ``g_free_rcu`` macros, which handle the common case where the
+ ``rcu_head`` member is the first of the struct.
- void call_rcu(T *p,
- void (*func)(T *p),
- field-name);
- void g_free_rcu(T *p,
- field-name);
+``void call_rcu(T *p, void (*func)(T *p), field-name);``
+ If the ``struct rcu_head`` is the first field in the struct, you can
+ use this macro instead of ``call_rcu1``.
- call_rcu1 is typically used through these macro, in the common case
- where the "struct rcu_head" is the first field in the struct. If
- the callback function is g_free, in particular, g_free_rcu can be
- used. In the above case, one could have written simply:
+``void g_free_rcu(T *p, field-name);``
+ This is a special-case version of ``call_rcu`` where the callback
+ function is ``g_free``.
+ In the example given in ``call_rcu1``, one could have written simply::
g_free_rcu(&foo, rcu);
- typeof(*p) qatomic_rcu_read(p);
+``typeof(*p) qatomic_rcu_read(p);``
+ ``qatomic_rcu_read()`` is similar to ``qatomic_load_acquire()``, but
+ it makes some assumptions on the code that calls it. This allows a
+ more optimized implementation.
- qatomic_rcu_read() is similar to qatomic_load_acquire(), but it makes
- some assumptions on the code that calls it. This allows a more
- optimized implementation.
-
- qatomic_rcu_read assumes that whenever a single RCU critical
+ ``qatomic_rcu_read`` assumes that whenever a single RCU critical
section reads multiple shared data, these reads are either
data-dependent or need no ordering. This is almost always the
case when using RCU, because read-side critical sections typically
@@ -144,7 +137,7 @@ The core RCU API is small:
every update) until reaching a data structure of interest,
and then read from there.
- RCU read-side critical sections must use qatomic_rcu_read() to
+ RCU read-side critical sections must use ``qatomic_rcu_read()`` to
read data, unless concurrent writes are prevented by another
synchronization mechanism.
@@ -152,18 +145,17 @@ The core RCU API is small:
data structure in a single direction, opposite to the direction
in which the updater initializes it.
- void qatomic_rcu_set(p, typeof(*p) v);
+``void qatomic_rcu_set(p, typeof(*p) v);``
+ ``qatomic_rcu_set()`` is similar to ``qatomic_store_release()``,
+ though it also makes assumptions on the code that calls it in
+ order to allow a more optimized implementation.
- qatomic_rcu_set() is similar to qatomic_store_release(), though it also
- makes assumptions on the code that calls it in order to allow a more
- optimized implementation.
-
- In particular, qatomic_rcu_set() suffices for synchronization
+ In particular, ``qatomic_rcu_set()`` suffices for synchronization
with readers, if the updater never mutates a field within a
data item that is already accessible to readers. This is the
case when initializing a new copy of the RCU-protected data
- structure; just ensure that initialization of *p is carried out
- before qatomic_rcu_set() makes the data item visible to readers.
+ structure; just ensure that initialization of ``*p`` is carried out
+ before ``qatomic_rcu_set()`` makes the data item visible to readers.
If this rule is observed, writes will happen in the opposite
order as reads in the RCU read-side critical sections (or if
there is just one update), and there will be no need for other
@@ -171,58 +163,54 @@ The core RCU API is small:
The following APIs must be used before RCU is used in a thread:
- void rcu_register_thread(void);
-
+``void rcu_register_thread(void);``
Mark a thread as taking part in the RCU mechanism. Such a thread
will have to report quiescent points regularly, either manually
- or through the QemuCond/QemuSemaphore/QemuEvent APIs.
-
- void rcu_unregister_thread(void);
+ or through the ``QemuCond``/``QemuSemaphore``/``QemuEvent`` APIs.
+``void rcu_unregister_thread(void);``
Mark a thread as not taking part anymore in the RCU mechanism.
It is not a problem if such a thread reports quiescent points,
- either manually or by using the QemuCond/QemuSemaphore/QemuEvent
- APIs.
+ either manually or by using the
+ ``QemuCond``/``QemuSemaphore``/``QemuEvent`` APIs.
-Note that these APIs are relatively heavyweight, and should _not_ be
+Note that these APIs are relatively heavyweight, and should **not** be
nested.
Convenience macros
-==================
+------------------
Two macros are provided that automatically release the read lock at the
end of the scope.
- RCU_READ_LOCK_GUARD()
-
+``RCU_READ_LOCK_GUARD()``
Takes the lock and will release it at the end of the block it's
used in.
- WITH_RCU_READ_LOCK_GUARD() { code }
-
+``WITH_RCU_READ_LOCK_GUARD() { code }``
Is used at the head of a block to protect the code within the block.
-Note that 'goto'ing out of the guarded block will also drop the lock.
+Note that a ``goto`` out of the guarded block will also drop the lock.
-DIFFERENCES WITH LINUX
-======================
+Differences with Linux
+----------------------
- Waiting on a mutex is possible, though discouraged, within an RCU critical
section. This is because spinlocks are rarely (if ever) used in userspace
programming; not allowing this would prevent upgrading an RCU read-side
critical section to become an updater.
-- qatomic_rcu_read and qatomic_rcu_set replace rcu_dereference and
- rcu_assign_pointer. They take a _pointer_ to the variable being accessed.
+- ``qatomic_rcu_read`` and ``qatomic_rcu_set`` replace ``rcu_dereference`` and
+ ``rcu_assign_pointer``. They take a **pointer** to the variable being accessed.
-- call_rcu is a macro that has an extra argument (the name of the first
- field in the struct, which must be a struct rcu_head), and expects the
+- ``call_rcu`` is a macro that has an extra argument (the name of the first
+ field in the struct, which must be a struct ``rcu_head``), and expects the
type of the callback's argument to be the type of the first argument.
- call_rcu1 is the same as Linux's call_rcu.
+ ``call_rcu1`` is the same as Linux's ``call_rcu``.
-RCU PATTERNS
-============
+RCU Patterns
+------------
Many patterns using read-writer locks translate directly to RCU, with
the advantages of higher scalability and deadlock immunity.
@@ -243,28 +231,28 @@ Here are some frequently-used RCU idioms that are worth noting.
RCU list processing
--------------------
+^^^^^^^^^^^^^^^^^^^
TBD (not yet used in QEMU)
RCU reference counting
-----------------------
+^^^^^^^^^^^^^^^^^^^^^^
Because grace periods are not allowed to complete while there is an RCU
read-side critical section in progress, the RCU read-side primitives
may be used as a restricted reference-counting mechanism. For example,
-consider the following code fragment:
+consider the following code fragment::
rcu_read_lock();
p = qatomic_rcu_read(&foo);
/* do something with p. */
rcu_read_unlock();
-The RCU read-side critical section ensures that the value of "p" remains
-valid until after the rcu_read_unlock(). In some sense, it is acquiring
-a reference to p that is later released when the critical section ends.
-The write side looks simply like this (with appropriate locking):
+The RCU read-side critical section ensures that the value of ``p`` remains
+valid until after the ``rcu_read_unlock()``. In some sense, it is acquiring
+a reference to ``p`` that is later released when the critical section ends.
+The write side looks simply like this (with appropriate locking)::
qemu_mutex_lock(&foo_mutex);
old = foo;
@@ -274,7 +262,7 @@ The write side looks simply like this (with appropriate locking):
free(old);
If the processing cannot be done purely within the critical section, it
-is possible to combine this idiom with a "real" reference count:
+is possible to combine this idiom with a "real" reference count::
rcu_read_lock();
p = qatomic_rcu_read(&foo);
@@ -283,7 +271,7 @@ is possible to combine this idiom with a "real" reference count:
/* do something with p. */
foo_unref(p);
-The write side can be like this:
+The write side can be like this::
qemu_mutex_lock(&foo_mutex);
old = foo;
@@ -292,7 +280,7 @@ The write side can be like this:
synchronize_rcu();
foo_unref(old);
-or with call_rcu:
+or with ``call_rcu``::
qemu_mutex_lock(&foo_mutex);
old = foo;
@@ -301,10 +289,10 @@ or with call_rcu:
call_rcu(foo_unref, old, rcu);
In both cases, the write side only performs removal. Reclamation
-happens when the last reference to a "foo" object is dropped.
-Using synchronize_rcu() is undesirably expensive, because the
+happens when the last reference to a ``foo`` object is dropped.
+Using ``synchronize_rcu()`` is undesirably expensive, because the
last reference may be dropped on the read side. Hence you can
-use call_rcu() instead:
+use ``call_rcu()`` instead::
foo_unref(struct foo *p) {
if (qatomic_fetch_dec(&p->refcount) == 1) {
@@ -314,7 +302,7 @@ use call_rcu() instead:
Note that the same idioms would be possible with reader/writer
-locks:
+locks::
read_lock(&foo_rwlock); write_mutex_lock(&foo_rwlock);
p = foo; p = foo;
@@ -334,15 +322,15 @@ locks:
foo_unref(p);
read_unlock(&foo_rwlock);
-foo_unref could use a mechanism such as bottom halves to move deallocation
+``foo_unref`` could use a mechanism such as bottom halves to move deallocation
out of the write-side critical section.
RCU resizable arrays
---------------------
+^^^^^^^^^^^^^^^^^^^^
Resizable arrays can be used with RCU. The expensive RCU synchronization
-(or call_rcu) only needs to take place when the array is resized.
+(or ``call_rcu``) only needs to take place when the array is resized.
The two items to take care of are:
- ensuring that the old version of the array is available between removal
@@ -351,10 +339,10 @@ The two items to take care of are:
- avoiding mismatches in the read side between the array data and the
array size.
-The first problem is avoided simply by not using realloc. Instead,
+The first problem is avoided simply by not using ``realloc``. Instead,
each resize will allocate a new array and copy the old data into it.
The second problem would arise if the size and the data pointers were
-two members of a larger struct:
+two members of a larger struct::
struct mystuff {
...
@@ -364,7 +352,7 @@ two members of a larger struct:
...
};
-Instead, we store the size of the array with the array itself:
+Instead, we store the size of the array with the array itself::
struct arr {
int size;
@@ -400,7 +388,7 @@ Instead, we store the size of the array with the array itself:
}
-SOURCES
-=======
+References
+----------
-* Documentation/RCU/ from the Linux kernel
+* The `Linux kernel RCU documentation <https://docs.kernel.org/RCU/>`__
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 6/7] include: Move QemuLockCnt APIs to their own header
2024-08-16 13:22 [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST Peter Maydell
` (4 preceding siblings ...)
2024-08-16 13:22 ` [PATCH 5/7] docs/devel/rcu: " Peter Maydell
@ 2024-08-16 13:22 ` Peter Maydell
2024-08-16 14:54 ` Philippe Mathieu-Daudé
2024-10-11 10:33 ` Paolo Bonzini
2024-08-16 13:22 ` [PATCH 7/7] docs/devel/lockcnt: Include kernel-doc API documentation Peter Maydell
2024-09-06 15:12 ` [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST Peter Maydell
7 siblings, 2 replies; 15+ messages in thread
From: Peter Maydell @ 2024-08-16 13:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
Currently the QemuLockCnt data structure and associated functions are
in the include/qemu/thread.h header. Move them to their own
qemu/lockcnt.h. The main reason for doing this is that it means we
can autogenerate the documentation comments into the docs/devel
documentation.
The copyright/author in the new header is drawn from lockcnt.c,
since the header changes were added in the same commit as
lockcnt.c; since neither thread.h nor lockcnt.c state an explicit
license, the standard default of GPL-2-or-later applies.
We include the new header (and the .c file, which was accidentally
omitted previously) in the "RCU" part of MAINTAINERS, since that
is where the lockcnt.rst documentation is categorized.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Paolo: could you confirm that you meant GPL2+ for this code?
---
MAINTAINERS | 2 +
docs/devel/lockcnt.rst | 2 +-
include/block/aio.h | 1 +
include/hw/core/cpu.h | 1 +
include/qemu/lockcnt.h | 130 +++++++++++++++++++++++++++++++++++++++++
include/qemu/thread.h | 111 -----------------------------------
accel/accel-blocker.c | 1 +
hw/core/cpu-common.c | 1 +
util/aio-posix.c | 1 +
util/aio-win32.c | 1 +
util/async.c | 1 +
util/fdmon-epoll.c | 1 +
util/lockcnt.c | 1 +
13 files changed, 142 insertions(+), 112 deletions(-)
create mode 100644 include/qemu/lockcnt.h
diff --git a/MAINTAINERS b/MAINTAINERS
index f8f4df44460..2da11411ff3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3115,8 +3115,10 @@ S: Maintained
F: docs/devel/lockcnt.rst
F: docs/devel/rcu.rst
F: include/qemu/rcu*.h
+F: include/qemu/lockcnt.h
F: tests/unit/rcutorture.c
F: tests/unit/test-rcu-*.c
+F: util/lockcnt.c
F: util/rcu.c
Human Monitor (HMP)
diff --git a/docs/devel/lockcnt.rst b/docs/devel/lockcnt.rst
index 994aeb57151..728594bcea3 100644
--- a/docs/devel/lockcnt.rst
+++ b/docs/devel/lockcnt.rst
@@ -175,7 +175,7 @@ three instructions in the critical path, two assignments and a ``smp_wmb()``.
``QemuLockCnt`` API
-------------------
-The ``QemuLockCnt`` API is described in ``include/qemu/thread.h``.
+The ``QemuLockCnt`` API is described in ``include/qemu/lockcnt.h``.
``QemuLockCnt`` usage
diff --git a/include/block/aio.h b/include/block/aio.h
index 4ee81936ed5..43883a8a33a 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -20,6 +20,7 @@
#include "qemu/coroutine-core.h"
#include "qemu/queue.h"
#include "qemu/event_notifier.h"
+#include "qemu/lockcnt.h"
#include "qemu/thread.h"
#include "qemu/timer.h"
#include "block/graph-lock.h"
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 1c9c775df65..ecbeeb1c0dd 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -33,6 +33,7 @@
#include "qemu/bitmap.h"
#include "qemu/rcu_queue.h"
#include "qemu/queue.h"
+#include "qemu/lockcnt.h"
#include "qemu/thread.h"
#include "qom/object.h"
diff --git a/include/qemu/lockcnt.h b/include/qemu/lockcnt.h
new file mode 100644
index 00000000000..2c92ae17c9e
--- /dev/null
+++ b/include/qemu/lockcnt.h
@@ -0,0 +1,130 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * QemuLockCnt implementation
+ *
+ * Copyright Red Hat, Inc. 2017
+ *
+ * Author:
+ * Paolo Bonzini <pbonzini@redhat.com>
+ *
+ */
+
+#ifndef QEMU_LOCKCNT_H
+#define QEMU_LOCKCNT_H
+
+#include "qemu/thread.h"
+
+typedef struct QemuLockCnt QemuLockCnt;
+
+struct QemuLockCnt {
+#ifndef CONFIG_LINUX
+ QemuMutex mutex;
+#endif
+ unsigned count;
+};
+
+/**
+ * qemu_lockcnt_init: initialize a QemuLockcnt
+ * @lockcnt: the lockcnt to initialize
+ *
+ * Initialize lockcnt's counter to zero and prepare its mutex
+ * for usage.
+ */
+void qemu_lockcnt_init(QemuLockCnt *lockcnt);
+
+/**
+ * qemu_lockcnt_destroy: destroy a QemuLockcnt
+ * @lockcnt: the lockcnt to destruct
+ *
+ * Destroy lockcnt's mutex.
+ */
+void qemu_lockcnt_destroy(QemuLockCnt *lockcnt);
+
+/**
+ * qemu_lockcnt_inc: increment a QemuLockCnt's counter
+ * @lockcnt: the lockcnt to operate on
+ *
+ * If the lockcnt's count is zero, wait for critical sections
+ * to finish and increment lockcnt's count to 1. If the count
+ * is not zero, just increment it.
+ *
+ * Because this function can wait on the mutex, it must not be
+ * called while the lockcnt's mutex is held by the current thread.
+ * For the same reason, qemu_lockcnt_inc can also contribute to
+ * AB-BA deadlocks. This is a sample deadlock scenario:
+ *
+ * thread 1 thread 2
+ * -------------------------------------------------------
+ * qemu_lockcnt_lock(&lc1);
+ * qemu_lockcnt_lock(&lc2);
+ * qemu_lockcnt_inc(&lc2);
+ * qemu_lockcnt_inc(&lc1);
+ */
+void qemu_lockcnt_inc(QemuLockCnt *lockcnt);
+
+/**
+ * qemu_lockcnt_dec: decrement a QemuLockCnt's counter
+ * @lockcnt: the lockcnt to operate on
+ */
+void qemu_lockcnt_dec(QemuLockCnt *lockcnt);
+
+/**
+ * qemu_lockcnt_dec_and_lock: decrement a QemuLockCnt's counter and
+ * possibly lock it.
+ * @lockcnt: the lockcnt to operate on
+ *
+ * Decrement lockcnt's count. If the new count is zero, lock
+ * the mutex and return true. Otherwise, return false.
+ */
+bool qemu_lockcnt_dec_and_lock(QemuLockCnt *lockcnt);
+
+/**
+ * qemu_lockcnt_dec_if_lock: possibly decrement a QemuLockCnt's counter and
+ * lock it.
+ * @lockcnt: the lockcnt to operate on
+ *
+ * If the count is 1, decrement the count to zero, lock
+ * the mutex and return true. Otherwise, return false.
+ */
+bool qemu_lockcnt_dec_if_lock(QemuLockCnt *lockcnt);
+
+/**
+ * qemu_lockcnt_lock: lock a QemuLockCnt's mutex.
+ * @lockcnt: the lockcnt to operate on
+ *
+ * Remember that concurrent visits are not blocked unless the count is
+ * also zero. You can use qemu_lockcnt_count to check for this inside a
+ * critical section.
+ */
+void qemu_lockcnt_lock(QemuLockCnt *lockcnt);
+
+/**
+ * qemu_lockcnt_unlock: release a QemuLockCnt's mutex.
+ * @lockcnt: the lockcnt to operate on.
+ */
+void qemu_lockcnt_unlock(QemuLockCnt *lockcnt);
+
+/**
+ * qemu_lockcnt_inc_and_unlock: combined unlock/increment on a QemuLockCnt.
+ * @lockcnt: the lockcnt to operate on.
+ *
+ * This is the same as
+ *
+ * qemu_lockcnt_unlock(lockcnt);
+ * qemu_lockcnt_inc(lockcnt);
+ *
+ * but more efficient.
+ */
+void qemu_lockcnt_inc_and_unlock(QemuLockCnt *lockcnt);
+
+/**
+ * qemu_lockcnt_count: query a LockCnt's count.
+ * @lockcnt: the lockcnt to query.
+ *
+ * Note that the count can change at any time. Still, while the
+ * lockcnt is locked, one can usefully check whether the count
+ * is non-zero.
+ */
+unsigned qemu_lockcnt_count(QemuLockCnt *lockcnt);
+
+#endif
diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index fb74e21c08a..7eba27a7049 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -293,115 +293,4 @@ static inline void qemu_spin_unlock(QemuSpin *spin)
#endif
}
-struct QemuLockCnt {
-#ifndef CONFIG_LINUX
- QemuMutex mutex;
-#endif
- unsigned count;
-};
-
-/**
- * qemu_lockcnt_init: initialize a QemuLockcnt
- * @lockcnt: the lockcnt to initialize
- *
- * Initialize lockcnt's counter to zero and prepare its mutex
- * for usage.
- */
-void qemu_lockcnt_init(QemuLockCnt *lockcnt);
-
-/**
- * qemu_lockcnt_destroy: destroy a QemuLockcnt
- * @lockcnt: the lockcnt to destruct
- *
- * Destroy lockcnt's mutex.
- */
-void qemu_lockcnt_destroy(QemuLockCnt *lockcnt);
-
-/**
- * qemu_lockcnt_inc: increment a QemuLockCnt's counter
- * @lockcnt: the lockcnt to operate on
- *
- * If the lockcnt's count is zero, wait for critical sections
- * to finish and increment lockcnt's count to 1. If the count
- * is not zero, just increment it.
- *
- * Because this function can wait on the mutex, it must not be
- * called while the lockcnt's mutex is held by the current thread.
- * For the same reason, qemu_lockcnt_inc can also contribute to
- * AB-BA deadlocks. This is a sample deadlock scenario:
- *
- * thread 1 thread 2
- * -------------------------------------------------------
- * qemu_lockcnt_lock(&lc1);
- * qemu_lockcnt_lock(&lc2);
- * qemu_lockcnt_inc(&lc2);
- * qemu_lockcnt_inc(&lc1);
- */
-void qemu_lockcnt_inc(QemuLockCnt *lockcnt);
-
-/**
- * qemu_lockcnt_dec: decrement a QemuLockCnt's counter
- * @lockcnt: the lockcnt to operate on
- */
-void qemu_lockcnt_dec(QemuLockCnt *lockcnt);
-
-/**
- * qemu_lockcnt_dec_and_lock: decrement a QemuLockCnt's counter and
- * possibly lock it.
- * @lockcnt: the lockcnt to operate on
- *
- * Decrement lockcnt's count. If the new count is zero, lock
- * the mutex and return true. Otherwise, return false.
- */
-bool qemu_lockcnt_dec_and_lock(QemuLockCnt *lockcnt);
-
-/**
- * qemu_lockcnt_dec_if_lock: possibly decrement a QemuLockCnt's counter and
- * lock it.
- * @lockcnt: the lockcnt to operate on
- *
- * If the count is 1, decrement the count to zero, lock
- * the mutex and return true. Otherwise, return false.
- */
-bool qemu_lockcnt_dec_if_lock(QemuLockCnt *lockcnt);
-
-/**
- * qemu_lockcnt_lock: lock a QemuLockCnt's mutex.
- * @lockcnt: the lockcnt to operate on
- *
- * Remember that concurrent visits are not blocked unless the count is
- * also zero. You can use qemu_lockcnt_count to check for this inside a
- * critical section.
- */
-void qemu_lockcnt_lock(QemuLockCnt *lockcnt);
-
-/**
- * qemu_lockcnt_unlock: release a QemuLockCnt's mutex.
- * @lockcnt: the lockcnt to operate on.
- */
-void qemu_lockcnt_unlock(QemuLockCnt *lockcnt);
-
-/**
- * qemu_lockcnt_inc_and_unlock: combined unlock/increment on a QemuLockCnt.
- * @lockcnt: the lockcnt to operate on.
- *
- * This is the same as
- *
- * qemu_lockcnt_unlock(lockcnt);
- * qemu_lockcnt_inc(lockcnt);
- *
- * but more efficient.
- */
-void qemu_lockcnt_inc_and_unlock(QemuLockCnt *lockcnt);
-
-/**
- * qemu_lockcnt_count: query a LockCnt's count.
- * @lockcnt: the lockcnt to query.
- *
- * Note that the count can change at any time. Still, while the
- * lockcnt is locked, one can usefully check whether the count
- * is non-zero.
- */
-unsigned qemu_lockcnt_count(QemuLockCnt *lockcnt);
-
#endif
diff --git a/accel/accel-blocker.c b/accel/accel-blocker.c
index e083f24aa80..75daaa29113 100644
--- a/accel/accel-blocker.c
+++ b/accel/accel-blocker.c
@@ -25,6 +25,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/lockcnt.h"
#include "qemu/thread.h"
#include "qemu/main-loop.h"
#include "hw/core/cpu.h"
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index 7982ecd39a5..09c79035949 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -24,6 +24,7 @@
#include "sysemu/hw_accel.h"
#include "qemu/log.h"
#include "qemu/main-loop.h"
+#include "qemu/lockcnt.h"
#include "exec/log.h"
#include "exec/gdbstub.h"
#include "sysemu/tcg.h"
diff --git a/util/aio-posix.c b/util/aio-posix.c
index 266c9dd35fa..06bf9f456cf 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -17,6 +17,7 @@
#include "block/block.h"
#include "block/thread-pool.h"
#include "qemu/main-loop.h"
+#include "qemu/lockcnt.h"
#include "qemu/rcu.h"
#include "qemu/rcu_queue.h"
#include "qemu/sockets.h"
diff --git a/util/aio-win32.c b/util/aio-win32.c
index d144f9391fb..6583d5c5f31 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -18,6 +18,7 @@
#include "qemu/osdep.h"
#include "block/block.h"
#include "qemu/main-loop.h"
+#include "qemu/lockcnt.h"
#include "qemu/queue.h"
#include "qemu/sockets.h"
#include "qapi/error.h"
diff --git a/util/async.c b/util/async.c
index 3e3e4fc7126..99db28389f6 100644
--- a/util/async.c
+++ b/util/async.c
@@ -30,6 +30,7 @@
#include "block/graph-lock.h"
#include "qemu/main-loop.h"
#include "qemu/atomic.h"
+#include "qemu/lockcnt.h"
#include "qemu/rcu_queue.h"
#include "block/raw-aio.h"
#include "qemu/coroutine_int.h"
diff --git a/util/fdmon-epoll.c b/util/fdmon-epoll.c
index c6413cb18fe..9fb8800dde8 100644
--- a/util/fdmon-epoll.c
+++ b/util/fdmon-epoll.c
@@ -5,6 +5,7 @@
#include "qemu/osdep.h"
#include <sys/epoll.h>
+#include "qemu/lockcnt.h"
#include "qemu/rcu_queue.h"
#include "aio-posix.h"
diff --git a/util/lockcnt.c b/util/lockcnt.c
index 5da36946b1b..d07c6cc5cee 100644
--- a/util/lockcnt.c
+++ b/util/lockcnt.c
@@ -7,6 +7,7 @@
* Paolo Bonzini <pbonzini@redhat.com>
*/
#include "qemu/osdep.h"
+#include "qemu/lockcnt.h"
#include "qemu/thread.h"
#include "qemu/atomic.h"
#include "trace.h"
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 6/7] include: Move QemuLockCnt APIs to their own header
2024-08-16 13:22 ` [PATCH 6/7] include: Move QemuLockCnt APIs to their own header Peter Maydell
@ 2024-08-16 14:54 ` Philippe Mathieu-Daudé
2024-10-11 10:33 ` Paolo Bonzini
1 sibling, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-08-16 14:54 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
On 16/8/24 15:22, Peter Maydell wrote:
> Currently the QemuLockCnt data structure and associated functions are
> in the include/qemu/thread.h header. Move them to their own
> qemu/lockcnt.h. The main reason for doing this is that it means we
> can autogenerate the documentation comments into the docs/devel
> documentation.
>
> The copyright/author in the new header is drawn from lockcnt.c,
> since the header changes were added in the same commit as
> lockcnt.c; since neither thread.h nor lockcnt.c state an explicit
> license, the standard default of GPL-2-or-later applies.
>
> We include the new header (and the .c file, which was accidentally
> omitted previously) in the "RCU" part of MAINTAINERS, since that
> is where the lockcnt.rst documentation is categorized.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Paolo: could you confirm that you meant GPL2+ for this code?
> ---
> MAINTAINERS | 2 +
> docs/devel/lockcnt.rst | 2 +-
> include/block/aio.h | 1 +
> include/hw/core/cpu.h | 1 +
> include/qemu/lockcnt.h | 130 +++++++++++++++++++++++++++++++++++++++++
> include/qemu/thread.h | 111 -----------------------------------
> accel/accel-blocker.c | 1 +
> hw/core/cpu-common.c | 1 +
> util/aio-posix.c | 1 +
> util/aio-win32.c | 1 +
> util/async.c | 1 +
> util/fdmon-epoll.c | 1 +
> util/lockcnt.c | 1 +
> 13 files changed, 142 insertions(+), 112 deletions(-)
> create mode 100644 include/qemu/lockcnt.h
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 6/7] include: Move QemuLockCnt APIs to their own header
2024-08-16 13:22 ` [PATCH 6/7] include: Move QemuLockCnt APIs to their own header Peter Maydell
2024-08-16 14:54 ` Philippe Mathieu-Daudé
@ 2024-10-11 10:33 ` Paolo Bonzini
1 sibling, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-10-11 10:33 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
On 8/16/24 15:22, Peter Maydell wrote:
> Currently the QemuLockCnt data structure and associated functions are
> in the include/qemu/thread.h header. Move them to their own
> qemu/lockcnt.h. The main reason for doing this is that it means we
> can autogenerate the documentation comments into the docs/devel
> documentation.
>
> The copyright/author in the new header is drawn from lockcnt.c,
> since the header changes were added in the same commit as
> lockcnt.c; since neither thread.h nor lockcnt.c state an explicit
> license, the standard default of GPL-2-or-later applies.
>
> We include the new header (and the .c file, which was accidentally
> omitted previously) in the "RCU" part of MAINTAINERS, since that
> is where the lockcnt.rst documentation is categorized.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Yeah, GPLv2+ is intended for both these declarations and lockcnt.c.
Thanks,
Paolo
> ---
> Paolo: could you confirm that you meant GPL2+ for this code?
> ---
> MAINTAINERS | 2 +
> docs/devel/lockcnt.rst | 2 +-
> include/block/aio.h | 1 +
> include/hw/core/cpu.h | 1 +
> include/qemu/lockcnt.h | 130 +++++++++++++++++++++++++++++++++++++++++
> include/qemu/thread.h | 111 -----------------------------------
> accel/accel-blocker.c | 1 +
> hw/core/cpu-common.c | 1 +
> util/aio-posix.c | 1 +
> util/aio-win32.c | 1 +
> util/async.c | 1 +
> util/fdmon-epoll.c | 1 +
> util/lockcnt.c | 1 +
> 13 files changed, 142 insertions(+), 112 deletions(-)
> create mode 100644 include/qemu/lockcnt.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f8f4df44460..2da11411ff3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3115,8 +3115,10 @@ S: Maintained
> F: docs/devel/lockcnt.rst
> F: docs/devel/rcu.rst
> F: include/qemu/rcu*.h
> +F: include/qemu/lockcnt.h
> F: tests/unit/rcutorture.c
> F: tests/unit/test-rcu-*.c
> +F: util/lockcnt.c
> F: util/rcu.c
>
> Human Monitor (HMP)
> diff --git a/docs/devel/lockcnt.rst b/docs/devel/lockcnt.rst
> index 994aeb57151..728594bcea3 100644
> --- a/docs/devel/lockcnt.rst
> +++ b/docs/devel/lockcnt.rst
> @@ -175,7 +175,7 @@ three instructions in the critical path, two assignments and a ``smp_wmb()``.
> ``QemuLockCnt`` API
> -------------------
>
> -The ``QemuLockCnt`` API is described in ``include/qemu/thread.h``.
> +The ``QemuLockCnt`` API is described in ``include/qemu/lockcnt.h``.
>
>
> ``QemuLockCnt`` usage
> diff --git a/include/block/aio.h b/include/block/aio.h
> index 4ee81936ed5..43883a8a33a 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -20,6 +20,7 @@
> #include "qemu/coroutine-core.h"
> #include "qemu/queue.h"
> #include "qemu/event_notifier.h"
> +#include "qemu/lockcnt.h"
> #include "qemu/thread.h"
> #include "qemu/timer.h"
> #include "block/graph-lock.h"
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 1c9c775df65..ecbeeb1c0dd 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -33,6 +33,7 @@
> #include "qemu/bitmap.h"
> #include "qemu/rcu_queue.h"
> #include "qemu/queue.h"
> +#include "qemu/lockcnt.h"
> #include "qemu/thread.h"
> #include "qom/object.h"
>
> diff --git a/include/qemu/lockcnt.h b/include/qemu/lockcnt.h
> new file mode 100644
> index 00000000000..2c92ae17c9e
> --- /dev/null
> +++ b/include/qemu/lockcnt.h
> @@ -0,0 +1,130 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * QemuLockCnt implementation
> + *
> + * Copyright Red Hat, Inc. 2017
> + *
> + * Author:
> + * Paolo Bonzini <pbonzini@redhat.com>
> + *
> + */
> +
> +#ifndef QEMU_LOCKCNT_H
> +#define QEMU_LOCKCNT_H
> +
> +#include "qemu/thread.h"
> +
> +typedef struct QemuLockCnt QemuLockCnt;
> +
> +struct QemuLockCnt {
> +#ifndef CONFIG_LINUX
> + QemuMutex mutex;
> +#endif
> + unsigned count;
> +};
> +
> +/**
> + * qemu_lockcnt_init: initialize a QemuLockcnt
> + * @lockcnt: the lockcnt to initialize
> + *
> + * Initialize lockcnt's counter to zero and prepare its mutex
> + * for usage.
> + */
> +void qemu_lockcnt_init(QemuLockCnt *lockcnt);
> +
> +/**
> + * qemu_lockcnt_destroy: destroy a QemuLockcnt
> + * @lockcnt: the lockcnt to destruct
> + *
> + * Destroy lockcnt's mutex.
> + */
> +void qemu_lockcnt_destroy(QemuLockCnt *lockcnt);
> +
> +/**
> + * qemu_lockcnt_inc: increment a QemuLockCnt's counter
> + * @lockcnt: the lockcnt to operate on
> + *
> + * If the lockcnt's count is zero, wait for critical sections
> + * to finish and increment lockcnt's count to 1. If the count
> + * is not zero, just increment it.
> + *
> + * Because this function can wait on the mutex, it must not be
> + * called while the lockcnt's mutex is held by the current thread.
> + * For the same reason, qemu_lockcnt_inc can also contribute to
> + * AB-BA deadlocks. This is a sample deadlock scenario:
> + *
> + * thread 1 thread 2
> + * -------------------------------------------------------
> + * qemu_lockcnt_lock(&lc1);
> + * qemu_lockcnt_lock(&lc2);
> + * qemu_lockcnt_inc(&lc2);
> + * qemu_lockcnt_inc(&lc1);
> + */
> +void qemu_lockcnt_inc(QemuLockCnt *lockcnt);
> +
> +/**
> + * qemu_lockcnt_dec: decrement a QemuLockCnt's counter
> + * @lockcnt: the lockcnt to operate on
> + */
> +void qemu_lockcnt_dec(QemuLockCnt *lockcnt);
> +
> +/**
> + * qemu_lockcnt_dec_and_lock: decrement a QemuLockCnt's counter and
> + * possibly lock it.
> + * @lockcnt: the lockcnt to operate on
> + *
> + * Decrement lockcnt's count. If the new count is zero, lock
> + * the mutex and return true. Otherwise, return false.
> + */
> +bool qemu_lockcnt_dec_and_lock(QemuLockCnt *lockcnt);
> +
> +/**
> + * qemu_lockcnt_dec_if_lock: possibly decrement a QemuLockCnt's counter and
> + * lock it.
> + * @lockcnt: the lockcnt to operate on
> + *
> + * If the count is 1, decrement the count to zero, lock
> + * the mutex and return true. Otherwise, return false.
> + */
> +bool qemu_lockcnt_dec_if_lock(QemuLockCnt *lockcnt);
> +
> +/**
> + * qemu_lockcnt_lock: lock a QemuLockCnt's mutex.
> + * @lockcnt: the lockcnt to operate on
> + *
> + * Remember that concurrent visits are not blocked unless the count is
> + * also zero. You can use qemu_lockcnt_count to check for this inside a
> + * critical section.
> + */
> +void qemu_lockcnt_lock(QemuLockCnt *lockcnt);
> +
> +/**
> + * qemu_lockcnt_unlock: release a QemuLockCnt's mutex.
> + * @lockcnt: the lockcnt to operate on.
> + */
> +void qemu_lockcnt_unlock(QemuLockCnt *lockcnt);
> +
> +/**
> + * qemu_lockcnt_inc_and_unlock: combined unlock/increment on a QemuLockCnt.
> + * @lockcnt: the lockcnt to operate on.
> + *
> + * This is the same as
> + *
> + * qemu_lockcnt_unlock(lockcnt);
> + * qemu_lockcnt_inc(lockcnt);
> + *
> + * but more efficient.
> + */
> +void qemu_lockcnt_inc_and_unlock(QemuLockCnt *lockcnt);
> +
> +/**
> + * qemu_lockcnt_count: query a LockCnt's count.
> + * @lockcnt: the lockcnt to query.
> + *
> + * Note that the count can change at any time. Still, while the
> + * lockcnt is locked, one can usefully check whether the count
> + * is non-zero.
> + */
> +unsigned qemu_lockcnt_count(QemuLockCnt *lockcnt);
> +
> +#endif
> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
> index fb74e21c08a..7eba27a7049 100644
> --- a/include/qemu/thread.h
> +++ b/include/qemu/thread.h
> @@ -293,115 +293,4 @@ static inline void qemu_spin_unlock(QemuSpin *spin)
> #endif
> }
>
> -struct QemuLockCnt {
> -#ifndef CONFIG_LINUX
> - QemuMutex mutex;
> -#endif
> - unsigned count;
> -};
> -
> -/**
> - * qemu_lockcnt_init: initialize a QemuLockcnt
> - * @lockcnt: the lockcnt to initialize
> - *
> - * Initialize lockcnt's counter to zero and prepare its mutex
> - * for usage.
> - */
> -void qemu_lockcnt_init(QemuLockCnt *lockcnt);
> -
> -/**
> - * qemu_lockcnt_destroy: destroy a QemuLockcnt
> - * @lockcnt: the lockcnt to destruct
> - *
> - * Destroy lockcnt's mutex.
> - */
> -void qemu_lockcnt_destroy(QemuLockCnt *lockcnt);
> -
> -/**
> - * qemu_lockcnt_inc: increment a QemuLockCnt's counter
> - * @lockcnt: the lockcnt to operate on
> - *
> - * If the lockcnt's count is zero, wait for critical sections
> - * to finish and increment lockcnt's count to 1. If the count
> - * is not zero, just increment it.
> - *
> - * Because this function can wait on the mutex, it must not be
> - * called while the lockcnt's mutex is held by the current thread.
> - * For the same reason, qemu_lockcnt_inc can also contribute to
> - * AB-BA deadlocks. This is a sample deadlock scenario:
> - *
> - * thread 1 thread 2
> - * -------------------------------------------------------
> - * qemu_lockcnt_lock(&lc1);
> - * qemu_lockcnt_lock(&lc2);
> - * qemu_lockcnt_inc(&lc2);
> - * qemu_lockcnt_inc(&lc1);
> - */
> -void qemu_lockcnt_inc(QemuLockCnt *lockcnt);
> -
> -/**
> - * qemu_lockcnt_dec: decrement a QemuLockCnt's counter
> - * @lockcnt: the lockcnt to operate on
> - */
> -void qemu_lockcnt_dec(QemuLockCnt *lockcnt);
> -
> -/**
> - * qemu_lockcnt_dec_and_lock: decrement a QemuLockCnt's counter and
> - * possibly lock it.
> - * @lockcnt: the lockcnt to operate on
> - *
> - * Decrement lockcnt's count. If the new count is zero, lock
> - * the mutex and return true. Otherwise, return false.
> - */
> -bool qemu_lockcnt_dec_and_lock(QemuLockCnt *lockcnt);
> -
> -/**
> - * qemu_lockcnt_dec_if_lock: possibly decrement a QemuLockCnt's counter and
> - * lock it.
> - * @lockcnt: the lockcnt to operate on
> - *
> - * If the count is 1, decrement the count to zero, lock
> - * the mutex and return true. Otherwise, return false.
> - */
> -bool qemu_lockcnt_dec_if_lock(QemuLockCnt *lockcnt);
> -
> -/**
> - * qemu_lockcnt_lock: lock a QemuLockCnt's mutex.
> - * @lockcnt: the lockcnt to operate on
> - *
> - * Remember that concurrent visits are not blocked unless the count is
> - * also zero. You can use qemu_lockcnt_count to check for this inside a
> - * critical section.
> - */
> -void qemu_lockcnt_lock(QemuLockCnt *lockcnt);
> -
> -/**
> - * qemu_lockcnt_unlock: release a QemuLockCnt's mutex.
> - * @lockcnt: the lockcnt to operate on.
> - */
> -void qemu_lockcnt_unlock(QemuLockCnt *lockcnt);
> -
> -/**
> - * qemu_lockcnt_inc_and_unlock: combined unlock/increment on a QemuLockCnt.
> - * @lockcnt: the lockcnt to operate on.
> - *
> - * This is the same as
> - *
> - * qemu_lockcnt_unlock(lockcnt);
> - * qemu_lockcnt_inc(lockcnt);
> - *
> - * but more efficient.
> - */
> -void qemu_lockcnt_inc_and_unlock(QemuLockCnt *lockcnt);
> -
> -/**
> - * qemu_lockcnt_count: query a LockCnt's count.
> - * @lockcnt: the lockcnt to query.
> - *
> - * Note that the count can change at any time. Still, while the
> - * lockcnt is locked, one can usefully check whether the count
> - * is non-zero.
> - */
> -unsigned qemu_lockcnt_count(QemuLockCnt *lockcnt);
> -
> #endif
> diff --git a/accel/accel-blocker.c b/accel/accel-blocker.c
> index e083f24aa80..75daaa29113 100644
> --- a/accel/accel-blocker.c
> +++ b/accel/accel-blocker.c
> @@ -25,6 +25,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/lockcnt.h"
> #include "qemu/thread.h"
> #include "qemu/main-loop.h"
> #include "hw/core/cpu.h"
> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
> index 7982ecd39a5..09c79035949 100644
> --- a/hw/core/cpu-common.c
> +++ b/hw/core/cpu-common.c
> @@ -24,6 +24,7 @@
> #include "sysemu/hw_accel.h"
> #include "qemu/log.h"
> #include "qemu/main-loop.h"
> +#include "qemu/lockcnt.h"
> #include "exec/log.h"
> #include "exec/gdbstub.h"
> #include "sysemu/tcg.h"
> diff --git a/util/aio-posix.c b/util/aio-posix.c
> index 266c9dd35fa..06bf9f456cf 100644
> --- a/util/aio-posix.c
> +++ b/util/aio-posix.c
> @@ -17,6 +17,7 @@
> #include "block/block.h"
> #include "block/thread-pool.h"
> #include "qemu/main-loop.h"
> +#include "qemu/lockcnt.h"
> #include "qemu/rcu.h"
> #include "qemu/rcu_queue.h"
> #include "qemu/sockets.h"
> diff --git a/util/aio-win32.c b/util/aio-win32.c
> index d144f9391fb..6583d5c5f31 100644
> --- a/util/aio-win32.c
> +++ b/util/aio-win32.c
> @@ -18,6 +18,7 @@
> #include "qemu/osdep.h"
> #include "block/block.h"
> #include "qemu/main-loop.h"
> +#include "qemu/lockcnt.h"
> #include "qemu/queue.h"
> #include "qemu/sockets.h"
> #include "qapi/error.h"
> diff --git a/util/async.c b/util/async.c
> index 3e3e4fc7126..99db28389f6 100644
> --- a/util/async.c
> +++ b/util/async.c
> @@ -30,6 +30,7 @@
> #include "block/graph-lock.h"
> #include "qemu/main-loop.h"
> #include "qemu/atomic.h"
> +#include "qemu/lockcnt.h"
> #include "qemu/rcu_queue.h"
> #include "block/raw-aio.h"
> #include "qemu/coroutine_int.h"
> diff --git a/util/fdmon-epoll.c b/util/fdmon-epoll.c
> index c6413cb18fe..9fb8800dde8 100644
> --- a/util/fdmon-epoll.c
> +++ b/util/fdmon-epoll.c
> @@ -5,6 +5,7 @@
>
> #include "qemu/osdep.h"
> #include <sys/epoll.h>
> +#include "qemu/lockcnt.h"
> #include "qemu/rcu_queue.h"
> #include "aio-posix.h"
>
> diff --git a/util/lockcnt.c b/util/lockcnt.c
> index 5da36946b1b..d07c6cc5cee 100644
> --- a/util/lockcnt.c
> +++ b/util/lockcnt.c
> @@ -7,6 +7,7 @@
> * Paolo Bonzini <pbonzini@redhat.com>
> */
> #include "qemu/osdep.h"
> +#include "qemu/lockcnt.h"
> #include "qemu/thread.h"
> #include "qemu/atomic.h"
> #include "trace.h"
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 7/7] docs/devel/lockcnt: Include kernel-doc API documentation
2024-08-16 13:22 [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST Peter Maydell
` (5 preceding siblings ...)
2024-08-16 13:22 ` [PATCH 6/7] include: Move QemuLockCnt APIs to their own header Peter Maydell
@ 2024-08-16 13:22 ` Peter Maydell
2024-09-06 15:12 ` [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST Peter Maydell
7 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2024-08-16 13:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
Pull in the kernel-doc API documentation into the lockcnt docs.
This requires us to fix one rST markup syntax error in the
header file comments.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
docs/devel/lockcnt.rst | 2 +-
include/qemu/lockcnt.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/devel/lockcnt.rst b/docs/devel/lockcnt.rst
index 728594bcea3..8b43578f6c7 100644
--- a/docs/devel/lockcnt.rst
+++ b/docs/devel/lockcnt.rst
@@ -175,7 +175,7 @@ three instructions in the critical path, two assignments and a ``smp_wmb()``.
``QemuLockCnt`` API
-------------------
-The ``QemuLockCnt`` API is described in ``include/qemu/lockcnt.h``.
+.. kernel-doc:: include/qemu/lockcnt.h
``QemuLockCnt`` usage
diff --git a/include/qemu/lockcnt.h b/include/qemu/lockcnt.h
index 2c92ae17c9e..f4b62a3f701 100644
--- a/include/qemu/lockcnt.h
+++ b/include/qemu/lockcnt.h
@@ -51,7 +51,7 @@ void qemu_lockcnt_destroy(QemuLockCnt *lockcnt);
* Because this function can wait on the mutex, it must not be
* called while the lockcnt's mutex is held by the current thread.
* For the same reason, qemu_lockcnt_inc can also contribute to
- * AB-BA deadlocks. This is a sample deadlock scenario:
+ * AB-BA deadlocks. This is a sample deadlock scenario::
*
* thread 1 thread 2
* -------------------------------------------------------
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST
2024-08-16 13:22 [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST Peter Maydell
` (6 preceding siblings ...)
2024-08-16 13:22 ` [PATCH 7/7] docs/devel/lockcnt: Include kernel-doc API documentation Peter Maydell
@ 2024-09-06 15:12 ` Peter Maydell
2024-09-19 12:12 ` Peter Maydell
7 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2024-09-06 15:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
Ping for review on these docs conversions, please?
thanks
-- PMM
On Fri, 16 Aug 2024 at 14:22, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset converts a collection of the remaining txt files in
> docs/devel to rST format. At this point in the release cycle
> I'm targeting 9.2 for these.
>
> Patches 1-5 are simple conversions. In patch 6 I move the
> QemuLockCnt APIs to their own header file; this is so that we can
> include the kernel-doc generated APIs in lockcnt.rst (since
> kernel-doc works on a per-file basis).
>
> Paolo: can you confirm that you mean GPLv2-or-later for the lockcnt
> code? There's no explicit license comment in either util/lockcnt.c
> or include/qemu/thread.h.
>
> (There's one .txt file left in docs/devel after this, but I don't
> think it's very useful so I propose to delete it and will send a
> separate patch for that.)
>
> thanks
> -- PMM
>
> Peter Maydell (7):
> docs/devel/blkdebug: Convert to rST format
> docs/devel/blkverify: Convert to rST format
> docs/devel/lockcnt: Convert to rST format
> docs/devel/multiple-iothreads: Convert to rST format
> docs/devel/rcu: Convert to rST format
> include: Move QemuLockCnt APIs to their own header
> docs/devel/lockcnt: Include kernel-doc API documentation
>
> MAINTAINERS | 8 +-
> docs/devel/blkdebug.rst | 177 ++++++++++++++++++++
> docs/devel/blkdebug.txt | 162 ------------------
> docs/devel/{blkverify.txt => blkverify.rst} | 30 ++--
> docs/devel/index-api.rst | 1 +
> docs/devel/index-build.rst | 2 +
> docs/devel/index-internals.rst | 2 +
> docs/devel/{lockcnt.txt => lockcnt.rst} | 89 +++++-----
> docs/devel/multiple-iothreads.rst | 139 +++++++++++++++
> docs/devel/multiple-iothreads.txt | 130 --------------
> docs/devel/{rcu.txt => rcu.rst} | 172 +++++++++----------
> include/block/aio.h | 1 +
> include/hw/core/cpu.h | 1 +
> include/qemu/lockcnt.h | 130 ++++++++++++++
> include/qemu/thread.h | 111 ------------
> accel/accel-blocker.c | 1 +
> hw/core/cpu-common.c | 1 +
> util/aio-posix.c | 1 +
> util/aio-win32.c | 1 +
> util/async.c | 1 +
> util/fdmon-epoll.c | 1 +
> util/lockcnt.c | 1 +
> 22 files changed, 608 insertions(+), 554 deletions(-)
> create mode 100644 docs/devel/blkdebug.rst
> delete mode 100644 docs/devel/blkdebug.txt
> rename docs/devel/{blkverify.txt => blkverify.rst} (77%)
> rename docs/devel/{lockcnt.txt => lockcnt.rst} (75%)
> create mode 100644 docs/devel/multiple-iothreads.rst
> delete mode 100644 docs/devel/multiple-iothreads.txt
> rename docs/devel/{rcu.txt => rcu.rst} (73%)
> create mode 100644 include/qemu/lockcnt.h
>
> --
> 2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST
2024-09-06 15:12 ` [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST Peter Maydell
@ 2024-09-19 12:12 ` Peter Maydell
2024-10-10 12:08 ` Peter Maydell
0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2024-09-19 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
Ping^2.
In particular: Paolo, it would be helpful if you could
confirm what license you intended for the lockcnt.c code.
thanks
-- PMM
On Fri, 6 Sept 2024 at 16:12, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Ping for review on these docs conversions, please?
>
> thanks
> -- PMM
>
> On Fri, 16 Aug 2024 at 14:22, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > This patchset converts a collection of the remaining txt files in
> > docs/devel to rST format. At this point in the release cycle
> > I'm targeting 9.2 for these.
> >
> > Patches 1-5 are simple conversions. In patch 6 I move the
> > QemuLockCnt APIs to their own header file; this is so that we can
> > include the kernel-doc generated APIs in lockcnt.rst (since
> > kernel-doc works on a per-file basis).
> >
> > Paolo: can you confirm that you mean GPLv2-or-later for the lockcnt
> > code? There's no explicit license comment in either util/lockcnt.c
> > or include/qemu/thread.h.
> >
> > (There's one .txt file left in docs/devel after this, but I don't
> > think it's very useful so I propose to delete it and will send a
> > separate patch for that.)
> >
> > thanks
> > -- PMM
> >
> > Peter Maydell (7):
> > docs/devel/blkdebug: Convert to rST format
> > docs/devel/blkverify: Convert to rST format
> > docs/devel/lockcnt: Convert to rST format
> > docs/devel/multiple-iothreads: Convert to rST format
> > docs/devel/rcu: Convert to rST format
> > include: Move QemuLockCnt APIs to their own header
> > docs/devel/lockcnt: Include kernel-doc API documentation
> >
> > MAINTAINERS | 8 +-
> > docs/devel/blkdebug.rst | 177 ++++++++++++++++++++
> > docs/devel/blkdebug.txt | 162 ------------------
> > docs/devel/{blkverify.txt => blkverify.rst} | 30 ++--
> > docs/devel/index-api.rst | 1 +
> > docs/devel/index-build.rst | 2 +
> > docs/devel/index-internals.rst | 2 +
> > docs/devel/{lockcnt.txt => lockcnt.rst} | 89 +++++-----
> > docs/devel/multiple-iothreads.rst | 139 +++++++++++++++
> > docs/devel/multiple-iothreads.txt | 130 --------------
> > docs/devel/{rcu.txt => rcu.rst} | 172 +++++++++----------
> > include/block/aio.h | 1 +
> > include/hw/core/cpu.h | 1 +
> > include/qemu/lockcnt.h | 130 ++++++++++++++
> > include/qemu/thread.h | 111 ------------
> > accel/accel-blocker.c | 1 +
> > hw/core/cpu-common.c | 1 +
> > util/aio-posix.c | 1 +
> > util/aio-win32.c | 1 +
> > util/async.c | 1 +
> > util/fdmon-epoll.c | 1 +
> > util/lockcnt.c | 1 +
> > 22 files changed, 608 insertions(+), 554 deletions(-)
> > create mode 100644 docs/devel/blkdebug.rst
> > delete mode 100644 docs/devel/blkdebug.txt
> > rename docs/devel/{blkverify.txt => blkverify.rst} (77%)
> > rename docs/devel/{lockcnt.txt => lockcnt.rst} (75%)
> > create mode 100644 docs/devel/multiple-iothreads.rst
> > delete mode 100644 docs/devel/multiple-iothreads.txt
> > rename docs/devel/{rcu.txt => rcu.rst} (73%)
> > create mode 100644 include/qemu/lockcnt.h
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH for-9.2 0/7] docs/devel: Convert txt files to rST
2024-09-19 12:12 ` Peter Maydell
@ 2024-10-10 12:08 ` Peter Maydell
0 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2024-10-10 12:08 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi
Ping^3 for review (or I'm going to just apply these anyway).
Paolo, I know we discussed this on IRC, but can I get an Acked-by
tag from you for the license clarification in patch 6, please?
thanks
-- PMM
On Thu, 19 Sept 2024 at 13:12, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Ping^2.
>
> In particular: Paolo, it would be helpful if you could
> confirm what license you intended for the lockcnt.c code.
>
> thanks
> -- PMM
>
> On Fri, 6 Sept 2024 at 16:12, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > Ping for review on these docs conversions, please?
> >
> > thanks
> > -- PMM
> >
> > On Fri, 16 Aug 2024 at 14:22, Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > This patchset converts a collection of the remaining txt files in
> > > docs/devel to rST format. At this point in the release cycle
> > > I'm targeting 9.2 for these.
> > >
> > > Patches 1-5 are simple conversions. In patch 6 I move the
> > > QemuLockCnt APIs to their own header file; this is so that we can
> > > include the kernel-doc generated APIs in lockcnt.rst (since
> > > kernel-doc works on a per-file basis).
> > >
> > > Paolo: can you confirm that you mean GPLv2-or-later for the lockcnt
> > > code? There's no explicit license comment in either util/lockcnt.c
> > > or include/qemu/thread.h.
> > >
> > > (There's one .txt file left in docs/devel after this, but I don't
> > > think it's very useful so I propose to delete it and will send a
> > > separate patch for that.)
> > >
> > > thanks
> > > -- PMM
> > >
> > > Peter Maydell (7):
> > > docs/devel/blkdebug: Convert to rST format
> > > docs/devel/blkverify: Convert to rST format
> > > docs/devel/lockcnt: Convert to rST format
> > > docs/devel/multiple-iothreads: Convert to rST format
> > > docs/devel/rcu: Convert to rST format
> > > include: Move QemuLockCnt APIs to their own header
> > > docs/devel/lockcnt: Include kernel-doc API documentation
> > >
> > > MAINTAINERS | 8 +-
> > > docs/devel/blkdebug.rst | 177 ++++++++++++++++++++
> > > docs/devel/blkdebug.txt | 162 ------------------
> > > docs/devel/{blkverify.txt => blkverify.rst} | 30 ++--
> > > docs/devel/index-api.rst | 1 +
> > > docs/devel/index-build.rst | 2 +
> > > docs/devel/index-internals.rst | 2 +
> > > docs/devel/{lockcnt.txt => lockcnt.rst} | 89 +++++-----
> > > docs/devel/multiple-iothreads.rst | 139 +++++++++++++++
> > > docs/devel/multiple-iothreads.txt | 130 --------------
> > > docs/devel/{rcu.txt => rcu.rst} | 172 +++++++++----------
> > > include/block/aio.h | 1 +
> > > include/hw/core/cpu.h | 1 +
> > > include/qemu/lockcnt.h | 130 ++++++++++++++
> > > include/qemu/thread.h | 111 ------------
> > > accel/accel-blocker.c | 1 +
> > > hw/core/cpu-common.c | 1 +
> > > util/aio-posix.c | 1 +
> > > util/aio-win32.c | 1 +
> > > util/async.c | 1 +
> > > util/fdmon-epoll.c | 1 +
> > > util/lockcnt.c | 1 +
> > > 22 files changed, 608 insertions(+), 554 deletions(-)
> > > create mode 100644 docs/devel/blkdebug.rst
> > > delete mode 100644 docs/devel/blkdebug.txt
> > > rename docs/devel/{blkverify.txt => blkverify.rst} (77%)
> > > rename docs/devel/{lockcnt.txt => lockcnt.rst} (75%)
> > > create mode 100644 docs/devel/multiple-iothreads.rst
> > > delete mode 100644 docs/devel/multiple-iothreads.txt
> > > rename docs/devel/{rcu.txt => rcu.rst} (73%)
> > > create mode 100644 include/qemu/lockcnt.h
^ permalink raw reply [flat|nested] 15+ messages in thread