Re: [PATCH] ext4: fix online resize with mballoc

[Mingming Cao <cmm@us.ibm.com>]

On Thu, 2008-06-26 at 17:26 -0600, Andreas Dilger wrote:
> On Jun 26, 2008  16:58 +0200, Fr�d�ric Boh� wrote:
> > From: Frederic Bohe <frederic.bohe@bull.net>

> 
> >  static int ext4_mb_init_backend(struct super_block *sb)
> >  {
> > +	/* This is the total number of blocks used by GDT including
> > +	* the number of reserved blocks for GDT.
> > +	* The s_group_info array is allocated with this value
> > +	* to allow a clean online resize without a complex
> > +	* manipulation of pointer.
> > +	* The drawback is the unused memory when no resize
> > +	* occurs but it's very low in terms of pages
> > +	* (see comments below)
> > +	*/
> 
> Also align comment '*' in the same columns.
> 
> > +	num_meta_group_infos_max = num_meta_group_infos +
> > +				le16_to_cpu(es->s_reserved_gdt_blocks);
> 
> The only drawback of NOT handling this properly is that once (eventually)
> we allow resizing with META_BG this code will be broken again.  It at
> least deserves a comment like "Need to handle this properly when META_BG
> resizing is allowed" so that it will show up on any grep for META_BG.
> 

It probably useful to document this limit in the change log too.

It would be nice to do it right now since we are here, if it's not too
complicated. The locking cost to do dynamic re-allocation of
s_groups_info seems low to me if we use read-write lock, as the write
path is relatively rare. But I guess there are a lot of there places we
need to worry to enable resize to any large with FLEX_BG/META_BG, and
could deal with that later.

Mingming
> It probably also makes sense to round this up to the next power-of-two
> value, since kmalloc will do that internally anyways, and it gives us
> some resizing headroom for no cost.
> 
> > @@ -2276,62 +2431,15 @@ static int ext4_mb_init_backend(struct s
> >  		sbi->s_group_info[i] = meta_group_info;
> >  	}
> >  
> >  	for (i = 0; i < sbi->s_groups_count; i++) {
> > +		if (ext4_mb_add_groupinfo(sb, i, desc) != 0)
> > +			goto err_freebuddy;
> 
> This is much nicer, great.
> 
> Cheers, Andreas
> --
> Andreas Dilger
> Sr. Staff Engineer, Lustre Group
> Sun Microsystems of Canada, Inc.
> 
> --
> 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

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