Re: [PATCH 05/12] ext4: let ext4_group_add_blocks return an error code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jul 18, 2011 at 2:48 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> On Mon, Jul 18, 2011 at 5:52 AM, Yongqiang Yang <xiaoqiangnk@xxxxxxxxx> wrote:
>> This patch lets ext4_group_add_blocks() return an error code if it fails,
>> so that upper functions can handle error correctly.
>>
>> Signed-off-by: Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>
>> ---
>>  fs/ext4/ext4.h    |    2 +-
>>  fs/ext4/mballoc.c |   19 +++++++++++++------
>>  fs/ext4/resize.c  |   10 +++++++---
>>  3 files changed, 21 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index bbe81db..da7ab48 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -1799,7 +1799,7 @@ extern void ext4_free_blocks(handle_t *handle, struct inode *inode,
>>                             unsigned long count, int flags);
>>  extern int ext4_mb_add_groupinfo(struct super_block *sb,
>>                ext4_group_t i, struct ext4_group_desc *desc);
>> -extern void ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
>> +extern int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
>>                                ext4_fsblk_t block, unsigned long count);
>>  extern int ext4_trim_fs(struct super_block *, struct fstrim_range *);
>>
>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>> index ea80c0b..33c41e6 100644
>> --- a/fs/ext4/mballoc.c
>> +++ b/fs/ext4/mballoc.c
>> @@ -4664,7 +4664,7 @@ error_return:
>>  *
>>  * This marks the blocks as free in the bitmap and buddy.
>>  */
>> -void ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
>> +int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
>>                         ext4_fsblk_t block, unsigned long count)
>>  {
>>        struct buffer_head *bitmap_bh = NULL;
>> @@ -4685,15 +4685,21 @@ void ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
>>         * Check to see if we are freeing blocks across a group
>>         * boundary.
>>         */
>> -       if (bit + count > EXT4_BLOCKS_PER_GROUP(sb))
>> -               goto error_return;
>> +       if (bit + count > EXT4_BLOCKS_PER_GROUP(sb)) {
>> +               ext4_warning(sb, "too much blocks added to group %u\n",
>> +                            block_group);
>> +               return -EINVAL;
>> +       }
>>
>>        bitmap_bh = ext4_read_block_bitmap(sb, block_group);
>>        if (!bitmap_bh)
>> -               goto error_return;
>> +               return -EIO;
>
> Any reason you are skipping the goto in the 2 error cases above and
> missing the ext4_std_error()?

Resizing is very different from normal freeing before the block bitmap
are modified,  because the added blocks in super block are marked
used, thus they could not be used, for a mounted fs, it can continue
working, and the wrong block count could be fixed easily by e2fsck.
So I think we'd better not report an error to the fs.   Just my humble
opinion.

What about your opinions?

Yongqiang.
>
>> +
>>        desc = ext4_get_group_desc(sb, block_group, &gd_bh);
>> -       if (!desc)
>> +       if (!desc) {
>> +               err = -EIO;
>>                goto error_return;
>> +       }
>>
>>        if (in_range(ext4_block_bitmap(sb, desc), block, count) ||
>>            in_range(ext4_inode_bitmap(sb, desc), block, count) ||
>> @@ -4703,6 +4709,7 @@ void ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
>>                ext4_error(sb, "Adding blocks in system zones - "
>>                           "Block = %llu, count = %lu",
>>                           block, count);
>> +               err = -EINVAL;
>>                goto error_return;
>>        }
>>
>> @@ -4771,7 +4778,7 @@ void ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
>>  error_return:
>>        brelse(bitmap_bh);
>>        ext4_std_error(sb, err);
>> -       return;
>> +       return err;
>>  }
>>
>>  /**
>> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
>> index 374fd1c..2e63376 100644
>> --- a/fs/ext4/resize.c
>> +++ b/fs/ext4/resize.c
>> @@ -990,7 +990,7 @@ int ext4_group_extend(struct super_block *sb, struct ext4_super_block *es,
>>        ext4_grpblk_t add;
>>        struct buffer_head *bh;
>>        handle_t *handle;
>> -       int err;
>> +       int err, err2;
>>        ext4_group_t group;
>>
>>        o_blocks_count = ext4_blocks_count(es);
>> @@ -1066,11 +1066,15 @@ int ext4_group_extend(struct super_block *sb, struct ext4_super_block *es,
>>        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 */
>> -       ext4_group_add_blocks(handle, sb, o_blocks_count, add);
>> +       err = ext4_group_add_blocks(handle, sb, o_blocks_count, add);
>>        ext4_handle_dirty_super(handle, sb);
>>        ext4_debug("freed blocks %llu through %llu\n", o_blocks_count,
>>                   o_blocks_count + add);
>> -       if ((err = ext4_journal_stop(handle)))
>> +       err2 = ext4_journal_stop(handle);
>> +       if (!err && err2)
>> +               err = err2;
>> +
>> +       if (err)
>>                goto exit_put;
>>
>>        if (test_opt(sb, DEBUG))
>> --
>> 1.7.5.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>



-- 
Best Wishes
Yongqiang Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux