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>
