Re: patch queue update

[Andreas Dilger <adilger@sun.com>]

On Jan 10, 2008  21:03 +0530, Aneesh Kumar K.V wrote:
>  	if (i >= sbi->s_mb_order2_reqs) {
> -		i--;
> -		if ((ac->ac_g_ex.fe_len & (~(1 << i))) == 0)
> +		/*
> +		 * This should tell if fe_len is exactly power of 2
> +		 */
> +		if ((ac->ac_g_ex.fe_len & (~(1 << (i - 1)))) == 0)
>  			ac->ac_2order = i;

While you changed i to (i - 1) in the "if" you didn't change it when
setting ac_2order...  Is that incorrect?

>  		/*
> +		 * Yield the CPU here so that we don't get soft lockup
>  		 */
> -		schedule_timeout(HZ);
> +		schedule();
>  		goto repeat;
>  	}
>  
> @@ -3808,7 +3820,7 @@ repeat:
>  			printk(KERN_ERR "uh-oh! used pa while discarding\n");
>  			dump_stack();
>  			current->state = TASK_UNINTERRUPTIBLE;
> -			schedule();
> +			schedule_timeout(HZ);
>  			goto repeat;

Is this change to schedule_timeout() intentional?  The earlier code is
removing the use of schedule_timeout.  I could be wrong, as I didn't
follow this discussion closely, but sometimes changes like this happen
accidentally and people don't look at the patch itself...

> +static unsigned long ext4_get_stripe_size(struct ext4_sb_info *sbi)
> +{
> +	unsigned long stride = le16_to_cpu(sbi->s_es->s_raid_stride);
> +	unsigned long stripe_width = le32_to_cpu(sbi->s_es->s_raid_stripe_width);
> +
> +	if (sbi->s_stripe && sbi->s_stripe <= sbi->s_blocks_per_group) {
> +		return sbi->s_stripe;
> +	} else if (stripe_width <= sbi->s_blocks_per_group) {
> +		return stripe_width;
> +	} else if (stride <= sbi->s_blocks_per_group) {
> +		return stride;
> +	}

If you are doing "return XXX" you don't need "else".

> +	/*
> +	 * set the stripe size. If we have specified it via mount option, then
> +	 * use the mount option value. If the value specified at mount time is
> +	 * greater than the blocks per group use the super block value.
> +	 * Allocator needs it be less than blocks per group.
> +	 */
> +	sbi->s_stripe = ext4_get_stripe_size(sbi);

This comment should probably go by ext4_get_stripe_size() definition instead
of here at the caller.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.