linux-ext4 - Re: [PATCH 04/12] ext4: Disable merging of uninitialized extents

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

Date:	Tue, 29 Jan 2013 11:41:13 +0400
From:	Dmitry Monakhov <dmonakhov@...nvz.org>
To:	Theodore Ts'o <tytso@....edu>
Cc:	Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org
Subject: Re: [PATCH 04/12] ext4: Disable merging of uninitialized extents

On Mon, 28 Jan 2013 10:38:36 -0500, Theodore Ts'o <tytso@....edu> wrote:
> On Mon, Jan 28, 2013 at 07:02:55PM +0400, Dmitry Monakhov wrote:
> > Actually this patch consists of two peaces
> > 1) disable merging of uninitialized extents. (1 line change) I'm
> > absolutely agree with it.
> 
> To be clear, that's this patch chunk (one line change not including
> comments :-), right?
Off course.
> 
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -1579,11 +1576,13 @@ int
>  ext4_can_extents_be_merged(struct inode *inode, struct ext4_extent *ex1,
>  				struct ext4_extent *ex2)
>  {
>  
>  	/*
> -	 * Make sure that either both extents are uninitialized, or
> -	 * both are _not_.
> +	 * Make sure that both extents are initialized. We don't merge
> +	 * uninitialized extents so that we can be sure that end_io code has
> +	 * the extent that was written properly split out and conversion to
> +	 * initialized is trivial.
>  	 */
> -	if (ext4_ext_is_uninitialized(ex1) ^ ext4_ext_is_uninitialized(ex2))
> +	if (ext4_ext_is_uninitialized(ex1) || ext4_ext_is_uninitialized(ex2))
>  		return 0;
>  
> 
> The one thing I'm a bit worried about is how much worse will extent
> fragmentation be once we do this, but it's clear we need to strive for
> correctness first.
This change should not affect fragmentation because
1) Most people call fallocate(2) on big chunks (>4M)
2) Once uninitialized extent filled with data and converted to
initialized extents  will be merged immediately.
3) Most people use fallocate(2) for preallocation before write(2)
   so effectively calls are interleaved so merging works as expected.

The only case where fragmentation will increase is when someone
performs many fallocate(2) calls for small chunks (4k) w/o writes.
As result leaf block will consist of 256 extents 4k each.
Later writes can't help us because we can not merge extents from two
leaf blocks. But I still think that this use case it inconvenient.

BTW why do we not try to merge extents from two leaf blocs?
I do not see any technical difficulties. If two adjacent leaf blocks
are covered by common index block merging is possible (but we need +1
journal block).  
> 
> 							- 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
--
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