svn commit: r338802 - head/sys/vm

[Mateusz Guzik]

On Wed, Sep 19, 2018 at 6:08 PM, Shawn Webb <shawn.webb at hardenedbsd.org>
wrote:

> 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 boxes
> 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 acquiring
> the
> >   lock.
> >
> >   Approved by:        re (kib)
> >
> > Modified:
> >   head/sys/vm/vm_glue.c
> >
> > Modified: head/sys/vm/vm_glue.c
> > ============================================================
> ==================
> > --- head/sys/vm/vm_glue.c     Wed Sep 19 15:39:16 2018        (r338801)
> > +++ head/sys/vm/vm_glue.c     Wed Sep 19 16:02:33 2018        (r338802)
> > @@ -327,7 +327,7 @@ vm_thread_new(struct thread *td, int pages)
> >       else if (pages > KSTACK_MAX_PAGES)
> >               pages = KSTACK_MAX_PAGES;
> >
> > -     if (pages == kstack_pages) {
> > +     if (pages == kstack_pages && kstack_cache != NULL) {
> >               mtx_lock(&kstack_cache_mtx);
> >               if (kstack_cache != NULL) {
> >                       ks_ce = 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.

Checking a condition before locking and then checking again is a common
optimization pattern for cases where the condition is likely false.

-- 
Mateusz Guzik <mjguzik gmail.com>