Patchwork [V6,RESEND,01/15] ext4: add a function which extends a group without checking parameters

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

Comments

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

This patch added a function named ext4_group_extend_no_check() whose code
is copied from ext4_group_extend().  ext4_group_extend_no_check() assumes
the parameter is valid and has been checked by caller.

Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/resize.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 53 insertions(+), 0 deletions(-)
Theodore Ts'o - Jan. 3, 2012, 9:07 p.m.
On Fri, Dec 30, 2011 at 06:59:58PM +0800, Yongqiang Yang wrote:
> +static int ext4_group_extend_no_check(struct super_block *sb,
> +			ext4_fsblk_t o_blocks_count, ext4_grpblk_t add)

I fixed the whitespace here (nit-picky, I know)

> +	handle = ext4_journal_start_sb(sb, 3);
> +	if (IS_ERR(handle)) {
> +		err = PTR_ERR(handle);
> +		ext4_warning(sb, "error %d on journal start", err);
> +		goto out;
> +	}

There's only two calls "goto out", and out: currently is just "return
err;".  So I just changed this to be "return err", which is clearer.
I tend to place more imporance for clarity of the code than rigid
rules which say that there should only be one control flow such that
"return err" must only occur once at the end of the function.

> +	err = ext4_journal_get_write_access(handle, EXT4_SB(sb)->s_sbh);
> +	if (err) {
> +		ext4_warning(sb, "error %d on journal write access", err);
> +		ext4_journal_stop(handle);
> +		goto out;
> +	}

Instead of calling ext4_journal_stop() here, it's simpler just to jump
to the "exit_journal" label, which is consistent with what we do
everywhere else.  Now all of the error gotos are to "exit_journal",
which I've renamed to "errout".

These are minor stylistic changes, but I thought I would mention them
so that other people understand what is considered the best way to do
things, I've just made these changes myself to avoid asking you to
respin the patches.

Regards,

					- 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, 3:59 a.m.
On Wed, Jan 4, 2012 at 5:07 AM, Ted Ts'o <tytso@mit.edu> wrote:
> On Fri, Dec 30, 2011 at 06:59:58PM +0800, Yongqiang Yang wrote:
>> +static int ext4_group_extend_no_check(struct super_block *sb,
>> +                     ext4_fsblk_t o_blocks_count, ext4_grpblk_t add)
>
> I fixed the whitespace here (nit-picky, I know)
>
>> +     handle = ext4_journal_start_sb(sb, 3);
>> +     if (IS_ERR(handle)) {
>> +             err = PTR_ERR(handle);
>> +             ext4_warning(sb, "error %d on journal start", err);
>> +             goto out;
>> +     }
>
> There's only two calls "goto out", and out: currently is just "return
> err;".  So I just changed this to be "return err", which is clearer.
> I tend to place more imporance for clarity of the code than rigid
> rules which say that there should only be one control flow such that
> "return err" must only occur once at the end of the function.
>
>> +     err = ext4_journal_get_write_access(handle, EXT4_SB(sb)->s_sbh);
>> +     if (err) {
>> +             ext4_warning(sb, "error %d on journal write access", err);
>> +             ext4_journal_stop(handle);
>> +             goto out;
>> +     }
>
> Instead of calling ext4_journal_stop() here, it's simpler just to jump
> to the "exit_journal" label, which is consistent with what we do
> everywhere else.  Now all of the error gotos are to "exit_journal",
> which I've renamed to "errout".
>
> These are minor stylistic changes, but I thought I would mention them
> so that other people understand what is considered the best way to do
> things, I've just made these changes myself to avoid asking you to
> respin the patches.
Thanks!!!

Yongqiang.
>
> Regards,
>
>                                        - Ted

Patch

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 996780a..ac5565c 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -969,6 +969,59 @@  exit_put:
 } /* ext4_group_add */
 
 /*
+ * extend a group without checking assuming that checking has been done.
+ */
+static int ext4_group_extend_no_check(struct super_block *sb,
+			ext4_fsblk_t o_blocks_count, ext4_grpblk_t add)
+{
+	struct ext4_super_block *es = EXT4_SB(sb)->s_es;
+	handle_t *handle;
+	int err = 0, err2;
+
+	/* We will update the superblock, one block bitmap, and
+	 * one group descriptor via ext4_group_add_blocks().
+	 */
+	handle = ext4_journal_start_sb(sb, 3);
+	if (IS_ERR(handle)) {
+		err = PTR_ERR(handle);
+		ext4_warning(sb, "error %d on journal start", err);
+		goto out;
+	}
+
+	err = ext4_journal_get_write_access(handle, EXT4_SB(sb)->s_sbh);
+	if (err) {
+		ext4_warning(sb, "error %d on journal write access", err);
+		ext4_journal_stop(handle);
+		goto out;
+	}
+
+	ext4_blocks_count_set(es, o_blocks_count + add);
+	ext4_debug("freeing blocks %llu through %llu\n", o_blocks_count,
+		   o_blocks_count + add);
+	/* We add the blocks to the bitmap and set the group need init bit */
+	err = ext4_group_add_blocks(handle, sb, o_blocks_count, add);
+	if (err)
+		goto exit_journal;
+	ext4_handle_dirty_super(handle, sb);
+	ext4_debug("freed blocks %llu through %llu\n", o_blocks_count,
+		   o_blocks_count + add);
+exit_journal:
+	err2 = ext4_journal_stop(handle);
+	if (err2 && !err)
+		err = err2;
+
+	if (!err) {
+		if (test_opt(sb, DEBUG))
+			printk(KERN_DEBUG "EXT4-fs: extended group to %llu "
+			       "blocks\n", ext4_blocks_count(es));
+		update_backups(sb, EXT4_SB(sb)->s_sbh->b_blocknr, (char *)es,
+			       sizeof(struct ext4_super_block));
+	}
+out:
+	return err;
+}
+
+/*
  * Extend the filesystem to the new number of blocks specified.  This entry
  * point is only used to extend the current filesystem to the end of the last
  * existing group.  It can be accessed via ioctl, or by "remount,resize=<size>"