All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Zhang Yi <yi.zhang@huawei.com>
Cc: linux-ext4@vger.kernel.org, tytso@mit.edu,
	adilger.kernel@dilger.ca, jack@suse.cz, yukuai3@huawei.com
Subject: Re: [PATCH v5 2/3] ext4: move ext4_fill_raw_inode() related functions
Date: Mon, 20 Sep 2021 15:57:57 +0200	[thread overview]
Message-ID: <20210920135757.GI6607@quack2.suse.cz> (raw)
In-Reply-To: <20210901020955.1657340-3-yi.zhang@huawei.com>

On Wed 01-09-21 10:09:54, Zhang Yi wrote:
> In preparation for calling ext4_fill_raw_inode() in
> __ext4_get_inode_loc(), move three related functions before
> __ext4_get_inode_loc(), no logical change.
> 
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/inode.c | 293 ++++++++++++++++++++++++------------------------
>  1 file changed, 147 insertions(+), 146 deletions(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c7186460c14d..3c36e701e30e 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4292,6 +4292,153 @@ int ext4_truncate(struct inode *inode)
>  	return err;
>  }
>  
> +static inline u64 ext4_inode_peek_iversion(const struct inode *inode)
> +{
> +	if (unlikely(EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL))
> +		return inode_peek_iversion_raw(inode);
> +	else
> +		return inode_peek_iversion(inode);
> +}
> +
> +static int ext4_inode_blocks_set(struct ext4_inode *raw_inode,
> +				 struct ext4_inode_info *ei)
> +{
> +	struct inode *inode = &(ei->vfs_inode);
> +	u64 i_blocks = READ_ONCE(inode->i_blocks);
> +	struct super_block *sb = inode->i_sb;
> +
> +	if (i_blocks <= ~0U) {
> +		/*
> +		 * i_blocks can be represented in a 32 bit variable
> +		 * as multiple of 512 bytes
> +		 */
> +		raw_inode->i_blocks_lo   = cpu_to_le32(i_blocks);
> +		raw_inode->i_blocks_high = 0;
> +		ext4_clear_inode_flag(inode, EXT4_INODE_HUGE_FILE);
> +		return 0;
> +	}
> +
> +	/*
> +	 * This should never happen since sb->s_maxbytes should not have
> +	 * allowed this, sb->s_maxbytes was set according to the huge_file
> +	 * feature in ext4_fill_super().
> +	 */
> +	if (!ext4_has_feature_huge_file(sb))
> +		return -EFSCORRUPTED;
> +
> +	if (i_blocks <= 0xffffffffffffULL) {
> +		/*
> +		 * i_blocks can be represented in a 48 bit variable
> +		 * as multiple of 512 bytes
> +		 */
> +		raw_inode->i_blocks_lo   = cpu_to_le32(i_blocks);
> +		raw_inode->i_blocks_high = cpu_to_le16(i_blocks >> 32);
> +		ext4_clear_inode_flag(inode, EXT4_INODE_HUGE_FILE);
> +	} else {
> +		ext4_set_inode_flag(inode, EXT4_INODE_HUGE_FILE);
> +		/* i_block is stored in file system block size */
> +		i_blocks = i_blocks >> (inode->i_blkbits - 9);
> +		raw_inode->i_blocks_lo   = cpu_to_le32(i_blocks);
> +		raw_inode->i_blocks_high = cpu_to_le16(i_blocks >> 32);
> +	}
> +	return 0;
> +}
> +
> +static int ext4_fill_raw_inode(struct inode *inode, struct ext4_inode *raw_inode)
> +{
> +	struct ext4_inode_info *ei = EXT4_I(inode);
> +	uid_t i_uid;
> +	gid_t i_gid;
> +	projid_t i_projid;
> +	int block;
> +	int err;
> +
> +	err = ext4_inode_blocks_set(raw_inode, ei);
> +
> +	raw_inode->i_mode = cpu_to_le16(inode->i_mode);
> +	i_uid = i_uid_read(inode);
> +	i_gid = i_gid_read(inode);
> +	i_projid = from_kprojid(&init_user_ns, ei->i_projid);
> +	if (!(test_opt(inode->i_sb, NO_UID32))) {
> +		raw_inode->i_uid_low = cpu_to_le16(low_16_bits(i_uid));
> +		raw_inode->i_gid_low = cpu_to_le16(low_16_bits(i_gid));
> +		/*
> +		 * Fix up interoperability with old kernels. Otherwise,
> +		 * old inodes get re-used with the upper 16 bits of the
> +		 * uid/gid intact.
> +		 */
> +		if (ei->i_dtime && list_empty(&ei->i_orphan)) {
> +			raw_inode->i_uid_high = 0;
> +			raw_inode->i_gid_high = 0;
> +		} else {
> +			raw_inode->i_uid_high =
> +				cpu_to_le16(high_16_bits(i_uid));
> +			raw_inode->i_gid_high =
> +				cpu_to_le16(high_16_bits(i_gid));
> +		}
> +	} else {
> +		raw_inode->i_uid_low = cpu_to_le16(fs_high2lowuid(i_uid));
> +		raw_inode->i_gid_low = cpu_to_le16(fs_high2lowgid(i_gid));
> +		raw_inode->i_uid_high = 0;
> +		raw_inode->i_gid_high = 0;
> +	}
> +	raw_inode->i_links_count = cpu_to_le16(inode->i_nlink);
> +
> +	EXT4_INODE_SET_XTIME(i_ctime, inode, raw_inode);
> +	EXT4_INODE_SET_XTIME(i_mtime, inode, raw_inode);
> +	EXT4_INODE_SET_XTIME(i_atime, inode, raw_inode);
> +	EXT4_EINODE_SET_XTIME(i_crtime, ei, raw_inode);
> +
> +	raw_inode->i_dtime = cpu_to_le32(ei->i_dtime);
> +	raw_inode->i_flags = cpu_to_le32(ei->i_flags & 0xFFFFFFFF);
> +	if (likely(!test_opt2(inode->i_sb, HURD_COMPAT)))
> +		raw_inode->i_file_acl_high =
> +			cpu_to_le16(ei->i_file_acl >> 32);
> +	raw_inode->i_file_acl_lo = cpu_to_le32(ei->i_file_acl);
> +	ext4_isize_set(raw_inode, ei->i_disksize);
> +
> +	raw_inode->i_generation = cpu_to_le32(inode->i_generation);
> +	if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
> +		if (old_valid_dev(inode->i_rdev)) {
> +			raw_inode->i_block[0] =
> +				cpu_to_le32(old_encode_dev(inode->i_rdev));
> +			raw_inode->i_block[1] = 0;
> +		} else {
> +			raw_inode->i_block[0] = 0;
> +			raw_inode->i_block[1] =
> +				cpu_to_le32(new_encode_dev(inode->i_rdev));
> +			raw_inode->i_block[2] = 0;
> +		}
> +	} else if (!ext4_has_inline_data(inode)) {
> +		for (block = 0; block < EXT4_N_BLOCKS; block++)
> +			raw_inode->i_block[block] = ei->i_data[block];
> +	}
> +
> +	if (likely(!test_opt2(inode->i_sb, HURD_COMPAT))) {
> +		u64 ivers = ext4_inode_peek_iversion(inode);
> +
> +		raw_inode->i_disk_version = cpu_to_le32(ivers);
> +		if (ei->i_extra_isize) {
> +			if (EXT4_FITS_IN_INODE(raw_inode, ei, i_version_hi))
> +				raw_inode->i_version_hi =
> +					cpu_to_le32(ivers >> 32);
> +			raw_inode->i_extra_isize =
> +				cpu_to_le16(ei->i_extra_isize);
> +		}
> +	}
> +
> +	if (i_projid != EXT4_DEF_PROJID &&
> +	    !ext4_has_feature_project(inode->i_sb))
> +		err = err ?: -EFSCORRUPTED;
> +
> +	if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
> +	    EXT4_FITS_IN_INODE(raw_inode, ei, i_projid))
> +		raw_inode->i_projid = cpu_to_le32(i_projid);
> +
> +	ext4_inode_csum_set(inode, raw_inode, ei);
> +	return err;
> +}
> +
>  /*
>   * ext4_get_inode_loc returns with an extra refcount against the inode's
>   * underlying buffer_head on success. If 'in_mem' is true, we have all
> @@ -4580,13 +4727,6 @@ static inline void ext4_inode_set_iversion_queried(struct inode *inode, u64 val)
>  	else
>  		inode_set_iversion_queried(inode, val);
>  }
> -static inline u64 ext4_inode_peek_iversion(const struct inode *inode)
> -{
> -	if (unlikely(EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL))
> -		return inode_peek_iversion_raw(inode);
> -	else
> -		return inode_peek_iversion(inode);
> -}
>  
>  struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
>  			  ext4_iget_flags flags, const char *function,
> @@ -4902,50 +5042,6 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
>  	return ERR_PTR(ret);
>  }
>  
> -static int ext4_inode_blocks_set(struct ext4_inode *raw_inode,
> -				 struct ext4_inode_info *ei)
> -{
> -	struct inode *inode = &(ei->vfs_inode);
> -	u64 i_blocks = READ_ONCE(inode->i_blocks);
> -	struct super_block *sb = inode->i_sb;
> -
> -	if (i_blocks <= ~0U) {
> -		/*
> -		 * i_blocks can be represented in a 32 bit variable
> -		 * as multiple of 512 bytes
> -		 */
> -		raw_inode->i_blocks_lo   = cpu_to_le32(i_blocks);
> -		raw_inode->i_blocks_high = 0;
> -		ext4_clear_inode_flag(inode, EXT4_INODE_HUGE_FILE);
> -		return 0;
> -	}
> -
> -	/*
> -	 * This should never happen since sb->s_maxbytes should not have
> -	 * allowed this, sb->s_maxbytes was set according to the huge_file
> -	 * feature in ext4_fill_super().
> -	 */
> -	if (!ext4_has_feature_huge_file(sb))
> -		return -EFSCORRUPTED;
> -
> -	if (i_blocks <= 0xffffffffffffULL) {
> -		/*
> -		 * i_blocks can be represented in a 48 bit variable
> -		 * as multiple of 512 bytes
> -		 */
> -		raw_inode->i_blocks_lo   = cpu_to_le32(i_blocks);
> -		raw_inode->i_blocks_high = cpu_to_le16(i_blocks >> 32);
> -		ext4_clear_inode_flag(inode, EXT4_INODE_HUGE_FILE);
> -	} else {
> -		ext4_set_inode_flag(inode, EXT4_INODE_HUGE_FILE);
> -		/* i_block is stored in file system block size */
> -		i_blocks = i_blocks >> (inode->i_blkbits - 9);
> -		raw_inode->i_blocks_lo   = cpu_to_le32(i_blocks);
> -		raw_inode->i_blocks_high = cpu_to_le16(i_blocks >> 32);
> -	}
> -	return 0;
> -}
> -
>  static void __ext4_update_other_inode_time(struct super_block *sb,
>  					   unsigned long orig_ino,
>  					   unsigned long ino,
> @@ -5006,101 +5102,6 @@ static void ext4_update_other_inodes_time(struct super_block *sb,
>  	rcu_read_unlock();
>  }
>  
> -static int ext4_fill_raw_inode(struct inode *inode, struct ext4_inode *raw_inode)
> -{
> -	struct ext4_inode_info *ei = EXT4_I(inode);
> -	uid_t i_uid;
> -	gid_t i_gid;
> -	projid_t i_projid;
> -	int block;
> -	int err;
> -
> -	err = ext4_inode_blocks_set(raw_inode, ei);
> -
> -	raw_inode->i_mode = cpu_to_le16(inode->i_mode);
> -	i_uid = i_uid_read(inode);
> -	i_gid = i_gid_read(inode);
> -	i_projid = from_kprojid(&init_user_ns, ei->i_projid);
> -	if (!(test_opt(inode->i_sb, NO_UID32))) {
> -		raw_inode->i_uid_low = cpu_to_le16(low_16_bits(i_uid));
> -		raw_inode->i_gid_low = cpu_to_le16(low_16_bits(i_gid));
> -		/*
> -		 * Fix up interoperability with old kernels. Otherwise,
> -		 * old inodes get re-used with the upper 16 bits of the
> -		 * uid/gid intact.
> -		 */
> -		if (ei->i_dtime && list_empty(&ei->i_orphan)) {
> -			raw_inode->i_uid_high = 0;
> -			raw_inode->i_gid_high = 0;
> -		} else {
> -			raw_inode->i_uid_high =
> -				cpu_to_le16(high_16_bits(i_uid));
> -			raw_inode->i_gid_high =
> -				cpu_to_le16(high_16_bits(i_gid));
> -		}
> -	} else {
> -		raw_inode->i_uid_low = cpu_to_le16(fs_high2lowuid(i_uid));
> -		raw_inode->i_gid_low = cpu_to_le16(fs_high2lowgid(i_gid));
> -		raw_inode->i_uid_high = 0;
> -		raw_inode->i_gid_high = 0;
> -	}
> -	raw_inode->i_links_count = cpu_to_le16(inode->i_nlink);
> -
> -	EXT4_INODE_SET_XTIME(i_ctime, inode, raw_inode);
> -	EXT4_INODE_SET_XTIME(i_mtime, inode, raw_inode);
> -	EXT4_INODE_SET_XTIME(i_atime, inode, raw_inode);
> -	EXT4_EINODE_SET_XTIME(i_crtime, ei, raw_inode);
> -
> -	raw_inode->i_dtime = cpu_to_le32(ei->i_dtime);
> -	raw_inode->i_flags = cpu_to_le32(ei->i_flags & 0xFFFFFFFF);
> -	if (likely(!test_opt2(inode->i_sb, HURD_COMPAT)))
> -		raw_inode->i_file_acl_high =
> -			cpu_to_le16(ei->i_file_acl >> 32);
> -	raw_inode->i_file_acl_lo = cpu_to_le32(ei->i_file_acl);
> -	ext4_isize_set(raw_inode, ei->i_disksize);
> -
> -	raw_inode->i_generation = cpu_to_le32(inode->i_generation);
> -	if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
> -		if (old_valid_dev(inode->i_rdev)) {
> -			raw_inode->i_block[0] =
> -				cpu_to_le32(old_encode_dev(inode->i_rdev));
> -			raw_inode->i_block[1] = 0;
> -		} else {
> -			raw_inode->i_block[0] = 0;
> -			raw_inode->i_block[1] =
> -				cpu_to_le32(new_encode_dev(inode->i_rdev));
> -			raw_inode->i_block[2] = 0;
> -		}
> -	} else if (!ext4_has_inline_data(inode)) {
> -		for (block = 0; block < EXT4_N_BLOCKS; block++)
> -			raw_inode->i_block[block] = ei->i_data[block];
> -	}
> -
> -	if (likely(!test_opt2(inode->i_sb, HURD_COMPAT))) {
> -		u64 ivers = ext4_inode_peek_iversion(inode);
> -
> -		raw_inode->i_disk_version = cpu_to_le32(ivers);
> -		if (ei->i_extra_isize) {
> -			if (EXT4_FITS_IN_INODE(raw_inode, ei, i_version_hi))
> -				raw_inode->i_version_hi =
> -					cpu_to_le32(ivers >> 32);
> -			raw_inode->i_extra_isize =
> -				cpu_to_le16(ei->i_extra_isize);
> -		}
> -	}
> -
> -	if (i_projid != EXT4_DEF_PROJID &&
> -	    !ext4_has_feature_project(inode->i_sb))
> -		err = err ?: -EFSCORRUPTED;
> -
> -	if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
> -	    EXT4_FITS_IN_INODE(raw_inode, ei, i_projid))
> -		raw_inode->i_projid = cpu_to_le32(i_projid);
> -
> -	ext4_inode_csum_set(inode, raw_inode, ei);
> -	return err;
> -}
> -
>  /*
>   * Post the struct inode info into an on-disk inode location in the
>   * buffer-cache.  This gobbles the caller's reference to the
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

  reply	other threads:[~2021-09-20 13:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-01  2:09 [PATCH v5 0/3] ext4: fix a inode checksum error Zhang Yi
2021-09-01  2:09 ` [PATCH v5 1/3] ext4: factor out ext4_fill_raw_inode() Zhang Yi
2021-09-20 13:57   ` Jan Kara
2021-09-01  2:09 ` [PATCH v5 2/3] ext4: move ext4_fill_raw_inode() related functions Zhang Yi
2021-09-20 13:57   ` Jan Kara [this message]
2021-09-01  2:09 ` [PATCH v5 3/3] ext4: prevent getting empty inode buffer Zhang Yi
2021-09-20 13:58   ` Jan Kara
2021-10-14 10:57 ` [PATCH v5 0/3] ext4: fix a inode checksum error Zhang Yi
2021-10-28 14:47 ` Theodore Ts'o

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210920135757.GI6607@quack2.suse.cz \
    --to=jack@suse.cz \
    --cc=adilger.kernel@dilger.ca \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    --cc=yi.zhang@huawei.com \
    --cc=yukuai3@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.