Re: svn commit: r331732 - head/sys/vm

[Tijl Coosemans]

On Sun, 1 Apr 2018 12:27:03 -0400 Mark Johnston  wrote:
> On Sun, Apr 01, 2018 at 05:20:21PM +0200, Tijl Coosemans wrote:
>> On Sat, 31 Mar 2018 18:54:32 -0400 Mark Johnston  wrote:  
>>> On Sat, Mar 31, 2018 at 08:21:18PM +0200, Tijl Coosemans wrote:  
 On Thu, 29 Mar 2018 14:27:40 + (UTC) Mark Johnston  
 wrote:
> Author: markj
> Date: Thu Mar 29 14:27:40 2018
> New Revision: 331732
> URL: https://svnweb.freebsd.org/changeset/base/331732
> 
> Log:
>   Fix the background laundering mechanism after r329882.
>   
>   Rather than using the number of inactive queue scans as a metric for
>   how many clean pages are being freed by the page daemon, have the
>   page daemon keep a running counter of the number of pages it has freed,
>   and have the laundry thread use that when computing the background
>   laundering threshold.
> [...]
 
 I'm seeing big processes being killed with an "out of swap space" message
 even though there's still plenty of swap available.  It seems to be fixed
 by making this division round upwards:
 
if (target == 0 && ndirty * isqrt((nfreed +
(vmd->vmd_free_target - vmd->vmd_free_min) - 1) /
(vmd->vmd_free_target - vmd->vmd_free_min)) >= nclean) {
 
 I don't know where this formula comes from, so I don't know if this
 change is correct.
>>> 
>>> Hm, that's somewhat surprising. This code shouldn't be executing in
>>> situations where the OOM kill logic is invoked (i.e., memory pressure
>>> plus a shortage of clean pages in the inactive queue).
>>> 
>>> How much RAM does the system have? Could you collect "sysctl vm" output
>>> around the time of an OOM kill?  
>> 
>> 1GiB RAM.  I've sampled sysctl vm every 5s from the moment the process
>> starts swapping until it is killed and uploaded that to
>> https://people.freebsd.org/~tijl/sysctl/  
> 
> Thank you. Now I agree with your change. Would you like to commit it?
> I can take care of it if you prefer.

Please go ahead.  You can probably write a better commit log.
___
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: r331732 - head/sys/vm

[Mark Johnston]

On Sun, Apr 01, 2018 at 05:20:21PM +0200, Tijl Coosemans wrote:
> On Sat, 31 Mar 2018 18:54:32 -0400 Mark Johnston  wrote:
> > On Sat, Mar 31, 2018 at 08:21:18PM +0200, Tijl Coosemans wrote:
> > > On Thu, 29 Mar 2018 14:27:40 + (UTC) Mark Johnston 
> > >  wrote:  
> > > > Author: markj
> > > > Date: Thu Mar 29 14:27:40 2018
> > > > New Revision: 331732
> > > > URL: https://svnweb.freebsd.org/changeset/base/331732
> > > > 
> > > > Log:
> > > >   Fix the background laundering mechanism after r329882.
> > > >   
> > > >   Rather than using the number of inactive queue scans as a metric for
> > > >   how many clean pages are being freed by the page daemon, have the
> > > >   page daemon keep a running counter of the number of pages it has 
> > > > freed,
> > > >   and have the laundry thread use that when computing the background
> > > >   laundering threshold.
> > > > [...]  
> > > 
> > > I'm seeing big processes being killed with an "out of swap space" message
> > > even though there's still plenty of swap available.  It seems to be fixed
> > > by making this division round upwards:
> > > 
> > >   if (target == 0 && ndirty * isqrt((nfreed +
> > >   (vmd->vmd_free_target - vmd->vmd_free_min) - 1) /
> > >   (vmd->vmd_free_target - vmd->vmd_free_min)) >= nclean) {
> > > 
> > > I don't know where this formula comes from, so I don't know if this
> > > change is correct.  
> > 
> > Hm, that's somewhat surprising. This code shouldn't be executing in
> > situations where the OOM kill logic is invoked (i.e., memory pressure
> > plus a shortage of clean pages in the inactive queue).
> > 
> > How much RAM does the system have? Could you collect "sysctl vm" output
> > around the time of an OOM kill?
> 
> 1GiB RAM.  I've sampled sysctl vm every 5s from the moment the process
> starts swapping until it is killed and uploaded that to
> https://people.freebsd.org/~tijl/sysctl/

Thank you. Now I agree with your change. Would you like to commit it?
I can take care of it if you prefer.

There is still a deeper problem here after r329882 in that the shortfall
laundering mechanism is not kicking in before the OOM killer. The
problem is that the PID controller may produce a positive output even
when the error is negative; this is seen in 22.txt, the last capture
before the OOM kill. Because the error is negative (i.e.,
v_free_count > v_free_target), we will not attempt to launder pages in
shortfall mode. The positive output means that the page daemon is
repeatedly scanning the (completely depleted) inactive queue, but since
the laundering mechanism is failing to produce clean pages, there is
nothing to reclaim and so we eventually invoke the OOM killer.

I'm not yet sure how best to address this, but your change is probably
sufficient to mitigate the problem in general and also corrects an
unintentional behaviour change in my commit.
___
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: r331732 - head/sys/vm

[Tijl Coosemans]

On Sat, 31 Mar 2018 18:54:32 -0400 Mark Johnston  wrote:
> On Sat, Mar 31, 2018 at 08:21:18PM +0200, Tijl Coosemans wrote:
> > On Thu, 29 Mar 2018 14:27:40 + (UTC) Mark Johnston  
> > wrote:  
> > > Author: markj
> > > Date: Thu Mar 29 14:27:40 2018
> > > New Revision: 331732
> > > URL: https://svnweb.freebsd.org/changeset/base/331732
> > > 
> > > Log:
> > >   Fix the background laundering mechanism after r329882.
> > >   
> > >   Rather than using the number of inactive queue scans as a metric for
> > >   how many clean pages are being freed by the page daemon, have the
> > >   page daemon keep a running counter of the number of pages it has freed,
> > >   and have the laundry thread use that when computing the background
> > >   laundering threshold.
> > > [...]  
> > 
> > I'm seeing big processes being killed with an "out of swap space" message
> > even though there's still plenty of swap available.  It seems to be fixed
> > by making this division round upwards:
> > 
> > if (target == 0 && ndirty * isqrt((nfreed +
> > (vmd->vmd_free_target - vmd->vmd_free_min) - 1) /
> > (vmd->vmd_free_target - vmd->vmd_free_min)) >= nclean) {
> > 
> > I don't know where this formula comes from, so I don't know if this
> > change is correct.  
> 
> Hm, that's somewhat surprising. This code shouldn't be executing in
> situations where the OOM kill logic is invoked (i.e., memory pressure
> plus a shortage of clean pages in the inactive queue).
> 
> How much RAM does the system have? Could you collect "sysctl vm" output
> around the time of an OOM kill?

1GiB RAM.  I've sampled sysctl vm every 5s from the moment the process
starts swapping until it is killed and uploaded that to
https://people.freebsd.org/~tijl/sysctl/

> I'm wondering if the higher inactive queue scan frequency after r329882
> might be responsible: OOM kills are performed after vm.pageout_oom_seq
> back-to-back scans fail to reclaim any pages. Does your problem persist
> if you increase the value of that sysctl, say to 60?

This didn't help.
___
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: r331732 - head/sys/vm

[Mark Johnston]

On Sat, Mar 31, 2018 at 08:21:18PM +0200, Tijl Coosemans wrote:
> On Thu, 29 Mar 2018 14:27:40 + (UTC) Mark Johnston  
> wrote:
> > Author: markj
> > Date: Thu Mar 29 14:27:40 2018
> > New Revision: 331732
> > URL: https://svnweb.freebsd.org/changeset/base/331732
> > 
> > Log:
> >   Fix the background laundering mechanism after r329882.
> >   
> >   Rather than using the number of inactive queue scans as a metric for
> >   how many clean pages are being freed by the page daemon, have the
> >   page daemon keep a running counter of the number of pages it has freed,
> >   and have the laundry thread use that when computing the background
> >   laundering threshold.
> > [...]
> 
> I'm seeing big processes being killed with an "out of swap space" message
> even though there's still plenty of swap available.  It seems to be fixed
> by making this division round upwards:
> 
>   if (target == 0 && ndirty * isqrt((nfreed +
>   (vmd->vmd_free_target - vmd->vmd_free_min) - 1) /
>   (vmd->vmd_free_target - vmd->vmd_free_min)) >= nclean) {
> 
> I don't know where this formula comes from, so I don't know if this
> change is correct.

Hm, that's somewhat surprising. This code shouldn't be executing in
situations where the OOM kill logic is invoked (i.e., memory pressure
plus a shortage of clean pages in the inactive queue).

How much RAM does the system have? Could you collect "sysctl vm" output
around the time of an OOM kill?

I'm wondering if the higher inactive queue scan frequency after r329882
might be responsible: OOM kills are performed after vm.pageout_oom_seq
back-to-back scans fail to reclaim any pages. Does your problem persist
if you increase the value of that sysctl, say to 60?
___
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: r331732 - head/sys/vm

[Tijl Coosemans]

On Thu, 29 Mar 2018 14:27:40 + (UTC) Mark Johnston  
wrote:
> Author: markj
> Date: Thu Mar 29 14:27:40 2018
> New Revision: 331732
> URL: https://svnweb.freebsd.org/changeset/base/331732
> 
> Log:
>   Fix the background laundering mechanism after r329882.
>   
>   Rather than using the number of inactive queue scans as a metric for
>   how many clean pages are being freed by the page daemon, have the
>   page daemon keep a running counter of the number of pages it has freed,
>   and have the laundry thread use that when computing the background
>   laundering threshold.
>   
>   Reviewed by:kib
>   Differential Revision:  https://reviews.freebsd.org/D14884
> 
> Modified:
>   head/sys/vm/vm_pageout.c
>   head/sys/vm/vm_pagequeue.h
> 
> Modified: head/sys/vm/vm_pageout.c
> ==
> --- head/sys/vm/vm_pageout.c  Thu Mar 29 13:55:23 2018(r331731)
> +++ head/sys/vm/vm_pageout.c  Thu Mar 29 14:27:40 2018(r331732)
> @@ -943,8 +943,7 @@ vm_pageout_laundry_worker(void *arg)
>  {
>   struct vm_domain *vmd;
>   struct vm_pagequeue *pq;
> - uint64_t nclean, ndirty;
> - u_int inactq_scans, last_launder;
> + uint64_t nclean, ndirty, nfreed;
>   int domain, last_target, launder, shortfall, shortfall_cycle, target;
>   bool in_shortfall;
>  
> @@ -958,8 +957,7 @@ vm_pageout_laundry_worker(void *arg)
>   in_shortfall = false;
>   shortfall_cycle = 0;
>   target = 0;
> - inactq_scans = 0;
> - last_launder = 0;
> + nfreed = 0;
>  
>   /*
>* Calls to these handlers are serialized by the swap syscall lock.
> @@ -1000,7 +998,6 @@ vm_pageout_laundry_worker(void *arg)
>   target = 0;
>   goto trybackground;
>   }
> - last_launder = inactq_scans;
>   launder = target / shortfall_cycle--;
>   goto dolaundry;
>  
> @@ -1009,24 +1006,23 @@ vm_pageout_laundry_worker(void *arg)
>* meet the conditions to perform background laundering:
>*
>* 1. The ratio of dirty to clean inactive pages exceeds the
> -  *background laundering threshold and the pagedaemon has
> -  *been woken up to reclaim pages since our last
> -  *laundering, or
> +  *background laundering threshold, or
>* 2. we haven't yet reached the target of the current
>*background laundering run.
>*
>* The background laundering threshold is not a constant.
>* Instead, it is a slowly growing function of the number of
> -  * page daemon scans since the last laundering.  Thus, as the
> -  * ratio of dirty to clean inactive pages grows, the amount of
> -  * memory pressure required to trigger laundering decreases.
> +  * clean pages freed by the page daemon since the last
> +  * background laundering.  Thus, as the ratio of dirty to
> +  * clean inactive pages grows, the amount of memory pressure
> +  * required to trigger laundering decreases.
>*/
>  trybackground:
>   nclean = vmd->vmd_free_count +
>   vmd->vmd_pagequeues[PQ_INACTIVE].pq_cnt;
>   ndirty = vmd->vmd_pagequeues[PQ_LAUNDRY].pq_cnt;
> - if (target == 0 && inactq_scans != last_launder &&
> - ndirty * isqrt(inactq_scans - last_launder) >= nclean) {
> + if (target == 0 && ndirty * isqrt(nfreed /
> + (vmd->vmd_free_target - vmd->vmd_free_min)) >= nclean) {

I'm seeing big processes being killed with an "out of swap space" message
even though there's still plenty of swap available.  It seems to be fixed
by making this division round upwards:

if (target == 0 && ndirty * isqrt((nfreed +
(vmd->vmd_free_target - vmd->vmd_free_min) - 1) /
(vmd->vmd_free_target - vmd->vmd_free_min)) >= nclean) {

I don't know where this formula comes from, so I don't know if this
change is correct.
___
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"

svn commit: r331732 - head/sys/vm

[Mark Johnston]

Author: markj
Date: Thu Mar 29 14:27:40 2018
New Revision: 331732
URL: https://svnweb.freebsd.org/changeset/base/331732

Log:
  Fix the background laundering mechanism after r329882.
  
  Rather than using the number of inactive queue scans as a metric for
  how many clean pages are being freed by the page daemon, have the
  page daemon keep a running counter of the number of pages it has freed,
  and have the laundry thread use that when computing the background
  laundering threshold.
  
  Reviewed by:  kib
  Differential Revision:https://reviews.freebsd.org/D14884

Modified:
  head/sys/vm/vm_pageout.c
  head/sys/vm/vm_pagequeue.h

Modified: head/sys/vm/vm_pageout.c
==
--- head/sys/vm/vm_pageout.cThu Mar 29 13:55:23 2018(r331731)
+++ head/sys/vm/vm_pageout.cThu Mar 29 14:27:40 2018(r331732)
@@ -943,8 +943,7 @@ vm_pageout_laundry_worker(void *arg)
 {
struct vm_domain *vmd;
struct vm_pagequeue *pq;
-   uint64_t nclean, ndirty;
-   u_int inactq_scans, last_launder;
+   uint64_t nclean, ndirty, nfreed;
int domain, last_target, launder, shortfall, shortfall_cycle, target;
bool in_shortfall;
 
@@ -958,8 +957,7 @@ vm_pageout_laundry_worker(void *arg)
in_shortfall = false;
shortfall_cycle = 0;
target = 0;
-   inactq_scans = 0;
-   last_launder = 0;
+   nfreed = 0;
 
/*
 * Calls to these handlers are serialized by the swap syscall lock.
@@ -1000,7 +998,6 @@ vm_pageout_laundry_worker(void *arg)
target = 0;
goto trybackground;
}
-   last_launder = inactq_scans;
launder = target / shortfall_cycle--;
goto dolaundry;
 
@@ -1009,24 +1006,23 @@ vm_pageout_laundry_worker(void *arg)
 * meet the conditions to perform background laundering:
 *
 * 1. The ratio of dirty to clean inactive pages exceeds the
-*background laundering threshold and the pagedaemon has
-*been woken up to reclaim pages since our last
-*laundering, or
+*background laundering threshold, or
 * 2. we haven't yet reached the target of the current
 *background laundering run.
 *
 * The background laundering threshold is not a constant.
 * Instead, it is a slowly growing function of the number of
-* page daemon scans since the last laundering.  Thus, as the
-* ratio of dirty to clean inactive pages grows, the amount of
-* memory pressure required to trigger laundering decreases.
+* clean pages freed by the page daemon since the last
+* background laundering.  Thus, as the ratio of dirty to
+* clean inactive pages grows, the amount of memory pressure
+* required to trigger laundering decreases.
 */
 trybackground:
nclean = vmd->vmd_free_count +
vmd->vmd_pagequeues[PQ_INACTIVE].pq_cnt;
ndirty = vmd->vmd_pagequeues[PQ_LAUNDRY].pq_cnt;
-   if (target == 0 && inactq_scans != last_launder &&
-   ndirty * isqrt(inactq_scans - last_launder) >= nclean) {
+   if (target == 0 && ndirty * isqrt(nfreed /
+   (vmd->vmd_free_target - vmd->vmd_free_min)) >= nclean) {
target = vmd->vmd_background_launder_target;
}
 
@@ -1039,8 +1035,8 @@ trybackground:
 * proceed at the background laundering rate.
 */
if (target > 0) {
-   if (inactq_scans != last_launder) {
-   last_launder = inactq_scans;
+   if (nfreed > 0) {
+   nfreed = 0;
last_target = target;
} else if (last_target - target >=
vm_background_launder_max * PAGE_SIZE / 1024) {
@@ -1089,7 +1085,8 @@ dolaundry:
 
if (target == 0)
vmd->vmd_laundry_request = VM_LAUNDRY_IDLE;
-   inactq_scans = vmd->vmd_inactq_scans;
+   nfreed += vmd->vmd_clean_pages_freed;
+   vmd->vmd_clean_pages_freed = 0;
vm_pagequeue_unlock(pq);
}
 }
@@ -1367,7 +1364,8 @@ drop_page:
VM_LAUNDRY_BACKGROUND;
wakeup(>vmd_laundry_request);
}
-   vmd->vmd_inactq_scans++;
+   vmd->vmd_clean_pages_freed +=
+   starting_page_shortage - page_shortage;
vm_pagequeue_unlock(pq);
}
 

Modified: head/sys/vm/vm_pagequeue.h