From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Fam Zheng <fam@euphon.net>,
qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [RFC v3 0/3] scsi: restart dma after vm change state handlers
Date: Thu, 30 May 2019 00:10:30 +0200 [thread overview]
Message-ID: <20190529221030.GD3471@localhost.localdomain> (raw)
In-Reply-To: <24b93cc5-edb1-a197-14be-e63ac356325d@redhat.com>
Am 24.05.2019 um 20:47 hat Paolo Bonzini geschrieben:
> On 24/05/19 20:36, Stefan Hajnoczi wrote:
> > v3:
> > * Fix s/k->vmstate_change/vdc->vmstate_change/
> > * Still RFC, waiting for customer to confirm this fixes the issue
> > v2:
> > * Do it properly with a clean API instead of deferring to a BH!
> > Thanks for encouraging me to do this, Kevin.
> >
> > These patches solve a deadlock when the 'cont' command is used and there are
> > failed requests on a virtio-scsi device with iothreads. The deadlock itself is
> > actually not the thing we need to fix because we should never reach that case
> > anyway. Instead we need to make sure DMA restart is only performed after the
> > virtio-scsi iothread is re-initialized.
>
> custom_dma_restart is a bit ugly... Do you think it would make sense to
> order the VMStateChange handlers using some kind of enum (with the order
> unspecified within the category)?
>
> We could start with
>
> VMSTATECHANGE_PRIO_UNKNOWN = 0 (if needed?)
> VMSTATECHANGE_PRIO_IOTHREAD = 100
> VMSTATECHANGE_PRIO_DEVICE = 200
>
> where higher priorities run first on stop and last on resume.
I don't think this is as nice because stopping or resuming a device
could involve some async operation (e.g. be delegated to a BH). In this
case, a device on a child bus must still wait for the BH (or other async
part) to be completed before it can resume its own operation.
Basically, a single flat list of global VM state handlers wasn't a good
design, because what we actually need in such cases is something
hierarchical.
Kevin
next prev parent reply other threads:[~2019-05-29 22:13 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-24 18:36 [Qemu-devel] [RFC v3 0/3] scsi: restart dma after vm change state handlers Stefan Hajnoczi
2019-05-24 18:36 ` [Qemu-devel] [RFC v3 1/3] virtio: add vdc->vmchange_state() callback Stefan Hajnoczi
2019-05-24 18:36 ` [Qemu-devel] [RFC v3 2/3] scsi: add scsi_bus_dma_restart() Stefan Hajnoczi
2019-05-24 18:36 ` [Qemu-devel] [RFC v3 3/3] virtio-scsi: fix iothread deadlock on 'cont' Stefan Hajnoczi
2019-05-24 18:47 ` [Qemu-devel] [RFC v3 0/3] scsi: restart dma after vm change state handlers Paolo Bonzini
2019-05-24 19:08 ` Michael S. Tsirkin
2019-05-29 22:10 ` Kevin Wolf [this message]
2019-05-30 8:27 ` Paolo Bonzini
2019-05-30 8:52 ` Kevin Wolf
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=20190529221030.GD3471@localhost.localdomain \
--to=kwolf@redhat.com \
--cc=fam@euphon.net \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--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).