linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chris Mason <chris.mason@oracle.com>
To: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-ext4@vger.kernel.org
Cc: jack@suse.cz, jbacik@redhat.com
Subject: Fun with fdatasync()
Date: Mon, 12 Oct 2009 10:00:49 -0400	[thread overview]
Message-ID: <20091012140049.GO2632@think> (raw)


Hello everyone,

Josef has been doing some benchmarking around rpm performance on the
filesystem and noticed that ext3 was going really fast on the
fdatasyncs.

It seems pretty surprising to me that rpm -Uvh should do fdatasync
without forcing fsyncs.  The files get overwritten, and any time we mark
an inode dirty I_DIRTY_DATASYNC is getting set.

Handling of I_DIRTY_DATASYNC seems to work like this:

mark_inode_dirty() will set I_DIRTY_DATASYNC

ext3_sync_file will force a full commit on I_DIRTY_DATASYNC

This part makes good sense.  If the inode has changed, we're supposed to
do a full commit.

writeback_single_inode is where things seem to go wrong:

        /* Set I_SYNC, reset I_DIRTY */
        dirty = inode->i_state & I_DIRTY;
        inode->i_state |= I_SYNC;
        inode->i_state &= ~I_DIRTY;

Whoops, we just lost I_DIRTY_DATASYNC.  So, if pdflush comes in and does some
writeback before we fdatasync, we'll skip the full commit because
I_DIRTY_DATASYNC is gone.

The solution to me seems to be that we need to keep I_DIRTY_DATASYNC
until the FS does an fsync/O_SYNC operation, and make the FS
responsible for clearing it.

This does risk extra full fsyncs if the FS does a transaction commit on
its own, but the FS should be responsible for keeping track of which
transaction last changed a given file and doing a shortcut in the fsync
code if the file is already on safely on disk.

Am I missing something?  I don't see how fdatasync is safe in our
current usage.

-chris

             reply	other threads:[~2009-10-12 14:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-12 14:00 Chris Mason [this message]
2009-10-12 22:00 ` Fun with fdatasync() Jan Kara
2009-10-13 16:00   ` Chris Mason
2009-10-13 19:23     ` Jan Kara
2009-10-13 20:01       ` Chris Mason
2009-10-13 20:18         ` Jan Kara

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=20091012140049.GO2632@think \
    --to=chris.mason@oracle.com \
    --cc=jack@suse.cz \
    --cc=jbacik@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.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 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).