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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to