Re: patch queue update

Andreas Dilger
2008-01-10T21:43:25Z

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.