qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "open list:Block layer core" <qemu-block@nongnu.org>,
	"Hajnoczi, Stefan" <stefanha@redhat.com>,
	Emanuele Giuseppe Esposito <eesposit@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [PATCH 03/20] block: bdrv/blk_co_unref() for calls in coroutine context
Date: Thu, 4 May 2023 13:18:53 +0200	[thread overview]
Message-ID: <ZFOUndq9rejgbYK4@redhat.com> (raw)
In-Reply-To: <CABgObfb1CuAEe53SiMbjSDZNvA+Hp4DB_HFZb-dxHg94vdJNDQ@mail.gmail.com>

Am 27.04.2023 um 22:49 hat Paolo Bonzini geschrieben:
> Il gio 27 apr 2023, 19:00 Kevin Wolf <kwolf@redhat.com> ha scritto:
> 
> > By the way, and slightly unrelated, can vrc somehow help with finding
> > places that call coroutine wrappers without holding the AioContext lock?
> > (This results in an abort() when AIO_WAIT_WHILE() tries to unlock the
> > AioContext.) This is one of the classes of bugs we're seeing in 8.0.
> >
> 
> Seems more like a task for TSA.
> 
> Even though C TSA doesn't let you check that the *right* AioContext lock is
> taken, it can check statically that *one* such lock is taken, and in
> general I would guess it's rare for the wrong AioContext to be locked.

The problem with TSA is that you need to add annotations everywhere, it
doesn't look at the full call graph. It would be almost the same effort
as for adding the graph lock annotation. I don't think we want to do
this for a lock that we want to remove relatively soon anyway.

I'd really prefer a tool where I don't need to modify the source code,
at least not more than saying "aio_context_acquire() takes the lock",
"aio_context_release() releases the lock" and "you must have the lock in
AIO_WAIT_WHILE()".

I guess the other option is manual auditing.

Kevin



  reply	other threads:[~2023-05-04 11:19 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-25 17:31 [PATCH 00/20] Graph locking, part 3 (more block drivers) Kevin Wolf
2023-04-25 17:31 ` [PATCH 01/20] qcow2: Don't call bdrv_getlength() in coroutine_fns Kevin Wolf
2023-04-25 18:37   ` Eric Blake
2023-04-27 11:12     ` Kevin Wolf
2023-04-27 15:07       ` Eric Blake
2023-05-01 15:19   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 02/20] block: Consistently call bdrv_activate() outside coroutine Kevin Wolf
2023-04-25 18:39   ` Eric Blake
2023-05-01 15:21   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 03/20] block: bdrv/blk_co_unref() for calls in coroutine context Kevin Wolf
2023-04-25 20:04   ` Eric Blake
2023-04-27 14:30   ` Paolo Bonzini
2023-04-27 17:00     ` Kevin Wolf
2023-04-27 20:49       ` Paolo Bonzini
2023-05-04 11:18         ` Kevin Wolf [this message]
2023-05-01 15:23   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 04/20] block: Don't call no_coroutine_fns in qmp_block_resize() Kevin Wolf
2023-04-25 20:08   ` Eric Blake
2023-04-27 11:15     ` Kevin Wolf
2023-04-27 15:09       ` Eric Blake
2023-05-01 15:24   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 05/20] test-bdrv-drain: Don't modify the graph in coroutines Kevin Wolf
2023-04-25 20:10   ` Eric Blake
2023-05-01 15:30   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 06/20] graph-lock: Add GRAPH_UNLOCKED(_PTR) Kevin Wolf
2023-04-25 20:20   ` Eric Blake
2023-05-01 15:32   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 07/20] graph-lock: Fix GRAPH_RDLOCK_GUARD*() to be reader lock Kevin Wolf
2023-04-25 20:36   ` Eric Blake
2023-04-27 11:28     ` Kevin Wolf
2023-04-27 15:15       ` Eric Blake
2023-05-01 15:34   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 08/20] block: .bdrv_open is non-coroutine and unlocked Kevin Wolf
2023-04-25 21:04   ` Eric Blake
2023-05-01 16:01   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 09/20] nbd: Remove nbd_co_flush() wrapper function Kevin Wolf
2023-04-25 21:05   ` Eric Blake
2023-05-01 16:02   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 10/20] nbd: Mark nbd_co_do_establish_connection() and callers GRAPH_RDLOCK Kevin Wolf
2023-04-25 21:07   ` Eric Blake
2023-05-01 18:57   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 11/20] vhdx: Take graph lock for accessing a node's parent list Kevin Wolf
2023-04-25 21:08   ` Eric Blake
2023-05-01 18:58   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 12/20] mirror: " Kevin Wolf
2023-04-25 21:09   ` Eric Blake
2023-05-01 18:59   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 13/20] block: Mark bdrv_co_get_allocated_file_size() and callers GRAPH_RDLOCK Kevin Wolf
2023-04-25 21:10   ` Eric Blake
2023-05-01 19:03   ` Stefan Hajnoczi
2023-05-04 10:52     ` Kevin Wolf
2023-04-25 17:31 ` [PATCH 14/20] block: Mark bdrv_co_get_info() " Kevin Wolf
2023-04-25 21:12   ` Eric Blake
2023-05-01 19:04   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 15/20] block: Mark bdrv_co_debug_event() GRAPH_RDLOCK Kevin Wolf
2023-04-25 21:14   ` Eric Blake
2023-05-04 11:12     ` Kevin Wolf
2023-05-01 19:05   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 16/20] block: Mark BlockDriver callbacks for amend job GRAPH_RDLOCK Kevin Wolf
2023-04-25 21:20   ` Eric Blake
2023-05-01 19:07   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 17/20] block: Mark bdrv_query_bds_stats() and callers GRAPH_RDLOCK Kevin Wolf
2023-04-25 21:20   ` Eric Blake
2023-05-01 19:08   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 18/20] block: Mark bdrv_query_block_graph_info() " Kevin Wolf
2023-04-25 21:21   ` Eric Blake
2023-05-01 19:08   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 19/20] block: Mark bdrv_recurse_can_replace() " Kevin Wolf
2023-04-25 21:22   ` Eric Blake
2023-05-01 19:10   ` Stefan Hajnoczi
2023-04-25 17:31 ` [PATCH 20/20] block: Mark bdrv_refresh_limits() " Kevin Wolf
2023-04-25 21:23   ` Eric Blake
2023-05-01 19:10   ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZFOUndq9rejgbYK4@redhat.com \
    --to=kwolf@redhat.com \
    --cc=eesposit@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).