linux-ext4 - Re: patch queue update

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080110214323.GL3351@webber.adilger.int>
Date:	Thu, 10 Jan 2008 14:43:23 -0700
From:	Andreas Dilger <adilger@....com>
To:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: patch queue update

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.

-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html