On Apr 28, 2009 01:04 +0530, Aneesh Kumar wrote:
> +static inline spinlock_t *ext4_group_lock(struct super_block *sb, ext4_group_t group)
> {
> + return bgl_lock_ptr(EXT4_SB(sb)->s_blockgroup_lock, group);
> +}
>
> +static inline void ext4_lock_group(struct super_block *sb,ext4_group_t group)
> +{
> + spin_lock(ext4_group_lock(sb, group));
> }
I find it a bit confusing to have both ext4_group_lock() and ext4_lock_group()
as it isn't obvious without looking at the functions which one is which.
I'd rather have a function name like "ext4_group_lock_ptr()" or similar,
which is pretty unambiguous.
> -static void mb_set_bits(spinlock_t *lock, void *bm, int cur, int len)
> +static void mb_set_bits(void *bm, int cur, int len)
It also wouldn't be a terrible idea to make the mb_set_bits() function
arguments match the name/order of mb_set_bit():
static inline void mb_set_bit(int bit, void *addr)
static void mb_set_bits(void *bm, int cur, int len)
They should be "bit, addr" and "bit, addr, len", to be more consistent
with ext4_set_bit(). Stuff for a separate patch, however.
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
