Re: uvm amap: Simplify amap traversal in amap_swap_off

2016-04-10 Thread Stefan Kempf
I'd like to commit this soon unless there are objections.

Stefan Kempf wrote:
> The recent uvm commits fixed hangs because machines went out of memory
> because of using too much space for amap slots.
> 
> It's possible to shrink memory requirements for amaps even more,
> but the current code needs some simplifications first. So here's another
> cleanup diff. There'll be one or two more before we get to the real stuff.
> 
> This one simplifies the traversal of an amap when swap is disabled.
> 
> There's no need to insert marker elements to find the next item in the
> amap list. The next amap can be determined by looking at the
> currently examined amap.
> 
> Care must be taken to get the next element before the current amap is
> possibly deleted, and after all the current amap's pages were read in
> from swap (because the page-in may sleep and remove items from the amap
> list).
> 
> ok?
> 
> Index: uvm/uvm_amap.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_amap.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 uvm_amap.c
> --- uvm/uvm_amap.c16 Mar 2016 16:53:43 -  1.62
> +++ uvm/uvm_amap.c17 Mar 2016 18:08:29 -
> @@ -902,21 +902,11 @@ amap_swap_off(int startslot, int endslot
>  {
>   struct vm_amap *am;
>   struct vm_amap *am_next;
> - struct vm_amap marker_prev;
> - struct vm_amap marker_next;
>   boolean_t rv = FALSE;
>  
> -#if defined(DIAGNOSTIC)
> - memset(_prev, 0, sizeof(marker_prev));
> - memset(_next, 0, sizeof(marker_next));
> -#endif /* defined(DIAGNOSTIC) */
> -
>   for (am = LIST_FIRST(_list); am != NULL && !rv; am = am_next) {
>   int i;
>  
> - LIST_INSERT_BEFORE(am, _prev, am_list);
> - LIST_INSERT_AFTER(am, _next, am_list);
> -
>   for (i = 0; i < am->am_nused; i++) {
>   int slot;
>   int swslot;
> @@ -935,23 +925,14 @@ amap_swap_off(int startslot, int endslot
>   rv = uvm_anon_pagein(anon);
>  
>   am->am_flags &= ~AMAP_SWAPOFF;
> - if (amap_refs(am) == 0) {
> - amap_wipeout(am);
> - am = NULL;
> + if (rv || amap_refs(am) == 0)
>   break;
> - }
> - if (rv) {
> - break;
> - }
>   i = 0;
>   }
>  
> - KASSERT(LIST_NEXT(_prev, am_list) == _next ||
> - LIST_NEXT(LIST_NEXT(_prev, am_list), am_list) ==
> - _next);
> - am_next = LIST_NEXT(_next, am_list);
> - LIST_REMOVE(_prev, am_list);
> - LIST_REMOVE(_next, am_list);
> + am_next = LIST_NEXT(am, am_list);
> + if (amap_refs(am) == 0)
> + amap_wipeout(am);
>   }
>  
>   return rv;



uvm amap: Simplify amap traversal in amap_swap_off

2016-03-19 Thread Stefan Kempf
The recent uvm commits fixed hangs because machines went out of memory
because of using too much space for amap slots.

It's possible to shrink memory requirements for amaps even more,
but the current code needs some simplifications first. So here's another
cleanup diff. There'll be one or two more before we get to the real stuff.

This one simplifies the traversal of an amap when swap is disabled.

There's no need to insert marker elements to find the next item in the
amap list. The next amap can be determined by looking at the
currently examined amap.

Care must be taken to get the next element before the current amap is
possibly deleted, and after all the current amap's pages were read in
from swap (because the page-in may sleep and remove items from the amap
list).

ok?

Index: uvm/uvm_amap.c
===
RCS file: /cvs/src/sys/uvm/uvm_amap.c,v
retrieving revision 1.62
diff -u -p -r1.62 uvm_amap.c
--- uvm/uvm_amap.c  16 Mar 2016 16:53:43 -  1.62
+++ uvm/uvm_amap.c  17 Mar 2016 18:08:29 -
@@ -902,21 +902,11 @@ amap_swap_off(int startslot, int endslot
 {
struct vm_amap *am;
struct vm_amap *am_next;
-   struct vm_amap marker_prev;
-   struct vm_amap marker_next;
boolean_t rv = FALSE;
 
-#if defined(DIAGNOSTIC)
-   memset(_prev, 0, sizeof(marker_prev));
-   memset(_next, 0, sizeof(marker_next));
-#endif /* defined(DIAGNOSTIC) */
-
for (am = LIST_FIRST(_list); am != NULL && !rv; am = am_next) {
int i;
 
-   LIST_INSERT_BEFORE(am, _prev, am_list);
-   LIST_INSERT_AFTER(am, _next, am_list);
-
for (i = 0; i < am->am_nused; i++) {
int slot;
int swslot;
@@ -935,23 +925,14 @@ amap_swap_off(int startslot, int endslot
rv = uvm_anon_pagein(anon);
 
am->am_flags &= ~AMAP_SWAPOFF;
-   if (amap_refs(am) == 0) {
-   amap_wipeout(am);
-   am = NULL;
+   if (rv || amap_refs(am) == 0)
break;
-   }
-   if (rv) {
-   break;
-   }
i = 0;
}
 
-   KASSERT(LIST_NEXT(_prev, am_list) == _next ||
-   LIST_NEXT(LIST_NEXT(_prev, am_list), am_list) ==
-   _next);
-   am_next = LIST_NEXT(_next, am_list);
-   LIST_REMOVE(_prev, am_list);
-   LIST_REMOVE(_next, am_list);
+   am_next = LIST_NEXT(am, am_list);
+   if (amap_refs(am) == 0)
+   amap_wipeout(am);
}
 
return rv;