[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-commits] [qemu/qemu] 98e3ab: coroutine: Rename qemu_coroutine_inc/
From: |
Richard Henderson |
Subject: |
[Qemu-commits] [qemu/qemu] 98e3ab: coroutine: Rename qemu_coroutine_inc/dec_pool_size() |
Date: |
Thu, 12 May 2022 10:02:15 -0700 |
Branch: refs/heads/master
Home: https://github.com/qemu/qemu
Commit: 98e3ab35054b946f7c2aba5408822532b0920b53
https://github.com/qemu/qemu/commit/98e3ab35054b946f7c2aba5408822532b0920b53
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2022-05-12 (Thu, 12 May 2022)
Changed paths:
M hw/block/virtio-blk.c
M include/qemu/coroutine.h
M util/qemu-coroutine.c
Log Message:
-----------
coroutine: Rename qemu_coroutine_inc/dec_pool_size()
It's true that these functions currently affect the batch size in which
coroutines are reused (i.e. moved from the global release pool to the
allocation pool of a specific thread), but this is a bug and will be
fixed in a separate patch.
In fact, the comment in the header file already just promises that it
influences the pool size, so reflect this in the name of the functions.
As a nice side effect, the shorter function name makes some line
wrapping unnecessary.
Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20220510151020.105528-2-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 9ec7a59b5aad4b736871c378d30f5ef5ec51cb52
https://github.com/qemu/qemu/commit/9ec7a59b5aad4b736871c378d30f5ef5ec51cb52
Author: Kevin Wolf <kwolf@redhat.com>
Date: 2022-05-12 (Thu, 12 May 2022)
Changed paths:
M util/qemu-coroutine.c
Log Message:
-----------
coroutine: Revert to constant batch size
Commit 4c41c69e changed the way the coroutine pool is sized because for
virtio-blk devices with a large queue size and heavy I/O, it was just
too small and caused coroutines to be deleted and reallocated soon
afterwards. The change made the size dynamic based on the number of
queues and the queue size of virtio-blk devices.
There are two important numbers here: Slightly simplified, when a
coroutine terminates, it is generally stored in the global release pool
up to a certain pool size, and if the pool is full, it is freed.
Conversely, when allocating a new coroutine, the coroutines in the
release pool are reused if the pool already has reached a certain
minimum size (the batch size), otherwise we allocate new coroutines.
The problem after commit 4c41c69e is that it not only increases the
maximum pool size (which is the intended effect), but also the batch
size for reusing coroutines (which is a bug). It means that in cases
with many devices and/or a large queue size (which defaults to the
number of vcpus for virtio-blk-pci), many thousand coroutines could be
sitting in the release pool without being reused.
This is not only a waste of memory and allocations, but it actually
makes the QEMU process likely to hit the vm.max_map_count limit on Linux
because each coroutine requires two mappings (its stack and the guard
page for the stack), causing it to abort() in qemu_alloc_stack() because
when the limit is hit, mprotect() starts to fail with ENOMEM.
In order to fix the problem, change the batch size back to 64 to avoid
uselessly accumulating coroutines in the release pool, but keep the
dynamic maximum pool size so that coroutines aren't freed too early
in heavy I/O scenarios.
Note that this fix doesn't strictly make it impossible to hit the limit,
but this would only happen if most of the coroutines are actually in use
at the same time, not just sitting in a pool. This is the same behaviour
as we already had before commit 4c41c69e. Fully preventing this would
require allowing qemu_coroutine_create() to return an error, but it
doesn't seem to be a scenario that people hit in practice.
Cc: qemu-stable@nongnu.org
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2079938
Fixes: 4c41c69e05fe28c0f95f8abd2ebf407e95a4f04b
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20220510151020.105528-3-kwolf@redhat.com>
Tested-by: Hiroki Narukawa <hnarukaw@yahoo-corp.jp>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 22d92e71c77c8690995315c9ef5a1136c6300047
https://github.com/qemu/qemu/commit/22d92e71c77c8690995315c9ef5a1136c6300047
Author: Hanna Reitz <hreitz@redhat.com>
Date: 2022-05-12 (Thu, 12 May 2022)
Changed paths:
M tests/qemu-iotests/testrunner.py
Log Message:
-----------
iotests/testrunner: Flush after run_test()
When stdout is not a terminal, the buffer may not be flushed at each end
of line, so we should flush after each test is done. This is especially
apparent when run by check-block, in two ways:
First, when running make check-block -jX with X > 1, progress indication
was missing, even though testrunner.py does theoretically print each
test's status once it has been run, even in multi-processing mode.
Flushing after each test restores this progress indication.
Second, sometimes make check-block failed altogether, with an error
message that "too few tests [were] run". I presume that's because one
worker process in the job pool did not get to flush its stdout before
the main process exited, and so meson did not get to see that worker's
test results. In any case, by flushing at the end of run_test(), the
problem has disappeared for me.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220506134215.10086-1-hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 5e781c700a6ebf089cb01eb4f612479bfcfe186d
https://github.com/qemu/qemu/commit/5e781c700a6ebf089cb01eb4f612479bfcfe186d
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: 2022-05-12 (Thu, 12 May 2022)
Changed paths:
M tests/qemu-iotests/testrunner.py
Log Message:
-----------
tests/qemu-iotests: print intent to run a test in TAP mode
When running I/O tests using TAP output mode, we get a single TAP test
with a sub-test reported for each I/O test that is run. The output looks
something like this:
1..123
ok qcow2 011
ok qcow2 012
ok qcow2 013
ok qcow2 217
...
If everything runs or fails normally this is fine, but periodically we
have been seeing the test harness abort early before all 123 tests have
been run, just leaving a fairly useless message like
TAP parsing error: Too few tests run (expected 123, got 107)
we have no idea which tests were running at the time the test harness
abruptly exited. This change causes us to print a message about our
intent to run each test, so we have a record of what is active at the
time the harness exits abnormally.
1..123
# running qcow2 011
ok qcow2 011
# running qcow2 012
ok qcow2 012
# running qcow2 013
ok qcow2 013
# running qcow2 217
ok qcow2 217
...
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20220509124134.867431-2-berrange@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 29a493765ecd63e2b8c1593c777877ee1f8f1cd1
https://github.com/qemu/qemu/commit/29a493765ecd63e2b8c1593c777877ee1f8f1cd1
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: 2022-05-12 (Thu, 12 May 2022)
Changed paths:
M .gitlab-ci.d/buildtest-template.yml
Log Message:
-----------
.gitlab-ci.d: export meson testlog.txt as an artifact
When running 'make check' we only get a summary of progress on the
console. Fortunately meson/ninja have saved the raw test output to a
logfile. Exposing this log will make it easier to debug failures that
happen in CI.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20220509124134.867431-3-berrange@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: defac5e2fbddf8423a354ff0454283a2115e1367
https://github.com/qemu/qemu/commit/defac5e2fbddf8423a354ff0454283a2115e1367
Author: Philippe Mathieu-Daudé <philmd@redhat.com>
Date: 2022-05-12 (Thu, 12 May 2022)
Changed paths:
M hw/block/fdc.c
Log Message:
-----------
hw/block/fdc: Prevent end-of-track overrun (CVE-2021-3507)
Per the 82078 datasheet, if the end-of-track (EOT byte in
the FIFO) is more than the number of sectors per side, the
command is terminated unsuccessfully:
* 5.2.5 DATA TRANSFER TERMINATION
The 82078 supports terminal count explicitly through
the TC pin and implicitly through the underrun/over-
run and end-of-track (EOT) functions. For full sector
transfers, the EOT parameter can define the last
sector to be transferred in a single or multisector
transfer. If the last sector to be transferred is a par-
tial sector, the host can stop transferring the data in
mid-sector, and the 82078 will continue to complete
the sector as if a hardware TC was received. The
only difference between these implicit functions and
TC is that they return "abnormal termination" result
status. Such status indications can be ignored if they
were expected.
* 6.1.3 READ TRACK
This command terminates when the EOT specified
number of sectors have been read. If the 82078
does not find an I D Address Mark on the diskette
after the second· occurrence of a pulse on the
INDX# pin, then it sets the IC code in Status Regis-
ter 0 to "01" (Abnormal termination), sets the MA bit
in Status Register 1 to "1", and terminates the com-
mand.
* 6.1.6 VERIFY
Refer to Table 6-6 and Table 6-7 for information
concerning the values of MT and EC versus SC and
EOT value.
* Table 6·6. Result Phase Table
* Table 6-7. Verify Command Result Phase Table
Fix by aborting the transfer when EOT > # Sectors Per Side.
Cc: qemu-stable@nongnu.org
Cc: Hervé Poussineau <hpoussin@reactos.org>
Fixes: baca51faff0 ("floppy driver: disk geometry auto detect")
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/339
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20211118115733.4038610-2-philmd@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 46609b90d9e3a6304def11038a76b58ff43f77bc
https://github.com/qemu/qemu/commit/46609b90d9e3a6304def11038a76b58ff43f77bc
Author: Philippe Mathieu-Daudé <philmd@redhat.com>
Date: 2022-05-12 (Thu, 12 May 2022)
Changed paths:
M tests/qtest/fdc-test.c
Log Message:
-----------
tests/qtest/fdc-test: Add a regression test for CVE-2021-3507
Add the reproducer from https://gitlab.com/qemu-project/qemu/-/issues/339
Without the previous commit, when running 'make check-qtest-i386'
with QEMU configured with '--enable-sanitizers' we get:
==4028352==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x619000062a00 at pc 0x5626d03c491a bp 0x7ffdb4199410 sp 0x7ffdb4198bc0
READ of size 786432 at 0x619000062a00 thread T0
#0 0x5626d03c4919 in __asan_memcpy (qemu-system-i386+0x1e65919)
#1 0x5626d1c023cc in flatview_write_continue softmmu/physmem.c:2787:13
#2 0x5626d1bf0c0f in flatview_write softmmu/physmem.c:2822:14
#3 0x5626d1bf0798 in address_space_write softmmu/physmem.c:2914:18
#4 0x5626d1bf0f37 in address_space_rw softmmu/physmem.c:2924:16
#5 0x5626d1bf14c8 in cpu_physical_memory_rw softmmu/physmem.c:2933:5
#6 0x5626d0bd5649 in cpu_physical_memory_write
include/exec/cpu-common.h:82:5
#7 0x5626d0bd0a07 in i8257_dma_write_memory hw/dma/i8257.c:452:9
#8 0x5626d09f825d in fdctrl_transfer_handler hw/block/fdc.c:1616:13
#9 0x5626d0a048b4 in fdctrl_start_transfer hw/block/fdc.c:1539:13
#10 0x5626d09f4c3e in fdctrl_write_data hw/block/fdc.c:2266:13
#11 0x5626d09f22f7 in fdctrl_write hw/block/fdc.c:829:9
#12 0x5626d1c20bc5 in portio_write softmmu/ioport.c:207:17
0x619000062a00 is located 0 bytes to the right of 512-byte region
[0x619000062800,0x619000062a00)
allocated by thread T0 here:
#0 0x5626d03c66ec in posix_memalign (qemu-system-i386+0x1e676ec)
#1 0x5626d2b988d4 in qemu_try_memalign util/oslib-posix.c:210:11
#2 0x5626d2b98b0c in qemu_memalign util/oslib-posix.c:226:27
#3 0x5626d09fbaf0 in fdctrl_realize_common hw/block/fdc.c:2341:20
#4 0x5626d0a150ed in isabus_fdc_realize hw/block/fdc-isa.c:113:5
#5 0x5626d2367935 in device_set_realized hw/core/qdev.c:531:13
SUMMARY: AddressSanitizer: heap-buffer-overflow (qemu-system-i386+0x1e65919)
in __asan_memcpy
Shadow bytes around the buggy address:
0x0c32800044f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c3280004500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c3280004510: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c3280004520: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c3280004530: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c3280004540:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c3280004550: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c3280004560: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c3280004570: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c3280004580: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c3280004590: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Heap left redzone: fa
Freed heap region: fd
==4028352==ABORTING
[ kwolf: Added snapshot=on to prevent write file lock failure ]
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: a5fced40212ed73c715ca298a2929dd4d99c9999
https://github.com/qemu/qemu/commit/a5fced40212ed73c715ca298a2929dd4d99c9999
Author: Eric Blake <eblake@redhat.com>
Date: 2022-05-12 (Thu, 12 May 2022)
Changed paths:
M blockdev-nbd.c
M include/block/nbd.h
M qemu-nbd.c
Log Message:
-----------
qemu-nbd: Pass max connections to blockdev layer
The next patch wants to adjust whether the NBD server code advertises
MULTI_CONN based on whether it is known if the server limits to
exactly one client. For a server started by QMP, this information is
obtained through nbd_server_start (which can support more than one
export); but for qemu-nbd (which supports exactly one export), it is
controlled only by the command-line option -e/--shared. Since we
already have a hook function used by qemu-nbd, it's easiest to just
alter its signature to fit our needs.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20220512004924.417153-2-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 58a6fdcc9efb2a7c1ef4893dca4aa5e8020ca3dc
https://github.com/qemu/qemu/commit/58a6fdcc9efb2a7c1ef4893dca4aa5e8020ca3dc
Author: Eric Blake <eblake@redhat.com>
Date: 2022-05-12 (Thu, 12 May 2022)
Changed paths:
M MAINTAINERS
M blockdev-nbd.c
M docs/interop/nbd.txt
M docs/tools/qemu-nbd.rst
M include/block/nbd.h
M nbd/server.c
M qapi/block-export.json
A tests/qemu-iotests/tests/nbd-multiconn
A tests/qemu-iotests/tests/nbd-multiconn.out
M tests/qemu-iotests/tests/nbd-qemu-allocation.out
Log Message:
-----------
nbd/server: Allow MULTI_CONN for shared writable exports
According to the NBD spec, a server that advertises
NBD_FLAG_CAN_MULTI_CONN promises that multiple client connections will
not see any cache inconsistencies: when properly separated by a single
flush, actions performed by one client will be visible to another
client, regardless of which client did the flush.
We always satisfy these conditions in qemu - even when we support
multiple clients, ALL clients go through a single point of reference
into the block layer, with no local caching. The effect of one client
is instantly visible to the next client. Even if our backend were a
network device, we argue that any multi-path caching effects that
would cause inconsistencies in back-to-back actions not seeing the
effect of previous actions would be a bug in that backend, and not the
fault of caching in qemu. As such, it is safe to unconditionally
advertise CAN_MULTI_CONN for any qemu NBD server situation that
supports parallel clients.
Note, however, that we don't want to advertise CAN_MULTI_CONN when we
know that a second client cannot connect (for historical reasons,
qemu-nbd defaults to a single connection while nbd-server-add and QMP
commands default to unlimited connections; but we already have
existing means to let either style of NBD server creation alter those
defaults). This is visible by no longer advertising MULTI_CONN for
'qemu-nbd -r' without -e, as in the iotest nbd-qemu-allocation.
The harder part of this patch is setting up an iotest to demonstrate
behavior of multiple NBD clients to a single server. It might be
possible with parallel qemu-io processes, but I found it easier to do
in python with the help of libnbd, and help from Nir and Vladimir in
writing the test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Suggested-by: Nir Soffer <nsoffer@redhat.com>
Suggested-by: Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>
Message-Id: <20220512004924.417153-3-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: f70625299ecc9ba577c87f3d1d75012c747c7d88
https://github.com/qemu/qemu/commit/f70625299ecc9ba577c87f3d1d75012c747c7d88
Author: Paolo Bonzini <pbonzini@redhat.com>
Date: 2022-05-12 (Thu, 12 May 2022)
Changed paths:
R tests/qemu-iotests/common.config
M tests/qemu-iotests/common.rc
Log Message:
-----------
qemu-iotests: inline common.config into common.rc
common.rc has some complicated logic to find the common.config that
dates back to xfstests and is completely unnecessary now. Just include
the contents of the file.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220505094723.732116-1-pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: b32b3897f8b8f2f17425c22ea229ea6ebcb7d552
https://github.com/qemu/qemu/commit/b32b3897f8b8f2f17425c22ea229ea6ebcb7d552
Author: Richard Henderson <richard.henderson@linaro.org>
Date: 2022-05-12 (Thu, 12 May 2022)
Changed paths:
M .gitlab-ci.d/buildtest-template.yml
M MAINTAINERS
M blockdev-nbd.c
M docs/interop/nbd.txt
M docs/tools/qemu-nbd.rst
M hw/block/fdc.c
M hw/block/virtio-blk.c
M include/block/nbd.h
M include/qemu/coroutine.h
M nbd/server.c
M qapi/block-export.json
M qemu-nbd.c
R tests/qemu-iotests/common.config
M tests/qemu-iotests/common.rc
M tests/qemu-iotests/testrunner.py
A tests/qemu-iotests/tests/nbd-multiconn
A tests/qemu-iotests/tests/nbd-multiconn.out
M tests/qemu-iotests/tests/nbd-qemu-allocation.out
M tests/qtest/fdc-test.c
M util/qemu-coroutine.c
Log Message:
-----------
Merge tag 'for-upstream' of git://repo.or.cz/qemu/kevin into staging
Block layer patches
- coroutine: Fix crashes due to too large pool batch size
- fdc: Prevent end-of-track overrun
- nbd: MULTI_CONN for shared writable exports
- iotests test runner improvements
# -----BEGIN PGP SIGNATURE-----
#
# iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmJ9KCkRHGt3b2xmQHJl
# ZGhhdC5jb20ACgkQfwmycsiPL9ZtSRAAmYDFBPqxfutpFXM7kIKwL6COXJC12MOx
# Tmu8cDiGB/jNChdi3kl6I5h5njzo3U0ZlL/Ign6EzHoeoXLAPSeUWmuRsARwsZ+A
# rL61gf6yrMjAo45FZuIS0GlMDk8BauRwPl9qPWeqQcrtOMYpxwZfyFGmcMpQgAOI
# MSC1I8p3FA7oJhGpKIHDPOjaZA97Lm2rLnDIwZ4f0YgssbybFBcFCXOQbhpsVhLy
# Tjp/L+qRUtna9xBsPHQvHZW0kITQbCQPdX+oVqqUmwzSvuHqfXKe1YppyPjBt/S0
# H7nxtx4HOgP0lP5Kea+wbIRAk9Da5uaOW8hlMWRLShEKv1iTUenQSKteBB6CD03t
# GD9ze1kGoR9b6szw795BXxZxcWii0cn359lIVHeKR/U8zDuz5w3zhyl0klK8xeJy
# nj+JErLwQ7BD8kNR+7WAfXTF3tk2dQao1AvsBjn087KjMiJ/Mg8HY4K2zrjBUrHL
# DLTyAIjzct3BWJDZ02fb5jb8pHmIP3JO6m9Zvjm7ibP65BqJOwIXUTFpbgnrOg45
# oFLDV4JgC4Hh4GEtdm+UhQE51A0VVW5pDaqWTdWkCcuk3QgxUdM3Wm3SW6pw1Gvb
# T0X0j5RgF/k3YrW576R/VIy6z4YPbzAtiG4O/zSlsujHoDcVNWnxApgSB/unaDh8
# LNkFPGEMeSs=
# =JmTm
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 12 May 2022 08:30:49 AM PDT
# gpg: using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg: issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
* tag 'for-upstream' of git://repo.or.cz/qemu/kevin:
qemu-iotests: inline common.config into common.rc
nbd/server: Allow MULTI_CONN for shared writable exports
qemu-nbd: Pass max connections to blockdev layer
tests/qtest/fdc-test: Add a regression test for CVE-2021-3507
hw/block/fdc: Prevent end-of-track overrun (CVE-2021-3507)
.gitlab-ci.d: export meson testlog.txt as an artifact
tests/qemu-iotests: print intent to run a test in TAP mode
iotests/testrunner: Flush after run_test()
coroutine: Revert to constant batch size
coroutine: Rename qemu_coroutine_inc/dec_pool_size()
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Compare: https://github.com/qemu/qemu/compare/ec11dc41eec5...b32b3897f8b8