Re: [PATCH 005 of 13] md: Allow stripes to be expanded in preparation for expanding an array.

[Neil Brown <neilb@suse.de>]

On Thursday March 16, akpm@osdl.org wrote:
> NeilBrown <neilb@suse.de> wrote:
> >
> > +static int resize_stripes(raid5_conf_t *conf, int newsize)
> >  +{
> >  +	/* make all the stripes able to hold 'newsize' devices.
> >  +	 * New slots in each stripe get 'page' set to a new page.
> >  +	 * We allocate all the new stripes first, then if that succeeds,
> >  +	 * copy everything across.
> >  +	 * Finally we add new pages.  This could fail, but we leave
> >  +	 * the stripe cache at it's new size, just with some pages empty.
> >  +	 *
> >  +	 * We use GFP_NOIO allocations as IO to the raid5 is blocked
> >  +	 * at some points in this operation.
> >  +	 */
> >  +	struct stripe_head *osh, *nsh;
> >  +	struct list_head newstripes, oldstripes;
> 
> You can use LIST_HEAD() here, avoid the separate INIT_LIST_HEAD().
> 

I guess.....
I have to have the declaration "miles" from where I use the variable.
Do I have to have the initialisation equally far?  Ok, I'll do that..


> 
> >  +	struct disk_info *ndisks;
> >  +	int err = 0;
> >  +	kmem_cache_t *sc;
> >  +	int i;
> >  +
> >  +	if (newsize <= conf->pool_size)
> >  +		return 0; /* never bother to shrink */
> >  +
> >  +	sc = kmem_cache_create(conf->cache_name[1-conf->active_name],
> >  +			       sizeof(struct stripe_head)+(newsize-1)*sizeof(struct r5dev),
> >  +			       0, 0, NULL, NULL);
> 
> kmem_cache_create() internally does a GFP_KERNEL allocation.
> 
> >  +	if (!sc)
> >  +		return -ENOMEM;
> >  +	INIT_LIST_HEAD(&newstripes);
> >  +	for (i = conf->max_nr_stripes; i; i--) {
> >  +		nsh = kmem_cache_alloc(sc, GFP_NOIO);
> 
> So either this can use GFP_KERNEL, or we have a problem.

Good point....  Maybe the comment about GFP_NOIO just needs to be
improved. 
We cannot risk waiting on IO after the 
	/* OK, we have enough stripes, start collecting inactive
	 * stripes and copying them over
	 */

comment, up to the second last while loop, that starts
	while(!list_empty(&newstripes)) {

Before the comment, which where the kmem_cache_create is, is OK.

Thanks!

NeilBrown

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/