Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 18 Aug 2007 19:27:19 +0200
From:      Pawel Jakub Dawidek <pjd@FreeBSD.org>
To:        Alfred Perlstein <alfred@freebsd.org>
Cc:        freebsd-arch@FreeBSD.org
Subject:   Re: Lockless uidinfo.
Message-ID:  <20070818172719.GF6498@garage.freebsd.pl>
In-Reply-To: <20070818171738.GB90381@elvis.mu.org>
References:  <20070818120056.GA6498@garage.freebsd.pl> <20070818142337.GW90381@elvis.mu.org> <20070818150028.GD6498@garage.freebsd.pl> <20070818155041.GY90381@elvis.mu.org> <20070818161449.GE6498@garage.freebsd.pl> <20070818171738.GB90381@elvis.mu.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--eDB11BtaWSyaBkpc
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Aug 18, 2007 at 10:17:38AM -0700, Alfred Perlstein wrote:
> * Pawel Jakub Dawidek <pjd@FreeBSD.org> [070818 09:14] wrote:
> > On Sat, Aug 18, 2007 at 08:50:41AM -0700, Alfred Perlstein wrote:
> > > * Pawel Jakub Dawidek <pjd@FreeBSD.org> [070818 07:59] wrote:
> > > > Yes, to lookup uidinfo you need to hold uihashtbl_mtx mutex, so onc=
e you
> > > > hold it and ui_ref is 0, noone will be able to reference it, becaus=
e it
> > > > has to wait to look it up.
> > >=20
> > > And the field doesn't need to be volatile to prevent cached/opportuni=
tic
> > > reads?
> >=20
> > The only chance of something like this will be the scenario below:
> >=20
> > thread1 (uifind)		thread2 (uifree)
> > ----------------		----------------
> > 				refcount_release(&uip->ui_ref))
> > 				/* ui_ref =3D=3D 0 */
> > mtx_lock(&uihashtbl_mtx);
> > refcount_acquire(&uip->ui_ref);
> > /* ui_ref =3D=3D 1 */
> > mtx_unlock(&uihashtbl_mtx);
> > 				mtx_lock(&uihashtbl_mtx);
> > 				if (uip->ui_ref > 0) {
> > 					mtx_unlock(&uihashtbl_mtx);
> > 					return;
> > 				}
> >=20
> > Now, you suggest that ui_ref in 'if (uip->ui_ref > 0)' may still have
> > cached 0? I don't think it is possible, first refcount_acquire() uses
> > read memory bariers (but we may still need ui_ref to volatile for this
> > to make any difference) and second, think of ui_ref as a field protected
> > by uihashtbl_mtx mutex in this very case.
> >=20
> > Is my thinking correct?
>=20
> I don't know, that's why I was asking you. :)

In previous version of the patch I had atomic_load() in there, but after
some thinking I decided it's not needed and I change it. Now, that you
asked about it I was afraid that maybe my thinking isn't correct.
Anyway, it'll be good if someone could confirm it's ok.

--=20
Pawel Jakub Dawidek                       http://www.wheel.pl
pjd@FreeBSD.org                           http://www.FreeBSD.org
FreeBSD committer                         Am I Evil? Yes, I Am!

--eDB11BtaWSyaBkpc
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4 (FreeBSD)

iD8DBQFGxyv3ForvXbEpPzQRAsJXAJ0e00hB+95tdLvWgtkiorckarjC0gCfVsDf
pWL2e7NV3LNx7NhWl9B6D2s=
=4DHl
-----END PGP SIGNATURE-----

--eDB11BtaWSyaBkpc--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070818172719.GF6498>