| Message ID | 1325242812-27005-2-git-send-email-xiaoqiangnk@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
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
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
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>"