linux-ext4 - Re: ext4-block-reservation.patch

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] [day] [month] [year] [list]

Date:	Tue, 19 Jun 2007 16:10:49 -0600
From:	Andreas Dilger <adilger@...sterfs.com>
To:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc:	Alex Tomas <alex@...sterfs.com>, linux-ext4@...r.kernel.org,
	Eric Sandeen <esandeen@...hat.com>
Subject: Re: ext4-block-reservation.patch

On Jun 19, 2007  22:38 +0530, Aneesh Kumar K.V wrote:
> This is what i have modified. I am yet to build test it. I am looking at 
> forward porting the
> mballoc patches and was planning to send it together.

> +int ext4_reserve_local(struct super_block *sb, int blocks)
> +{
> +	preempt_disable();
> +	rs = sbi->s_reservation_slots + smp_processor_id();

Should this be instead "rs = sbi->s_reservation_slots + get_cpu()"

> +	spin_lock(&rs->rs_lock);
> +	if (likely(rs->rs_reserved >= blocks)) {
> +		rs->rs_reserved -= blocks;
> +		rc = 0;
> +	}
> +	spin_unlock(&rs->rs_lock);
> +
> +	preempt_enable();

And "put_cpu()" here?

> +void ext4_rebalance_reservation(struct ext4_reservation_slot *rs, __u64 free)
> +{
> +	/* chunk is a number of block every used
> +	 * slot will get. make sure it isn't 0 */
> +	chunk = free + used_slots - 1;
> +	do_div(chunk, used_slots);
> +
> +	for_each_possible_cpu(i) {
> +		if (free < chunk)
> +			chunk = free;
> +		if (rs[i].rs_reserved || i == smp_processor_id()) {
> +			rs[i].rs_reserved = chunk;
> +			free -= chunk;
> +			BUG_ON(free < 0);
> +		}
> +	}

Should we be assigning reservations to offline CPUs?  Doesn't it make sense
to assign 0 reservation to offline CPUs until they come back?  In the first
loop, if it is "for_each_possible_cpu()" it would drop reservations from
offline CPUs, and then the bottom one is "for_each_online_cpu()".

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, 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