All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcelo Tosatti <marcelo@kvack.org>
To: Jamie Lokier <jamie@shareable.org>, Paul Brook <paul@codesourcery.com>
Cc: kvm-devel <kvm-devel@lists.sourceforge.net>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] QEMU: fsync AIO writes on flush request
Date: Fri, 28 Mar 2008 15:36:28 -0300	[thread overview]
Message-ID: <20080328183628.GB19547@dmt> (raw)
In-Reply-To: <20080328180324.GA22555@shareable.org>


On Fri, Mar 28, 2008 at 06:03:25PM +0000, Jamie Lokier wrote:
> 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 is enough, fdatasync() becomes redundant.

> 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?

I don't think the first qemu_aio_flush() is necessary because the fsync
request will be enqueued after pending ones: 

       aio_fsync()  function  does a sync on all outstanding asynchronous
       I/O operations associated with aiocbp->aio_fildes.

       More precisely, if op is O_SYNC, then all currently queued  I/O  opera-
       tions  shall  be  completed  as  if by a call of fsync(2), and if op is
       O_DSYNC, this call is the asynchronous analog  of  fdatasync(2).   Note
       that  this  is a request only — this call does not wait for I/O comple-
       tion.

glibc sets the priority for fsync as 0, which is the same priority AIO
reads and writes are submitted by QEMU.


-------------------------------------------------------------------------
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
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

WARNING: multiple messages have this Message-ID (diff)
From: Marcelo Tosatti <marcelo@kvack.org>
To: Jamie Lokier <jamie@shareable.org>, Paul Brook <paul@codesourcery.com>
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 15:36:28 -0300	[thread overview]
Message-ID: <20080328183628.GB19547@dmt> (raw)
In-Reply-To: <20080328180324.GA22555@shareable.org>


On Fri, Mar 28, 2008 at 06:03:25PM +0000, Jamie Lokier wrote:
> 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 is enough, fdatasync() becomes redundant.

> 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?

I don't think the first qemu_aio_flush() is necessary because the fsync
request will be enqueued after pending ones: 

       aio_fsync()  function  does a sync on all outstanding asynchronous
       I/O operations associated with aiocbp->aio_fildes.

       More precisely, if op is O_SYNC, then all currently queued  I/O  opera-
       tions  shall  be  completed  as  if by a call of fsync(2), and if op is
       O_DSYNC, this call is the asynchronous analog  of  fdatasync(2).   Note
       that  this  is a request only — this call does not wait for I/O comple-
       tion.

glibc sets the priority for fsync as 0, which is the same priority AIO
reads and writes are submitted by QEMU.

  reply	other threads:[~2008-03-28 18:36 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
2008-03-28 18:03       ` [kvm-devel] " Jamie Lokier
2008-03-28 18:36       ` Marcelo Tosatti [this message]
2008-03-28 18:36         ` 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=20080328183628.GB19547@dmt \
    --to=marcelo@kvack.org \
    --cc=jamie@shareable.org \
    --cc=kvm-devel@lists.sourceforge.net \
    --cc=paul@codesourcery.com \
    --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.