Re: [PATCH 2/3] sysfs: slim down sysfs_dirent->s_active

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Tue, 29 May 2007 01:24:06 +0900,
Tejun Heo <htejun@gmail.com> wrote:

> Make sysfs_dirent->s_active an atomic_t instead of rwsem.  This
> reduces the size of sysfs_dirent from 136 to 104 on 64bit and from 76
> to 60 on 32bit with lock debugging turned off.  With lock debugging
> turned on the reduction is much larger.

Cool.

> s_active starts at zero and each active reference increments s_active.
> Putting a reference decrements s_active.  Deactivation subtracts
> SD_DEACTIVATED_BIAS which is currently INT_MIN and assumed to be small
> enough to make s_active negative.  If s_active is negative,
> sysfs_get() no longer grants new references.  Deactivation succeeds
> immediately if there is no active user; otherwise, it waits using a
> completion for the last put.
> 
> Due to the removal of lockdep tricks, this change makes things less
> trickier in release_sysfs_dirent().  As all the complexity is
> contained in three s_active functions, I think it's more readable this
> way.

Agreed.


> @@ -32,11 +33,24 @@ static DEFINE_IDA(sysfs_ino_ida);
>   */
>  struct sysfs_dirent *sysfs_get_active(struct sysfs_dirent *sd)
>  {
> -	if (sd) {
> -		if (unlikely(!down_read_trylock(&sd->s_active)))
> -			sd = NULL;
> +	if (unlikely(!sd))
> +		return NULL;
> +
> +	while (1) {
> +		int v, t;
> +
> +		v = atomic_read(&sd->s_active);
> +		if (unlikely(v < 0))
> +			return NULL;
> +
> +		t = atomic_cmpxchg(&sd->s_active, v, v + 1);
> +		if (likely(t == v))
> +			return sd;
> +		if (t < 0)
> +			return NULL;
> +
> +		cpu_relax();
>  	}
> -	return sd;
>  }

I don't quite like v and t, but don't have a better naming suggestion
either.