On Mon, Nov 24, 2008 at 10:14:27AM +0300, Alex Zhuravlev wrote:
> Theodore Tso wrote:
>> My bigger concern is given that we are playing games like *this*:
>>
>> if ((cur & 31) == 0 && (len - cur) >= 32) {
>> /* fast path: set whole word at once */
>> addr = bm + (cur >> 3);
>> *addr = 0xffffffff;
>> cur += 32;
>> continue;
>> }
>
> this is to avoid expensive LOCK prefix in some cases.
>
>> without taking a lock, I'm a little surprised we haven't been
>> seriously burned by other race conditions. What's the point of
>> calling mb_set_bit_atomic() and passing in a spinlock if we are doing
>> this kind of check without the protection of the same spinlock?!?
>
> why would we need a lock for a whole word bitop ?
Ok the changes was not done for this purpose. I need to make sure we
update bitmap and clear group_desc uninit flag after taking sb_bgl_lock
That means when we claim blocks we can't use mb_set_bits with
sb_bgl_lock because we would already be holding it. How about the below
change
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 444ad99..53180b1 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -1031,7 +1031,10 @@ static void mb_clear_bits(spinlock_t *lock, void *bm, int cur, int len)
cur += 32;
continue;
}
- mb_clear_bit_atomic(lock, cur, bm);
+ if (lock)
+ mb_clear_bit_atomic(lock, cur, bm);
+ else
+ mb_clear_bit(cur, bm);
cur++;
}
}
@@ -1049,7 +1052,10 @@ static void mb_set_bits(spinlock_t *lock, void *bm, int cur, int len)
cur += 32;
continue;
}
- mb_set_bit_atomic(lock, cur, bm);
+ if (lock)
+ mb_set_bit_atomic(lock, cur, bm);
+ else
+ mb_set_bit(cur, bm);
cur++;
}
}
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
