Re: [PATCH 09/12] ext4: simplify journal handling in ext4_setup_new_group_blocks

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

 



On Mon, Jul 18, 2011 at 11:40 AM, Tao Ma <tm@xxxxxx> wrote:
> On 07/18/2011 11:21 AM, Yongqiang Yang wrote:
>> On Mon, Jul 18, 2011 at 11:09 AM, Tao Ma <tm@xxxxxx> wrote:
>>> On 07/18/2011 10:52 AM, Yongqiang Yang wrote:
>>>> This patch simplifies journal handling in ext4_setup_new_group_blocks().
>>>>
>>>>
>>>> Signed-off-by: Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>
>>>> ---
>>>>  fs/ext4/resize.c |   39 ++++++++++++++++++++-------------------
>>>>  1 files changed, 20 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
>>>> index c91653c..ac5b63d 100644
>>>> --- a/fs/ext4/resize.c
>>>> +++ b/fs/ext4/resize.c
>>>> @@ -171,8 +171,7 @@ static struct buffer_head *bclean(handle_t *handle, struct super_block *sb,
>>>>   * If that fails, restart the transaction & regain write access for the
>>>>   * buffer head which is used for block_bitmap modifications.
>>>>   */
>>>> -static int extend_or_restart_transaction(handle_t *handle, int thresh,
>>>> -                                      struct buffer_head *bh)
>>>> +static int extend_or_restart_transaction(handle_t *handle, int thresh)
>>>>  {
>>>>       int err;
>>>>
>>>> @@ -183,9 +182,8 @@ static int extend_or_restart_transaction(handle_t *handle, int thresh,
>>>>       if (err < 0)
>>>>               return err;
>>>>       if (err) {
>>>> -             if ((err = ext4_journal_restart(handle, EXT4_MAX_TRANS_DATA)))
>>>> -                     return err;
>>>> -             if ((err = ext4_journal_get_write_access(handle, bh)))
>>>> +             err = ext4_journal_restart(handle, EXT4_MAX_TRANS_DATA);
>>>> +             if (err)
>>>>                       return err;
>>> you removed an ext4_journal_get_write_access here, and I didn't see you
>>> add it back anywhere.
>> This get_write_access() was used to guarantee modification on the
>> block bitmap is in the new handle in previous code.  In previous code,
>> bitmap is modified everywhere in setup_group_blocks(),  actually, this
>> makes things complicated, in this patch, the modifications on block
>> bitmap are batched and are done by ext4_set_bits() after
>> extend_or_restart_transaction(), so it is ok.
> oh, I see. You moved the initialization of bh after the extension of
> journal. OK, but please do describe all your changes in more detail in
> the commit log.
Yes, my carelessness.

Thank you for your review.

Yongqiang.
>
> Thanks
> Tao
>>
>> Thanks,
>> Yongqiang.
>>>
>>> Thanks
>>> Tao
>>>>       }
>>>>
>>>> @@ -222,29 +220,24 @@ static int setup_new_group_blocks(struct super_block *sb,
>>>>
>>>>       BUG_ON(input->group != sbi->s_groups_count);
>>>>
>>>> -     if (IS_ERR(bh = bclean(handle, sb, input->block_bitmap))) {
>>>> -             err = PTR_ERR(bh);
>>>> -             goto exit_journal;
>>>> -     }
>>>> -
>>>>       /* Copy all of the GDT blocks into the backup in this group */
>>>>       for (i = 0, bit = 1, block = start + 1;
>>>>            i < gdblocks; i++, block++, bit++) {
>>>>               struct buffer_head *gdb;
>>>>
>>>>               ext4_debug("update backup group %#04llx (+%d)\n", block, bit);
>>>> -
>>>> -             if ((err = extend_or_restart_transaction(handle, 1, bh)))
>>>> -                     goto exit_bh;
>>>> +             err = extend_or_restart_transaction(handle, 1);
>>>> +             if (err)
>>>> +                     goto exit_journal;
>>>>
>>>>               gdb = sb_getblk(sb, block);
>>>>               if (!gdb) {
>>>>                       err = -EIO;
>>>> -                     goto exit_bh;
>>>> +                     goto exit_journal;
>>>>               }
>>>>               if ((err = ext4_journal_get_write_access(handle, gdb))) {
>>>>                       brelse(gdb);
>>>> -                     goto exit_bh;
>>>> +                     goto exit_journal;
>>>>               }
>>>>               lock_buffer(gdb);
>>>>               memcpy(gdb->b_data, sbi->s_group_desc[i]->b_data, gdb->b_size);
>>>> @@ -253,7 +246,7 @@ static int setup_new_group_blocks(struct super_block *sb,
>>>>               err = ext4_handle_dirty_metadata(handle, NULL, gdb);
>>>>               if (unlikely(err)) {
>>>>                       brelse(gdb);
>>>> -                     goto exit_bh;
>>>> +                     goto exit_journal;
>>>>               }
>>>>               brelse(gdb);
>>>>       }
>>>> @@ -264,7 +257,17 @@ static int setup_new_group_blocks(struct super_block *sb,
>>>>       err = sb_issue_zeroout(sb, gdblocks + start + 1, reserved_gdb,
>>>>                              GFP_NOFS);
>>>>       if (err)
>>>> -             goto exit_bh;
>>>> +             goto exit_journal;
>>>> +
>>>> +     err = extend_or_restart_transaction(handle, 2);
>>>> +     if (err)
>>>> +             goto exit_journal;
>>>> +
>>>> +     bh = bclean(handle, sb, input->block_bitmap);
>>>> +     if (IS_ERR(bh)) {
>>>> +             err = PTR_ERR(bh);
>>>> +             goto exit_journal;
>>>> +     }
>>>>
>>>>       if (ext4_bg_has_super(sb, input->group)) {
>>>>               ext4_debug("mark backup group tables %#04llx (+0)\n", start);
>>>> @@ -288,8 +291,6 @@ static int setup_new_group_blocks(struct super_block *sb,
>>>>       ext4_set_bits(bh->b_data, input->inode_table - start,
>>>>                     sbi->s_itb_per_group);
>>>>
>>>> -     if ((err = extend_or_restart_transaction(handle, 2, bh)))
>>>> -             goto exit_bh;
>>>>
>>>>       ext4_mark_bitmap_end(input->blocks_count, sb->s_blocksize * 8,
>>>>                            bh->b_data);
>>>
>>>
>>
>>
>>
>
>



-- 
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