linux-ext4 - Re: [PATCH] ext4: remove filemap_fdatawrite() when synchronizing a data journaled file

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-prev] [day] [month] [year] [list]

Date:	Tue, 28 Jul 2015 08:07:13 -0400
From:	Theodore Ts'o <tytso@....edu>
To:	Daeho Jeong <daeho.jeong@...sung.com>
Cc:	linux-ext4@...r.kernel.org, daehojng@...il.com
Subject: Re: [PATCH] ext4: remove filemap_fdatawrite() when synchronizing a
 data journaled file

On Tue, Jul 28, 2015 at 02:17:26AM +0000, Daeho Jeong wrote:
> When fsync() system call is executed, all data of the file should be written on the storage.
> But, in a case of that the file is operated in data journal mode, the data doesn't need to be
> flushed out and waited for its I/O completion. Moreover, by invoking filemap_fdatawrite()
> function on the data journaled file, ext4_sync_file() suffers from an unnecessary delay
> which is caused by flushing out and waiting for the I/O completion of a ton of newly-dirtied
> pages which were dirtied by the previous commit.

No, this patch is incorrect.  Data journalling works by having the
buffers getting dirtied, and that doesn't happen until we call
filemap_fdatawrite_range().  It's true that we don't have to call
filemap_write_and_wait_range(), but simply skipping the writeback
entirely will cause data writes to get lost after an fsync() if we
crash immediately afterwards.

I'm guessing you didn't actually test this patch for correctness?

    	     	 		      	   	 - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html