[PATCH] 2.2.1{3,4,5} VM fix

[PATCH] 2.2.1{3,4,5} VM fix

[Rik van Riel]

Hi Alan,

here's the patch for the 2.2.15* out of memory problem.
This patch does a number of things:
- make kswapd sleep just HZ, just like in 2.3
- don't start freeing memory from a user process when
  we're at or just below the limit:
	- we need the kernel lock so freeing memory
	  together with kswapd isn't possible anyway
	- if kswapd can keep up, there's no latency
	  penalty for (kernel) memory allocations
	- when we truly are low on memory or kswapd
	  can't keep up, then any __GFP_WAIT allocations
	  _will_ wait while trying to free memory
	  (so there's less chance of running out for
	  GFP_ATOMIC ones)
- wake up kswapd somewhat earlier (instead of too late)

This should be enough to make sure that 2.2.1* doesn't
run out of memory on a slight network flooding any more.
Note that I have only compiled the code with this patch
and not tried it, this wouldn't have worked anyway because
I can't create the problem situation (fast network, etc)
at home...

regards,

Rik
--
The Internet is not a network of computers. It is a network
of people. That is its real strength.



--- linux-2.2.15-pre3/mm/vmscan.c.orig	Wed Jan 19 21:18:54 2000
+++ linux-2.2.15-pre3/mm/vmscan.c	Wed Jan 19 22:06:34 2000
@@ -490,12 +490,13 @@
 		{
 			if (do_try_to_free_pages(GFP_KSWAPD))
 			{
+				run_task_queue(&tq_disk);
 				if (tsk->need_resched)
 					schedule();
 				continue;
 			}
 			tsk->state = TASK_INTERRUPTIBLE;
-			schedule_timeout(10*HZ);
+			schedule_timeout(HZ);
 		}
 	}
 }
@@ -509,18 +510,16 @@
  * from user processes, because the locking issues are
  * nasty to the extreme (file write locks, and MM locking)
  *
- * One option might be to let kswapd do all the page-out
- * and VM page table scanning that needs locking, and this
- * process thread could do just the mmap shrink stage that
- * can be done by just dropping cached pages without having
- * any deadlock issues.
+ * If we're on or just slighly below freepages.low, kswapd
+ * should manage on its own, we just give it a nudge. This
+ * should also reduce contention for the kernel lock above.
  */
 int try_to_free_pages(unsigned int gfp_mask)
 {
 	int retval = 1;
 
 	wake_up_interruptible(&kswapd_wait);
-	if (gfp_mask & __GFP_WAIT)
+	if ((gfp_mask & __GFP_WAIT) && (nr_free_pages < (freepages.low - 4)))
 		retval = do_try_to_free_pages(gfp_mask);
 	return retval;
 }
