qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <famz@redhat.com>,
	qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [RFC 0/5] nbd: Adapt for dataplane
Date: Thu, 05 Jun 2014 19:41:07 +0200	[thread overview]
Message-ID: <5390ABB3.9050509@redhat.com> (raw)
In-Reply-To: <20140603143800.GG723@stefanha-thinkpad.muc.redhat.com>

On 03.06.2014 16:38, Stefan Hajnoczi wrote:
> On Sat, May 31, 2014 at 08:43:07PM +0200, Max Reitz wrote:
>> For the NBD server to work with dataplane, it needs to correctly access
>> the exported BDS. It makes the most sense to run both in the same
>> AioContext, therefore this series implements methods for tracking a
>> BDS's AioContext and makes NBD make use of this for keeping the clients
>> connected to that BDS in the same AioContext.
>>
>> The reason this is an RFC and not a PATCH is my inexperience with AIO,
>> coroutines and the like. Also, I'm not sure about what to do about the
>> coroutines. The NBD server has up to two coroutines per client: One for
>> receiving and one for sending. Theoretically, both have to be
>> "transferred" to the new AioContext if it is changed; however, as far as
>> I see it, coroutines are not really bound to an AioContext, they are
>> simply run in the AioContext entering them. Therefore, I think a
>> transfer is unnecessary. All coroutines are entered from nbd_read() and
>> nbd_restart_write(), both of which are AIO routines registered via
>> aio_set_fd_handler2().
>>
>> As bs_aio_detach() unregisters all of these routines, the coroutines can
>> no longer be entered, but only after bs_aio_attach() is called again.
>> Then, when they are called, they will enter the coroutines in the new
>> AioContext. Therefore, I think an explicit transfer unnecessary.
> This reasoning sounds correct.
>
>> However, if bs_aio_detach() is called from a different thread than the
>> old AioContext is running in, we may still have coroutines running for
>> which we should wait before returning from bs_aio_detach().
> The bdrv_attach/detach_aio_context() APIs have rules regarding where
> these functions are called from:
>
>    /**
>     * bdrv_set_aio_context:
>     *
>     * Changes the #AioContext used for fd handlers, timers, and BHs by this
>     * BlockDriverState and all its children.
>     *
>     * This function must be called from the old #AioContext or with a lock held so
>     * the old #AioContext is not executing.

Oh, that makes things easier. *g*

>     */
>    void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
>
> and:
>
>    /* Remove fd handlers, timers, and other event loop callbacks so the event
>     * loop is no longer in use.  Called with no in-flight requests and in
>     * depth-first traversal order with parents before child nodes.
>     */
>    void (*bdrv_detach_aio_context)(BlockDriverState *bs);
>
>    /* Add fd handlers, timers, and other event loop callbacks so I/O requests
>     * can be processed again.  Called with no in-flight requests and in
>     * depth-first traversal order with child nodes before parent nodes.
>     */
>    void (*bdrv_attach_aio_context)(BlockDriverState *bs,
>                                    AioContext *new_context);
>
> These rules ensure that it's safe to perform these operations.  You
> don't have to support arbitrary callers in NBD either.
>
>> But because of my inexperience with coroutines, I'm not sure. I now have
>> these patches nearly unchanged here for about a week and I'm looking for
>> ways of testing them, but so far I could only test whether the old use
>> cases work, but not whether they will work for what they are intended to
>> do: With BDS changing their AioContext.
>>
>> So, because I'm not sure what else to do and because I don't know how to
>> test multiple AIO threads (how do I move a BDS into another iothread?)
>> I'm just sending this out as an RFC.
> Use a Linux guest with virtio-blk:
>
>    qemu -drive if=none,file=test.img,id=drive0 \
>         -object iothread,id=iothread0 \
>         -device virtio-blk-pci,drive=drive0,x-iothread=iothread0 \
>         ...
>
> Once the guest has booted the virtio-blk device will be in dataplane
> mode.  That means drive0's BlockDriverState ->aio_context will be the
> IOThread AioContext and not the global qemu_aio_context.

Ah, thank you.

> Now you can exercise the run-time NBD server over QMP and check that
> things still work.  For example, try running a few instance of dd
> if=/dev/vdb of=/dev/null iflag=direct inside the guest to stress guest
> I/O.
>
> Typically what happens if code is not dataplane-aware is that a deadlock
> or crash occurs due to race conditions between the QEMU main loop and
> the IOThread for this virtio-blk device.
>
> For an overview of dataplane programming concepts, see:
> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg01436.html

Yes, I took this email as a reference, however it only said how to 
create a new iothread, but not how to use it. :-)

Max

> Stefan

  reply	other threads:[~2014-06-05 17:41 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1401561792-13410-1-git-send-email-mreitz@redhat.com>
2014-06-03 14:38 ` [Qemu-devel] [RFC 0/5] nbd: Adapt for dataplane Stefan Hajnoczi
2014-06-05 17:41   ` Max Reitz [this message]
     [not found] ` <1401561792-13410-3-git-send-email-mreitz@redhat.com>
2014-06-03 17:55   ` [Qemu-devel] [RFC 2/5] aio: Add io_read_poll() callback Paolo Bonzini
2014-06-05 17:29     ` Max Reitz
2014-06-04 11:59   ` Stefan Hajnoczi
     [not found] ` <1401561792-13410-2-git-send-email-mreitz@redhat.com>
2014-06-04 11:52   ` [Qemu-devel] [RFC 1/5] nbd: Correct name comparison for export_set_name() Stefan Hajnoczi
2014-06-05 17:28     ` Max Reitz
     [not found] ` <1401561792-13410-4-git-send-email-mreitz@redhat.com>
2014-06-04 12:37   ` [Qemu-devel] [RFC 3/5] nbd: Use aio_set_fd_handler2() Stefan Hajnoczi
2014-06-04 18:02     ` Paolo Bonzini
2014-06-05  8:12       ` Stefan Hajnoczi
2014-06-05  9:27         ` Paolo Bonzini
2014-06-05 13:32           ` Stefan Hajnoczi
2014-06-05 18:18     ` Max Reitz
2014-06-06  7:44       ` Paolo Bonzini
2014-06-07 19:27         ` Max Reitz
2014-06-09 13:35           ` Stefan Hajnoczi
     [not found] ` <1401561792-13410-5-git-send-email-mreitz@redhat.com>
2014-06-04 12:41   ` [Qemu-devel] [RFC 4/5] block: Add AIO followers Stefan Hajnoczi
2014-06-05 17:31     ` Max Reitz
     [not found] ` <538A3A8F.3060508@redhat.com>
2014-06-04 12:47   ` [Qemu-devel] [RFC 0/5] nbd: Adapt for dataplane Stefan Hajnoczi
2014-06-04 12:50 ` 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=5390ABB3.9050509@redhat.com \
    --to=mreitz@redhat.com \
    --cc=famz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --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).