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:	Thu, 17 Sep 2009 11:04:15 -0600
From:	Andreas Dilger <adilger@....com>
To:	"Theodore Ts'o" <tytso@....edu>
Cc:	Ext4 Developers List <linux-ext4@...r.kernel.org>,
	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Subject: Re: [PATCH] ext4: store EXT4_EXT_MIGRATE in i_state instead of	i_flags

On Sep 17, 2009  09:25 -0400, Theodore Ts'o wrote:
> EXT4_EXT_MIGRATE is only intended to be used for an in-memory flag,
> and the hex value assigned to it collides with FS_DIRECTIO_FL (which
> is also stored in i_flags).  There's no reason for the
> EXT4_EXT_MIGRATE bit to be stored in i_flags, so we switch it to use
> i_state instead.
> 
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
> Signed-off-by: "Theodore Ts'o" <tytso@....edu>

Reviewed-by: Andreas Dilger <adilger@....com>

> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index b558de8..98cf2b3 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -268,7 +268,6 @@ struct flex_groups {
>  #define EXT4_TOPDIR_FL			0x00020000 /* Top of directory hierarchies*/
>  #define EXT4_HUGE_FILE_FL               0x00040000 /* Set to each huge file */
>  #define EXT4_EXTENTS_FL			0x00080000 /* Inode uses extents */
> -#define EXT4_EXT_MIGRATE		0x00100000 /* Inode is migrating */

You might consider adding in an "EXT4_DIRECTIO_FL" here as a placeholder,
to ensure it is not used for some other conflicting purpose.

>  #define EXT4_RESERVED_FL		0x80000000 /* reserved for ext4 lib */
>  
>  #define EXT4_FL_USER_VISIBLE		0x000BDFFF /* User visible flags */
> @@ -306,6 +305,7 @@ static inline __u32 ext4_mask_flags(umode_t mode, __u32 flags)
>  #define EXT4_STATE_XATTR		0x00000004 /* has in-inode xattrs */
>  #define EXT4_STATE_NO_EXPAND		0x00000008 /* No space for expansion */
>  #define EXT4_STATE_DA_ALLOC_CLOSE	0x00000010 /* Alloc DA blks on close */
> +#define EXT4_STATE_EXT_MIGRATE		0x00000020 /* Inode is migrating */
>  
>  /* Used to pass group descriptor data when online resize is done */
>  struct ext4_new_group_input {
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 5a89792..a5b4ce4 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1255,8 +1255,7 @@ int ext4_get_blocks(handle_t *handle, struct inode *inode, sector_t block,
>  			 * i_data's format changing.  Force the migrate
>  			 * to fail by clearing migrate flags
>  			 */
> -			EXT4_I(inode)->i_flags = EXT4_I(inode)->i_flags &
> -							~EXT4_EXT_MIGRATE;
> +			EXT4_I(inode)->i_state &= ~EXT4_STATE_EXT_MIGRATE;
>  		}
>  	}
>  
> @@ -4596,8 +4595,7 @@ static int ext4_do_update_inode(handle_t *handle,
>  	if (ext4_inode_blocks_set(handle, raw_inode, ei))
>  		goto out_brelse;
>  	raw_inode->i_dtime = cpu_to_le32(ei->i_dtime);
> -	/* clear the migrate flag in the raw_inode */
> -	raw_inode->i_flags = cpu_to_le32(ei->i_flags & ~EXT4_EXT_MIGRATE);
> +	raw_inode->i_flags = cpu_to_le32(ei->i_flags);
>  	if (EXT4_SB(inode->i_sb)->s_es->s_creator_os !=
>  	    cpu_to_le32(EXT4_OS_HURD))
>  		raw_inode->i_file_acl_high =
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index 05361ad..bf519f2 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -353,17 +353,16 @@ static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
>  
>  	down_write(&EXT4_I(inode)->i_data_sem);
>  	/*
> -	 * if EXT4_EXT_MIGRATE is cleared a block allocation
> +	 * if EXT4_STATE_EXT_MIGRATE is cleared a block allocation
>  	 * happened after we started the migrate. We need to
>  	 * fail the migrate
>  	 */
> -	if (!(EXT4_I(inode)->i_flags & EXT4_EXT_MIGRATE)) {
> +	if (!(EXT4_I(inode)->i_state & EXT4_STATE_EXT_MIGRATE)) {
>  		retval = -EAGAIN;
>  		up_write(&EXT4_I(inode)->i_data_sem);
>  		goto err_out;
>  	} else
> -		EXT4_I(inode)->i_flags = EXT4_I(inode)->i_flags &
> -							~EXT4_EXT_MIGRATE;
> +		EXT4_I(inode)->i_state &= ~EXT4_STATE_EXT_MIGRATE;
>  	/*
>  	 * We have the extent map build with the tmp inode.
>  	 * Now copy the i_data across
> @@ -517,14 +516,15 @@ int ext4_ext_migrate(struct inode *inode)
>  	 * when we add extents we extent the journal
>  	 */
>  	/*
> -	 * Even though we take i_mutex we can still cause block allocation
> -	 * via mmap write to holes. If we have allocated new blocks we fail
> -	 * migrate.  New block allocation will clear EXT4_EXT_MIGRATE flag.
> -	 * The flag is updated with i_data_sem held to prevent racing with
> -	 * block allocation.
> +	 * Even though we take i_mutex we can still cause block
> +	 * allocation via mmap write to holes. If we have allocated
> +	 * new blocks we fail migrate.  New block allocation will
> +	 * clear EXT4_STATE_EXT_MIGRATE flag.  The flag is updated
> +	 * with i_data_sem held to prevent racing with block
> +	 * allocation.
>  	 */
>  	down_read((&EXT4_I(inode)->i_data_sem));
> -	EXT4_I(inode)->i_flags = EXT4_I(inode)->i_flags | EXT4_EXT_MIGRATE;
> +	EXT4_I(inode)->i_state |= EXT4_STATE_EXT_MIGRATE;
>  	up_read((&EXT4_I(inode)->i_data_sem));
>  
>  	handle = ext4_journal_start(inode, 1);
> -- 
> 1.6.3.2.1.gb9f7d.dirty
> 
> --
> 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

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

--
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