All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org,
	"Michael S. Tsirkin" <mst@redhat.com>,
	peterx@redhat.com, Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 2/2] virtio-scsi/virtio-blk: Disable poll handlers when stopping vq handler
Date: Wed, 12 Sep 2018 19:50:20 +0800	[thread overview]
Message-ID: <20180912115020.GH2526@lemon.usersys.redhat.com> (raw)
In-Reply-To: <3266e1a2-d4af-e8b3-1239-21778396718e@redhat.com>

On Wed, 09/12 13:11, Paolo Bonzini wrote:
> On 12/09/2018 03:31, Fam Zheng wrote:
> >>>
> >>> ctx is qemu_aio_context here, so there's no interaction with IOThread.
> >> In this case, it should be okay to have the reentrancy, what is the bug
> >> that this patch is fixing?
> > The same symptom as in the previous patch: virtio_scsi_handle_cmd_vq hangs. The
> > reason it hangs is fixed by the previous patch, but I don't think it should be
> > invoked as we're in the middle of virtio_scsi_dataplane_stop(). Applying either
> > one of the two patches avoids the problem, but this one is more superficial.
> > What do you think?
> 
> I think it's okay if it is invoked.  The sequence is first you stop the
> vq, then you drain the BlockBackends, then you switch AioContext.  All
> that matters is the outcome when virtio_scsi_dataplane_stop returns.

Yes, but together with vIOMMU, it also effectively leads to a virtio_error(),
which is not clean. QEMU stderr when this call happens (with patch 1 but not
this patch):

2018-09-12T11:48:10.193023Z qemu-system-x86_64: vtd_iommu_translate: detected translation failure (dev=02:00:00, iova=0x0)
2018-09-12T11:48:10.193044Z qemu-system-x86_64: New fault is not recorded due to compression of faults
2018-09-12T11:48:10.193061Z qemu-system-x86_64: virtio: zero sized buffers are not allowed

Fam

  reply	other threads:[~2018-09-12 11:56 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-10 14:56 [Qemu-devel] [PATCH 0/2] virtio-scsi: Fix QEMU hang with vIOMMU and ATS Fam Zheng
2018-09-10 14:56 ` [Qemu-devel] [PATCH 1/2] virtio: Return true from virtio_queue_empty if broken Fam Zheng
2018-09-10 14:56 ` [Qemu-devel] [PATCH 2/2] virtio-scsi/virtio-blk: Disable poll handlers when stopping vq handler Fam Zheng
2018-09-11 11:32   ` Paolo Bonzini
2018-09-11 14:12     ` Fam Zheng
2018-09-11 15:30       ` Paolo Bonzini
2018-09-12  1:31         ` Fam Zheng
2018-09-12 11:11           ` Paolo Bonzini
2018-09-12 11:50             ` Fam Zheng [this message]
2018-09-12 12:42               ` Paolo Bonzini
2018-09-13  6:03                 ` Fam Zheng
2018-09-13  9:11                   ` Paolo Bonzini
2018-09-13 10:04                     ` [Qemu-devel] [Qemu-block] " Paolo Bonzini
2018-09-13 16:00                       ` Alex Williamson
2018-09-14  2:45                         ` Peter Xu

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=20180912115020.GH2526@lemon.usersys.redhat.com \
    --to=famz@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.