Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 22 Mar 2018 19:40:35 -0700
From:      Cy Schubert <Cy.Schubert@cschubert.com>
To:        Jeff Roberson <jroberson@jroberson.net>
Cc:        Cy Schubert <Cy.Schubert@cschubert.com>, Justin Hibbits <jrh29@alumni.cwru.edu>, Jeff Roberson <jeff@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r331369 - head/sys/vm
Message-ID:  <201803230240.w2N2eZDb005402@slippy.cwsent.com>
In-Reply-To: Message from Jeff Roberson <jroberson@jroberson.net> of "Thu, 22 Mar 2018 15:53:51 -1000." <alpine.BSF.2.21.1803221553380.2307@desktop>

next in thread | previous in thread | raw e-mail | index | archive | help
In message <alpine.BSF.2.21.1803221553380.2307@desktop>, Jeff Roberson 
writes:
> On Thu, 22 Mar 2018, Cy Schubert wrote:
>
> > In message <alpine.BSF.2.21.1803221518200.2307@desktop>, Jeff Roberson
> > writes:
> >> On Thu, 22 Mar 2018, Cy Schubert wrote:
> >>
> >>> It broke i386 too.
> >>
> >> I just did
> >> TARGET_ARCH=i386 make buildworld
> >> TARGET_ARCH=i386 make buildkernel
> >>
> >> This worked for me?
> >>
> >> Jeff
> >
> > hmmm.
> >
> > make TARGET=i386 TARGET_ARCH=i386 buildkernel
>
> Do you have changes to GENERIC?

Not to GENERIC but I used a different kernel by default that removes 
most drivers (as I load them dynamically -- single kernel different 
modules on each machine).

Ahh but this particular i386 kernel I was building does not include 
INVARIANTS. sys/i386/include/counter.h at line 35 has:

#ifdef INVARIANTS
#include <sys/proc.h>
#endif

sys/proc.h includes sys/systm.h.

i386 without INVARIANTS will fail to build. With INVARIANTS i386 does 
build.

I'll commit the fix.

~cy


