lkml.org 
[lkml]   [2008]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v2] mballoc: fix hot spins after err_freebuddy and err_freemeta
    On Fri, Apr 18, 2008 at 10:44:14AM +0530, Aneesh Kumar K.V wrote:
    > On Thu, Apr 17, 2008 at 08:29:18PM +0200, Roel Kluin wrote:
    > > Aneesh Kumar K.V wrote:
    > >
    > > > The function needs more changes. For ex:
    > > >
    > > > 2279 if (meta_group_info[j] == NULL) {
    > > > 2280 printk(KERN_ERR "EXT4-fs: can't allocate buddy mem\n");
    > > > 2281 i--;
    > > > 2282 goto err_freebuddy;
    > > > 2283 }
    > > >
    > > > That decrement i--; could result in bad value if i == 0;.
    > >
    > > Thanks Aneesh,
    > > ---
    > > Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
    > > ext4_mb_init_backend() has a variable i of type ext4_group_t. which is typedefined
    > > in include/linux/ext4_fs_i.h:34 to unsigned long. Since unsigned, i >= 0 is always
    > > true, so fix hot spins after err_freebuddy and err_freemeta.
    > > Also when meta_group_info cannot be allocated prevent a decrement of i when zero.
    > >
    > > Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
    > > ---
    > > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
    > > index ef97f19..2c13dca 100644
    > > --- a/fs/ext4/mballoc.c
    > > +++ b/fs/ext4/mballoc.c
    > > @@ -2572,8 +2572,13 @@ static int ext4_mb_init_backend(struct super_block *sb)
    > > meta_group_info[j] = kzalloc(len, GFP_KERNEL);
    > > if (meta_group_info[j] == NULL) {
    > > printk(KERN_ERR "EXT4-fs: can't allocate buddy mem\n");
    > > - i--;
    > > - goto err_freebuddy;
    > > + if (i != 0) {
    > > + i--;
    > > + goto err_freebuddy;
    > > + } else {
    > > + i = num_meta_group_infos - 1;
    > > + goto err_freemeta;
    > > + }
    > > }
    > > desc = ext4_get_group_desc(sb, i, NULL);
    > > if (desc == NULL) {
    > > @@ -2618,14 +2623,14 @@ static int ext4_mb_init_backend(struct super_block *sb)
    > > return 0;
    > >
    > > err_freebuddy:
    > > - while (i >= 0) {
    > > + do {
    > > kfree(ext4_get_group_info(sb, i));
    > > - i--;
    > > - }
    > > - i = num_meta_group_infos;
    > > + } while (i-- != 0);
    > > + i = num_meta_group_infos - 1;
    > > err_freemeta:
    > > - while (--i >= 0)
    > > + do {
    > > kfree(sbi->s_group_info[i]);
    > > + } while (i-- != 0);
    > > iput(sbi->s_buddy_cache);
    > > err_freesgi:
    > > kfree(sbi->s_group_info);
    > >
    >
    > Won't this also have a memory corruption ? Let's say we fail in the first
    > loop itslef. That's with i = 0, and since we are using kmalloc.
    > we may find sbi->s_group_info[0] having some random values. So the
    > kfree can crash. Why not a simple change like below ?
    >

    Updated one.

    diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
    index 28b5ada..572d809 100644
    --- a/fs/ext4/mballoc.c
    +++ b/fs/ext4/mballoc.c
    @@ -2223,7 +2223,7 @@ static noinline void ext4_mb_store_history(struct ext4_allocation_context *ac)

    static int ext4_mb_init_backend(struct super_block *sb)
    {
    - ext4_group_t i;
    + long i; /* should be able to store group number */
    int j, len, metalen;
    struct ext4_sb_info *sbi = EXT4_SB(sb);
    int num_meta_group_infos =
    @@ -2257,6 +2257,7 @@ static int ext4_mb_init_backend(struct super_block *sb)
    if (meta_group_info == NULL) {
    printk(KERN_ERR "EXT4-fs: can't allocate mem for a "
    "buddy group\n");
    + i--;
    goto err_freemeta;
    }
    sbi->s_group_info[i] = meta_group_info;
    @@ -2328,10 +2329,12 @@ err_freebuddy:
    kfree(ext4_get_group_info(sb, i));
    i--;
    }
    - i = num_meta_group_infos;
    + i = num_meta_group_infos - 1;
    err_freemeta:
    - while (--i >= 0)
    + while (i >= 0) {
    kfree(sbi->s_group_info[i]);
    + i--;
    + }
    iput(sbi->s_buddy_cache);
    err_freesgi:
    kfree(sbi->s_group_info);

    \
     
     \ /
      Last update: 2008-04-18 07:23    [W:0.029 / U:29.340 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site