Patchwork [V6,RESEND,03/15] ext4: add a function which sets up a new group desc

login
register
mail settings
Submitter Yongqiang Yang
Date Dec. 30, 2011, 11 a.m.
Message ID <1325242812-27005-4-git-send-email-xiaoqiangnk@gmail.com>
Download mbox | patch
Permalink /patch/133663/
State Superseded
Headers show

Comments

Yongqiang Yang - Dec. 30, 2011, 11 a.m.
From: Yongqiang Yang <xiaoqiangnk@gmail.com>

This patch adds a function named ext4_setup_new_desc() which sets
up a new group descriptor and whose code is copied from ext4_group_add().

The function will be used by new resize implementation.

Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/resize.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 55 insertions(+), 0 deletions(-)
Theodore Ts'o - Jan. 4, 2012, 2:24 a.m.
On Fri, Dec 30, 2011 at 07:00:00PM +0800, Yongqiang Yang wrote:
> +
> +	memset(gdp, 0, EXT4_DESC_SIZE(sb));
> +	 /* LV FIXME */
> +	memset(gdp, 0, EXT4_DESC_SIZE(sb));
> +	ext4_block_bitmap_set(sb, gdp, input->block_bitmap); /* LV FIXME */
> +	ext4_inode_bitmap_set(sb, gdp, input->inode_bitmap); /* LV FIXME */
> +	ext4_inode_table_set(sb, gdp, input->inode_table); /* LV FIXME */

There is a duplicated (and unneeded) call to memset above.  Also, the
LV FIXME was a comment introduced by Laurent Vivier in commit bd81d8ee
because at the time input->block_bitmap was a 32-bit value.  In the
new ext4_new_group_data structure, input->block_bitmap is now a 64-bit
field, so the need for "LV FIXME" is gone, and should be removed.

I'll remove the duplicate memset() calls and the "LV FIXME".

For future reference, this is why it's better to use something
descriptive about the FIXME "64-bit FIXME", as opposed to your
initials.  And it's best if there's an explicit comment explaining why
the fixme is there (and what the consequences of leaving partially
broken code in the kernel :-), so that future code maintainers won't
have to do code archeology to figure out what the FIXME is all about.

     	   		      	     	 - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yongqiang Yang - Jan. 4, 2012, 4:02 a.m.
On Wed, Jan 4, 2012 at 10:24 AM, Ted Ts'o <tytso@mit.edu> wrote:
> On Fri, Dec 30, 2011 at 07:00:00PM +0800, Yongqiang Yang wrote:
>> +
>> +     memset(gdp, 0, EXT4_DESC_SIZE(sb));
>> +      /* LV FIXME */
>> +     memset(gdp, 0, EXT4_DESC_SIZE(sb));
>> +     ext4_block_bitmap_set(sb, gdp, input->block_bitmap); /* LV FIXME */
>> +     ext4_inode_bitmap_set(sb, gdp, input->inode_bitmap); /* LV FIXME */
>> +     ext4_inode_table_set(sb, gdp, input->inode_table); /* LV FIXME */
>
> There is a duplicated (and unneeded) call to memset above.  Also, the
> LV FIXME was a comment introduced by Laurent Vivier in commit bd81d8ee
> because at the time input->block_bitmap was a 32-bit value.  In the
> new ext4_new_group_data structure, input->block_bitmap is now a 64-bit
> field, so the need for "LV FIXME" is gone, and should be removed.
>
> I'll remove the duplicate memset() calls and the "LV FIXME".
>
> For future reference, this is why it's better to use something
> descriptive about the FIXME "64-bit FIXME", as opposed to your
> initials.  And it's best if there's an explicit comment explaining why
> the fixme is there (and what the consequences of leaving partially
> broken code in the kernel :-), so that future code maintainers won't
> have to do code archeology to figure out what the FIXME is all about.
Thanks for your explanation.  I did not figure out what 'LV FIXME'
means, so I left them.   Now I am understood.

Yongqiang.
>
>                                         - Ted

Patch

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 9a5486e..a50eab3 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -777,6 +777,61 @@  out:
 	return err;
 }
 
+/*
+ * ext4_setup_new_desc() sets up group descriptors specified by @input.
+ *
+ * @handle: journal handle
+ * @sb: super block
+ */
+static int ext4_setup_new_desc(handle_t *handle, struct super_block *sb,
+			       struct ext4_new_group_data *input)
+{
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+	ext4_group_t group;
+	struct ext4_group_desc *gdp;
+	struct buffer_head *gdb_bh;
+	int gdb_off, gdb_num, err = 0;
+
+	group = input->group;
+
+	gdb_off = group % EXT4_DESC_PER_BLOCK(sb);
+	gdb_num = group / EXT4_DESC_PER_BLOCK(sb);
+
+	/*
+	 * get_write_access() has been called on gdb_bh by ext4_add_new_desc().
+	 */
+	gdb_bh = sbi->s_group_desc[gdb_num];
+	/* Update group descriptor block for new group */
+	gdp = (struct ext4_group_desc *)((char *)gdb_bh->b_data +
+				 gdb_off * EXT4_DESC_SIZE(sb));
+
+	memset(gdp, 0, EXT4_DESC_SIZE(sb));
+	 /* LV FIXME */
+	memset(gdp, 0, EXT4_DESC_SIZE(sb));
+	ext4_block_bitmap_set(sb, gdp, input->block_bitmap); /* LV FIXME */
+	ext4_inode_bitmap_set(sb, gdp, input->inode_bitmap); /* LV FIXME */
+	ext4_inode_table_set(sb, gdp, input->inode_table); /* LV FIXME */
+	ext4_free_group_clusters_set(sb, gdp,
+				     EXT4_B2C(sbi, input->free_blocks_count));
+	ext4_free_inodes_set(sb, gdp, EXT4_INODES_PER_GROUP(sb));
+	gdp->bg_flags = cpu_to_le16(EXT4_BG_INODE_ZEROED);
+	gdp->bg_checksum = ext4_group_desc_csum(sbi, input->group, gdp);
+
+	err = ext4_handle_dirty_metadata(handle, NULL, gdb_bh);
+	if (unlikely(err)) {
+		ext4_std_error(sb, err);
+		return err;
+	}
+
+	/*
+	 * We can allocate memory for mb_alloc based on the new group
+	 * descriptor
+	 */
+	err = ext4_mb_add_groupinfo(sb, group, gdp);
+
+	return err;
+}
+
 /* Add group descriptor data to an existing or new group descriptor block.
  * Ensure we handle all possible error conditions _before_ we start modifying
  * the filesystem, because we cannot abort the transaction and not have it