>
> Thanks,
> Jeff
> >
> > --- vm_reserv.o ---
> > In file included from /opt/src/svn-current/sys/vm/vm_reserv.c:48:
> > In file included from /opt/src/svn-current/sys/sys/counter.h:37:
> > ./machine/counter.h:174:3: error: implicit declaration of function
> > 'critical_enter' is invalid in C99 [-Werror,-Wimplicit-function-declarat
> > ion]
> >                critical_enter();
> >                ^
> > ./machine/counter.h:174:3: error: this function declaration is not a
> > prototype [-Werror,-Wstrict-prototypes]
> > ./machine/counter.h:176:3: error: implicit declaration of function
> > 'critical_exit' is invalid in C99 [-Werror,-Wimplicit-function-declarati
> > on]
> >                critical_exit();
> >                ^
> > ./machine/counter.h:176:3: note: did you mean 'critical_enter'?
> > ./machine/counter.h:174:3: note: 'critical_enter' declared here
> >                critical_enter();
> >                ^
> > ./machine/counter.h:176:3: error: this function declaration is not a
> > prototype [-Werror,-Wstrict-prototypes]
> >                critical_exit();
> >                ^
> > In file included from /opt/src/svn-current/sys/vm/vm_reserv.c:57:
> > /opt/src/svn-current/sys/sys/systm.h:217:6: error: conflicting types
> > for 'critical_enter'
> > void    critical_enter(void);
> >        ^
> > ./machine/counter.h:174:3: note: previous implicit declaration is here
> >                critical_enter();
> >                ^
> > In file included from /opt/src/svn-current/sys/vm/vm_reserv.c:57:
> > /opt/src/svn-current/sys/sys/systm.h:218:6: error: conflicting types
> > for 'critical_exit'
> > void    critical_exit(void);
> >        ^
> > ./machine/counter.h:176:3: note: previous implicit declaration is here
> >                critical_exit();
> >                ^
> > 6 errors generated.
> > *** [vm_reserv.o] Error code 1
> >
> > make[2]: stopped in /export/obj/opt/src/svn-current/i386.i386/sys/BREAK
> > --- modules-all ---
> > A failure has been detected in another branch of the parallel make
> >
> > make[3]: stopped in /opt/src/svn-current/sys/modules
> > *** [modules-all] Error code 2
> >
> > make[2]: stopped in /export/obj/opt/src/svn-current/i386.i386/sys/BREAK
> > 2 errors
> >
> > make[2]: stopped in /export/obj/opt/src/svn-current/i386.i386/sys/BREAK
> > *** [buildkernel] Error code 2
> >
> > make[1]: stopped in /opt/src/svn-current
> > 1 error
> >
> > make[1]: stopped in /opt/src/svn-current
> > *** [buildkernel] Error code 2
> >
> > make: stopped in /opt/src/svn-current
> > 1 error
> >
> > make: stopped in /opt/src/svn-current
> >
> > ~cy
> >
> >>
> >>>
> >>> Index: sys/vm/vm_reserv.c
> >>> ===================================================================
> >>> --- sys/vm/vm_reserv.c	(revision 331399)
> >>> +++ sys/vm/vm_reserv.c	(working copy)
> >>> @@ -45,8 +45,6 @@
> >>>
> >>> #include <sys/param.h>
> >>> #include <sys/kernel.h>
> >>> -#include <sys/counter.h>
> >>> -#include <sys/ktr.h>
> >>> #include <sys/lock.h>
> >>> #include <sys/malloc.h>
> >>> #include <sys/mutex.h>
> >>> @@ -55,6 +53,8 @@
> >>> #include <sys/sbuf.h>
> >>> #include <sys/sysctl.h>
> >>> #include <sys/systm.h>
> >>> +#include <sys/counter.h>
> >>> +#include <sys/ktr.h>
> >>> #include <sys/vmmeter.h>
> >>> #include <sys/smp.h>
> >>>
> >>> This is because sys/i386/include/machine.h uses critical_enter() and
> >>> critical_exit() which are defined in sys/systm.h.
> >>>
> >>> It built nicely on my amd64's though.
> >>>
> >>> ~cy
> >>>
> >>> In message <alpine.BSF.2.21.1803221451420.2307@desktop>, Jeff Roberson
> >>> writes:
> >>>> Thank you, working on it.  I had done a make universe before getting
> >>>> review feedback.
> >>>>
> >>>> Jeff
> >>>>
> >>>> On Thu, 22 Mar 2018, Justin Hibbits wrote:
> >>>>
> >>>>> This broke gcc builds.
> >>>>>
> >>>>> On Thu, Mar 22, 2018 at 2:21 PM, Jeff Roberson <jeff@freebsd.org> wrote
> :
> >>>>>> Author: jeff
> >>>>>> Date: Thu Mar 22 19:21:11 2018
> >>>>>> New Revision: 331369
> >>>>>> URL: https://svnweb.freebsd.org/changeset/base/331369
> >>>>>>
> >>>>>> Log:
> >>>>>>   Lock reservations with a dedicated lock in each reservation.  Protec
> t
> >> th
> >>>> e
> >>>>>>   vmd_free_count with atomics.
> >>>>>>
> >>>>>>   This allows us to allocate and free from reservations without the fr
> ee
> >>  l
> >>>> ock
> >>>>>>   except where a superpage is allocated from the physical layer, which
>  i
> >> s
> >>>>>>   roughly 1/512 of the operations on amd64.
> >>>>>>
> >>>>>>   Use the counter api to eliminate cache conention on counters.
> >>>>>>
> >>>>>>   Reviewed by:  markj
> >>>>>>   Tested by:    pho
> >>>>>>   Sponsored by: Netflix, Dell/EMC Isilon
> >>>>>>   Differential Revision:        https://reviews.freebsd.org/D14707
> >>>>>>
> >>>>>> Modified:
> >>>>>>   head/sys/vm/vm_page.c
> >>>>>>   head/sys/vm/vm_pagequeue.h
> >>>>>>   head/sys/vm/vm_reserv.c
> >>>>>>   head/sys/vm/vm_reserv.h
> >>>>>>
> >>>>>> Modified: head/sys/vm/vm_page.c
> >>>>>> ======================================================================
> ==
> >> ==
> >>>> ====
> >>>>>> --- head/sys/vm/vm_page.c       Thu Mar 22 19:11:43 2018        (r3313
> 68
> >> )
> >>>>>> +++ head/sys/vm/vm_page.c       Thu Mar 22 19:21:11 2018        (r3313
> 69
> >> )
> >>>>>> @@ -177,7 +177,6 @@ static uma_zone_t fakepg_zone;
> >>>>>>  static void vm_page_alloc_check(vm_page_t m);
> >>>>>>  static void vm_page_clear_dirty_mask(vm_page_t m, vm_page_bits_t page
> bi
> >> ts
> >>>> );
> >>>>>>  static void vm_page_enqueue(uint8_t queue, vm_page_t m);
> >>>>>> -static void vm_page_free_phys(struct vm_domain *vmd, vm_page_t m);
> >>>>>>  static void vm_page_init(void *dummy);
> >>>>>>  static int vm_page_insert_after(vm_page_t m, vm_object_t object,
> >>>>>>      vm_pindex_t pindex, vm_page_t mpred);
> >>>>>> @@ -1677,10 +1676,10 @@ vm_page_alloc_after(vm_object_t object, vm_pin
> de
> >> x_
> >>>> t pi
> >>>>>>   * for the request class and false otherwise.
> >>>>>>   */
> >>>>>>  int
> >>>>>> -vm_domain_available(struct vm_domain *vmd, int req, int npages)
> >>>>>> +vm_domain_allocate(struct vm_domain *vmd, int req, int npages)
> >>>>>>  {
> >>>>>> +       u_int limit, old, new;
> >>>>>>
> >>>>>> -       vm_domain_free_assert_locked(vmd);
> >>>>>>         req = req & VM_ALLOC_CLASS_MASK;
> >>>>>>
> >>>>>>         /*
> >>>>>> @@ -1688,15 +1687,34 @@ vm_domain_available(struct vm_domain *vmd, int
>  r
> >> eq
> >>>> , in
> >>>>>>          */
> >>>>>>         if (curproc == pageproc && req != VM_ALLOC_INTERRUPT)
> >>>>>>                 req = VM_ALLOC_SYSTEM;
> >>>>>> +       if (req == VM_ALLOC_INTERRUPT)
> >>>>>> +               limit = 0;
> >>>>>> +       else if (req == VM_ALLOC_SYSTEM)
> >>>>>> +               limit = vmd->vmd_interrupt_free_min;
> >>>>>> +       else
> >>>>>> +               limit = vmd->vmd_free_reserved;
> >>>>>>
> >>>>>> -       if (vmd->vmd_free_count >= npages + vmd->vmd_free_reserved ||
> >>>>>> -           (req == VM_ALLOC_SYSTEM &&
> >>>>>> -           vmd->vmd_free_count >= npages + vmd->vmd_interrupt_free_mi
> n)
> >>  |
> >>>> |
> >>>>>> -           (req == VM_ALLOC_INTERRUPT &&
> >>>>>> -           vmd->vmd_free_count >= npages))
> >>>>>> -               return (1);
> >>>>>> +       /*
> >>>>>> +        * Attempt to reserve the pages.  Fail if we're below the limi
> t.
> >>>>>> +        */
> >>>>>> +       limit += npages;
> >>>>>> +       old = vmd->vmd_free_count;
> >>>>>> +       do {
> >>>>>> +               if (old < limit)
> >>>>>> +                       return (0);
> >>>>>> +               new = old - npages;
> >>>>>> +       } while (atomic_fcmpset_int(&vmd->vmd_free_count, &old, new) =
> =
> >> 0)
> >>>> ;
> >>>>>>
> >>>>>> -       return (0);
> >>>>>> +       /* Wake the page daemon if we've crossed the threshold. */
> >>>>>> +       if (vm_paging_needed(vmd, new) && !vm_paging_needed(vmd, old))
> >>>>>> +               pagedaemon_wakeup(vmd->vmd_domain);
> >>>>>> +
> >>>>>> +       /* Only update bitsets on transitions. */
> >>>>>> +       if ((old >= vmd->vmd_free_min && new < vmd->vmd_free_min) ||
> >>>>>> +           (old >= vmd->vmd_free_severe && new < vmd->vmd_free_severe
> ))
> >>>>>> +               vm_domain_set(vmd);
> >>>>>> +
> >>>>>> +       return (1);
> >>>>>>  }
> >>>>>>
> >>>>>>  vm_page_t
> >>>>>> @@ -1723,44 +1741,34 @@ vm_page_alloc_domain_after(vm_object_t object,
>  v
> >> m_
> >>>> pind
> >>>>>>  again:
> >>>>>>         m = NULL;
> >>>>>>  #if VM_NRESERVLEVEL > 0
> >>>>>> +       /*
> >>>>>> +        * Can we allocate the page from a reservation?
> >>>>>> +        */
> >>>>>>         if (vm_object_reserv(object) &&
> >>>>>> -           (m = vm_reserv_extend(req, object, pindex, domain, mpred))
> >>>>>> -           != NULL) {
> >>>>>> +           ((m = vm_reserv_extend(req, object, pindex, domain, mpred)
> )
> >> !=
> >>>>  NULL ||
> >>>>>> +           (m = vm_reserv_alloc_page(req, object, pindex, domain, mpr
> ed
> >> ))
> >>>>  != NULL)) {
> >>>>>>                 domain = vm_phys_domain(m);
> >>>>>>                 vmd = VM_DOMAIN(domain);
> >>>>>>                 goto found;
> >>>>>>         }
> >>>>>>  #endif
> >>>>>>         vmd = VM_DOMAIN(domain);
> >>>>>> -       vm_domain_free_lock(vmd);
> >>>>>> -       if (vm_domain_available(vmd, req, 1)) {
> >>>>>> +       if (vm_domain_allocate(vmd, req, 1)) {
> >>>>>>                 /*
> >>>>>> -                * Can we allocate the page from a reservation?
> >>>>>> +                * If not, allocate it from the free page queues.
> >>>>>>                  */
> >>>>>> +               vm_domain_free_lock(vmd);
> >>>>>> +               m = vm_phys_alloc_pages(domain, object != NULL ?
> >>>>>> +                   VM_FREEPOOL_DEFAULT : VM_FREEPOOL_DIRECT, 0);
> >>>>>> +               vm_domain_free_unlock(vmd);
> >>>>>> +               if (m == NULL) {
> >>>>>> +                       vm_domain_freecnt_inc(vmd, 1);
> >>>>>>  #if VM_NRESERVLEVEL > 0
> >>>>>> -               if (!vm_object_reserv(object) ||
> >>>>>> -                   (m = vm_reserv_alloc_page(object, pindex,
> >>>>>> -                   domain, mpred)) == NULL)
> >>>>>> +                       if (vm_reserv_reclaim_inactive(domain))
> >>>>>> +                               goto again;
> >>>>>>  #endif
> >>>>>> -               {
> >>>>>> -                       /*
> >>>>>> -                        * If not, allocate it from the free page queu
> es
> >> .
> >>>>>> -                        */
> >>>>>> -                       m = vm_phys_alloc_pages(domain, object != NULL
>  ?
> >>>>>> -                           VM_FREEPOOL_DEFAULT : VM_FREEPOOL_DIRECT, 
> 0)
> >> ;
> >>>>>> -#if VM_NRESERVLEVEL > 0
> >>>>>> -                       if (m == NULL && vm_reserv_reclaim_inactive(do
> ma
> >> in
> >>>> )) {
> >>>>>> -                               m = vm_phys_alloc_pages(domain,
> >>>>>> -                                   object != NULL ?
> >>>>>> -                                   VM_FREEPOOL_DEFAULT : VM_FREEPOOL_
> DI
> >> RE
> >>>> CT,
> >>>>>> -                                   0);
> >>>>>> -                       }
> >>>>>> -#endif
> >>>>>>                 }
> >>>>>>         }
> >>>>>> -       if (m != NULL)
> >>>>>> -               vm_domain_freecnt_dec(vmd, 1);
> >>>>>> -       vm_domain_free_unlock(vmd);
> >>>>>>         if (m == NULL) {
> >>>>>>                 /*
> >>>>>>                  * Not allocatable, give up.
> >>>>>> @@ -1775,9 +1783,7 @@ again:
> >>>>>>          */
> >>>>>>         KASSERT(m != NULL, ("missing page"));
> >>>>>>
> >>>>>> -#if VM_NRESERVLEVEL > 0
> >>>>>>  found:
> >>>>>> -#endif
> >>>>>
> >>>>> 'found' is now declared, but unused on powerpc64.
> >>>>>
> >>>>> - Justin
> >>>>>
> >>>>
> >>>
> >>> --
> >>> Cheers,
> >>> Cy Schubert <Cy.Schubert@cschubert.com>
> >>> FreeBSD UNIX:  <cy@FreeBSD.org>   Web:  http://www.FreeBSD.org
> >>>
> >>> 	The need of the many outweighs the greed of the few.
> >>>
> >>>
> >
> > -- 
> > Cheers,
> > Cy Schubert <Cy.Schubert@cschubert.com>
> > FreeBSD UNIX:  <cy@FreeBSD.org>   Web:  http://www.FreeBSD.org
> >
> > 	The need of the many outweighs the greed of the few.
> >
> >

-- 
Cheers,
Cy Schubert <Cy.Schubert@cschubert.com>
FreeBSD UNIX:  <cy@FreeBSD.org>   Web:  http://www.FreeBSD.org

	The need of the many outweighs the greed of the few.





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