Re: [PATCH 11/12] ext4: simplify parameters of add_new_gdb()
From: Andreas Dilger <hidden>
Date: 2011-07-18 06:46:03
On 2011-07-17, at 8:52 PM, Yongqiang Yang wrote:
add_new_gdb() only needs the no. of a group, there is no need to pass a pointer to struct ext4_new_group_data to add_new_gdb(). static int add_new_gdb(handle_t *handle, struct inode *inode, - struct ext4_new_group_data *input, - struct buffer_head **primary) + ext4_group_t group)
This patch is changing the code in a subtle, but not incorrect way. At a minimum, the commit comment should also mention why "primary" can be removed as an argument. That is because add_new_gdb() is storing the bh pointer for the new group descriptor block into s_group_desc[] internally:
quoted hunk ↗ jump to hunk
@@ -513,7 +514,7 @@ static int add_new_gdb(handle_t *handle, struct inode *inode,o_group_desc = EXT4_SB(sb)->s_group_desc; memcpy(n_group_desc, o_group_desc, EXT4_SB(sb)->s_gdb_count * sizeof(struct buffer_head *)); - n_group_desc[gdb_num] = *primary; + n_group_desc[gdb_num] = gdb_bh;
Here...
quoted hunk ↗ jump to hunk
@@ -843,8 +844,12 @@ int ext4_group_add(struct super_block *sb, struct ext4_new_group_data *input)if (reserved_gdb && ext4_bg_num_gdb(sb, input->group) && (err = reserve_backup_gdb(handle, inode, input))) goto exit_journal; - } else if ((err = add_new_gdb(handle, inode, input, &primary))) - goto exit_journal; + } else { + err = add_new_gdb(handle, inode, input->group); + if (err) + goto exit_journal; + primary = sbi->s_group_desc[gdb_num];
and the caller can safely access this after the function returns. It would be incorrect to "optimize" this code to consolidate the setting of "primary" from sbi->s_group_desc[gdb_num] before the if/else clause, so this should get a comment that this array element is only valid after add_new_gdb() returns. Cheers, Andreas