Re: [RFC PATCH] Convert ext4_lock_group to use sb_bgl_lock

[Andreas Dilger <adilger@sun.com>]

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.