svn commit: r331369 - head/sys/vm
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. Protect the
vmd_free_count with atomics.
This allows us to allocate and free from reservations without the free lock
except where a superpage is allocated from the physical layer, which is
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(r331368)
+++ head/sys/vm/vm_page.c Thu Mar 22 19:21:11 2018(r331369)
@@ -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 pagebits);
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_pindex_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 req, 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_min) ||
- (req == VM_ALLOC_INTERRUPT &&
- vmd->vmd_free_count >= npages))
- return (1);
+ /*
+* Attempt to reserve the pages. Fail if we're below the limit.
+*/
+ 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, vm_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, mpred)) !=
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
-
Re: svn commit: r331369 - head/sys/vm
This broke gcc builds.
On Thu, Mar 22, 2018 at 2:21 PM, Jeff Roberson 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. Protect the
> vmd_free_count with atomics.
>
> This allows us to allocate and free from reservations without the free lock
> except where a superpage is allocated from the physical layer, which is
> 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(r331368)
> +++ head/sys/vm/vm_page.c Thu Mar 22 19:21:11 2018(r331369)
> @@ -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 pagebits);
> 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_pindex_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 req, 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_min) ||
> - (req == VM_ALLOC_INTERRUPT &&
> - vmd->vmd_free_count >= npages))
> - return (1);
> + /*
> +* Attempt to reserve the pages. Fail if we're below the limit.
> +*/
> + 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, vm_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, mpred)) !=
> 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_in
Re: svn commit: r331369 - head/sys/vm
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 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. Protect the
vmd_free_count with atomics.
This allows us to allocate and free from reservations without the free lock
except where a superpage is allocated from the physical layer, which is
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(r331368)
+++ head/sys/vm/vm_page.c Thu Mar 22 19:21:11 2018(r331369)
@@ -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 pagebits);
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_pindex_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 req, 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_min) ||
- (req == VM_ALLOC_INTERRUPT &&
- vmd->vmd_free_count >= npages))
- return (1);
+ /*
+* Attempt to reserve the pages. Fail if we're below the limit.
+*/
+ 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, vm_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, mpred)) !=
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) ||
-
Re: svn commit: r331369 - head/sys/vm
It broke i386 too.
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
#include
-#include
-#include
#include
#include
#include
@@ -55,6 +53,8 @@
#include
#include
#include
+#include
+#include
#include
#include
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 , 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 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. Protect th
> e
> >> vmd_free_count with atomics.
> >>
> >> This allows us to allocate and free from reservations without the free l
> ock
> >> except where a superpage is allocated from the physical layer, which is
> >> 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(r331368)
> >> +++ head/sys/vm/vm_page.c Thu Mar 22 19:21:11 2018(r331369)
> >> @@ -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 pagebits
> );
> >> 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_pindex_
> 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 req
> , 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_min) |
> |
> >> - (req == VM_ALLOC_INTERRUPT &&
> >> - vmd->vmd_free_count >= npages))
> >> - return (1);
> >> + /*
> >> +* Attempt to reserve the pages. Fail if we're below the limit.
> >> +*/
> >> + 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, vm_
> pind
> >> again:
> >> m = NULL;
> >> #if VM_NRESERVLEVEL > 0
> >> + /*
> >> +* Can we allocate the page from a reservation?
> >> +*/
> >> if (vm_object_reserv(obj
Re: svn commit: r331369 - head/sys/vm
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
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
#include
-#include
-#include
#include
#include
#include
@@ -55,6 +53,8 @@
#include
#include
#include
+#include
+#include
#include
#include
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 , 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 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. Protect th
e
vmd_free_count with atomics.
This allows us to allocate and free from reservations without the free l
ock
except where a superpage is allocated from the physical layer, which is
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(r331368)
+++ head/sys/vm/vm_page.c Thu Mar 22 19:21:11 2018(r331369)
@@ -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 pagebits
);
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_pindex_
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 req
, 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_min) |
|
- (req == VM_ALLOC_INTERRUPT &&
- vmd->vmd_free_count >= npages))
- return (1);
+ /*
+* Attempt to reserve the pages. Fail if we're below the limit.
+*/
+ 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, vm_
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, mpred))
!= NULL)) {
domain = vm_phys_domain(m);
vmd = VM_DOMAIN(domain);
Re: svn commit: r331369 - head/sys/vm
On Thu, 22 Mar 2018, Cy Schubert wrote:
It broke i386 too.
I believe I'm not able to reproduce this because it is a result of changes
in kernel config. I can not reproduce it so I can't verify the fix.
Since you can, would you mind committing? I see no problem with the diff
below.
Thanks,
Jeff
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
#include
-#include
-#include
#include
#include
#include
@@ -55,6 +53,8 @@
#include
#include
#include
+#include
+#include
#include
#include
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 , 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 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. Protect th
e
vmd_free_count with atomics.
This allows us to allocate and free from reservations without the free l
ock
except where a superpage is allocated from the physical layer, which is
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(r331368)
+++ head/sys/vm/vm_page.c Thu Mar 22 19:21:11 2018(r331369)
@@ -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 pagebits
);
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_pindex_
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 req
, 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_min) |
|
- (req == VM_ALLOC_INTERRUPT &&
- vmd->vmd_free_count >= npages))
- return (1);
+ /*
+* Attempt to reserve the pages. Fail if we're below the limit.
+*/
+ 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, vm_
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,
Re: svn commit: r331369 - head/sys/vm
In message , 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 --- 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' voidcritical_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' voidcritical_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 > > #include > > -#include > > -#include > > #include > > #include > > #include > > @@ -55,6 +53,8 @@ > > #include > > #include > > #include > > +#include > > +#include > > #include > > #include > > > > 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 , 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 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. Protect > th > >> e > vmd_free_count with atomics. > > This allows us to allocate and free from reservations without the free > 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(r331368 > ) > +++ head/sys/vm/vm_page.c Thu Mar 22 19:21:11 2018(r331369 > ) > @@ -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
Re: svn commit: r331369 - head/sys/vm
On Thu, 22 Mar 2018, Cy Schubert wrote: In message , 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? 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' voidcritical_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' voidcritical_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 #include -#include -#include #include #include #include @@ -55,6 +53,8 @@ #include #include #include +#include +#include #include #include 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 , 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 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. Protect th e vmd_free_count with atomics. This allows us to allocate and free from reservations without the free 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(r331368 ) +++ head/sys/vm/vm_page.c Thu Mar 22 19:21:11 2018(r331369 ) @@ -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 pagebi 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 obj
Re: svn commit: r331369 - head/sys/vm
In message , Jeff Roberson
writes:
> On Thu, 22 Mar 2018, Cy Schubert wrote:
>
> > It broke i386 too.
>
> I believe I'm not able to reproduce this because it is a result of changes
> in kernel config. I can not reproduce it so I can't verify the fix.
> Since you can, would you mind committing? I see no problem with the diff
> below.
I don't mind.
>
> Thanks,
> Jeff
>
> >
> > 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
> > #include
> > -#include
> > -#include
> > #include
> > #include
> > #include
> > @@ -55,6 +53,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > #include
> > #include
> >
> > 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 , 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 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. Protect
> th
> >> e
> vmd_free_count with atomics.
>
> This allows us to allocate and free from reservations without the free
> 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(r331368
> )
> +++ head/sys/vm/vm_page.c Thu Mar 22 19:21:11 2018(r331369
> )
> @@ -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 pagebi
> 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_pinde
> 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_min)
> |
> >> |
> - (req == VM_ALLOC_INTERRUPT &&
> - vmd->vmd_free_count >= npages))
> - return (1);
> + /*
> +* Attempt to reserve the pages. Fail if we're below the limit.
> +*/
> + 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. */
> +
Re: svn commit: r331369 - head/sys/vm
In message , Jeff Roberson writes: > On Thu, 22 Mar 2018, Cy Schubert wrote: > > > In message , 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 #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' > > voidcritical_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' > > voidcritical_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 > >>> #include > >>> -#include > >>> -#include > >>> #include > >>> #include > >>> #include > >>> @@ -55,6 +53,8 @@ > >>> #include > >>> #include > >>> #include > >>> +#include > >>> +#include > >>> #include > >>> #include > >>> > >>> 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 , 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 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
Re: svn commit: r331369 - head/sys/vm
On Thu, 22 Mar 2018, Jeff Roberson wrote: 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? 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 #include -#include -#include #include #include #include @@ -55,6 +53,8 @@ #include #include #include +#include +#include #include #include This is because sys/i386/include/machine.h uses critical_enter() and critical_exit() which are defined in sys/systm.h. Wrong fix. I see you committed this. Now there are more bugs to fix. is a prerequisite for all kernel headers except , since it defines and declares things like KASSERT() and critical_enter() which might be used in other headers (except sys/param.h and its standard pollution). Sometimes sys/systm.h is included as undocumented namespace pollution in headers that are accidentally included before the (other) ones that use KASSERT(), etc. The headers that have this bug have it to work around bugs in .c files like the one above. It is more usual to have this bug by not including sys/systm.h at all than to have it by including it in a wrong order. Sorting it alphabetically almost always gives a wrong order. It must be included after sys/param.h and that must be included first. It is a related bug to include only sys/types.h and not sys/param.h. This requires chumminess with the current implementation and all future implementations. sys/param.h provides certain undocumented but standard namespace pollution which might vary with the implementation, as necessary to satisfy some of the pollution requirements of all current and future implementations of other headers. (The pollution should be monotonically decreasing but it was only that for a few years about 20 years ago when I worked on fixing it.) .c files that include sys/types.h instead of sys/param.h have do some subset of the includes in sys/param.h. Since nothing is documented and the subset might depend on the arch and user options, it is hard to know the minimal subset. .c files that include sys/types.h tend to have lots of other #include bugs like not including sys/systm.h. Again it is hard to know the minimal replacement for sys/systm.h and its undocumented but standard pollution. It is a style bug to include both sys/types.h and sys/param.h. style(9) even explicitly forbids including both. It is a larger style bug to include the standard pollution in sys/systm.h direction. This includes especially and . These should be considered as being implemented in sys/systm.h, with the headers for them only and implementation detail. Similarly for . It built nicely on my amd64's though. amd64 apparently has more namespace pollution which breaks detection of the bug. But I couldn't find where it is. sys/systm.h isn't included nested in any amd64 or x86 headers. Apparently some amd64 option gives it. Bruce ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r331369 - head/sys/vm
In message <20180323150709.h...@besplex.bde.org>, Bruce Evans writes: > On Thu, 22 Mar 2018, Jeff Roberson wrote: > > > 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? > >> > >> 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 > >> #include > >> -#include > >> -#include > >> #include > >> #include > >> #include > >> @@ -55,6 +53,8 @@ > >> #include > >> #include > >> #include > >> +#include > >> +#include > >> #include > >> #include > >> > >> This is because sys/i386/include/machine.h uses critical_enter() and > >> critical_exit() which are defined in sys/systm.h. > > Wrong fix. I see you committed this. Now there are more bugs to fix. > > is a prerequisite for all kernel headers except > , since it defines and declares things like KASSERT() and > critical_enter() which might be used in other headers (except > sys/param.h and its standard pollution). Sometimes sys/systm.h is > included as undocumented namespace pollution in headers that are > accidentally included before the (other) ones that use KASSERT(), etc. > The headers that have this bug have it to work around bugs in .c files > like the one above. It is more usual to have this bug by not including > sys/systm.h at all than to have it by including it in a wrong order. > Sorting it alphabetically almost always gives a wrong order. It must > be included after sys/param.h and that must be included first. Agreed on alphabetic sorting. > > It is a related bug to include only sys/types.h and not sys/param.h. > This requires chumminess with the current implementation and all > future implementations. sys/param.h provides certain undocumented > but standard namespace pollution which might vary with the implementation, > as necessary to satisfy some of the pollution requirements of all current > and future implementations of other headers. (The pollution should be > monotonically decreasing but it was only that for a few years about 20 > years ago when I worked on fixing it.) .c files that include sys/types.h > instead of sys/param.h have do some subset of the includes in sys/param.h. > Since nothing is documented and the subset might depend on the arch and > user options, it is hard to know the minimal subset. That's not the case here. sys/types.h is not included in this file but point taken. > > .c files that include sys/types.h tend to have lots of other #include > bugs like not including sys/systm.h. Again it is hard to know the > minimal replacement for sys/systm.h and its undocumented but standard > pollution. It is a style bug to include both sys/types.h and sys/param.h. > style(9) even explicitly forbids including both. It is a larger style > bug to include the standard pollution in sys/systm.h direction. This > includes especially and . These > should be considered as being implemented in sys/systm.h, with the > headers for them only and implementation detail. Similarly > for . > > >> It built nicely on my amd64's though. > > amd64 apparently has more namespace pollution which breaks detection > of the bug. But I couldn't find where it is. sys/systm.h isn't included > nested in any amd64 or x86 headers. Apparently some amd64 option gives > it. The reason is amd64 doesn't use critical_enter() and critical_exit() because counter_enter() and counter_exit() are NOPs. The reason they are NOPs in amd64 and not in i386 is not all i386 processors support cmpxchg8b. It is only then that the critical_*() functions are called. > > Bruce I can create a phabricator revision to clean this instance up and move sys/systm.h just after sys/param.h. I'm just about to head out of town so I'll create it after I get back, after April 4. Thank you for your input Bruce. -- Cheers, Cy Schubert FreeBSD UNIX: Web: http://www.FreeBSD.org The need of the many outweighs the greed of the few. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r331369 - head/sys/vm
In message <201803230615.w2n6ftmj040...@slippy.cwsent.com>, Cy Schubert
writes:
> In message <20180323150709.h...@besplex.bde.org>, Bruce Evans writes:
> > On Thu, 22 Mar 2018, Jeff Roberson wrote:
> >
> > > 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?
> > >>
> > >> 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
> > >> #include
> > >> -#include
> > >> -#include
> > >> #include
> > >> #include
> > >> #include
> > >> @@ -55,6 +53,8 @@
> > >> #include
> > >> #include
> > >> #include
> > >> +#include
> > >> +#include
> > >> #include
> > >> #include
> > >>
> > >> This is because sys/i386/include/machine.h uses critical_enter() and
> > >> critical_exit() which are defined in sys/systm.h.
> >
> > Wrong fix. I see you committed this. Now there are more bugs to fix.
> >
> > is a prerequisite for all kernel headers except
> > , since it defines and declares things like KASSERT() and
> > critical_enter() which might be used in other headers (except
> > sys/param.h and its standard pollution). Sometimes sys/systm.h is
> > included as undocumented namespace pollution in headers that are
> > accidentally included before the (other) ones that use KASSERT(), etc.
> > The headers that have this bug have it to work around bugs in .c files
> > like the one above. It is more usual to have this bug by not including
> > sys/systm.h at all than to have it by including it in a wrong order.
> > Sorting it alphabetically almost always gives a wrong order. It must
> > be included after sys/param.h and that must be included first.
>
> Agreed on alphabetic sorting.
>
> >
> > It is a related bug to include only sys/types.h and not sys/param.h.
> > This requires chumminess with the current implementation and all
> > future implementations. sys/param.h provides certain undocumented
> > but standard namespace pollution which might vary with the implementation,
> > as necessary to satisfy some of the pollution requirements of all current
> > and future implementations of other headers. (The pollution should be
> > monotonically decreasing but it was only that for a few years about 20
> > years ago when I worked on fixing it.) .c files that include sys/types.h
> > instead of sys/param.h have do some subset of the includes in sys/param.h.
> > Since nothing is documented and the subset might depend on the arch and
> > user options, it is hard to know the minimal subset.
>
> That's not the case here. sys/types.h is not included in this file but
> point taken.
>
> >
> > .c files that include sys/types.h tend to have lots of other #include
> > bugs like not including sys/systm.h. Again it is hard to know the
> > minimal replacement for sys/systm.h and its undocumented but standard
> > pollution. It is a style bug to include both sys/types.h and sys/param.h.
> > style(9) even explicitly forbids including both. It is a larger style
> > bug to include the standard pollution in sys/systm.h direction. This
> > includes especially and . These
> > should be considered as being implemented in sys/systm.h, with the
> > headers for them only and implementation detail. Similarly
> > for .
> >
> > >> It built nicely on my amd64's though.
> >
> > amd64 apparently has more namespace pollution which breaks detection
> > of the bug. But I couldn't find where it is. sys/systm.h isn't included
> > nested in any amd64 or x86 headers. Apparently some amd64 option gives
> > it.
>
> The reason is amd64 doesn't use critical_enter() and critical_exit()
> because counter_enter() and counter_exit() are NOPs. The reason they
> are NOPs in amd64 and not in i386 is not all i386 processors support
> cmpxchg8b. It is only then that the critical_*() functions are called.
>
> >
> > Bruce
>
> I can create a phabricator revision to clean this instance up and move
> sys/systm.h just after sys/param.h. I'm just about to head out of town
> so I'll create it after I get back, after April 4.
>
> Thank you for your input Bruce.
Hi Bruce,
Can you please give this a once over?
diff --git a/sys/vm/vm_reserv.c b/sys/vm/vm_reserv.c
index d8869e3bdbe..6d31d79da39 100644
--- a/sys/vm/vm_reserv.c
+++ b/sys/vm/vm_reserv.c
@@ -44,6 +44,7 @@ __FBSDID("$FreeBSD$");
#include "opt_vm.h"
#include
+#include
#include
#include
#include
@@ -52,7 +53,6 @@ __FBSDID("$FreeBSD$");
#include
#include
#include
-#include
#include
#include
#include
--
Cheers,
Cy Schubert
FreeBSD UNIX: Web: http://www.FreeBSD.org
The need of the many outweighs the greed of the few.
___
svn-src-head@freebsd.org mailing list
https://lists.freebs
Re: svn commit: r331369 - head/sys/vm
On Sat, 7 Apr 2018, Cy Schubert wrote:
In message <201803230615.w2n6ftmj040...@slippy.cwsent.com>, Cy Schubert
writes:
In message <20180323150709.h...@besplex.bde.org>, Bruce Evans writes:
On Thu, 22 Mar 2018, Jeff Roberson wrote:
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?
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
#include
-#include
-#include
#include
#include
#include
@@ -55,6 +53,8 @@
#include
#include
#include
+#include
+#include
#include
#include
This is because sys/i386/include/machine.h uses critical_enter() and
critical_exit() which are defined in sys/systm.h.
Wrong fix. I see you committed this. Now there are more bugs to fix.
is a prerequisite for all kernel headers except
, since it defines and declares things like KASSERT() and
...
Can you please give this a once over?
diff --git a/sys/vm/vm_reserv.c b/sys/vm/vm_reserv.c
index d8869e3bdbe..6d31d79da39 100644
--- a/sys/vm/vm_reserv.c
+++ b/sys/vm/vm_reserv.c
@@ -44,6 +44,7 @@ __FBSDID("$FreeBSD$");
#include "opt_vm.h"
#include
+#include
#include
#include
#include
@@ -52,7 +53,6 @@ __FBSDID("$FreeBSD$");
#include
#include
#include
-#include
#include
#include
#include
It is missing backing out the unsorting of counter.h and ktr.h.
This should be fixed in the same commit, so that it only takes 3 instead
of 4 commits to do the sorting. Reverting changes in historical order
is too much for small patches, and doesn't work when the intermediate
version is broken.
Bruce
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
