> 2010/3/26, Aneesh Kumar K. V <aneesh.kumar@linux.vnet.ibm.com>:
> > On Tue, 23 Mar 2010 20:47:39 +0800, jing zhang <zj.barak@gmail.com> wrote:
> >> 2010/3/22,
tytso@mit.edu <tytso@mit.edu>:
> >> > On Sun, Mar 21, 2010 at 10:01:06PM +0800, jing zhang wrote:
> >> >> From: Jing Zhang <zj.barak@gmail.com>
> >> >>
> >> >> Date: Sun Mar 21 21:59:35 2010
> >> >>
> >> >> Cc: Theodore Ts'o <tytso@mit.edu>
> >> >> Cc: Andreas Dilger <adilger@sun.com>
> >> >> Cc: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> >> >> Signed-off-by: Jing Zhang <zj.barak@gmail.com>
> >> >>
> >> >> ---
> >> >>
> >> >> --- linux-2.6.32/fs/ext4/mballoc.c 2009-12-03 11:51:22.000000000 +0800
> >> >> +++ ext4_mm_leak/mballoc3.c 2010-03-21 21:37:18.000000000 +0800
> >> >> @@ -2360,6 +2360,24 @@ err_freesgi:
> >> >> return -ENOMEM;
> >> >> }
> >> >>
> >> >> +static void ext4_mb_destroy_backend(struct super_block *sb)
> >> >> +{
> >> >> + ext4_group_t ngroups = ext4_get_groups_count(sb);
> >> >> + ext4_group_t i;
> >> >> + struct ext4_sb_info *sbi = EXT4_SB(sb);
> >> >> + int j;
> >> >> + int num_meta_group_infos = (ngroups + EXT4_DESC_PER_BLOCK(sb) -1)
> >> >> + >> EXT4_DESC_PER_BLOCK_BITS(sb);
> >> >> + for (i = 0; i < ngroups; i++)
> >> >> + kfree(ext4_get_group_info(sb, i));
> >> >> +
> >> >> + for (j = 0; j < num_meta_group_infos; j++)
> >> >> + kfree(sbi->s_group_info[j]);
> >> >> +
> >> >> + kfree(sbi->s_group_info);
> >> >> + iput(sbi->s_buddy_cache);
> >> >> +}
> >> >> +
> >> >
> >> > It would be better if this could be done by making ext4_mb_release()
> >> > more flexible, and then calling ext4_mb_release() if there is an error
> >> > setting up the data structures in ext4_mb_init().
> >> >
> >> > - Ted
> >> >
> >>
> >> Yeah, Ted, going through ext4_mb_release() is clearer.
> >>
> >> ---
> >>
> >> diff --git a/linux-2.6.32/fs/ext4/mballoc.c b/ext4_mm_leak/mballoc3.c
> >> index bba1282..99ca2de 100644
> >> --- a/linux-2.6.32/fs/ext4/mballoc.c
> >> +++ b/ext4_mm_leak/mballoc3.c
> >> @@ -2417,8 +2417,7 @@ int ext4_mb_init(struct super_block *sb, int
> >> needs_recovery)
> >>
> >> sbi->s_locality_groups = alloc_percpu(struct ext4_locality_group);
> >> if (sbi->s_locality_groups == NULL) {
> >> - kfree(sbi->s_mb_offsets);
> >> - kfree(sbi->s_mb_maxs);
> >> + ext4_mb_release(sb);
> >
> > We may want to make sure that we can safely call ext4_mb_release that
> > early. what i would suggest is to move s_locality_group allocation
> > before ext4_mb_init. that makes error handling easy
> >
>
> cool idea, Aneesh, and I got it.
>
> ---
>
> --- old-linux-2.6.32/fs/ext4/mballoc.c 2009-12-03 11:51:22.000000000 +0800
> +++ ext4_mm_leak/mballoc3-2.c 2010-03-28 16:13:20.000000000 +0800
> @@ -2397,11 +2397,27 @@ int ext4_mb_init(struct super_block *sb,
> i++;
> } while (i <= sb->s_blocksize_bits + 1);
>
> + sbi->s_locality_groups = alloc_percpu(struct ext4_locality_group);
> + if (sbi->s_locality_groups == NULL) {
> + kfree(sbi->s_mb_offsets);
> + kfree(sbi->s_mb_maxs);
> + return -ENOMEM;
> + }
> + for_each_possible_cpu(i) {
> + struct ext4_locality_group *lg;
> + lg = per_cpu_ptr(sbi->s_locality_groups, i);
> + mutex_init(&lg->lg_mutex);
> + for (j = 0; j < PREALLOC_TB_SIZE; j++)
> + INIT_LIST_HEAD(&lg->lg_prealloc_list[j]);
> + spin_lock_init(&lg->lg_prealloc_lock);
> + }
> +
> /* init file for buddy data */
> ret = ext4_mb_init_backend(sb);
> if (ret != 0) {
> kfree(sbi->s_mb_offsets);
> kfree(sbi->s_mb_maxs);
> + free_percpu(sbi->s_locality_groups);
> return ret;
> }
>
> @@ -2415,20 +2431,6 @@ int ext4_mb_init(struct super_block *sb,
> sbi->s_mb_order2_reqs = MB_DEFAULT_ORDER2_REQS;
> sbi->s_mb_group_prealloc = MB_DEFAULT_GROUP_PREALLOC;
>
> - sbi->s_locality_groups = alloc_percpu(struct ext4_locality_group);
> - if (sbi->s_locality_groups == NULL) {
> - kfree(sbi->s_mb_offsets);
> - kfree(sbi->s_mb_maxs);
> - return -ENOMEM;
> - }
> - for_each_possible_cpu(i) {
> - struct ext4_locality_group *lg;
> - lg = per_cpu_ptr(sbi->s_locality_groups, i);
> - mutex_init(&lg->lg_mutex);
> - for (j = 0; j < PREALLOC_TB_SIZE; j++)
> - INIT_LIST_HEAD(&lg->lg_prealloc_list[j]);
> - spin_lock_init(&lg->lg_prealloc_lock);
> - }
>
> if (sbi->s_proc)
> proc_create_data("mb_groups", S_IRUGO, sbi->s_proc,
>
>
> >
> >
> >> return -ENOMEM;
> >> }
> >> for_each_possible_cpu(i) {
> >> @@ -2511,7 +2510,8 @@ int ext4_mb_release(struct super_block *sb)
> >> atomic_read(&sbi->s_mb_discarded));
> >> }
> >>
> >> - free_percpu(sbi->s_locality_groups);
> >> + if (sbi->s_locality_groups)
> >> + free_percpu(sbi->s_locality_groups);
> >> if (sbi->s_proc)
> >> remove_proc_entry("mb_groups", sbi->s_proc);
> >> --
> >> 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
> >
> > -aneesh
> >
