linux-ext4 - Re: [PATCH] ext4: restart ext4_ext_remove

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87wrw03pr5.fsf@openvz.org>
Date:	Thu, 22 Apr 2010 11:33:50 +0400
From:	Dmitry Monakhov <dmonakhov@...nvz.org>
To:	linux-ext4@...r.kernel.org
Cc:	jack@...e.cz, aneesh.kumar@...ux.vnet.ibm.com, tytso@....edu
Subject: Re: [PATCH] ext4: restart ext4_ext_remove_space() after transaction restart

Dmitry Monakhov <dmonakhov@...nvz.org> writes:

Oops. i've missed last 'u' in 'edu', the Ted's email.
Add him to cc.
BTW seems that there is only one reproducible bug is left after
227's xfstest case. The bug with wrong i_blocks count:
Pass 1: Checking inodes, blocks, and sizes
Inode 3855, i_blocks is 288, should be 304.  Fix? no
This will be the hardest one because i'm able to discover wrong blocks
count only on fsck stage. I'm suspecting what quota charge is missed
somewhere for metablocks. Continue digging. 
> If i_data_sem was internally dropped due to transaction restart, it is
> necessary to restart path look-up because extents tree was possibly
> modified by ext4_get_block().
>
> https://bugzilla.kernel.org/show_bug.cgi?id=15827
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@...nvz.org>
> ---
>  fs/ext4/ext4.h    |    1 +
>  fs/ext4/extents.c |   18 +++++++++++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 6641c58..c69efb2 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1089,6 +1089,7 @@ enum {
>  	EXT4_STATE_DA_ALLOC_CLOSE,	/* Alloc DA blks on close */
>  	EXT4_STATE_EXT_MIGRATE,		/* Inode is migrating */
>  	EXT4_STATE_DIO_UNWRITTEN,	/* need convert on dio done*/
> +	EXT4_STATE_EXT_TRUNC,		/* truncate is in progress, modified under i_data_sem */
>  };
>  
>  #define EXT4_INODE_BIT_FNS(name, field)					\
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 4fa103c..6856272 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -107,11 +107,8 @@ static int ext4_ext_truncate_extend_restart(handle_t *handle,
>  	if (err <= 0)
>  		return err;
>  	err = ext4_truncate_restart_trans(handle, inode, needed);
> -	/*
> -	 * We have dropped i_data_sem so someone might have cached again
> -	 * an extent we are going to truncate.
> -	 */
> -	ext4_ext_invalidate_cache(inode);
> +	if (!err && !ext4_test_inode_state(inode, EXT4_STATE_EXT_TRUNC))
> +		err = -EAGAIN;
>  
>  	return err;
>  }
> @@ -2370,12 +2367,15 @@ static int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start)
>  	if (IS_ERR(handle))
>  		return PTR_ERR(handle);
>  
> +again:
>  	ext4_ext_invalidate_cache(inode);
>  
>  	/*
>  	 * We start scanning from right side, freeing all the blocks
>  	 * after i_size and walking into the tree depth-wise.
>  	 */
> +	ext4_set_inode_state(inode, EXT4_STATE_EXT_TRUNC);
> +	depth = ext_depth(inode);
>  	path = kzalloc(sizeof(struct ext4_ext_path) * (depth + 1), GFP_NOFS);
>  	if (path == NULL) {
>  		ext4_journal_stop(handle);
> @@ -2480,6 +2480,11 @@ static int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start)
>  out:
>  	ext4_ext_drop_refs(path);
>  	kfree(path);
> +	if (err == EAGAIN) {
> +		err = 0;
> +		goto again;
> +	}
> +	ext4_clear_inode_state(inode, EXT4_STATE_EXT_TRUNC);
>  	ext4_journal_stop(handle);
>  
>  	return err;
> @@ -3338,6 +3343,9 @@ int ext4_ext_get_blocks(handle_t *handle, struct inode *inode,
>  	ext_debug("blocks %u/%u requested for inode %lu\n",
>  			iblock, max_blocks, inode->i_ino);
>  
> +	if (unlikely((flags & EXT4_GET_BLOCKS_CREATE)) &&
> +		ext4_test_inode_state(inode, EXT4_STATE_EXT_TRUNC))
> +		ext4_clear_inode_state(inode, EXT4_STATE_EXT_TRUNC);
>  	/* check in cache */
>  	cache_type = ext4_ext_in_cache(inode, iblock, &newex);
>  	if (cache_type) {

>  LocalWords:  xfstest
--
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