All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Fam Zheng <famz@redhat.com>,
	qemu-block@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>,
	qemu-devel@nongnu.org,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	tu bo <tubo@linux.vnet.ibm.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 0/4] Tweaks around virtio-blk start/stop
Date: Tue, 22 Mar 2016 13:11:05 +0100	[thread overview]
Message-ID: <56F13659.1020605@redhat.com> (raw)
In-Reply-To: <20160322125958.13afc4e7.cornelia.huck@de.ibm.com>



On 22/03/2016 12:59, Cornelia Huck wrote:
>> > They can be fixed with just an extra object_ref/object_unref.
>> > 
>> > I didn't understand that Tu Bo also needed the BH fix, and with that
>> > information it makes sense.  Passing the assign value ensures that
>> > ioeventfd remains always assigned.  With the CPU threads out of the
>> > picture, the BH becomes enough to make everything thread-safe.
> Yes, this makes sense.
> 
> Might we still have a hole somewhere in dataplane teardown? Probably
> not, from reading the code, even if it runs in cpu thread context.

The bug arises when the main thread sets started = true, a CPU thread
comes along while the ioeventfd is reset, and as soon as the BQL is
released by the main thread the CPU thread thinks it is a dataplane
thread.  This does horrible things to non-reentrant code.  For stop we
should be safe because the CPU thread is the one that sets started = false.

IOW, we should be safe as long as the ioeventfd is never unassigned
(your fix) _and_ we ensure serialization between threads that touch
dataplane_started (Fam's fix).

Paolo

  reply	other threads:[~2016-03-22 12:11 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-16 10:10 [Qemu-devel] [PATCH 0/4] Tweaks around virtio-blk start/stop Fam Zheng
2016-03-16 10:10 ` [Qemu-devel] [PATCH 1/4] block: Use drained section in bdrv_set_aio_context Fam Zheng
2016-03-16 10:27   ` Paolo Bonzini
2016-03-16 10:51     ` Fam Zheng
2016-03-16 10:10 ` [Qemu-devel] [PATCH 2/4] block-backend: Introduce blk_drained_begin/end Fam Zheng
2016-03-16 10:10 ` [Qemu-devel] [PATCH 3/4] virtio-blk: Use blk_drained_begin/end around dataplane stop Fam Zheng
2016-03-16 10:10 ` [Qemu-devel] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH Fam Zheng
2016-03-17 15:00   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-03-17 15:07     ` Paolo Bonzini
2016-03-22 12:52       ` Fam Zheng
2016-03-22 18:05         ` Paolo Bonzini
2016-03-23  8:10           ` Cornelia Huck
2016-03-23  9:08             ` Paolo Bonzini
2016-03-23  9:12               ` Christian Borntraeger
2016-03-24  8:19                 ` tu bo
2016-03-24  8:32                   ` Cornelia Huck
2016-03-24  8:47                     ` Cornelia Huck
2016-03-24  9:31                       ` Cornelia Huck
2016-03-16 10:28 ` [Qemu-devel] [PATCH 0/4] Tweaks around virtio-blk start/stop Paolo Bonzini
2016-03-16 10:49   ` Christian Borntraeger
2016-03-16 11:09     ` Paolo Bonzini
2016-03-16 11:24       ` Christian Borntraeger
2016-03-16 12:55         ` Paolo Bonzini
2016-03-16 13:38           ` Christian Borntraeger
2016-03-16 13:45             ` Paolo Bonzini
2016-03-17  0:39               ` Fam Zheng
2016-03-17 11:03                 ` tu bo
2016-03-21 10:57                   ` Fam Zheng
2016-03-21 11:15                     ` Cornelia Huck
2016-03-21 12:45                       ` Fam Zheng
2016-03-21 13:02                         ` Cornelia Huck
2016-03-21 23:45                           ` Fam Zheng
2016-03-22  8:06                             ` Cornelia Huck
2016-03-22  7:10                     ` tu bo
2016-03-22  7:18                       ` Fam Zheng
2016-03-22  9:07                         ` Cornelia Huck
2016-03-22  9:46                           ` Paolo Bonzini
2016-03-22 11:59                             ` Cornelia Huck
2016-03-22 12:11                               ` Paolo Bonzini [this message]
2016-03-22 12:54                                 ` Cornelia Huck
2016-03-17 12:22             ` tu bo
2016-03-17 12:39               ` Christian Borntraeger
2016-03-17 13:02                 ` Cornelia Huck
2016-03-17 15:02                 ` Paolo Bonzini
2016-03-17 15:07                   ` Christian Borntraeger
2016-03-17 15:15                   ` Christian Borntraeger
2016-03-17 15:16                     ` Christian Borntraeger
2016-03-17 16:08                       ` Christian Borntraeger
2016-03-18 15:03                         ` Paolo Bonzini
2016-03-21  9:42                           ` Fam Zheng
2016-03-21 11:10                             ` Christian Borntraeger
2016-03-21 12:17                             ` Cornelia Huck
2016-03-21 13:47                           ` TU BO
2016-03-21 13:54                             ` Paolo Bonzini
2016-03-21 14:19                               ` Cornelia Huck
2016-03-22  0:31                                 ` Fam Zheng
2016-03-16 11:32       ` Cornelia Huck
2016-03-16 11:48         ` Paolo Bonzini
2016-03-16 11:56           ` Cornelia Huck
2016-03-16 11:59             ` Paolo Bonzini
2016-03-16 12:22               ` Cornelia Huck
2016-03-16 12:32                 ` Paolo Bonzini
2016-03-16 12:42                   ` Cornelia Huck
2016-03-16 12:49                     ` Paolo Bonzini
2016-03-16 13:04                       ` Cornelia Huck
2016-03-16 13:10                         ` Paolo Bonzini
2016-03-16 13:14                           ` Cornelia Huck
2016-03-16 13:15                             ` Paolo Bonzini
2016-03-16 11:52         ` Cornelia Huck
2016-03-16 11:54           ` Paolo Bonzini

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=56F13659.1020605@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=cornelia.huck@de.ibm.com \
    --cc=famz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=tubo@linux.vnet.ibm.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.