linux-kernel - Re: [PATCH 2/3] sysfs: slim down sysfs_dirent->s

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070529181315.37e31832@gondolin.boeblingen.de.ibm.com>
Date:	Tue, 29 May 2007 18:13:15 +0200
From:	Cornelia Huck <cornelia.huck@...ibm.com>
To:	Tejun Heo <htejun@...il.com>
Cc:	gregkh@...e.de, dmitry.torokhov@...il.com, oneukum@...e.de,
	rpurdie@...ys.net, stern@...land.harvard.edu, maneesh@...ibm.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] sysfs: slim down sysfs_dirent->s_active

On Tue, 29 May 2007 01:24:06 +0900,
Tejun Heo <htejun@...il.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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/