Date: Wed, 19 Sep 2018 18:12:29 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Shawn Webb <shawn.webb@hardenedbsd.org> 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: <CAGudoHF6DbqFpT2jSFrbEOTvmN0e=Oz8ZZco1PRhN36PNMDpQA@mail.gmail.com> In-Reply-To: <20180919160819.lfzrey3upzri4tll@mutt-hbsd> References: <201809191602.w8JG2Y0X046254@repo.freebsd.org> <20180919160819.lfzrey3upzri4tll@mutt-hbsd>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Sep 19, 2018 at 6:08 PM, Shawn Webb <shawn.webb@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>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHF6DbqFpT2jSFrbEOTvmN0e=Oz8ZZco1PRhN36PNMDpQA>