Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.
Signed-off-by: Stefan Hajnoczi
---
block/blkio.c | 40 +---
1 file changed, 21 insertions(+), 19
No block driver implements .bdrv_co_io_plug() anymore. Get rid of the
function pointers.
Signed-off-by: Stefan Hajnoczi
---
include/block/block-io.h | 3 ---
include/block/block_int-common.h | 11 --
block/io.c | 37
3
Introduce a new API for thread-local blk_io_plug() that does not
traverse the block graph. The goal is to make blk_io_plug() multi-queue
friendly.
Instead of having block drivers track whether or not we're in a plugged
section, provide an API that allows them to defer a function call until
we're
Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.
Signed-off-by: Stefan Hajnoczi
---
block/nvme.c | 44
1 file changed, 12 insertions(+), 32
Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.
Signed-off-by: Stefan Hajnoczi
---
include/block/raw-aio.h | 7 ---
block/file-posix.c | 28
Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.
Signed-off-by: Stefan Hajnoczi
---
include/block/raw-aio.h | 7 ---
block/file-posix.c | 10 -
block/io_uring.c
The existing blk_io_plug() API is not block layer multi-queue friendly because
the plug state is per-BlockDriverState.
Change blk_io_plug()'s implementation so it is thread-local. This is done by
introducing the blk_io_plug_call() function that block drivers use to batch
calls while plugged. It
On Fri, Feb 24, 2023 at 10:34 AM Philippe Mathieu-Daudé
wrote:
>
> First, IDE_DMA__COUNT represents the number of enumerated
> values, but is incorrectly listed as part of the enum.
>
> Second, IDE_DMA_CMD_str() is internal to core.c and only
> takes sane enums from ide_dma_cmd. So no need to
On Fri, Apr 28, 2023 at 9:23 AM Niklas Cassel wrote:
>
> From: Niklas Cassel
>
> When encountering an NCQ error, you should not write the NCQ tag to the
> SError register. This is completely wrong.
Mea culpa ... !
>
> The SError register has a clear definition, where each bit represents a
>
On Fri, Apr 28, 2023 at 9:23 AM Niklas Cassel wrote:
>
> From: Niklas Cassel
>
> According to AHCI 1.3.1, 5.3.8.1 RegFIS:Entry, if ERR_STAT is set,
> we jump to state ERR:FatalTaskfile, which will raise a TFES IRQ
> unconditionally, regardless if the I bit is set in the FIS or not.
>
> Thus, we
On Fri, Apr 28, 2023 at 9:23 AM Niklas Cassel wrote:
>
> From: Niklas Cassel
>
> According to AHCI 1.3.1 definition of PxSACT:
> This field is cleared when PxCMD.ST is written from a '1' to a '0' by
> software. This field is not cleared by a COMRESET or a software reset.
>
> According to AHCI
On Fri, Apr 28, 2023 at 9:22 AM Niklas Cassel wrote:
>
> From: Niklas Cassel
>
> The way that BUSY + PxCI is cleared for NCQ (FPDMA QUEUED) commands is
> described in SATA 3.5a Gold:
>
> 11.15 FPDMA QUEUED command protocol
> DFPDMAQ2: ClearInterfaceBsy
> "Transmit Register Device to Host FIS
On Fri, Apr 28, 2023 at 9:22 AM Niklas Cassel wrote:
>
> From: Niklas Cassel
>
> Currently, the first time sending an unsupported command
> (e.g. READ LOG DMA EXT) will not have ERR_STAT set in the completion.
> Sending the unsupported command again, will correctly have ERR_STAT set.
>
> When
On Wed, May 10, 2023 at 08:31:13AM +0200, Juan Quintela wrote:
> Peter Xu wrote:
>
> Hi
>
> [Adding Kevin to the party]
>
> > On Fri, May 05, 2023 at 03:46:52PM +0200, Fiona Ebner wrote:
> >> To fix it, ensure that the BQL is held during setup. To avoid changing
> >> the behavior for migration
On 5/17/23 09:51, Kevin Wolf wrote:
From: Stefan Hajnoczi
Signed-off-by: Stefan Hajnoczi
Message-Id: <20230502184134.534703-3-stefa...@redhat.com>
Tested-by: Kevin Wolf
Signed-off-by: Kevin Wolf
---
tests/unit/test-nested-aio-poll.c | 130 ++
On Fri, Apr 28, 2023 at 9:22 AM Niklas Cassel wrote:
>
> From: Niklas Cassel
>
> This backslash obviously does not belong here, so remove it.
Reviewed-by: John Snow
>
> Signed-off-by: Niklas Cassel
> ---
> hw/ide/ahci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
On Fri, Apr 28, 2023 at 9:22 AM Niklas Cassel wrote:
>
> From: Niklas Cassel
>
> Hello John,
>
Hi Niklas!
I haven't been actively involved with AHCI for a while, so I am not
sure I can find the time to give this a deep scrub. I'm going to
assume based on '@wdc.com` that you probably know a
Skip TestBlockdevReopen.test_insert_compress_filter() if the 'compress'
driver isn't available.
In order to make the test succeed when the case is skipped, we also need
to remove any output from it (which would be missing in the case where
we skip it). This is done by replacing qemu_io_log() with
If we take a reader lock, we can't call any functions that take a writer
lock internally without causing deadlocks once the reader lock is
actually enforced in the main thread, too. Take the reader lock only
where it is actually needed.
Signed-off-by: Kevin Wolf
Message-Id:
If we take a reader lock, we can't call any functions that take a writer
lock internally without causing deadlocks once the reader lock is
actually enforced in the main thread, too. Take the reader lock only
where it is actually needed.
Signed-off-by: Kevin Wolf
Message-Id:
From: Vladimir Sementsov-Ogievskiy
Let's simplify things:
First, actions generally don't need access to common BlkActionState
structure. The only exclusion are backup actions that need
block_job_txn.
Next, for transaction actions of Transaction API is more native to
allocated state structure
These are functions that modify the graph, so they must be able to take
a writer lock. This is impossible if they already hold the reader lock.
If they need a reader lock for some of their operations, they should
take it internally.
Many of them go through blk_*(), which will always take the lock
From: Stefan Hajnoczi
QEMU's event loop supports nesting, which means that event handler
functions may themselves call aio_poll(). The condition that triggered a
handler must be reset before the nested aio_poll() call, otherwise the
same handler will be called and immediately re-enter aio_poll.
When jobs are sleeping, for example to enforce a given rate limit, they
can be reentered early, in particular in order to get paused, to update
the rate limit or to get cancelled.
Before this patch, they behave in this case as if they had fully
completed their rate limiting delay. This means that
From: Vladimir Sementsov-Ogievskiy
We are going to add more block-graph modifying transaction actions,
and block-graph modifying functions are already based on Transaction
API.
Next, we'll need to separately update permissions after several
graph-modifying actions, and this would be simple with
There are some error paths in blk_exp_add() that jump to 'fail:' before
'exp' is even created. So we can't just unconditionally access exp->blk.
Add a NULL check, and switch from exp->blk to blk, which is available
earlier, just to be extra sure that we really cover all cases where
BlockDevOps
There are some conditions under which we don't actually need to do
anything for taking a reader lock: Writing the graph is only possible
from the main context while holding the BQL. So if a reader is running
in the main context under the BQL and knows that it won't be interrupted
until the next
From: Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Kevin Wolf
Message-Id: <20230510150624.310640-4-vsement...@yandex-team.ru>
Signed-off-by: Kevin Wolf
---
blockdev.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git
From: Vladimir Sementsov-Ogievskiy
Look at qmp_transaction(): dev_list is not obvious name for list of
actions. Let's look at qapi spec, this argument is "actions". Let's
follow the common practice of using same argument names in qapi scheme
and code.
To be honest, rename props to properties
qcow2_do_open() calls a few no_co_wrappers that wrap functions taking
the graph lock internally as a writer. Therefore, it can't hold the
reader lock across these calls, it causes deadlocks. Drop the lock
temporarily around the calls.
Signed-off-by: Kevin Wolf
Message-Id:
From: Stefan Hajnoczi
Signed-off-by: Stefan Hajnoczi
Message-Id: <20230502184134.534703-3-stefa...@redhat.com>
Tested-by: Kevin Wolf
Signed-off-by: Kevin Wolf
---
tests/unit/test-nested-aio-poll.c | 130 ++
tests/unit/meson.build| 1 +
2 files
bdrv_unref() is a no_coroutine_fn, so calling it from coroutine context
is invalid. Use bdrv_co_unref() instead.
Signed-off-by: Kevin Wolf
Message-Id: <20230510203601.418015-7-kw...@redhat.com>
Reviewed-by: Eric Blake
Signed-off-by: Kevin Wolf
---
tests/unit/test-bdrv-drain.c | 2 +-
1 file
From: Vladimir Sementsov-Ogievskiy
Only backup supports GROUPED mode. Make this logic more clear. And
avoid passing extra thing to each action.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Message-Id: <20230510150624.310640-5-vsement...@yandex-team.ru>
Reviewed-by: Kevin Wolf
Signed-off-by:
From: Vladimir Sementsov-Ogievskiy
Other bitmap related actions use the .bitmap pointer in .abort action,
let's do same here:
1. It helps further refactoring, as bitmap-add is the only bitmap
action that uses state.action in .abort
2. It must be safe: transaction actions rely on the fact
From: Akihiro Suda
"zlib" clusters are actually raw deflate (RFC1951) clusters without
zlib headers.
Signed-off-by: Akihiro Suda
Message-Id: <168424874322.11954.134094204635185952...@git.sr.ht>
Reviewed-by: Eric Blake
Reviewed-by: Kevin Wolf
Signed-off-by: Kevin Wolf
---
The following changes since commit 6972ef1440a9d685482d78672620a7482f2bd09a:
Merge tag 'pull-tcg-20230516-3' of https://gitlab.com/rth7680/qemu into
staging (2023-05-16 21:30:27 -0700)
are available in the Git repository at:
https://repo.or.cz/qemu/kevin.git tags/for-upstream
for you to
Am 02.05.2023 um 20:41 hat Stefan Hajnoczi geschrieben:
> The following stack exhaustion was reported in
> https://bugzilla.redhat.com/show_bug.cgi?id=2186181:
>
> ...
> #51 0x55884fca7451 aio_poll (qemu-kvm + 0x9d6451)
> #52 0x55884fab9cbd bdrv_poll_co (qemu-kvm + 0x7e8cbd)
> #53
Am 16.05.2023 um 16:32 hat ~akihirosuda geschrieben:
> From: Akihiro Suda
>
> "zlib" clusters are actually raw deflate (RFC1951) clusters without
> zlib headers.
>
> Signed-off-by: Akihiro Suda
Thanks, applied to the block branch.
Kevin
This tests exercises graph locking, draining, and graph modifications
with AioContext switches a lot. Amongst others, it serves as a
regression test for bdrv_graph_wrlock() deadlocking because it is called
with a locked AioContext and for AioContext handling in the NBD server.
Signed-off-by:
nbd_drained_poll() generally runs in the main thread, not whatever
iothread the NBD server coroutine is meant to run in, so it can't
directly reenter the coroutines to wake them up.
The code seems to have the right intention, it specifies the correct
AioContext when it calls
In QEMU 8.0, we've been seeing deadlocks in bdrv_graph_wrlock(). They
come from callers that hold an AioContext lock, which is not allowed
during polling. In theory, we could temporarily release the lock, but
callers are inconsistent about whether they hold a lock, and if they do,
some are also
tl;dr is that graph locking introduced deadlocks in 8.0, and disabling
it for now fixes them again. See patch 1 for the details.
I still intend the fix this properly before we remove the AioContext
lock (which is when the deadlock would be automatically solved), but
it's not trivial enough for
On Wed, May 17, 2023 at 09:19:26AM +0200, Stefano Garzarella wrote:
> CCing Markus for some advice.
>
> On Tue, May 16, 2023 at 11:04:21AM -0500, Jonathon Jongsma wrote:
> > On 5/15/23 5:10 AM, Stefano Garzarella wrote:
> > > On Thu, May 11, 2023 at 11:03:22AM -0500, Jonathon Jongsma wrote:
> > >
CCing Markus for some advice.
On Tue, May 16, 2023 at 11:04:21AM -0500, Jonathon Jongsma wrote:
On 5/15/23 5:10 AM, Stefano Garzarella wrote:
On Thu, May 11, 2023 at 11:03:22AM -0500, Jonathon Jongsma wrote:
On 5/11/23 4:15 AM, Stefano Garzarella wrote:
The virtio-blk-vhost-vdpa driver in
44 matches
Mail list logo