--- linux-2.2.15-pre3/mm/page_alloc.c.orig	Wed Jan 19 21:32:05 2000
+++ linux-2.2.15-pre3/mm/page_alloc.c	Wed Jan 19 21:42:00 2000
@@ -212,7 +212,7 @@
 	if (!(current->flags & PF_MEMALLOC)) {
 		int freed;
 
-		if (nr_free_pages > freepages.min) {
+		if (nr_free_pages > freepages.low) {
 			if (!low_on_memory)
 				goto ok_to_allocate;
 			if (nr_free_pages >= freepages.high) {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.nl.linux.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Andrea Arcangeli]

On Wed, 19 Jan 2000, Rik van Riel wrote:

>--- linux-2.2.15-pre3/mm/vmscan.c.orig	Wed Jan 19 21:18:54 2000
>+++ linux-2.2.15-pre3/mm/vmscan.c	Wed Jan 19 22:06:34 2000
>@@ -490,12 +490,13 @@
> 		{
> 			if (do_try_to_free_pages(GFP_KSWAPD))
> 			{
>+				run_task_queue(&tq_disk);
> 				if (tsk->need_resched)
> 					schedule();
> 				continue;
> 			}

There's a limit of max swap request, after that a wait_on_page will
trigger I/O. So you'll only decrease performance by doing so as far I can 
tell.

> 			tsk->state = TASK_INTERRUPTIBLE;
>-			schedule_timeout(10*HZ);
>+			schedule_timeout(HZ);

I used 10 sec because if you run oom into kswapd it means you can't hardly
do anything within kswapd in the next 10 seconds. If userspace is
triggering OOM all will continue to run fine and it's way better to remove
kswapd from the game to gracefully allow userspace to detect oom. Note
that a do_try_to_free_pages can take quite some time before failing in a
9giga machine with 9giga allocated in userspace (I am talking 2.2.x, in
2.3.x my page-LRU will allow shrink_mmap to not waste time trying to free
9giga/PAGE_SIZE pages).

If instead atomic allocations caused oom and kswapd failed then you should
give up for some time as well since you know all memory is not freeable
and so only a __free_pages will release memory. Thus kswapd will be
wasted time in such case too and you should instead wait a process to
trigger some allocation to be able to kill it.

> 	wake_up_interruptible(&kswapd_wait);
>-	if (gfp_mask & __GFP_WAIT)
>+	if ((gfp_mask & __GFP_WAIT) && (nr_free_pages < (freepages.low - 4)))
> 		retval = do_try_to_free_pages(gfp_mask);

-4 make no sense as ".low" could be as well set to 3 and theorically the
system should remains stable.

And you are trying to wakeup kswapd instead of blocking. This will make
oom condition worse and it brekas my trashing mem watermark heuristic. The
heuristic is necessary to penalize the hog. It should be a per-process
thing btw.

>--- linux-2.2.15-pre3/mm/page_alloc.c.orig	Wed Jan 19 21:32:05 2000
>+++ linux-2.2.15-pre3/mm/page_alloc.c	Wed Jan 19 21:42:00 2000
>@@ -212,7 +212,7 @@
> 	if (!(current->flags & PF_MEMALLOC)) {
> 		int freed;
> 
>-		if (nr_free_pages > freepages.min) {
>+		if (nr_free_pages > freepages.low) {
> 			if (!low_on_memory)
> 				goto ok_to_allocate;
> 			if (nr_free_pages >= freepages.high) {

freepages.low is unused. With this change you are using low and making min
unused. min seems the right name there. Nobody is allowed to allocate
memory without first free some memory if the system is under the
_min_ memory watermark. If you don't like the current defaults just change
the dyanmic setting at boot in page_alloc.c. They can be changed as well
via /proc/sys/vm/freepages (you just know :).

Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.nl.linux.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Rik van Riel]

On Thu, 20 Jan 2000, Andrea Arcangeli wrote:

> There's a limit of max swap request, after that a wait_on_page
> will trigger I/O. So you'll only decrease performance by doing so
> as far I can tell.

Not really. We want to get the I/O done and we don't want
to wait until the queue fills up.

> > 			tsk->state = TASK_INTERRUPTIBLE;
> >-			schedule_timeout(10*HZ);
> >+			schedule_timeout(HZ);
> 
> I used 10 sec because if you run oom into kswapd it means you
> can't hardly do anything within kswapd in the next 10 seconds.

OOM is not the normal state the system is in. You really want
kswapd to do some work in the background (allowing user programs
to allocate their bits of memory without stalling).

> If instead atomic allocations caused oom and kswapd failed then
> you should give up for some time as well since you know all memory
> is not freeable and so only a __free_pages will release memory.

In the OOM case we'll have to kill a process. Stalling the
system forever is not a solution.

> > 	wake_up_interruptible(&kswapd_wait);
> >-	if (gfp_mask & __GFP_WAIT)
> >+	if ((gfp_mask & __GFP_WAIT) && (nr_free_pages < (freepages.low - 4)))
> > 		retval = do_try_to_free_pages(gfp_mask);
> 
> -4 make no sense as ".low" could be as well set to 3 and
> theorically the system should remains stable.

It does make sense in reality. Please read the explanation
I sent with the patch. I agree that somewhere halfway
freepages.low and freepages.min would be better, but there
is no need to complicate the calculation...

> And you are trying to wakeup kswapd instead of blocking. This will
> make oom condition worse and it brekas my trashing mem watermark
> heuristic. The heuristic is necessary to penalize the hog. It
> should be a per-process thing btw.

If kswapd can keep up, there's no need at all to slow down
processes, not even hogs. If kswapd can't keep up then we'll
start stalling processes (just 5 allocations further away,
_and_ kswapd will have been started by that time and have had
the time to do something).

> >--- linux-2.2.15-pre3/mm/page_alloc.c.orig	Wed Jan 19 21:32:05 2000
> >+++ linux-2.2.15-pre3/mm/page_alloc.c	Wed Jan 19 21:42:00 2000
> >@@ -212,7 +212,7 @@
> > 	if (!(current->flags & PF_MEMALLOC)) {
> > 		int freed;
> > 
> >-		if (nr_free_pages > freepages.min) {
> >+		if (nr_free_pages > freepages.low) {
> > 			if (!low_on_memory)
> > 				goto ok_to_allocate;
> > 			if (nr_free_pages >= freepages.high) {
> 
> freepages.low is unused. With this change you are using low and
> making min unused. min seems the right name there.

I consider the absense of the third boundary a HUGE bug
in current 2.2 VM. You need three boundaries:

- min:  below this boundary only ATOMIC (and GFP_HIGH?)
        allocations can be done
- low:  below this boundary we start swapping agressively
- high: below this boundary we start background swapping
        (once a second, without impact on processes), above
        this boundary we stop swapping

Current (2-border) code makes for bursty swap behaviour,
poor handling of kernel allocations (see the complaints
Alan got about systems which ran OOM on atomic allocations).

There is a good reason why there is a difference between
background swapping and agressive swapping, why there is
a separate swapout daemon, etc...

Performance-wise, current 2.2 went back to the stone age,
we should do something about that.

> Nobody is allowed to allocate memory without first free some
> memory if the system is under the _min_ memory watermark.

That's not the way things were meant to be. Below freepages.low
low-priority allocations should stall, below freepages.min higher
priority allocations should stall and only GFP_ATOMIC and
PF_MEMALLOC allocations should be able to proceed.

regards,

Rik
--
The Internet is not a network of computers. It is a network
of people. That is its real strength.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.nl.linux.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Andrea Arcangeli]

On Thu, 20 Jan 2000, Rik van Riel wrote:

>On Thu, 20 Jan 2000, Andrea Arcangeli wrote:
>
>> There's a limit of max swap request, after that a wait_on_page
>> will trigger I/O. So you'll only decrease performance by doing so
>> as far I can tell.
>
>Not really. We want to get the I/O done and we don't want
>to wait until the queue fills up.

The bank gives us 32 pages of credit. We don't need to get the I/O on
them. We have a credit that we can use to optimze the I/O.

>> > 			tsk->state = TASK_INTERRUPTIBLE;
>> >-			schedule_timeout(10*HZ);
>> >+			schedule_timeout(HZ);
>> 
>> I used 10 sec because if you run oom into kswapd it means you
>> can't hardly do anything within kswapd in the next 10 seconds.
>
>OOM is not the normal state the system is in. You really want

And infact such schedule_timeout(10*HZ) will happen _only_ when you are
OOM.

>kswapd to do some work in the background (allowing user programs
>to allocate their bits of memory without stalling).

This should be accomplished by a pre-wakeup once the high watermark
triggers. The HZ polling that is just doing is only for atomic
allocations. To parallelize memory freeing a polling with HZ frequency is
too slow.

>> If instead atomic allocations caused oom and kswapd failed then
>> you should give up for some time as well since you know all memory
>> is not freeable and so only a __free_pages will release memory.
>
>In the OOM case we'll have to kill a process. Stalling the
>system forever is not a solution.

The system is not stalled. atomic stuff that doesn't need to alloc memory
will continue to run. It will then complete and release memory later. Or
it will fail if it needs futher allocation to complete and it will release
memory for other atomic stuff that can now start. Allowing the TCP stack
to kill netscape is not a 2.2.x thing IMHO. BTW, you are not doing that in
your patch either.

>> > 	wake_up_interruptible(&kswapd_wait);
>> >-	if (gfp_mask & __GFP_WAIT)
>> >+	if ((gfp_mask & __GFP_WAIT) && (nr_free_pages < (freepages.low - 4)))
>> > 		retval = do_try_to_free_pages(gfp_mask);
>> 
>> -4 make no sense as ".low" could be as well set to 3 and
>> theorically the system should remains stable.
>
>It does make sense in reality. Please read the explanation
>I sent with the patch. I agree that somewhere halfway
>freepages.low and freepages.min would be better, but there
>is no need to complicate the calculation...

The calc is a linear scale 1/3 2/3 3/3. The difference between 512 and 508
is zero in RL.

>> And you are trying to wakeup kswapd instead of blocking. This will
>> make oom condition worse and it brekas my trashing mem watermark
>> heuristic. The heuristic is necessary to penalize the hog. It
>> should be a per-process thing btw.
>
>If kswapd can keep up, there's no need at all to slow down
>processes, not even hogs. If kswapd can't keep up then we'll

If you don't want to slow down not-hogs-processes, you must make sure that
who is allocating memory will block. If kswapd will be the only blocking
the system will trash because the hog will continue to run.

>start stalling processes (just 5 allocations further away,
>_and_ kswapd will have been started by that time and have had
>the time to do something).

That's not a matter of swapout but of shrink_mmap. See my previous point
about a pre-wakeup when the high watermark triggers. When swap is involved
instead kswapd won't help anymore IMHO.

>> >--- linux-2.2.15-pre3/mm/page_alloc.c.orig	Wed Jan 19 21:32:05 2000
>> >+++ linux-2.2.15-pre3/mm/page_alloc.c	Wed Jan 19 21:42:00 2000
>> >@@ -212,7 +212,7 @@
>> > 	if (!(current->flags & PF_MEMALLOC)) {
>> > 		int freed;
>> > 
>> >-		if (nr_free_pages > freepages.min) {
>> >+		if (nr_free_pages > freepages.low) {
>> > 			if (!low_on_memory)
>> > 				goto ok_to_allocate;
>> > 			if (nr_free_pages >= freepages.high) {
>> 
>> freepages.low is unused. With this change you are using low and
>> making min unused. min seems the right name there.
>
>I consider the absense of the third boundary a HUGE bug
>in current 2.2 VM. You need three boundaries:
>
>- min:  below this boundary only ATOMIC (and GFP_HIGH?)
>        allocations can be done
>- low:  below this boundary we start swapping agressively
>- high: below this boundary we start background swapping
>        (once a second, without impact on processes), above
>        this boundary we stop swapping
>
>Current (2-border) code makes for bursty swap behaviour,
>poor handling of kernel allocations (see the complaints
>Alan got about systems which ran OOM on atomic allocations).

I missed Alan problems (probably they are hided by the l-k heavy
traffic).

high is just implemented in your way. The once a second is not enough for
RL, you need a pre-wakeup if you want to take try to pre-free memory. And
the pre-wakeup won't help in the swap case.

the need of low and min make no sense to me. You may be more happy to know
that all three freepages values in /proc are used for something but for
your machine it will make no differences. Micro tuning make no sense to
me. Only tuning that make differences in RL make sense to me.

>There is a good reason why there is a difference between
>background swapping and agressive swapping, why there is
>a separate swapout daemon, etc...

The separate swapout daemon is there only for allowing the system to free
cache and userspace memory for atomic allocations. It has nothing to do
with aggressive or not aggressive swapping.

>Performance-wise, current 2.2 went back to the stone age,
>we should do something about that.

Please run 2.2.14aa1.

>> Nobody is allowed to allocate memory without first free some
>> memory if the system is under the _min_ memory watermark.
>
>That's not the way things were meant to be. Below freepages.low
>low-priority allocations should stall, below freepages.min higher
>priority allocations should stall and only GFP_ATOMIC and
>PF_MEMALLOC allocations should be able to proceed.

The only difference between high and low prio allocation is that high prio
allocation are not triggered by an userspace action. Thus they'd better
complete trying to allocate memory even if we are low on memory. low prio
allocation are triggered by userspace so we want to fail ASAP if userspace
triggered an oom. It has nothing to do with performance and it has not to
do with the selection if somebody should stall or not.

If you are planning to change the current semantics of the GFP_* variables
please work on 2.3.x only.

Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.nl.linux.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Alan Cox]

> The bank gives us 32 pages of credit. We don't need to get the I/O on
> them. We have a credit that we can use to optimze the I/O.

32 pages, thats 87 ethernet packets. At 100Mbit thats a rather short period
of time. I make it 1/60th of a second

Alan

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.nl.linux.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Andrea Arcangeli]

On Fri, 21 Jan 2000, Alan Cox wrote:

>> The bank gives us 32 pages of credit. We don't need to get the I/O on
>> them. We have a credit that we can use to optimze the I/O.
>
>32 pages, thats 87 ethernet packets. At 100Mbit thats a rather short period
>of time. I make it 1/60th of a second

Actually the 32 pages I was talking about aren't the freepages.min but the
SWAP_CUSTER_MAX.

Anyway I think to see the problem described by Rik and actually I think it
can be fixed by killing the useless polling of 1 second and replacing it
with an unconiditional pre-wakeup of kswapd when GFP touch the
freepages.high watermark. This will in turn should also help performances
and it will try to free the cache from inside kswapd before a process have
to free it itself.

I said "useless polling" exactly because of what you said, that is a
network allocation will eat the freepages.min pages in less than 1 second.

Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Rik van Riel]

On Fri, 21 Jan 2000, Andrea Arcangeli wrote:

> Anyway I think to see the problem described by Rik and actually I
> think it can be fixed by killing the useless polling of 1 second
> and replacing it with an unconiditional pre-wakeup of kswapd when
> GFP touch the freepages.high watermark. This will in turn should
> also help performances and it will try to free the cache from
> inside kswapd before a process have to free it itself.

It will damage performance. We want the hysteresis provided
by freepages.{min,low,high}...

Between .low and .high the one-second poll should do
the trick, making sure that all allocations are done
without delay.

Just around .low kswapd should be unconditionally
woken up.

Between .min and .low kswapd should be woken and
__GFP_WAIT allocations should wait.

Below .min only GFP_ATOMIC and PF_MEMALLOC allocations
should be allowed.

This is how the priorities have been intended
from the start on (except that we didn't have the
waiting part in the beginning, this needed fixing
later).

regards,

Rik
--
The Internet is not a network of computers. It is a network
of people. That is its real strength.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Andrea Arcangeli]

On Fri, 21 Jan 2000, Rik van Riel wrote:

>Below .min only GFP_ATOMIC and PF_MEMALLOC allocations
>should be allowed.
>
>This is how the priorities have been intended
>from the start on (except that we didn't have the

Very wrong. Since 2.1.x all GFP_KERNEL allocations (not atomic) succeed
too.

Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Rik van Riel]

On Fri, 21 Jan 2000, Andrea Arcangeli wrote:

> Since 2.1.x all GFP_KERNEL allocations (not atomic) succeed too.

Alan, I think we've located the bug that made 2.2 kernels
run completely out of memory :)

Andrea, the last few pages are meant for ATOMIC and
PF_MEMALLOC allocations only, otherwise you'll get
deadlock situations.

And don't point me at buggy code that crashes if its
GFP_KERNEL allocation fails. That same code would
also crash if it were allowed to fill up freepages.min
and then run really out of memory...

regards,

Rik
--
The Internet is not a network of computers. It is a network
of people. That is its real strength.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Andrea Arcangeli]

On Fri, 21 Jan 2000, Rik van Riel wrote:

>On Fri, 21 Jan 2000, Andrea Arcangeli wrote:
>
>> Since 2.1.x all GFP_KERNEL allocations (not atomic) succeed too.
>
>Alan, I think we've located the bug that made 2.2 kernels
>run completely out of memory :)

Yes, the fix is to kill the meaningless 1 second polling loop and to
replace it with a proper wakeup. It has definitely nothing to do with
GFP_KERNEL semantics.

	ftp://ftp.*.kernel.org/pub/linux/kernel/people/andrea/patches/v2.2/2.2.14/atomic-allocations-2.gz

>Andrea, the last few pages are meant for ATOMIC and
>PF_MEMALLOC allocations only, otherwise you'll get
>deadlock situations.

Deadlock happens only due a bug in the caller. That has nothing to do with
failed atomic allocations or with the MM core.

About your proposed change (as I just said), the semantic you want to
change whith your diff that I am quoting here:

>-               if (!freed && !(gfp_mask & (__GFP_MED | __GFP_HIGH)))
>+               if (!freed && !(gfp_mask & __GFP_HIGH))

makes a difference _only_ if the machine goes _OOM_ (so when
try_to_free_pages fails) and that's completly _unrelated_ with the
failed-atomic-allocation case we are talking about. If the machine is OOM
it's perfectly normal that atomic allocations fails. That's expected.

You are basically making GFP_KERNEL equal to GFP_USER and that's wrong. It
makes sense that allocation triggered by the kernel have access to the
whole free memory available: kernel allocations have the same privilegies
of atomic allocations. The reason atomic allocations are atomic is that
they can't sleep, that's all. If they could sleep they would be GFP_KERNEL
allocations instead. The only difference between the two, is that the
non-atomic allocations will free memory themselfs before accessing _all_
the free memory because they _can_ do that.

The reason GFP_USER allocations won't access the whole free memory is
_only_ that we want to kill userspace as soon as we detect OOM. It's not
because we want to reserve some memory for atomic allocations _during_
oom. During OOM we must try to kill userspace or eat the last memory free
if possible.

The problem we have now with atomic allcations is instead that they can
fail even if there's plenty of _freeable_unused_ cache allocated (_not_
during oom).

IMHO the problem we have is that atomic allocations only rely on the way
too slow 1 sec polling of kswapd, while they should wakeup kswapd
immediatly and my patch will address exactly that.

>GFP_KERNEL allocation fails. That same code would
>also crash if it were allowed to fill up freepages.min
>and then run really out of memory...

If something crashes when an allocation fails that's a caller bug and it's
unrelated with the VM core. If nr_free_pages == 0 the system must remains
stable. If the system crashes it's not an MM bug. Having nr_free_pages ==
0 is an expected condition.

NOTE: we actually have getblk a bit deadlock prone so getblk very much
prefer to succeed if possible. Fixing it it's not a 2.2.x thing IMHO also
considering I never succeed to reproduce that in RL. So we have to take in
mind this getblk thing while playing with the MM in 2.2.x. That's why I
said that your change could also harm getblk during oom, but nevertheless
it wasn't your change the source of the problem. Without your proposed
change instead getblk may get in troubles only if nr_free_pages == 0 that
rarely happens, but again: that's a getblk problem, and not an MM one.

Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Benjamin C.R. LaHaise]

On Sat, 22 Jan 2000, Andrea Arcangeli wrote:

> On Fri, 21 Jan 2000, Rik van Riel wrote:
> 
> >On Fri, 21 Jan 2000, Andrea Arcangeli wrote:
> >
> >> Since 2.1.x all GFP_KERNEL allocations (not atomic) succeed too.
> >
> >Alan, I think we've located the bug that made 2.2 kernels
> >run completely out of memory :)
> 
> Yes, the fix is to kill the meaningless 1 second polling loop and to
> replace it with a proper wakeup. It has definitely nothing to do with
> GFP_KERNEL semantics.
> 
> 	ftp://ftp.*.kernel.org/pub/linux/kernel/people/andrea/patches/v2.2/2.2.14/atomic-allocations-2.gz
> 
> >Andrea, the last few pages are meant for ATOMIC and
> >PF_MEMALLOC allocations only, otherwise you'll get
> >deadlock situations.
> 
> Deadlock happens only due a bug in the caller. That has nothing to do with
> failed atomic allocations or with the MM core.

I think the deadlock being referred by Rik to is that of getting stuck
trying to allocate memory while trying to perform a swapout.  Sure, it's
rare now, but not on low memory machines.

> About your proposed change (as I just said), the semantic you want to
> change whith your diff that I am quoting here:
> 
> >-               if (!freed && !(gfp_mask & (__GFP_MED | __GFP_HIGH)))
> >+               if (!freed && !(gfp_mask & __GFP_HIGH))
> 
> makes a difference _only_ if the machine goes _OOM_ (so when
> try_to_free_pages fails) and that's completly _unrelated_ with the
> failed-atomic-allocation case we are talking about. If the machine is OOM
> it's perfectly normal that atomic allocations fails. That's expected.

Wrong.  I have to agree with Rik on this one: even during normal use, you
want to keep at least freepages.min pages reserved only for atomic memory
allocations.  We did this back in 2.0, even in 1.2, and for good reason:
if an interrupt comes along and needs to allocate memory, we do *not* want
to be calling do_try_to_free_pages from irq context -- it throws the
latency of the interrupt through the roof!  Keeping that small pool of
atomic-only memory around means that the machine can continue routing
packets while netscape is starting up and eating all your memory, without
needlessly dropping packets or adding latency.  It also means that there
are a few pages grace for helping to combat memory fragmentation, and
gives heavy NFS traffic a better chance at getting thru.

> You are basically making GFP_KERNEL equal to GFP_USER and that's wrong. It
> makes sense that allocation triggered by the kernel have access to the
> whole free memory available: kernel allocations have the same privilegies
> of atomic allocations. The reason atomic allocations are atomic is that
> they can't sleep, that's all. If they could sleep they would be GFP_KERNEL
> allocations instead. The only difference between the two, is that the
> non-atomic allocations will free memory themselfs before accessing _all_
> the free memory because they _can_ do that.

GFP_KERNEL for the normal case *is* equal to GFP_USER.  It's GFP_BUFFER
and GFP_ATOMIC that are special and need to have access to a normally
untouched pool of memory. 

		-ben

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Andrea Arcangeli]

On Fri, 21 Jan 2000, Benjamin C.R. LaHaise wrote:

>I think the deadlock being referred by Rik to is that of getting stuck
>trying to allocate memory while trying to perform a swapout.  Sure, it's
>rare now, but not on low memory machines.

It's exactly the same problem for getblk. I am aware of the create_buffers
async allocation loop too but that's even less unlikely to happens
thanks to the async-reserved bh heads.

>> About your proposed change (as I just said), the semantic you want to
>> change whith your diff that I am quoting here:
>> 
>> >-               if (!freed && !(gfp_mask & (__GFP_MED | __GFP_HIGH)))
>> >+               if (!freed && !(gfp_mask & __GFP_HIGH))
>> 
>> makes a difference _only_ if the machine goes _OOM_ (so when
>> try_to_free_pages fails) and that's completly _unrelated_ with the
>> failed-atomic-allocation case we are talking about. If the machine is OOM
>> it's perfectly normal that atomic allocations fails. That's expected.
>
>Wrong.  I have to agree with Rik on this one: even during normal use, you
>want to keep at least freepages.min pages reserved only for atomic memory
>allocations.  We did this back in 2.0, even in 1.2, and for good reason:

During normal use you _just_ take freepages.min pages reserved only for
atomic memory allocations.

It's during OOM that you want to eat all memory. Why should you left
3mbyte of memory unused if there aren't atomic allocations at all?

>if an interrupt comes along and needs to allocate memory, we do *not* want
>to be calling do_try_to_free_pages from irq context -- it throws the
>latency of the interrupt through the roof!  Keeping that small pool of
>atomic-only memory around means that the machine can continue routing
>packets while netscape is starting up and eating all your memory, without
>needlessly dropping packets or adding latency.  It also means that there
>are a few pages grace for helping to combat memory fragmentation, and
>gives heavy NFS traffic a better chance at getting thru.

That just happens. I definitely don't see your point. We just reserve such
atomic pool during normal use.

What you want to change is to left such pool reserved also during OOM and
that make no sense at all to me. During OOM atomic allocation should fail
exactly like all other allocations.

The current problem isn't that GFP_KERNEL is eating the atomic memory, but
that GFP_ATOMIC is eating all the pool before kswapd can notice that. My
patch will address exactly that.

>> You are basically making GFP_KERNEL equal to GFP_USER and that's wrong. It
>> makes sense that allocation triggered by the kernel have access to the
>> whole free memory available: kernel allocations have the same privilegies
>> of atomic allocations. The reason atomic allocations are atomic is that
>> they can't sleep, that's all. If they could sleep they would be GFP_KERNEL
>> allocations instead. The only difference between the two, is that the
>> non-atomic allocations will free memory themselfs before accessing _all_
>> the free memory because they _can_ do that.
>
>GFP_KERNEL for the normal case *is* equal to GFP_USER.  It's GFP_BUFFER

_Exactly_ I just know that. The only difference between GFP_KERNEL and
GFP_USER happens during OOM and you want to make GFP_KERNEL and GFP_USER
equal also during OOM and that make no sense to me. During OOM there's no
one point to not use the remaining 3mbyte for kernel progresses. We just
make sure to kill userspace ASAP anyway (that's the point for the 
GFP_USER difference between GFP_KERNEL).

>and GFP_ATOMIC that are special and need to have access to a normally
>untouched pool of memory. 

IMHO only GFP_USER shouldn't have access to the whole memory for a subtle
reason: we want to kill userspace as soon as we detect OOM. Everything
else can eat all the memory. This not only make sense to me but it will
also give us more probability to not fail anything except the killed task.

BTW, I just noticed GFP_BUFFER has a strange thing. He uses a low prio and
so it can't access the whole memory. This seems wrong. I think it should
be changed this way:

--- 2.2.14/include/linux/mm.h	Fri Jan 21 03:31:05 2000
+++ /tmp/mm.h	Sat Jan 22 14:57:40 2000
@@ -334,7 +334,7 @@
 
 #define __GFP_DMA	0x80
 
-#define GFP_BUFFER	(__GFP_LOW | __GFP_WAIT)
+#define GFP_BUFFER	(__GFP_MID | __GFP_WAIT)
 #define GFP_ATOMIC	(__GFP_HIGH)
 #define GFP_USER	(__GFP_LOW | __GFP_WAIT | __GFP_IO)
 #define GFP_KERNEL	(__GFP_MED | __GFP_WAIT | __GFP_IO)


NOTE: the above patch is unrelated to the atomic allocation problems, it's
just another thing I noticed now.

	ftp://ftp.*.kernel.org/pub/linux/kernel/people/andrea/patches/v2.2/2.2.14/GFP_BUFFER-1.gz

Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

GFP_XXX semantics (was: Re: [PATCH] 2.2.1{3,4,5} VM fix)

[Ingo Oeser]

On Sat, 22 Jan 2000, Andrea Arcangeli wrote:

[GFP-Mask semantics discussion]

ok, once we are about it here, could you please explain the
_exact_ semantics for the GFP_XXX constants?

GFP_BUFFER
GFP_ATOMIC
GFP_BIGUSER
GFP_USER
GFP_KERNEL
GFP_NFS
GFP_KSWAPD

So which steps are tried to allocate these pages (freeing
process, freeing globally, waiting, failing, kswapd-wakeup)? 

Because it is not easy to decide from a driver writers point of
view, which one to use for which requests :(

Thanks and Regards

Ingo Oeser
-- 
Feel the power of the penguin - run linux@your.pc
<esc>:x

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: GFP_XXX semantics (was: Re: [PATCH] 2.2.1{3,4,5} VM fix)

[Andrea Arcangeli]

On Mon, 24 Jan 2000, Ingo Oeser wrote:

>ok, once we are about it here, could you please explain the
>_exact_ semantics for the GFP_XXX constants?

NOTE: this first explanation is relative to the semantics of the GFP_* in
2.2.14aa2.

>GFP_BUFFER

GFP_BUFFER is equal to GFP_KERNEL (see below), with the difference that
while trying to free memory it will never generate I/O on disk, so if you
are allocating memory with potential FS lock held you _must_ use
GFP_BUFFER to avoid deadlocking by recursing on the lock. This is used for
example to allocate buffer memory in the getblk code that gets recalled
also with the superblock locked.

>GFP_ATOMIC

GFP_ATOMIC has to be called if you can't afford sleeping (for example to
allocate memory within an IRQ or a BH handler).

There's also at least one subtle usage of GFP_ATOMIC that makes perfect
sense (the bigmem swap-bounce code) but that is a quite special case.

>GFP_BIGUSER

Equal to GFP_USER (see below) but it will try to return you BIGMEMORY if
possible. BIGMEMORY is memory that can't be used all over the place and
you should never use GFP_BIGUSER unless you know well what are you doing.
The kernel just try to allocate bigmemory where it knows he can handle it
(like while allocating userspace anonymous memory inside page faults or
for vmalloced memory).

BTW, GFP_BIGUSER in 2.2.14aa2 == GFP_HIGHUSER in 2.3.40.

>GFP_USER

GFP_USER fails just while the system is only going OOM. Going OOM means
that there's nothing of freeable anymore and at the same time the system
is low on memory.

GFP_USER should be used on all allocations that are triggered directly
from userspace and usually that by failing the allocation you'll be able
recover the out of memory condition (for example in the page fault handler
we use GFP_USER to kill ASAP the task that is making the system to go
OOM).

>GFP_KERNEL

GFP kernel should be used for all normal kernel allocations where you can
sleep.

>GFP_NFS

Equal to GFP_KERNEL but supposed to have more priority (actually they have
the same prio).

>GFP_KSWAPD

It simply tells kswapd that it should free all kinds of memory (in the
bigmem case only non-bigmem memory to not break atomic allocations
reliability). Only kswapd uses GFP_KSWAPD.

>So which steps are tried to allocate these pages (freeing
>process, freeing globally, waiting, failing, kswapd-wakeup)? 

Actually in 2.2.14aa2 it's something like that:

------------------------------------------
if (no GFP_ATOMIC && system low on memory)
{
	succeed = try_to_free_some_memory(do IO only if it's not GFP_BUFFER);

	if (!succeed)
		if GFP_USER then fail even there's still something in the freelist;
}

grab from the freelist and return the memory if there's still something available
------------------------------------------

In 2.2.15pre4 instead GFP_KERNEL is equal to the 2.2.14aa2 (above
described) GFP_USER. And in 2.2.15pre4 (and 2.2.14) GFP_BUFFER is equal to
GFP_USER too with the exception that GFP_BUFFER is not allowed to do I/O
to release memory as usual).

I understood that the argument for the 2.2.15pre4 differences is that they
want to disallow GFP_BUFFER and GFP_KERNEL to eat all the free memory to
left such memory for the PF_MEMALLOC path. But the whole point is that
during the PF_MEMALLOC path everybody can ping flood the machine and eat
such last memory from interrupts anyway (or GFP_NFS could be used as
well). So if the machine deadlocks with 2.2.14aa2 it will deadlock with
2.2.15pre4 too.

If the machine deadlocks inside the PG_MEMALLOC path that's plain a bug in
the PF_MEMALLOC path and not something that can be fixed inside GFP in any
way.

And during OOM it make sense to me that we allow the system to use the
last memory available (unless it's an USER allocation that we want to kill
immediatly before nr_free_pages goes to zero so we'll have more changes
that the kernel-stuff won't be affected by the temporary memory shortage).

Note that during OOM you are going to have all the swap space allocated
thus the PF_MEMALLOC path is not going to allocate memory or do I/O
anyway.

Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: GFP_XXX semantics (was: Re: [PATCH] 2.2.1{3,4,5} VM fix)

[Rik van Riel]

On Mon, 24 Jan 2000, Andrea Arcangeli wrote:

> >GFP_KERNEL
> 
> GFP kernel should be used for all normal kernel allocations where you can
> sleep.
> 
> >GFP_NFS
> 
> Equal to GFP_KERNEL but supposed to have more priority (actually they have
> the same prio).

Do I smell a deadlock here? :)

HINT: deadlocks of this kind have been observed in 2.2.13
and 2.2.14. GFP_KERNEL allocations had eaten into the last
free pages (below freepages.min) and seriously confused
some algorithms that depend on the emergency pages.

(yeah, I know that's broken but it's all we've got)

> I understood that the argument for the 2.2.15pre4 differences is
> that they want to disallow GFP_BUFFER and GFP_KERNEL to eat all
> the free memory to left such memory for the PF_MEMALLOC path.

Indeed. If you cannot rely on PF_MEMALLOC any more, you're
bust.

> But the whole point is that during the PF_MEMALLOC path everybody
> can ping flood the machine and eat such last memory from
> interrupts anyway (or GFP_NFS could be used as well).

Of course it can. Now think about likelyhood...

> If the machine deadlocks inside the PG_MEMALLOC path that's plain
> a bug in the PF_MEMALLOC path and not something that can be fixed
> inside GFP in any way.

Indeed, the real solution is to fix the buggy code.

> And during OOM it make sense to me that we allow the system to use
> the last memory available (unless it's an USER allocation that we
> want to kill immediatly before nr_free_pages goes to zero so we'll
> have more changes that the kernel-stuff won't be affected by the
> temporary memory shortage).

I'm now running my system (and NL.linux.org) with a
temporary kludge. I only allow __GFP_MED allocations
to eat into the first half of freepages.min.

I hope this will (for now) avoid the worst crashes
from both sides. Again, this is not the solution. We
should search for the buggy code and try to fix as
much as possible before 2.2.15...

regards,

Rik
--
The Internet is not a network of computers. It is a network
of people. That is its real strength.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: GFP_XXX semantics (was: Re: [PATCH] 2.2.1{3,4,5} VM fix)

[Andi Kleen]

On Mon, Jan 24, 2000 at 02:22:45PM +0100, Ingo Oeser wrote:
> On Sat, 22 Jan 2000, Andrea Arcangeli wrote:
> 
> [GFP-Mask semantics discussion]
> 
> ok, once we are about it here, could you please explain the
> _exact_ semantics for the GFP_XXX constants?
> 
> GFP_BUFFER
> GFP_ATOMIC
> GFP_BIGUSER
> GFP_USER
> GFP_KERNEL
> GFP_NFS
> GFP_KSWAPD
> 
> So which steps are tried to allocate these pages (freeing
> process, freeing globally, waiting, failing, kswapd-wakeup)? 
> 
> Because it is not easy to decide from a driver writers point of
> view, which one to use for which requests :(

As device driver writer you should only use two:
GFP_ATOMIC in interrupts/bottom halves/when you cannot sleep and
GFP_KERNEL when you're in user context and able to sleep.
All others are internal and only used by specific subsystems you
shouldn't care about.

-Andi

-- 
This is like TV. I don't like TV.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Rik van Riel]

On Fri, 21 Jan 2000, Andrea Arcangeli wrote:
> On Thu, 20 Jan 2000, Rik van Riel wrote:
> >On Thu, 20 Jan 2000, Andrea Arcangeli wrote:
> >
> >> There's a limit of max swap request, after that a wait_on_page
> >> will trigger I/O. So you'll only decrease performance by doing so
> >> as far I can tell.
> >
> >Not really. We want to get the I/O done and we don't want
> >to wait until the queue fills up.
> 
> The bank gives us 32 pages of credit. We don't need to get the I/O
> on them. We have a credit that we can use to optimze the I/O.

We must clean up those 32 pages because we also use them
for swapin readaround.

> >> > 			tsk->state = TASK_INTERRUPTIBLE;
> >> >-			schedule_timeout(10*HZ);
> >> >+			schedule_timeout(HZ);
> >> 
> >> I used 10 sec because if you run oom into kswapd it means you
> >> can't hardly do anything within kswapd in the next 10 seconds.
> >
> >OOM is not the normal state the system is in. You really want
> 
> And infact such schedule_timeout(10*HZ) will happen _only_ when
> you are OOM.

Hmmm, you're right. It's even worse than I thought...

Once we've reached free_pages.high, kswapd will sleep
and not wake up again until we've reached an emergency
situation. And when we are in an even worse emergency
kswapd will bloody SLEEP for 10 seconds!

How much more braindead can it get?
I'll make a new version of the patch
in a moment...

> >kswapd to do some work in the background (allowing user programs
> >to allocate their bits of memory without stalling).
> 
> This should be accomplished by a pre-wakeup once the high
> watermark triggers. The HZ polling that is just doing is only for
> atomic allocations. To parallelize memory freeing a polling with
> HZ frequency is too slow.

We want kswapd to free memory before we reach an emergency
situation. That is, if nr_free_pages < freepages.high,
kswapd should free a few pages. We should check that every
second or so...

> >> If instead atomic allocations caused oom and kswapd failed then
> >> you should give up for some time as well since you know all memory
> >> is not freeable and so only a __free_pages will release memory.
> >
> >In the OOM case we'll have to kill a process. Stalling the
> >system forever is not a solution.
> 
> The system is not stalled. atomic stuff that doesn't need to alloc
> memory will continue to run. It will then complete and release
> memory later.

You hope. And even if it does, that's no excuse for stopping
kswapd for such a long time. We need kswapd to continue
background scanning because the `OOM' might just as well be
caused by our inability to unmap process pages that are in
heavy use...

> >> > 	wake_up_interruptible(&kswapd_wait);
> >> >-	if (gfp_mask & __GFP_WAIT)
> >> >+	if ((gfp_mask & __GFP_WAIT) && (nr_free_pages < (freepages.low - 4)))
> >> > 		retval = do_try_to_free_pages(gfp_mask);
> >> 
> >> -4 make no sense as ".low" could be as well set to 3 and
> >> theorically the system should remains stable.
> >
> >It does make sense in reality. Please read the explanation
> >I sent with the patch. I agree that somewhere halfway
> >freepages.low and freepages.min would be better, but there
> >is no need to complicate the calculation...
> 
> The calc is a linear scale 1/3 2/3 3/3. The difference between 512
> and 508 is zero in RL.

It's not about that. The 4 extra pages allow us to let the
process continue _immediately_ to let kswapd handle the dirty
work later on.

This is a Good Thing(tm) in a lot of situations since it
allows us to defer the page freeing until after we pushed
that HTML document out the door (do the page freeing in
what would otherwise have been idle time).

> >If kswapd can keep up, there's no need at all to slow down
> >processes, not even hogs. If kswapd can't keep up then we'll
> 
> If you don't want to slow down not-hogs-processes, you must make
> sure that who is allocating memory will block. If kswapd will be
> the only blocking the system will trash because the hog will
> continue to run.

`Who is allocating memory' is _not_ always the memory hog.
If there's a memory hog in the system, smaller (often
sleeping, interactive) processes will be swapped out. The
hog is using its memory (otherwise it wouldn't be considered
a hog) so it will have less of a chance of being swapped out.

Your idea will punish the wrong processes!

> >start stalling processes (just 5 allocations further away,
> >_and_ kswapd will have been started by that time and have had
> >the time to do something).
> 
> That's not a matter of swapout but of shrink_mmap. See my previous
> point about a pre-wakeup when the high watermark triggers. When
> swap is involved instead kswapd won't help anymore IMHO.

A shrink_mmap() will cost time (especially on 2.2), you want
to do that _later_, when the system is idle again.
FYI, non-idle systems are in the minority by far.

> >I consider the absense of the third boundary a HUGE bug
> >in current 2.2 VM. You need three boundaries:
> >
> >- min:  below this boundary only ATOMIC (and GFP_HIGH?)
> >        allocations can be done
> >- low:  below this boundary we start swapping agressively
> >- high: below this boundary we start background swapping
> >        (once a second, without impact on processes), above
> >        this boundary we stop swapping
> >
> >Current (2-border) code makes for bursty swap behaviour,
> >poor handling of kernel allocations (see the complaints
> >Alan got about systems which ran OOM on atomic allocations).
> 
> I missed Alan problems (probably they are hided by the l-k heavy
> traffic).

Systems ran out of memory for network buffers. This is
because kswapd isn't woken up early enough (pre-scanning
pages in the background _will_ make it easier to free
something in an emergency too, we'll have scanned memory
more often and have unmapped/cleaned more pages).

> high is just implemented in your way. The once a second is not
> enough for RL, you need a pre-wakeup if you want to take try to
> pre-free memory. And the pre-wakeup won't help in the swap case.

The pre-wakeup is triggered in part by the `-4' thing
and partly disfunctional because kswapd in 2.2 is more
braindead than I thought before :(

> the need of low and min make no sense to me. You may be more happy
> to know that all three freepages values in /proc are used for
> something but for your machine it will make no differences. Micro
> tuning make no sense to me. Only tuning that make differences in
> RL make sense to me.

The `micro tuning' _does_ make sense in real life. When
there are only a few allocations/second, kswapd can free
memory in the background, in idle time. This allows all
allocations to continue without stalling.

> >There is a good reason why there is a difference between
> >background swapping and agressive swapping, why there is
> >a separate swapout daemon, etc...
> 
> The separate swapout daemon is there only for allowing the system
> to free cache and userspace memory for atomic allocations. It has
> nothing to do with aggressive or not aggressive swapping.

Yes it has. It is clearly visible that it works that
way and the use of idle time for freeing helps for a
lot of types of system load.

> The only difference between high and low prio allocation is that
> high prio allocation are not triggered by an userspace action.
> Thus they'd better complete trying to allocate memory even if we
> are low on memory. low prio allocation are triggered by userspace
> so we want to fail ASAP if userspace triggered an oom. It has
> nothing to do with performance and it has not to do with the
> selection if somebody should stall or not.

True in a theoretical way. In practice `fast failing'
and `we can wait a little longer to see if things clear
up' are equal. So are `this must succeed' and `don't
hang around, continue'.

> If you are planning to change the current semantics of the GFP_*
> variables please work on 2.3.x only.

You have silently changed the semantics of various
things in the VM subsystem in 2.2. I'd like to
change it back to something that works.

regards,

Rik
--
The Internet is not a network of computers. It is a network
of people. That is its real strength.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.nl.linux.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Andrea Arcangeli]

On Fri, 21 Jan 2000, Rik van Riel wrote:

>We must clean up those 32 pages because we also use them
>for swapin readaround.

During the swapout that doesn't matter much since in the mix of swapin
swapout you may need the readaround in the middle of swap_out and not when
do_try_to_free_pages completed. If there's high memory and you do a
swapin, the first swapin will be a sync operation so you'll flush the I/O
and all further swapins will take advantage of readaround so that's a
minor issue too.

Anyway your fix wouldn't be correct either, because if it would be
necessary you should trigger I/O after all swapouts not only inside
kswapd. And you should never run_task_queue if you didn't recalled
swap_out().

>Hmmm, you're right. It's even worse than I thought...
>
>Once we've reached free_pages.high, kswapd will sleep
>and not wake up again until we've reached an emergency
>situation. And when we are in an even worse emergency
>kswapd will bloody SLEEP for 10 seconds!

That's fine instead.

>We want kswapd to free memory before we reach an emergency
>situation. That is, if nr_free_pages < freepages.high,
>kswapd should free a few pages. We should check that every
>second or so...

_It's_ checking every second (not or so).

But what I just said is that the check every second is almost useless for
normal allocations. In one second you just eaten all free memory. And I
think such pool loop should be shutted down completly. I think that kind
of poll is the real source of atomic-allocation problems. I'll do a patch
against 2.2.14 to show you what I think to have seen as problematic for
atomic allocations but IIRC nothing is changed in this respect between
2.2.13 and 2.2.14. Is the atomic allocation troubles started with 2.2.14
(I guess no)?

>`Who is allocating memory' is _not_ always the memory hog.

red herring. You know that we are based on probability.

>Your idea will punish the wrong processes!

Run the trashing_mem patch I pointed out to Alan. I debugged it with
printk and it punish the right process here. Give it a try. It will only
bias a bit more the current heuristic.

>The pre-wakeup is triggered in part by the `-4' thing
>and partly disfunctional because kswapd in 2.2 is more
>braindead than I thought before :(

I remeber how you driven kswapd in the past. You let it running all the
time in RT. Then when people come up and complained about oom-deadlocks,
so you started sending sigkill to processes from inside kswapd and you
given this as a solution. NOTE: I like your selection of the "best" task
but sending sigkill from kswapd is wrong IMHO. Sorry I don't agree in your
way and I merely suggest you to not take it again.

I got lots of oom lockup reports for 2.2.x. Now 2.2.14 has all my fixes
included and a kswapd that is been finally dominated, please don't
resurrect it to the old behaviour.

I think to see the atomic allocation problem and it will get fixed with
a one liner. But the problem cames from 2.2.13, not from my changes.

>> If you are planning to change the current semantics of the GFP_*
>> variables please work on 2.3.x only.
>
>You have silently changed the semantics of various
>things in the VM subsystem in 2.2. I'd like to
>change it back to something that works.

2.2.13 definitely doesn't work. If you don't believe me I'll send you a
flood of lockup reports (I got one security-advisory about oom lockups
even _today_ that 2.2.14 just includes all the fixes and doesn't deadlock
anymore with such exploits :).

And I am not changing the semantics of anything. Please read the diff
before complaining.

_You_ are the one that proposed new semantics for high/low priority
allocations. I only said you how things works since the late 2.1.x and I
agree with the current semanitc.

I'll try to send a patch against 2.2.14 for the atomic allocation thing
that I think to see ASAP.

Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Andrea Arcangeli]

On Fri, 21 Jan 2000, Andrea Arcangeli wrote:

>I'll try to send a patch against 2.2.14 for the atomic allocation thing
>that I think to see ASAP.

Ok this should fix the atomic allocation problem (that was present as well
in 2.2.13 and previous). It should also improve performance freeing memory
in parallel with the task that is allocating memory.

Way too late to try it out now in practice, but I checked it compiles and
looks strightforward:

--- 2.2.14-kswapd/mm/vmscan.c.~1~	Wed Jan  5 14:16:56 2000
+++ 2.2.14-kswapd/mm/vmscan.c	Fri Jan 21 03:25:59 2000
@@ -437,7 +437,7 @@
        printk ("Starting kswapd v%.*s\n", i, s);
 }
 
-static struct wait_queue * kswapd_wait = NULL;
+struct wait_queue * kswapd_wait;
 
 /*
  * The background pageout daemon, started as a kernel thread
@@ -485,7 +485,7 @@
 		 * the processes needing more memory will wake us
 		 * up on a more timely basis.
 		 */
-		interruptible_sleep_on_timeout(&kswapd_wait, HZ);
+		interruptible_sleep_on(&kswapd_wait);
 		while (nr_free_pages < freepages.high)
 		{
 			if (do_try_to_free_pages(GFP_KSWAPD))
@@ -519,7 +519,6 @@
 {
 	int retval = 1;
 
-	wake_up_interruptible(&kswapd_wait);
 	if (gfp_mask & __GFP_WAIT)
 		retval = do_try_to_free_pages(gfp_mask);
 	return retval;
--- 2.2.14-kswapd/mm/page_alloc.c.~1~	Wed Jan  5 14:16:56 2000
+++ 2.2.14-kswapd/mm/page_alloc.c	Fri Jan 21 03:29:40 2000
@@ -211,14 +211,20 @@
 	 */
 	if (!(current->flags & PF_MEMALLOC)) {
 		int freed;
+		extern struct wait_queue * kswapd_wait;
 
-		if (nr_free_pages > freepages.min) {
-			if (!low_on_memory)
-				goto ok_to_allocate;
-			if (nr_free_pages >= freepages.high) {
+		if (nr_free_pages >= freepages.high)
+		{
+			/* share RO cachelines in fast path */
+			if (low_on_memory)
 				low_on_memory = 0;
+			goto ok_to_allocate;
+		}
+		else
+		{
+			wake_up_interruptible(&kswapd_wait);
+			if (nr_free_pages > freepages.min && !low_on_memory)
 				goto ok_to_allocate;
-			}
 		}
 
 		low_on_memory = 1;

Please if you have problematic atomic allocation try the above patch.

It's downloadable also from here:

	ftp://ftp.*.kernel.org/pub/linux/kernel/people/andrea/patches/v2.2/2.2.14/atomic-allocations-1.gz

Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Rik van Riel]

On Fri, 21 Jan 2000, Andrea Arcangeli wrote:
> On Fri, 21 Jan 2000, Rik van Riel wrote:
> 
> >We must clean up those 32 pages because we also use them
> >for swapin readaround.
> 
> Anyway your fix wouldn't be correct either, because if it would be
> necessary you should trigger I/O after all swapouts not only
> inside kswapd. And you should never run_task_queue if you didn't
> recalled swap_out().

Fixed in the second version of the patch.

> >Once we've reached free_pages.high, kswapd will sleep
> >and not wake up again until we've reached an emergency
> >situation. And when we are in an even worse emergency
> >kswapd will bloody SLEEP for 10 seconds!
> 
> That's fine instead.

Why is this a good thing? I really don't see why it
helps us any bit...

> >We want kswapd to free memory before we reach an emergency
> >situation. That is, if nr_free_pages < freepages.high,
> >kswapd should free a few pages. We should check that every
> >second or so...
> 
> _It's_ checking every second (not or so).

True, my fault.

> But what I just said is that the check every second is almost
> useless for normal allocations. In one second you just eaten all
> free memory.

Not on a quiet machine. Or on a somewhat larger machine.
Think a webserver, or a desktop machine that's streaming
mp3s from disk or reading email.

In those loads there is enough idle time that kswapd can
free up memory in the background and memory consumption
is so slow that normal processes never stall or even leave
the fast path. That is definately a good thing.

> >`Who is allocating memory' is _not_ always the memory hog.
> 
> red herring. You know that we are based on probability.

Indeed. And the probability is that the most unused
process will be continuously swapping while the hog
is using up most of system memory. We've all seen
it happen.

> >Your idea will punish the wrong processes!
> 
> Run the trashing_mem patch I pointed out to Alan. I debugged it
> with printk and it punish the right process here. Give it a try.
> It will only bias a bit more the current heuristic.

Sounds like a good idea, but for 2.3. I'll check it out.

> >The pre-wakeup is triggered in part by the `-4' thing
> >and partly disfunctional because kswapd in 2.2 is more
> >braindead than I thought before :(
> 
> I remeber how you driven kswapd in the past. You let it running
> all the time in RT. Then when people come up and complained about
> oom-deadlocks, so you started sending sigkill to processes from
> inside kswapd and you given this as a solution. NOTE: I like your
> selection of the "best" task but sending sigkill from kswapd is
> wrong IMHO. Sorry I don't agree in your way and I merely suggest
> you to not take it again.

You're completely right about this one. We've both made our
mistakes and we should definately fight this argument.
May the best code win!

> I got lots of oom lockup reports for 2.2.x. Now 2.2.14 has all my
> fixes included and a kswapd that is been finally dominated, please
> don't resurrect it to the old behaviour.
>
> I think to see the atomic allocation problem and it will get fixed with
> a one liner. But the problem cames from 2.2.13, not from my changes.

Both 2.2.13, .14 and the pre-15s have the atomic allocation
problem. I hope to have it fixed with my patch.

> And I am not changing the semantics of anything. Please read the
> diff before complaining.

What diff? I haven't seen you post anything.
And if it's too big to post, it's definately
too big to go into the stable kernel...

> I'll try to send a patch against 2.2.14 for the atomic allocation
> thing that I think to see ASAP.

OK, great. Let's try to use each other's ideas and
make the code as good as possible.

kind regards,

Rik
--
The Internet is not a network of computers. It is a network
of people. That is its real strength.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/

Re: [PATCH] 2.2.1{3,4,5} VM fix

[Andrea Arcangeli]

On Fri, 21 Jan 2000, Rik van Riel wrote:

>> >Once we've reached free_pages.high, kswapd will sleep
>> >and not wake up again until we've reached an emergency
>> >situation. And when we are in an even worse emergency
>> >kswapd will bloody SLEEP for 10 seconds!
>> 
>> That's fine instead.
>
>Why is this a good thing? I really don't see why it
>helps us any bit...

The worse emergency is oom. During oom kswapd can only harm. Only real
application can do something getting killed or releasing memory thus
having kswapd out of the game will allow to handle oom gracefully.

>Not on a quiet machine. Or on a somewhat larger machine.
>Think a webserver, or a desktop machine that's streaming
>mp3s from disk or reading email.

Reading emails my mailer executable grow with a 35mbyte/sec rate
regularly.

>In those loads there is enough idle time that kswapd can
>free up memory in the background and memory consumption
>is so slow that normal processes never stall or even leave
>the fast path. That is definately a good thing.

This is true only for shrink_mmap (for swap there's not enough time). The
pre-wakeup patch I posted this night will take care of this memory-freeing
in background (potentially in a parallel cpu).

>Indeed. And the probability is that the most unused
>process will be continuously swapping while the hog
>is using up most of system memory. We've all seen
>it happen.

If you'll apply my trashing_mem patch that bias the current trashing_mem
heuristic making it a per-task thing you would see what happens instead to
a trashing hog.

>> Run the trashing_mem patch I pointed out to Alan. I debugged it
>> with printk and it punish the right process here. Give it a try.
>> It will only bias a bit more the current heuristic.
>
>Sounds like a good idea, but for 2.3. I'll check it out.

My per-task trashing_mem is used for production just now in 2.2.x since
quite some time ago. it's not a news. it's also how I designed the code
originally (that make more sense to me).

>> And I am not changing the semantics of anything. Please read the
>> diff before complaining.
>
>What diff? [..]

	ftp://ftp.*.kernel.org/pub/linux/kernel/v2.2/patch-2.2.14.gz

(or precisely the interesting part with the sparc updates excluded is:

	ftp://ftp.*.kernel.org/pub/linux/kernel/people/andrea/kernels/v2.2/2.2.13aa6/oom-2.2.12-I.gz
)

>And if it's too big to post, it's definately
>too big to go into the stable kernel...

So backout patch-2.2.14 if you want to return to get oom deadlocks :). I
took the small patch approch and then Linus didn't liked it and he liked
the way that 2.3.x took and to do so I had to break all archs but then I
am been able to do things like sending SIGTERM to process in iopl so that
they can recover the oom gracefully without screwing up the console.

>> I'll try to send a patch against 2.2.14 for the atomic allocation
>> thing that I think to see ASAP.
>
>OK, great. Let's try to use each other's ideas and
>make the code as good as possible.

I have not used your ideas, but someway discussing with you
I seen what could be the allocation problem, so the discussion IMHO had a
positive result :). You didn't either changed GFP in your patches (while I
only reworked GFP) and you didn't killed the 1 second polling (while I
killed the 1 second polling).

IMHO, the right fix is to wakeup kswapd from GFP unconditionally when the
low watermark triggers as I did in my patch. In the patch I posted I am
waking up on the _high_ watermark, but I just did a new version of the
patch that wakeups at the _low_ watermark. Using the _high_ is safer (it's
better for atomic allocations) but it will produce worse performance
because kswapd will be wakenup too often for a little work. While waking
up at the low (and it's not low but "low" here means middle) watermark
will wakeup kswapd only when there is some good work to do. Hopefully
waking up at the low watermark won't be too late... But I'd like to try it
out because for the normal allocations the low limit seems way better than
the high one.

The new patch against 2.2.14 is here:

	ftp://ftp.*.kernel.org/pub/linux/kernel/people/andrea/patches/v2.2/2.2.14/atomic-allocations-2.gz

The only difference between my last patch and the new patch is this:

--- 2.2.14-kswapd/mm/page_alloc.c.~2~	Fri Jan 21 03:29:40 2000
+++ 2.2.14-kswapd/mm/page_alloc.c	Fri Jan 21 13:50:58 2000
@@ -222,7 +222,8 @@
 		}
 		else
 		{
-			wake_up_interruptible(&kswapd_wait);
+			if (nr_free_pages < freepages.low)
+				wake_up_interruptible(&kswapd_wait);
 			if (nr_free_pages > freepages.min && !low_on_memory)
 				goto ok_to_allocate;
 		}

Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/