Re: [PATCH 06/13] ext4: factor out ext4_inode_info_init()

[Jan Kara <jack@suse.cz>]

On Tue 30-08-22 20:04:04, Jason Yan wrote:
> Factor out ext4_inode_info_init(). No functional change.
> 
> Signed-off-by: Jason Yan <yanaijie@huawei.com>

Looks good. Feel free to add:

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

								Honza

> ---
>  fs/ext4/super.c | 137 ++++++++++++++++++++++++++----------------------
>  1 file changed, 75 insertions(+), 62 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index d355eda2f184..458d146149b2 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4448,6 +4448,79 @@ static void ext4_fast_commit_init(struct super_block *sb)
>  	sbi->s_fc_replay_state.fc_modified_inodes_used = 0;
>  }
>  
> +static int ext4_inode_info_init(struct super_block *sb,
> +				struct ext4_super_block *es,
> +				int blocksize)
> +{
> +	struct ext4_sb_info *sbi = EXT4_SB(sb);
> +
> +	if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV) {
> +		sbi->s_inode_size = EXT4_GOOD_OLD_INODE_SIZE;
> +		sbi->s_first_ino = EXT4_GOOD_OLD_FIRST_INO;
> +	} else {
> +		sbi->s_inode_size = le16_to_cpu(es->s_inode_size);
> +		sbi->s_first_ino = le32_to_cpu(es->s_first_ino);
> +		if (sbi->s_first_ino < EXT4_GOOD_OLD_FIRST_INO) {
> +			ext4_msg(sb, KERN_ERR, "invalid first ino: %u",
> +				 sbi->s_first_ino);
> +			return -EINVAL;
> +		}
> +		if ((sbi->s_inode_size < EXT4_GOOD_OLD_INODE_SIZE) ||
> +		    (!is_power_of_2(sbi->s_inode_size)) ||
> +		    (sbi->s_inode_size > blocksize)) {
> +			ext4_msg(sb, KERN_ERR,
> +			       "unsupported inode size: %d",
> +			       sbi->s_inode_size);
> +			ext4_msg(sb, KERN_ERR, "blocksize: %d", blocksize);
> +			return -EINVAL;
> +		}
> +		/*
> +		 * i_atime_extra is the last extra field available for
> +		 * [acm]times in struct ext4_inode. Checking for that
> +		 * field should suffice to ensure we have extra space
> +		 * for all three.
> +		 */
> +		if (sbi->s_inode_size >= offsetof(struct ext4_inode, i_atime_extra) +
> +			sizeof(((struct ext4_inode *)0)->i_atime_extra)) {
> +			sb->s_time_gran = 1;
> +			sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX;
> +		} else {
> +			sb->s_time_gran = NSEC_PER_SEC;
> +			sb->s_time_max = EXT4_NON_EXTRA_TIMESTAMP_MAX;
> +		}
> +		sb->s_time_min = EXT4_TIMESTAMP_MIN;
> +	}
> +
> +	if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE) {
> +		sbi->s_want_extra_isize = sizeof(struct ext4_inode) -
> +			EXT4_GOOD_OLD_INODE_SIZE;
> +		if (ext4_has_feature_extra_isize(sb)) {
> +			unsigned v, max = (sbi->s_inode_size -
> +					   EXT4_GOOD_OLD_INODE_SIZE);
> +
> +			v = le16_to_cpu(es->s_want_extra_isize);
> +			if (v > max) {
> +				ext4_msg(sb, KERN_ERR,
> +					 "bad s_want_extra_isize: %d", v);
> +				return -EINVAL;
> +			}
> +			if (sbi->s_want_extra_isize < v)
> +				sbi->s_want_extra_isize = v;
> +
> +			v = le16_to_cpu(es->s_min_extra_isize);
> +			if (v > max) {
> +				ext4_msg(sb, KERN_ERR,
> +					 "bad s_min_extra_isize: %d", v);
> +				return -EINVAL;
> +			}
> +			if (sbi->s_want_extra_isize < v)
> +				sbi->s_want_extra_isize = v;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  {
>  	struct buffer_head *bh, **group_desc;
> @@ -4590,68 +4663,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  	if (blocksize == PAGE_SIZE)
>  		set_opt(sb, DIOREAD_NOLOCK);
>  
> -	if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV) {
> -		sbi->s_inode_size = EXT4_GOOD_OLD_INODE_SIZE;
> -		sbi->s_first_ino = EXT4_GOOD_OLD_FIRST_INO;
> -	} else {
> -		sbi->s_inode_size = le16_to_cpu(es->s_inode_size);
> -		sbi->s_first_ino = le32_to_cpu(es->s_first_ino);
> -		if (sbi->s_first_ino < EXT4_GOOD_OLD_FIRST_INO) {
> -			ext4_msg(sb, KERN_ERR, "invalid first ino: %u",
> -				 sbi->s_first_ino);
> -			goto failed_mount;
> -		}
> -		if ((sbi->s_inode_size < EXT4_GOOD_OLD_INODE_SIZE) ||
> -		    (!is_power_of_2(sbi->s_inode_size)) ||
> -		    (sbi->s_inode_size > blocksize)) {
> -			ext4_msg(sb, KERN_ERR,
> -			       "unsupported inode size: %d",
> -			       sbi->s_inode_size);
> -			ext4_msg(sb, KERN_ERR, "blocksize: %d", blocksize);
> -			goto failed_mount;
> -		}
> -		/*
> -		 * i_atime_extra is the last extra field available for
> -		 * [acm]times in struct ext4_inode. Checking for that
> -		 * field should suffice to ensure we have extra space
> -		 * for all three.
> -		 */
> -		if (sbi->s_inode_size >= offsetof(struct ext4_inode, i_atime_extra) +
> -			sizeof(((struct ext4_inode *)0)->i_atime_extra)) {
> -			sb->s_time_gran = 1;
> -			sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX;
> -		} else {
> -			sb->s_time_gran = NSEC_PER_SEC;
> -			sb->s_time_max = EXT4_NON_EXTRA_TIMESTAMP_MAX;
> -		}
> -		sb->s_time_min = EXT4_TIMESTAMP_MIN;
> -	}
> -	if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE) {
> -		sbi->s_want_extra_isize = sizeof(struct ext4_inode) -
> -			EXT4_GOOD_OLD_INODE_SIZE;
> -		if (ext4_has_feature_extra_isize(sb)) {
> -			unsigned v, max = (sbi->s_inode_size -
> -					   EXT4_GOOD_OLD_INODE_SIZE);
> -
> -			v = le16_to_cpu(es->s_want_extra_isize);
> -			if (v > max) {
> -				ext4_msg(sb, KERN_ERR,
> -					 "bad s_want_extra_isize: %d", v);
> -				goto failed_mount;
> -			}
> -			if (sbi->s_want_extra_isize < v)
> -				sbi->s_want_extra_isize = v;
> -
> -			v = le16_to_cpu(es->s_min_extra_isize);
> -			if (v > max) {
> -				ext4_msg(sb, KERN_ERR,
> -					 "bad s_min_extra_isize: %d", v);
> -				goto failed_mount;
> -			}
> -			if (sbi->s_want_extra_isize < v)
> -				sbi->s_want_extra_isize = v;
> -		}
> -	}
> +	if (ext4_inode_info_init(sb, es, blocksize))
> +		goto failed_mount;
>  
>  	err = parse_apply_sb_mount_options(sb, ctx);
>  	if (err < 0)
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR