linux-kernel - Fun with fdatasync()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20091012140049.GO2632@think>
Date:	Mon, 12 Oct 2009 10:00:49 -0400
From:	Chris Mason <chris.mason@...cle.com>
To:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-ext4@...r.kernel.org
Cc:	jack@...e.cz, jbacik@...hat.com
Subject: Fun with fdatasync()

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/