Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 19 Sep 2018 12:13:43 -0400
From:      Shawn Webb <shawn.webb@hardenedbsd.org>
To:        Mateusz Guzik <mjguzik@gmail.com>
Cc:        Mateusz Guzik <mjg@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r338802 - head/sys/vm
Message-ID:  <20180919161343.ltn7ykiop2lzzgcj@mutt-hbsd>
In-Reply-To: <CAGudoHF6DbqFpT2jSFrbEOTvmN0e=Oz8ZZco1PRhN36PNMDpQA@mail.gmail.com>
References:  <201809191602.w8JG2Y0X046254@repo.freebsd.org> <20180919160819.lfzrey3upzri4tll@mutt-hbsd> <CAGudoHF6DbqFpT2jSFrbEOTvmN0e=Oz8ZZco1PRhN36PNMDpQA@mail.gmail.com>

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

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

On Wed, Sep 19, 2018 at 06:12:29PM +0200, Mateusz Guzik wrote:
> On Wed, Sep 19, 2018 at 6:08 PM, Shawn Webb <shawn.webb@hardenedbsd.org>
> wrote:
>=20
> > On Wed, Sep 19, 2018 at 04:02:34PM +0000, Mateusz Guzik wrote:
> > > Author: mjg
> > > Date: Wed Sep 19 16:02:33 2018
> > > New Revision: 338802
> > > URL: https://svnweb.freebsd.org/changeset/base/338802
> > >
> > > Log:
> > >   vm: check for empty kstack cache before locking
> > >
> > >   The current cache logic checks the total number of stacks in the
> > kernel,
> > >   which even on small boxes significantly exceeds the 128 limit (e.g.=
 an
> > >   8-way box with zfs has almost 800 stacks allocated).
> > >
> > >   Stacks are cached earlier for each main thread.
> > >
> > >   As a result the code is rarely executed, but when it is then (on bo=
xes
> > like
> > >   the above) it always fails. Since there are no provisions made for
> > NUMA and
> > >   release time is approaching, just do a quick check to avoid acquiri=
ng
> > the
> > >   lock.
> > >
> > >   Approved by:        re (kib)
> > >
> > > Modified:
> > >   head/sys/vm/vm_glue.c
> > >
> > > Modified: head/sys/vm/vm_glue.c
> > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > > --- head/sys/vm/vm_glue.c     Wed Sep 19 15:39:16 2018        (r33880=
1)
> > > +++ head/sys/vm/vm_glue.c     Wed Sep 19 16:02:33 2018        (r33880=
2)
> > > @@ -327,7 +327,7 @@ vm_thread_new(struct thread *td, int pages)
> > >       else if (pages > KSTACK_MAX_PAGES)
> > >               pages =3D KSTACK_MAX_PAGES;
> > >
> > > -     if (pages =3D=3D kstack_pages) {
> > > +     if (pages =3D=3D kstack_pages && kstack_cache !=3D NULL) {
> > >               mtx_lock(&kstack_cache_mtx);
> > >               if (kstack_cache !=3D NULL) {
> > >                       ks_ce =3D kstack_cache;
> >
> > Since kstack_cache is guaranteed not to be NULL, can the second
> > conditional that checks for kstack_cache not being NULL be removed?
> >
> >
> The one with the lock held? By the time we get there someone else might
> have removed the last cached stack making the pointer NULL.
>=20
> Checking a condition before locking and then checking again is a common
> optimization pattern for cases where the condition is likely false.

Ah, I understand now. Thanks for the clarification!

--=20
Shawn Webb
Cofounder and Security Engineer
HardenedBSD

Tor-ified Signal:    +1 443-546-8752
Tor+XMPP+OTR:        lattera@is.a.hacker.sx
GPG Key ID:          0x6A84658F52456EEE
GPG Key Fingerprint: 2ABA B6BD EF6A F486 BE89  3D9E 6A84 658F 5245 6EEE

--7k4dlpxs4gushcjn
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEKrq2ve9q9Ia+iT2eaoRlj1JFbu4FAluidbYACgkQaoRlj1JF
bu47pw//ZB5VcPYu/cXoOkHWlSxfUgIN1xwQqbEQiI4O50S3DSUBi97JJD3s9zRx
b8x4Cc4Cq1C5KdJzrg7KBOWZLOJL/Y3K4r1q6T2ADgn3Hd8qjCriqZhvLz3lZvQ5
q4eBwo19G/M6v6lcPJIZEwrUaZh3GgVqGbqvntlzmOkbrNwogE1x6f0EUp0RSbRQ
qCEePYihRFlKuwdVzykcQj8bzbK4BaT3oEKvTq4aJ71JAdWDo/CebDA6eKMAeS9q
nq9EKZNT9u9E+jjONIJ6JGQ/q3LF+4QHH+Omkj8JQD05/7oef6Otoq/yaSw4ghu0
2WdgT8PuZAdi/sS84Ma0C8ujXO3g9bflyYcS2smMyFXXEqPzDgEBRJQ5v6JnvTOn
kr5yR70EdefLbcTHhg2YbnjlBnQ7Wv3Vo5LclpNOEPLHCjrjLzAvyvh7Xg9cLbQX
6JaOsWXkA2uVIsGnMahu2dvFnFIzguBcgEB1gzaGsSVdYJgiS0BryIhbTpigbuv+
0ydKXKr2qFdJw+iBJMkCuFRqE1SwqlF6MIYb7LJOJGbJxtQI8mNEMq+77Kt1Fb1E
ZyAJLkcIZmo8W7nYxOPjuB+u/oCjcaRKsyioY2qNcdbQMwvBXeZQgEpHobnV4/3q
zGY8qBGKTISTxox7aqsa4ltIRwKfyv0PlLJjOunOcO/9/6m3CG0=
=n6/0
-----END PGP SIGNATURE-----

--7k4dlpxs4gushcjn--



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