All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jamie Lokier <jamie@shareable.org>
To: Marcelo Tosatti <marcelo@kvack.org>
Cc: kvm-devel <kvm-devel@lists.sourceforge.net>,
	qemu-devel@nongnu.org, Avi Kivity <avi@qumranet.com>
Subject: Re: [Qemu-devel] [PATCH] QEMU: fsync AIO writes on flush request
Date: Fri, 28 Mar 2008 18:03:25 +0000	[thread overview]
Message-ID: <20080328180324.GA22555@shareable.org> (raw)
In-Reply-To: <20080328163116.GA18853@dmt>

Marcelo Tosatti wrote:
> On Fri, Mar 28, 2008 at 03:07:03PM +0000, Jamie Lokier wrote:
> > Marcelo Tosatti wrote:
> > > Its necessary to guarantee that pending AIO writes have reached stable
> > > storage when the flush request returns.
> > > 
> > > Also change fsync() to fdatasync(), since the modification time is not
> > > critical data.
> > > +    if (aio_fsync(O_DSYNC, &acb->aiocb) < 0) {
> > 
> > >      BDRVRawState *s = bs->opaque;
> > > -    fsync(s->fd);
> > > +    raw_aio_flush(bs);
> > > +    fdatasync(s->fd);
> > > +
> > > +    /* We rely on the fact that no other AIO will be submitted
> > > +     * in parallel, but this should be fixed by per-device
> > > +     * AIO queues when allowing multiple CPU's to process IO
> > > +     * in QEMU.
> > > +     */
> > > +    qemu_aio_flush();
> > 
> > I'm a bit confused by this.  Why do you need aio_fsync(O_DSYNC) _and_
> > synchronous fdatasync() calls?  Aren't they equivalent?
> 
> fdatasync() will write and wait for completion of dirty file data
> present in memory.
> 
> aio_write() only queues data for submission:
> 
>        The "asynchronous" means that this call returns as soon as the  request
>        has  been  enqueued;  the  write may or may not have completed when the
>        call returns. One tests for completion using aio_error(3).
> 

> So fdatasync() is not enough because data written via AIO may not
> have been reflected as "dirty file data" through write() by the time
> raw_flush() is called.

Sure.  But why isn't the aio_fsync(O_DSYNC) enough by itself?

It seems to me you should have something like this:

    /* Flush pending aio_writes until they are dirty data,
       and wait before the aio_fsync. */
    qemu_aio_flush();

    /* Call aio_fsync(O_DSYNC). */
    raw_aio_flush(bs);

    /* Wait for the aio_fsync to complete. */
    qemu_aio_flush();

What am I missing?

-- Jamie

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

WARNING: multiple messages have this Message-ID (diff)
From: Jamie Lokier <jamie@shareable.org>
To: Marcelo Tosatti <marcelo@kvack.org>
Cc: kvm-devel <kvm-devel@lists.sourceforge.net>, qemu-devel@nongnu.org
Subject: Re: [kvm-devel] [Qemu-devel] [PATCH] QEMU: fsync AIO writes on flush request
Date: Fri, 28 Mar 2008 18:03:25 +0000	[thread overview]
Message-ID: <20080328180324.GA22555@shareable.org> (raw)
In-Reply-To: <20080328163116.GA18853@dmt>

Marcelo Tosatti wrote:
> On Fri, Mar 28, 2008 at 03:07:03PM +0000, Jamie Lokier wrote:
> > Marcelo Tosatti wrote:
> > > Its necessary to guarantee that pending AIO writes have reached stable
> > > storage when the flush request returns.
> > > 
> > > Also change fsync() to fdatasync(), since the modification time is not
> > > critical data.
> > > +    if (aio_fsync(O_DSYNC, &acb->aiocb) < 0) {
> > 
> > >      BDRVRawState *s = bs->opaque;
> > > -    fsync(s->fd);
> > > +    raw_aio_flush(bs);
> > > +    fdatasync(s->fd);
> > > +
> > > +    /* We rely on the fact that no other AIO will be submitted
> > > +     * in parallel, but this should be fixed by per-device
> > > +     * AIO queues when allowing multiple CPU's to process IO
> > > +     * in QEMU.
> > > +     */
> > > +    qemu_aio_flush();
> > 
> > I'm a bit confused by this.  Why do you need aio_fsync(O_DSYNC) _and_
> > synchronous fdatasync() calls?  Aren't they equivalent?
> 
> fdatasync() will write and wait for completion of dirty file data
> present in memory.
> 
> aio_write() only queues data for submission:
> 
>        The "asynchronous" means that this call returns as soon as the  request
>        has  been  enqueued;  the  write may or may not have completed when the
>        call returns. One tests for completion using aio_error(3).
> 

> So fdatasync() is not enough because data written via AIO may not
> have been reflected as "dirty file data" through write() by the time
> raw_flush() is called.

Sure.  But why isn't the aio_fsync(O_DSYNC) enough by itself?

It seems to me you should have something like this:

    /* Flush pending aio_writes until they are dirty data,
       and wait before the aio_fsync. */
    qemu_aio_flush();

    /* Call aio_fsync(O_DSYNC). */
    raw_aio_flush(bs);

    /* Wait for the aio_fsync to complete. */
    qemu_aio_flush();

What am I missing?

-- Jamie

  parent reply	other threads:[~2008-03-28 18:03 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-28 15:05 [PATCH] QEMU: fsync AIO writes on flush request Marcelo Tosatti
2008-03-28 15:05 ` [Qemu-devel] " Marcelo Tosatti
2008-03-28 15:07 ` Jamie Lokier
2008-03-28 15:07   ` Jamie Lokier
2008-03-28 16:31   ` Marcelo Tosatti
2008-03-28 16:31     ` [kvm-devel] " Marcelo Tosatti
2008-03-28 16:40     ` Paul Brook
2008-03-28 16:40       ` [kvm-devel] " Paul Brook
2008-03-28 16:59       ` Marcelo Tosatti
2008-03-28 16:59         ` [kvm-devel] " Marcelo Tosatti
2008-03-28 17:00         ` Paul Brook
2008-03-28 17:00           ` [kvm-devel] " Paul Brook
2008-03-28 18:13           ` Marcelo Tosatti
2008-03-28 18:13             ` [kvm-devel] " Marcelo Tosatti
2008-03-29  1:17             ` Jamie Lokier
2008-03-29  1:17               ` [kvm-devel] " Jamie Lokier
2008-03-29  2:02               ` Paul Brook
2008-03-29  2:02                 ` [kvm-devel] " Paul Brook
2008-03-29  2:11                 ` Jamie Lokier
2008-03-29  2:11                   ` [kvm-devel] " Jamie Lokier
2008-03-29  2:43                   ` Paul Brook
2008-03-29  2:43                     ` [kvm-devel] " Paul Brook
2008-03-28 18:03     ` Jamie Lokier [this message]
2008-03-28 18:03       ` Jamie Lokier
2008-03-28 18:36       ` Marcelo Tosatti
2008-03-28 18:36         ` [kvm-devel] " Marcelo Tosatti
2008-03-29  1:09         ` Jamie Lokier
2008-03-29  1:09           ` [kvm-devel] " Jamie Lokier
2008-03-29  6:49           ` Marcelo Tosatti
2008-03-29  6:49             ` [kvm-devel] " Marcelo Tosatti
2008-03-28 17:25 ` Ian Jackson
2008-03-28 17:25   ` Ian Jackson
2008-03-28 19:11   ` [kvm-devel] " Marcelo Tosatti

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=20080328180324.GA22555@shareable.org \
    --to=jamie@shareable.org \
    --cc=avi@qumranet.com \
    --cc=kvm-devel@lists.sourceforge.net \
    --cc=marcelo@kvack.org \
    --cc=qemu-devel@nongnu.org \
    /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.