kernel/trace:check the val against the available mem

It is reported that some user app would like to echo a huge number to "/sys/kernel/debug/tracing/buffer_size_kb" regardless
of the available memory, which will cause the coinstantaneous
page allocation failed and introduce OOM. The commit checking the val against the available mem first to avoid the consequence allocation.

Quoted 6 lines · On Thu, 29 Mar 2018 18:41:44 +0800 Zhaoyang Huang <[email protected]>…

It is reported that some user app would like to echo a huge number to "/sys/kernel/debug/tracing/buffer_size_kb" regardless
of the available memory, which will cause the coinstantaneous
page allocation failed and introduce OOM. The commit checking the val against the available mem first to avoid the consequence allocation.

One of my tests is to stress buffer_size_kb, and it fails nicely if you try to get too much. Although, it may cause an OOM, but that's expected.

The application should do the test (try "free" on the command line). This isn't something that the kernel should be responsible for. If someone wants to allocate all memory for tracing, that's their prerogative.

-- Steve

Steve, thanks for your quick feedback. The original purpose is to avoid such kind
of OOM as kernel can not define the behavior of the application. I think it is no need
to do the alloc->free process if we have known the number of pages difinitly lead to failure.
Furthermore, the app which screw up the thing usually escape the OOM and cause the innocent to be sacrificed.

It is reported that some user app would like to echo a huge number to "/sys/kernel/debug/tracing/buffer_size_kb" regardless
of the available memory, which will cause the coinstantaneous
page allocation failed and introduce OOM. The commit checking the val against the available mem first to avoid the consequence allocation.

Signed-off-by Zhaoyang Huang <[email protected]>

---
 kernel/trace/trace.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

kernel/trace/trace.c +25 -0

index 2d0ffcc..a4a4237 100644
@@ -43,6 +43,8 @@
 #include <linux/trace.h>
 #include <linux/sched/rt.h>
 
+#include <linux/mm.h>
+#include <linux/swap.h>
 #include "trace.h"
 #include "trace_output.h"
 

@@ -5967,6 +5969,39 @@ static ssize_t tracing_splice_read_pipe(struct file *filp,
 	return ret;
 }
 
+static long get_available_mem(void)
+{
+	struct sysinfo i;
+	long available;
+	unsigned long pagecache;
+	unsigned long wmark_low = 0;
+	unsigned long pages[NR_LRU_LISTS];
+	struct zone *zone;
+	int lru;
+
+	si_meminfo(&i);
+	si_swapinfo(&i);
+
+	for (lru = LRU_BASE; lru < NR_LRU_LISTS; lru++)
+		pages[lru] = global_page_state(NR_LRU_BASE + lru);
+
+	for_each_zone(zone)
+		wmark_low += zone->watermark[WMARK_LOW];
+
+	available = i.freeram - wmark_low;
+
+	pagecache = pages[LRU_ACTIVE_FILE] + pages[LRU_INACTIVE_FILE];
+	pagecache -= min(pagecache / 2, wmark_low);

kernel/trace/trace.c +3 -1

 static ssize_t
 tracing_entries_write(struct file *filp, const char __user *ubuf,
 		      size_t cnt, loff_t *ppos)

@@ -5975,13 +6010,15 @@ static ssize_t tracing_splice_read_pipe(struct file *filp,
 	struct trace_array *tr = inode->i_private;
 	unsigned long val;
 	int ret;
+	long available;
 
+	available = get_available_mem();
 	ret = kstrtoul_from_user(ubuf, cnt, 10, &val);
 	if (ret)
 		return ret;
 
 	/* must have at least 1 entry */
-	if (!val)
+	if (!val || (val > available))
 		return -EINVAL;
 
 	/* value is in KB */

[ Adding memory management folks to discuss the issue ]

Quoted 51 lines · On Thu, 29 Mar 2018 18:41:44 +0800 Zhaoyang Huang <[email protected]>…

It is reported that some user app would like to echo a huge number to "/sys/kernel/debug/tracing/buffer_size_kb" regardless
of the available memory, which will cause the coinstantaneous
page allocation failed and introduce OOM. The commit checking the val against the available mem first to avoid the consequence allocation.

Signed-off-by Zhaoyang Huang <[email protected]>

---
 kernel/trace/trace.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

kernel/trace/trace.c +25 -0

index 2d0ffcc..a4a4237 100644
@@ -43,6 +43,8 @@
 #include <linux/trace.h>
 #include <linux/sched/rt.h>
 
+#include <linux/mm.h>
+#include <linux/swap.h>
 #include "trace.h"
 #include "trace_output.h"
 

@@ -5967,6 +5969,39 @@ static ssize_t tracing_splice_read_pipe(struct file *filp,
      return ret;
 }
 
+static long get_available_mem(void)
+{
+     struct sysinfo i;
+     long available;
+     unsigned long pagecache;
+     unsigned long wmark_low = 0;
+     unsigned long pages[NR_LRU_LISTS];
+     struct zone *zone;
+     int lru;
+
+     si_meminfo(&i);
+     si_swapinfo(&i);
+
+     for (lru = LRU_BASE; lru < NR_LRU_LISTS; lru++)
+             pages[lru] = global_page_state(NR_LRU_BASE + lru);
+
+     for_each_zone(zone)
+             wmark_low += zone->watermark[WMARK_LOW];
+
+     available = i.freeram - wmark_low;
+
+     pagecache = pages[LRU_ACTIVE_FILE] + pages[LRU_INACTIVE_FILE];
+     pagecache -= min(pagecache / 2, wmark_low);

kernel/trace/trace.c static ssize_t tracing_splice_read_pipe(struct file *filp, · line 6004

Quoted 10 lines

kernel/trace/trace.c +10 -0

+     available += pagecache;
+
+     available += global_page_state(NR_SLAB_RECLAIMABLE) -
+             min(global_page_state(NR_SLAB_RECLAIMABLE) / 2, wmark_low);
+
+     if (available < 0)
+             available = 0;
+     return available;
+}
+

As I stated in my other reply, the above function does not belong in tracing.

That said, it appears you are having issues that were caused by the change by commit 848618857d2 ("tracing/ring_buffer: Try harder to allocate"), where we replaced NORETRY with RETRY_MAYFAIL. The point of NORETRY was to keep allocations of the tracing ring-buffer from causing OOMs. But the RETRY was too strong in that case, because there were

those that wanted to allocate large ring buffers but it would fail due to memory being used that could be reclaimed. Supposedly, RETRY_MAYFAIL is to allocate with reclaim but still allow to fail, and isn't suppose to trigger an OOM. From my own tests, this is obviously not the case.

Perhaps this is because the ring buffer allocates one page at a time, and by doing so, it can get every last available page, and if anything in the mean time does an allocation without MAYFAIL, it will cause an OOM. For example, when I stressed this I triggered this:

pool invoked oom-killer: gfp_mask=0x14200ca(GFP_HIGHUSER_MOVABLE), nodemask=(null), order=0, oom_score_adj=0 pool cpuset=/ mems_allowed=0

 CPU: 7 PID: 1040 Comm: pool Not tainted 4.16.0-rc4-test+ #663
 Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016
 Call Trace:
  dump_stack+0x8e/0xce
  dump_header.isra.30+0x6e/0x28f
  ? _raw_spin_unlock_irqrestore+0x30/0x60
  oom_kill_process+0x218/0x400
  ? has_capability_noaudit+0x17/0x20
  out_of_memory+0xe3/0x5c0
  __alloc_pages_slowpath+0xa8e/0xe50
  __alloc_pages_nodemask+0x206/0x220
  alloc_pages_current+0x6a/0xe0
  __page_cache_alloc+0x6a/0xa0
  filemap_fault+0x208/0x5f0
  ? __might_sleep+0x4a/0x80

  ext4_filemap_fault+0x31/0x44
  __do_fault+0x20/0xd0
  __handle_mm_fault+0xc08/0x1160
  handle_mm_fault+0x76/0x110
  __do_page_fault+0x299/0x580
  do_page_fault+0x2d/0x110
  ? page_fault+0x2f/0x50
  page_fault+0x45/0x50

I wonder if I should have the ring buffer allocate groups of pages, to avoid this. Or try to allocate with NORETRY, one page at a time, and when that fails, allocate groups of pages with RETRY_MAYFAIL, and that may keep it from causing an OOM?

Quoted 5 lines

That said, it appears you are having issues that were caused by the change by commit 848618857d2 ("tracing/ring_buffer: Try harder to allocate"), where we replaced NORETRY with RETRY_MAYFAIL. The point of NORETRY was to keep allocations of the tracing ring-buffer from causing OOMs. But the RETRY was too strong in that case, because there were

Yes this was discussed with -mm folks. Basically the problem we were seeing is devices with tonnes of free memory (but free as in free but used by page cache) were not being used so it was unnecessarily failing to allocate ring buffer on the system with otherwise lots of memory.

IIRC, the OOM that my patch was trying to avoid, was being triggered in the path/context of the write to buffer_size_kb itself (when not doing the NORETRY), not by other processes.

Quoted 23 lines

Perhaps this is because the ring buffer allocates one page at a time, and by doing so, it can get every last available page, and if anything in the mean time does an allocation without MAYFAIL, it will cause an OOM. For example, when I stressed this I triggered this:

pool invoked oom-killer: gfp_mask=0x14200ca(GFP_HIGHUSER_MOVABLE), nodemask=(null), order=0, oom_score_adj=0 pool cpuset=/ mems_allowed=0

 CPU: 7 PID: 1040 Comm: pool Not tainted 4.16.0-rc4-test+ #663
 Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016
 Call Trace:
  dump_stack+0x8e/0xce
  dump_header.isra.30+0x6e/0x28f
  ? _raw_spin_unlock_irqrestore+0x30/0x60
  oom_kill_process+0x218/0x400
  ? has_capability_noaudit+0x17/0x20
  out_of_memory+0xe3/0x5c0
  __alloc_pages_slowpath+0xa8e/0xe50
  __alloc_pages_nodemask+0x206/0x220
  alloc_pages_current+0x6a/0xe0
  __page_cache_alloc+0x6a/0xa0
  filemap_fault+0x208/0x5f0
  ? __might_sleep+0x4a/0x80

Quoted 8 lines

  ext4_filemap_fault+0x31/0x44
  __do_fault+0x20/0xd0
  __handle_mm_fault+0xc08/0x1160
  handle_mm_fault+0x76/0x110
  __do_page_fault+0x299/0x580
  do_page_fault+0x2d/0x110
  ? page_fault+0x2f/0x50
  page_fault+0x45/0x50  

But this OOM is not in the path of the buffer_size_kb write, right? So then what does it have to do with buffer_size_kb write failure?

I guess the original issue reported is that the buffer_size_kb write causes *other* applications to fail allocation. So in that case, capping the amount that ftrace writes makes sense. Basically my point is I don't see how the patch you mentioned introduces the problem here
- in the sense the patch just makes ftrace allocate from memory it couldn't before and to try harder.

Quoted 7 lines

I wonder if I should have the ring buffer allocate groups of pages, to avoid this. Or try to allocate with NORETRY, one page at a time, and when that fails, allocate groups of pages with RETRY_MAYFAIL, and that may keep it from causing an OOM?

I don't see immediately how that can prevent an OOM in other applications here? If ftrace allocates lots of memory with RETRY_MAYFAIL, then we would still OOM in other applications if memory isn't available. Sorry if I missed something.

Quoted 10 lines

Quoted 5 lines

I wonder if I should have the ring buffer allocate groups of pages, to avoid this. Or try to allocate with NORETRY, one page at a time, and when that fails, allocate groups of pages with RETRY_MAYFAIL, and that may keep it from causing an OOM?

I don't see immediately how that can prevent an OOM in other applications here? If ftrace allocates lots of memory with RETRY_MAYFAIL, then we would still OOM in other applications if memory isn't available. Sorry if I missed something.

Here's the idea.

Allocate one page at a time with NORETRY. If that fails, then allocate larger amounts (higher order of pages) with RETRY_MAYFAIL. Then if it can't get all the memory it needs, it wont take up all memory in the system before it finds out that it can't have any more.

Or perhaps the memory management system can provide a get_available_mem() function that ftrace could call before it tries to increase the ring buffer and take up all the memory of the system before it realizes that it can't get all the memory it wants.

The main problem I have with Zhaoyang's patch is that get_available_mem() does not belong in the tracing code. It should be something that the mm subsystem provides.

That said, it appears you are having issues that were caused by the change by commit 848618857d2 ("tracing/ring_buffer: Try harder to allocate"), where we replaced NORETRY with RETRY_MAYFAIL. The point of NORETRY was to keep allocations of the tracing ring-buffer from causing OOMs. But the RETRY was too strong in that case, because there were those that wanted to allocate large ring buffers but it would fail due to memory being used that could be reclaimed. Supposedly, RETRY_MAYFAIL is to allocate with reclaim but still allow to fail, and isn't suppose to trigger an OOM. From my own tests, this is obviously not the case.

It seems to me that what you're asking for at the moment is lower-likelihood-of-failure-than-GFP_KERNEL, and it's not entirely clear to me why your allocation is so much more important than other allocations in the kernel.

Also, the pattern you have is very close to that of vmalloc. You're allocating one page at a time to satisfy a multi-page request. In lieu of actually thinking about what you should do, I might recommend using the same GFP flags as vmalloc() which works out to GFP_KERNEL | __GFP_NOWARN (possibly | __GFP_HIGHMEM if you can tolerate having to kmap the pages when accessed from within the kernel).

I'll take a look at si_mem_available() that Joel suggested and see if we can make that work.

Quoted 2 lines · On Fri, 30 Mar 2018 17:30:31 -0400 Steven Rostedt <[email protected]> wrote:

I'll take a look at si_mem_available() that Joel suggested and see if we can make that work.

Wow, this appears to work great! Joel and Zhaoyang, can you test this?

-- Steve

kernel/trace/ring_buffer.c +0 -0

index a2fd3893cc02..32a803626ee2 100644

Quoted 6 lines

kernel/trace/ring_buffer.c +0 -0

@@ -1164,6 +1164,11 @@ static int __rb_allocate_pages(long nr_pages, struct list_head *pages, int cpu)
        struct buffer_page *bpage, *tmp;
        long i;
 

kernel/trace/ring_buffer.c static int __rb_allocate_pages(long nr_pages, struct list_head *pages, int cpu) · line 1169

Quoted 3 lines

kernel/trace/ring_buffer.c +3 -0

+       /* Check if the available memory is there first */
+       i = si_mem_available();
+       if (i < nr_pages)  

Does it make sense to add a small margin here so that after ftrace finishes allocating, we still have some memory left for the system? But then then we have to define a magic number :-|

kernel/trace/ring_buffer.c static int __rb_allocate_pages(long nr_pages, struct list_head *pages, int cpu) · line 1171

Quoted 1 line

kernel/trace/ring_buffer.c +1 -0

+  

I tested in Qemu with 1GB memory, I am always able to get it to fail allocation even without this patch without causing an OOM. Maybe I am not running enough allocations in parallel or something :)

The patch you shared using si_mem_available is working since I'm able to allocate till the end without a page allocation failure:

bash-4.3# echo 237800 > /d/tracing/buffer_size_kb
bash: echo: write error: Cannot allocate memory
bash-4.3# echo 237700 > /d/tracing/buffer_size_kb
bash-4.3# free -m
total used free shared buffers

Mem:           985          977            7           10            0
-/+ buffers:                977            7
Swap:            0            0            0

bash-4.3#

I think this patch is still good to have, since IMO we should not go and get page allocation failure (even if its a non-OOM) and subsequent stack dump from mm's allocator, if we can avoid it.

Tested-by Joel Fernandes <[email protected]>

Quoted 6 lines · On Fri, 30 Mar 2018 16:38:52 -0700 Joel Fernandes <[email protected]> wrote:

Quoted 6 lines

kernel/trace/ring_buffer.c +0 -0

@@ -1164,6 +1164,11 @@ static int __rb_allocate_pages(long nr_pages, struct list_head *pages, int cpu)
        struct buffer_page *bpage, *tmp;
        long i;
 

Quoted 7 lines

kernel/trace/ring_buffer.c static int __rb_allocate_pages(long nr_pages, struct list_head *pages, int cpu) · line 1169

Quoted 3 lines

kernel/trace/ring_buffer.c +3 -0

+       /* Check if the available memory is there first */
+       i = si_mem_available();
+       if (i < nr_pages)  

Does it make sense to add a small margin here so that after ftrace finishes allocating, we still have some memory left for the system? But then then we have to define a magic number :-|

I don't think so. The memory is allocated by user defined numbers. They can do "free" to see what is available. The original patch from Zhaoyang was due to a script that would just try a very large number and cause issues.

If the memory is available, I just say let them have it. This is borderline user space issue and not a kernel one.

Again though, this is the same pattern as vmalloc. There are any number of places where userspace can cause an arbitrarily large vmalloc to be attempted (grep for kvmalloc_array for a list of promising candidates). I'm pretty sure that just changing your GFP flags to GFP_KERNEL | __GFP_NOWARN will give you the exact behaviour that you want with no need to grub around in the VM to find out if your huge allocation is likely to succeed.

Quoted 7 lines · On Fri, 30 Mar 2018 19:18:57 -0700 Matthew Wilcox <[email protected]> wrote:

Again though, this is the same pattern as vmalloc. There are any number of places where userspace can cause an arbitrarily large vmalloc to be attempted (grep for kvmalloc_array for a list of promising candidates). I'm pretty sure that just changing your GFP flags to GFP_KERNEL | __GFP_NOWARN will give you the exact behaviour that you want with no need to grub around in the VM to find out if your huge allocation is likely to succeed.

Not sure how this helps. Note, I don't care about consecutive pages, so this is not an array. It's a link list of thousands of pages. How do you suggest allocating them? The ring buffer is a link list of pages.

Quoted 2 lines · On Fri, 30 Mar 2018 17:30:31 -0400 Steven Rostedt <[email protected]> wrote:

I'll take a look at si_mem_available() that Joel suggested and see if we can make that work.

Wow, this appears to work great! Joel and Zhaoyang, can you test this?

-- Steve

kernel/trace/ring_buffer.c +0 -0

index a2fd3893cc02..32a803626ee2 100644
@@ -1164,6 +1164,11 @@ static int __rb_allocate_pages(long nr_pages, struct list_head *pages, int cpu)
        struct buffer_page *bpage, *tmp;

[ Adding memory management folks to discuss the issue ]

Quoted 51 lines · On Thu, 29 Mar 2018 18:41:44 +0800 Zhaoyang Huang <[email protected]>…

It is reported that some user app would like to echo a huge number to "/sys/kernel/debug/tracing/buffer_size_kb" regardless
of the available memory, which will cause the coinstantaneous
page allocation failed and introduce OOM. The commit checking the val against the available mem first to avoid the consequence allocation.

Signed-off-by Zhaoyang Huang <[email protected]>

---
 kernel/trace/trace.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

kernel/trace/trace.c +25 -0

index 2d0ffcc..a4a4237 100644
@@ -43,6 +43,8 @@
 #include <linux/trace.h>
 #include <linux/sched/rt.h>
 
+#include <linux/mm.h>
+#include <linux/swap.h>
 #include "trace.h"
 #include "trace_output.h"
 

@@ -5967,6 +5969,39 @@ static ssize_t tracing_splice_read_pipe(struct file *filp,
 	return ret;
 }
 
+static long get_available_mem(void)
+{
+	struct sysinfo i;
+	long available;
+	unsigned long pagecache;
+	unsigned long wmark_low = 0;
+	unsigned long pages[NR_LRU_LISTS];
+	struct zone *zone;
+	int lru;
+
+	si_meminfo(&i);
+	si_swapinfo(&i);
+
+	for (lru = LRU_BASE; lru < NR_LRU_LISTS; lru++)
+		pages[lru] = global_page_state(NR_LRU_BASE + lru);
+
+	for_each_zone(zone)
+		wmark_low += zone->watermark[WMARK_LOW];
+
+	available = i.freeram - wmark_low;
+
+	pagecache = pages[LRU_ACTIVE_FILE] + pages[LRU_INACTIVE_FILE];
+	pagecache -= min(pagecache / 2, wmark_low);

kernel/trace/trace.c static ssize_t tracing_splice_read_pipe(struct file *filp, · line 6004

Quoted 10 lines

kernel/trace/trace.c +10 -0

+	available += pagecache;
+
+	available += global_page_state(NR_SLAB_RECLAIMABLE) -
+		min(global_page_state(NR_SLAB_RECLAIMABLE) / 2, wmark_low);
+
+	if (available < 0)
+		available = 0;
+	return available;
+}
+

As I stated in my other reply, the above function does not belong in tracing.

That said, it appears you are having issues that were caused by the change by commit 848618857d2 ("tracing/ring_buffer: Try harder to allocate"), where we replaced NORETRY with RETRY_MAYFAIL. The point of NORETRY was to keep allocations of the tracing ring-buffer from causing OOMs. But the RETRY was too strong in that case, because there were those that wanted to allocate large ring buffers but it would fail due to memory being used that could be reclaimed. Supposedly, RETRY_MAYFAIL is to allocate with reclaim but still allow to fail, and isn't suppose to trigger an OOM. From my own tests, this is obviously not the case.

Perhaps this is because the ring buffer allocates one page at a time, and by doing so, it can get every last available page, and if anything in the mean time does an allocation without MAYFAIL, it will cause an OOM. For example, when I stressed this I triggered this:

pool invoked oom-killer: gfp_mask=0x14200ca(GFP_HIGHUSER_MOVABLE), nodemask=(null), order=0, oom_score_adj=0 pool cpuset=/ mems_allowed=0

 CPU: 7 PID: 1040 Comm: pool Not tainted 4.16.0-rc4-test+ #663
 Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016
 Call Trace:
  dump_stack+0x8e/0xce
  dump_header.isra.30+0x6e/0x28f
  ? _raw_spin_unlock_irqrestore+0x30/0x60
  oom_kill_process+0x218/0x400
  ? has_capability_noaudit+0x17/0x20
  out_of_memory+0xe3/0x5c0
  __alloc_pages_slowpath+0xa8e/0xe50
  __alloc_pages_nodemask+0x206/0x220
  alloc_pages_current+0x6a/0xe0
  __page_cache_alloc+0x6a/0xa0
  filemap_fault+0x208/0x5f0
  ? __might_sleep+0x4a/0x80
  ext4_filemap_fault+0x31/0x44
  __do_fault+0x20/0xd0
  __handle_mm_fault+0xc08/0x1160
  handle_mm_fault+0x76/0x110
  __do_page_fault+0x299/0x580
  do_page_fault+0x2d/0x110
  ? page_fault+0x2f/0x50
  page_fault+0x45/0x50

I wonder if I should have the ring buffer allocate groups of pages, to avoid this. Or try to allocate with NORETRY, one page at a time, and when that fails, allocate groups of pages with RETRY_MAYFAIL, and that may keep it from causing an OOM?

Quoted 4 lines

I wonder if I should have the ring buffer allocate groups of pages, to avoid this. Or try to allocate with NORETRY, one page at a time, and when that fails, allocate groups of pages with RETRY_MAYFAIL, and that may keep it from causing an OOM?

I wonder why it really matters. The interface is root only and we expect some sanity from an admin, right? So allocating such a large ring buffer that it sends the system to the OOM is a sign that the admin should be more careful. Balancing on the OOM edge is always a risk and the result will highly depend on the workload running in parallel.

Quoted 10 lines · On Tue, 3 Apr 2018 13:06:12 +0200 Michal Hocko <[email protected]> wrote:

Quoted 4 lines

I wonder if I should have the ring buffer allocate groups of pages, to avoid this. Or try to allocate with NORETRY, one page at a time, and when that fails, allocate groups of pages with RETRY_MAYFAIL, and that may keep it from causing an OOM?

I wonder why it really matters. The interface is root only and we expect some sanity from an admin, right? So allocating such a large ring buffer that it sends the system to the OOM is a sign that the admin should be more careful. Balancing on the OOM edge is always a risk and the result will highly depend on the workload running in parallel.

This came up because there's scripts or programs that set the size of the ring buffer. The complaint was that the application would just set the size to something bigger than what was available and cause an OOM killing other applications. The final solution is to simply check the available memory before allocating the ring buffer:

	/* Check if the available memory is there first */
	i = si_mem_available();
	if (i < nr_pages)
		return -ENOMEM;

And it works well.

Quoted 11 lines

This came up because there's scripts or programs that set the size of the ring buffer. The complaint was that the application would just set the size to something bigger than what was available and cause an OOM killing other applications. The final solution is to simply check the available memory before allocating the ring buffer:

	/* Check if the available memory is there first */
	i = si_mem_available();
	if (i < nr_pages)
		return -ENOMEM;

Quoted 1 line

And it works well.

Except that it doesn't work. si_mem_available is not really suitable for any allocation estimations. Its only purpose is to provide a very rough estimation for userspace. Any other use is basically abuse. The situation can change really quickly. Really it is really hard to be clever here with the volatility the memory allocations can cause.

Quoted 11 lines · On Tue, 3 Apr 2018 14:16:14 +0200 Michal Hocko <[email protected]> wrote:

Quoted 11 lines

This came up because there's scripts or programs that set the size of the ring buffer. The complaint was that the application would just set the size to something bigger than what was available and cause an OOM killing other applications. The final solution is to simply check the available memory before allocating the ring buffer:

	/* Check if the available memory is there first */
	i = si_mem_available();
	if (i < nr_pages)
		return -ENOMEM;

Quoted 7 lines

Quoted 1 line

And it works well.

Except that it doesn't work. si_mem_available is not really suitable for any allocation estimations. Its only purpose is to provide a very rough estimation for userspace. Any other use is basically abuse. The situation can change really quickly. Really it is really hard to be clever here with the volatility the memory allocations can cause.

OK, then what do you suggest? Because currently, it appears to work. A rough estimate may be good enough.

If we use NORETRY, then we have those that complain that we do not try hard enough to reclaim memory. If we use RETRY_MAYFAIL we have this issue of taking up all memory before we get what we want.

Perhaps I should try to allocate a large group of pages with RETRY_MAYFAIL, and if that fails go back to NORETRY, with the thinking that the large allocation may reclaim some memory that would allow the NORETRY to succeed with smaller allocations (one page at a time)?

Quoted 3 lines

If we use NORETRY, then we have those that complain that we do not try hard enough to reclaim memory. If we use RETRY_MAYFAIL we have this issue of taking up all memory before we get what we want.

Just try to do what admin asks for and trust it will not try to shoot his foot? I mean there are other ways admin can shoot the machine down.

Being clever is OK if it doesn't add a tricky code. And relying on si_mem_available is definitely tricky and obscure.

Quoted 5 lines

Perhaps I should try to allocate a large group of pages with RETRY_MAYFAIL, and if that fails go back to NORETRY, with the thinking that the large allocation may reclaim some memory that would allow the NORETRY to succeed with smaller allocations (one page at a time)?

That again relies on a subtle dependencies of the current implementation. So I would rather ask whether this is something that really deserves special treatment. If admin asks for a buffer of a certain size then try to do so. If we get OOM then bad luck you cannot get large memory buffers for free...

On Tue, 3 Apr 2018 14:35:14 +0200
Michal Hocko <[email protected]> wrote:

Quoted 2 lines

Being clever is OK if it doesn't add a tricky code. And relying on si_mem_available is definitely tricky and obscure.

Can we get the mm subsystem to provide a better method to know if an allocation will possibly succeed or not before trying it? It doesn't have to be free of races. Just "if I allocate this many pages right now, will it work?" If that changes from the time it asks to the time it allocates, that's fine. I'm not trying to prevent OOM to never trigger. I just don't want to to trigger consistently.

Quoted 10 lines

Quoted 4 lines

Perhaps I should try to allocate a large group of pages with RETRY_MAYFAIL, and if that fails go back to NORETRY, with the thinking that the large allocation may reclaim some memory that would allow the NORETRY to succeed with smaller allocations (one page at a time)?

That again relies on a subtle dependencies of the current implementation. So I would rather ask whether this is something that really deserves special treatment. If admin asks for a buffer of a certain size then try to do so. If we get OOM then bad luck you cannot get large memory buffers for free...

That is not acceptable to me nor to the people asking for this.

The problem is known. The ring buffer allocates memory page by page, and this can allow it to easily take all memory in the system before it fails to allocate and free everything it had done.

If you don't like the use of si_mem_available() I'll do the larger pages method. Yes it depends on the current implementation of memory allocation. It will depend on RETRY_MAYFAIL trying to allocate a large number of pages, and fail if it can't (leaving memory for other allocations to succeed).

The allocation of the ring buffer isn't critical. It can fail to expand, and we can tell the user -ENOMEM. I original had NORETRY because I rather have it fail than cause an OOM. But there's folks (like Joel) that want it to succeed when there's available memory in page caches.

I'm fine if the admin shoots herself in the foot if the ring buffer gets big enough to start causing OOMs, but I don't want it to cause OOMs if there's not even enough memory to fulfill the ring buffer size itself.

Quoted 2 lines · Steven Rostedt

On Tue, 3 Apr 2018 14:35:14 +0200
Michal Hocko <[email protected]> wrote:

[...]

Quoted 9 lines

Quoted 2 lines

Being clever is OK if it doesn't add a tricky code. And relying on si_mem_available is definitely tricky and obscure.

Can we get the mm subsystem to provide a better method to know if an allocation will possibly succeed or not before trying it? It doesn't have to be free of races. Just "if I allocate this many pages right now, will it work?" If that changes from the time it asks to the time it allocates, that's fine. I'm not trying to prevent OOM to never trigger. I just don't want to to trigger consistently.

How do you do that without an actuall allocation request? And more fundamentally, what if your _particular_ request is just fine but it will get us so close to the OOM edge that the next legit allocation request simply goes OOM? There is simply no sane interface I can think of that would satisfy a safe/sensible "will it cause OOM" semantic.

Quoted 14 lines

Quoted 11 lines

Quoted 5 lines

Perhaps I should try to allocate a large group of pages with RETRY_MAYFAIL, and if that fails go back to NORETRY, with the thinking that the large allocation may reclaim some memory that would allow the NORETRY to succeed with smaller allocations (one page at a time)?

That again relies on a subtle dependencies of the current implementation. So I would rather ask whether this is something that really deserves special treatment. If admin asks for a buffer of a certain size then try to do so. If we get OOM then bad luck you cannot get large memory buffers for free...

That is not acceptable to me nor to the people asking for this.

Quoted 3 lines

The problem is known. The ring buffer allocates memory page by page, and this can allow it to easily take all memory in the system before it fails to allocate and free everything it had done.

Then do not allow buffers that are too large. How often do you need buffers that are larger than few megs or small % of the available memory? Consuming excessive amount of memory just to trace workload which will need some memory on its own sounds just dubious to me.

Quoted 7 lines

If you don't like the use of si_mem_available() I'll do the larger pages method. Yes it depends on the current implementation of memory allocation. It will depend on RETRY_MAYFAIL trying to allocate a large number of pages, and fail if it can't (leaving memory for other allocations to succeed).

Quoted 5 lines

The allocation of the ring buffer isn't critical. It can fail to expand, and we can tell the user -ENOMEM. I original had NORETRY because I rather have it fail than cause an OOM. But there's folks (like Joel) that want it to succeed when there's available memory in page caches.

Then implement a retry logic on top of NORETRY. You can control how hard to retry to satisfy the request yourself. You still risk that your allocation will get us close to OOM for _somebody_ else though.

Quoted 4 lines

I'm fine if the admin shoots herself in the foot if the ring buffer gets big enough to start causing OOMs, but I don't want it to cause OOMs if there's not even enough memory to fulfill the ring buffer size itself.

I simply do not see the difference between the two. Both have the same deadly effect in the end. The direct OOM has an arguable advantage that the effect is immediate rather than subtle with potential performance side effects until the machine OOMs after crawling for quite some time.

On Tue, 3 Apr 2018 15:56:07 +0200
Michal Hocko <[email protected]> wrote:

Quoted 4 lines

I simply do not see the difference between the two. Both have the same deadly effect in the end. The direct OOM has an arguable advantage that the effect is immediate rather than subtle with potential performance side effects until the machine OOMs after crawling for quite some time.

The difference is if the allocation succeeds or not. If it doesn't succeed, we free all memory that we tried to allocate. If it succeeds and causes issues, then yes, that's the admins fault.

I'm worried about
the accidental putting in too big of a number, either by an admin by mistake, or some stupid script that just thinks the current machines has terabytes of memory.

I'm under the assumption that if I allocate an allocation of 32 pages with RETRY_MAYFAIL, and there's 2 pages available, but not 32, and while my allocation is reclaiming memory, and another task comes in and asks for a single page, it can still succeed. This would be why I would be using RETRY_MAYFAIL with higher orders of pages, that it doesn't take all memory in the system if it fails. Is this assumption incorrect?

The current approach of allocating 1 page at a time with RETRY_MAYFAIL is that it will succeed to get any pages that are available, until there are none, and if some unlucky task asks for memory during that time, it is guaranteed to fail its allocation triggering an OOM.

I was thinking of doing something like:

	large_pages = nr_pages / 32;
	if (large_pages) {
		pages = alloc_pages_node(cpu_to_node(cpu),
				GFP_KERNEL | __GFP_RETRY_MAYFAIL, 5);
		if (pages)
			/* break up pages */
		else
			/* try to allocate with NORETRY */
	}

Now it will allocate memory in 32 page chunks using reclaim. If it fails to allocate them, it would not have taken up any smaller chunks that were available, leaving them for other users. It would then go back to singe pages, allocating with RETRY. Or I could just say screw it, and make the allocation of the ring buffer always be 32 page chunks (or at least make it user defined).

yes a fallback is questionable. Whether to make the batch size configuration is a matter of how much internal details you want to expose to userspace.

You can do so, of course. In fact it would have some advantages over single pages because you would fragment the memory less but this is not a reliable prevention from OOM killing and the complete memory depletion if you allow arbitrary trace buffer sizes.

Except that it doesn't work. si_mem_available is not really suitable for any allocation estimations. Its only purpose is to provide a very rough estimation for userspace. Any other use is basically abuse. The situation can change really quickly. Really it is really hard to be clever here with the volatility the memory allocations can cause.

From your earlier email:

Quoted 5 lines

Except that it doesn't work. si_mem_available is not really suitable for any allocation estimations. Its only purpose is to provide a very rough estimation for userspace. Any other use is basically abuse. The situation can change really quickly. Really it is really hard to be clever here with the volatility the memory allocations can cause.

Now can you please explain to me why si_mem_available is not suitable for my purpose.

On Tue 03-04-18 18:56:27, Steven Rostedt wrote:
[...]

Quoted 10 lines

From your earlier email:

Quoted 5 lines

Except that it doesn't work. si_mem_available is not really suitable for any allocation estimations. Its only purpose is to provide a very rough estimation for userspace. Any other use is basically abuse. The situation can change really quickly. Really it is really hard to be clever here with the volatility the memory allocations can cause.

Now can you please explain to me why si_mem_available is not suitable for my purpose.

Several problems. It is overly optimistic especially when we are close to OOM. The available pagecache or slab reclaimable objects might be pinned long enough that your allocation based on that estimation will just make the situation worse and result in OOM. More importantly though, your allocations are GFP_KERNEL, right, that means that such an allocation will not reach to ZONE_MOVABLE or ZONE_HIGMEM (32b systems) while the pagecache will. So you will get an overestimate of how much you can allocate.

Really si_mem_available is for proc/meminfo and a rough estimate of the free memory because users tend to be confused by seeing MemFree too low and complaining that the system has eaten all their memory. I have some

skepticism about how useful it is in practice apart from showing it in top or alike tools. The memory is simply not usable immediately or without an overall and visible effect on the whole system.

Quoted 2 lines

Now can you please explain to me why si_mem_available is not suitable for my purpose.

Several problems. It is overly optimistic especially when we are close to OOM. The available pagecache or slab reclaimable objects might be pinned long enough that your allocation based on that estimation will just make the situation worse and result in OOM. More importantly though, your allocations are GFP_KERNEL, right, that means that such an allocation will not reach to ZONE_MOVABLE or ZONE_HIGMEM (32b systems) while the pagecache will. So you will get an overestimate of how much you can allocate.

Really si_mem_available is for proc/meminfo and a rough estimate of the free memory because users tend to be confused by seeing MemFree too low and complaining that the system has eaten all their memory. I have some skepticism about how useful it is in practice apart from showing it in top or alike tools. The memory is simply not usable immediately or without an overall and visible effect on the whole system.

(quoted diff) +11 -0

+       /*
+        * Check if the available memory is there first.
+        * Note, si_mem_available() only gives us a rough estimate of available
+        * memory. It may not be accurate. But we don't care, we just want
+        * to prevent doing any allocation when it is obvious that it is
+        * not going to succeed.
+        */
+       i = si_mem_available();
+       if (i < nr_pages)
+               return -ENOMEM;
+

Better?

On Wed 04-04-18 08:59:01, Steven Rostedt wrote:
[...]

Quoted 12 lines

(quoted diff) +11 -0

+       /*
+        * Check if the available memory is there first.
+        * Note, si_mem_available() only gives us a rough estimate of available
+        * memory. It may not be accurate. But we don't care, we just want
+        * to prevent doing any allocation when it is obvious that it is
+        * not going to succeed.
+        */
+       i = si_mem_available();
+       if (i < nr_pages)
+               return -ENOMEM;
+

Quoted 1 line

Better?

I must be really missing something here. How can that work at all for e.g. the zone_{highmem/movable}. You will get false on the above tests even when you will have hard time to allocate anything from your destination zones.

Quoted 16 lines · On Wed, 4 Apr 2018 16:10:52 +0200 Michal Hocko <[email protected]> wrote:

On Wed 04-04-18 08:59:01, Steven Rostedt wrote:
[...]

Quoted 13 lines

(quoted diff) +11 -0

+       /*
+        * Check if the available memory is there first.
+        * Note, si_mem_available() only gives us a rough estimate of available
+        * memory. It may not be accurate. But we don't care, we just want
+        * to prevent doing any allocation when it is obvious that it is
+        * not going to succeed.
+        */
+       i = si_mem_available();
+       if (i < nr_pages)
+               return -ENOMEM;
+

Better?

Quoted 4 lines

I must be really missing something here. How can that work at all for e.g. the zone_{highmem/movable}. You will get false on the above tests even when you will have hard time to allocate anything from your destination zones.

You mean we will get true on the above tests? Again, the current method is to just say screw it and try to allocate.

I'm looking for something where "yes" means "there may be enough, but there may not be, buyer beware", and "no" means "forget it, don't even start, because you just asked for more than possible".

Quoted 24 lines · Steven Rostedt

Quoted 20 lines · On Wed, 4 Apr 2018 16:10:52 +0200 Michal Hocko <[email protected]> wrote:

On Wed 04-04-18 08:59:01, Steven Rostedt wrote:
[...]

Quoted 12 lines

(quoted diff) +11 -0

+       /*
+        * Check if the available memory is there first.
+        * Note, si_mem_available() only gives us a rough estimate of available
+        * memory. It may not be accurate. But we don't care, we just want
+        * to prevent doing any allocation when it is obvious that it is
+        * not going to succeed.
+        */
+       i = si_mem_available();
+       if (i < nr_pages)
+               return -ENOMEM;
+

Quoted 1 line

Better?

I must be really missing something here. How can that work at all for e.g. the zone_{highmem/movable}. You will get false on the above tests even when you will have hard time to allocate anything from your destination zones.

Quoted 2 lines

You mean we will get true on the above tests? Again, the current method is to just say screw it and try to allocate.

No, you will get false on that test. Say that you have a system with

large ZONE_MOVABLE. Now your kernel allocations can fit only into !movable zones (say we have 1G for !movable and 3G for movable). Now say that !movable zones are getting close to the edge while movable zones are full of reclaimable pages. si_mem_available will tell you there is a _lot_ of memory available while your GFP_KERNEL request will happily consume the rest of !movable zones and trigger OOM. See?

[...]

Quoted 3 lines

I'm looking for something where "yes" means "there may be enough, but there may not be, buyer beware", and "no" means "forget it, don't even start, because you just asked for more than possible".

We do not have _that_ something other than try to opportunistically allocate and see what happens. Sucks? Maybe yes but I really cannot think of an interface with sane semantic that would catch all the different scenarios.

I'm not looking for perfect. In fact, I love what si_mem_available() gives me now! Sure, it can say "there's enough memory" even if I can't use it. Because most of the OOM allocations that happen with increasing the size of the ring buffer isn't due to "just enough memory allocated", but it's due to "trying to allocate crazy amounts of memory". That's because it does the allocation one page at a time, and if you try to allocate crazy amounts of memory, it will allocate all memory before it fails. I don't want that. I want crazy allocations to fail from the start. A "maybe this will allocate" is fine even if it will end up causing an OOM.

I am afraid I cannot help you much more though.

Quoted 2 lines · Steven Rostedt

On Tue, 3 Apr 2018 14:35:14 +0200
Michal Hocko <[email protected]> wrote:

[...]

Quoted 9 lines

Quoted 2 lines

Being clever is OK if it doesn't add a tricky code. And relying on si_mem_available is definitely tricky and obscure.

Can we get the mm subsystem to provide a better method to know if an allocation will possibly succeed or not before trying it? It doesn't have to be free of races. Just "if I allocate this many pages right now, will it work?" If that changes from the time it asks to the time it allocates, that's fine. I'm not trying to prevent OOM to never trigger. I just don't want to to trigger consistently.

How do you do that without an actuall allocation request? And more fundamentally, what if your _particular_ request is just fine but it will get us so close to the OOM edge that the next legit allocation request simply goes OOM? There is simply no sane interface I can think of that would satisfy a safe/sensible "will it cause OOM" semantic.

Quoted 11 lines

Quoted 10 lines

Quoted 4 lines

Perhaps I should try to allocate a large group of pages with RETRY_MAYFAIL, and if that fails go back to NORETRY, with the thinking that the large allocation may reclaim some memory that would allow the NORETRY to succeed with smaller allocations (one page at a time)?

That again relies on a subtle dependencies of the current implementation. So I would rather ask whether this is something that really deserves special treatment. If admin asks for a buffer of a certain size then try to do so. If we get OOM then bad luck you cannot get large memory buffers for free...

Quoted 5 lines

That is not acceptable to me nor to the people asking for this.

The problem is known. The ring buffer allocates memory page by page, and this can allow it to easily take all memory in the system before it fails to allocate and free everything it had done.

Then do not allow buffers that are too large. How often do you need buffers that are larger than few megs or small % of the available memory? Consuming excessive amount of memory just to trace workload which will need some memory on its own sounds just dubious to me.

Quoted 11 lines

If you don't like the use of si_mem_available() I'll do the larger pages method. Yes it depends on the current implementation of memory allocation. It will depend on RETRY_MAYFAIL trying to allocate a large number of pages, and fail if it can't (leaving memory for other allocations to succeed).

The allocation of the ring buffer isn't critical. It can fail to expand, and we can tell the user -ENOMEM. I original had NORETRY because I rather have it fail than cause an OOM. But there's folks (like Joel) that want it to succeed when there's available memory in page caches.

Then implement a retry logic on top of NORETRY. You can control how hard to retry to satisfy the request yourself. You still risk that your allocation will get us close to OOM for _somebody_ else though.

Quoted 4 lines

I'm fine if the admin shoots herself in the foot if the ring buffer gets big enough to start causing OOMs, but I don't want it to cause OOMs if there's not even enough memory to fulfill the ring buffer size itself.

I simply do not see the difference between the two. Both have the same deadly effect in the end. The direct OOM has an arguable advantage that the effect is immediate rather than subtle with potential performance side effects until the machine OOMs after crawling for quite some time.

--
Michal Hocko
SUSE Labs

Quoted 14 lines · Michal Hocko

Quoted 2 lines · Steven Rostedt

On Tue, 3 Apr 2018 14:35:14 +0200
Michal Hocko <[email protected]> wrote:

[...]

Quoted 9 lines

Quoted 2 lines

Being clever is OK if it doesn't add a tricky code. And relying on si_mem_available is definitely tricky and obscure.

Can we get the mm subsystem to provide a better method to know if an allocation will possibly succeed or not before trying it? It doesn't have to be free of races. Just "if I allocate this many pages right now, will it work?" If that changes from the time it asks to the time it allocates, that's fine. I'm not trying to prevent OOM to never trigger. I just don't want to to trigger consistently.

Quoted 6 lines

How do you do that without an actuall allocation request? And more fundamentally, what if your _particular_ request is just fine but it will get us so close to the OOM edge that the next legit allocation request simply goes OOM? There is simply no sane interface I can think of that would satisfy a safe/sensible "will it cause OOM" semantic.

The point is the app which try to allocate the size over the line will escape the OOM and let other innocent to be sacrificed. However, the one which you mentioned above will be possibly selected by OOM that triggered by consequnce failed allocation.

Quoted 21 lines · Zhaoyang Huang

Quoted 20 lines · Michal Hocko

Quoted 2 lines · Steven Rostedt

On Tue, 3 Apr 2018 14:35:14 +0200
Michal Hocko <[email protected]> wrote:

[...]

Quoted 9 lines

Quoted 2 lines

Being clever is OK if it doesn't add a tricky code. And relying on si_mem_available is definitely tricky and obscure.

Can we get the mm subsystem to provide a better method to know if an allocation will possibly succeed or not before trying it? It doesn't have to be free of races. Just "if I allocate this many pages right now, will it work?" If that changes from the time it asks to the time it allocates, that's fine. I'm not trying to prevent OOM to never trigger. I just don't want to to trigger consistently.

How do you do that without an actuall allocation request? And more fundamentally, what if your _particular_ request is just fine but it will get us so close to the OOM edge that the next legit allocation request simply goes OOM? There is simply no sane interface I can think of that would satisfy a safe/sensible "will it cause OOM" semantic.

Quoted 4 lines

The point is the app which try to allocate the size over the line will escape the OOM and let other innocent to be sacrificed. However, the one which you mentioned above will be possibly selected by OOM that triggered by consequnce failed allocation.

If you are afraid of that then you can have a look at {set,clear}_current_oom_origin() which will automatically select the current process as an oom victim and kill it.

--
Michal Hocko
SUSE Labs

If you are afraid of that then you can have a look at {set,clear}_current_oom_origin() which will automatically select the current process as an oom victim and kill it.

Quoted 3 lines · On Wed, 4 Apr 2018 08:23:40 +0200 Michal Hocko <[email protected]> wrote:

If you are afraid of that then you can have a look at {set,clear}_current_oom_origin() which will automatically select the current process as an oom victim and kill it.

Would it even receive the signal? Does alloc_pages_node() even respond to signals? Because the OOM happens while the allocation loop is running.

I tried it out, I did the following:

	set_current_oom_origin();
	for (i = 0; i < nr_pages; i++) {
		struct page *page;
		/*
		 * __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
		 * gracefully without invoking oom-killer and the system is not
		 * destabilized.
		 */
		bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
				    GFP_KERNEL | __GFP_RETRY_MAYFAIL,
				    cpu_to_node(cpu));
		if (!bpage)

			goto free_pages;

		list_add(&bpage->list, pages);

		page = alloc_pages_node(cpu_to_node(cpu),
					GFP_KERNEL | __GFP_RETRY_MAYFAIL, 0);
		if (!page)
			goto free_pages;

		bpage->page = page_address(page);
		rb_init_page(bpage->page);
	}
	clear_current_oom_origin();

Quoted 10 lines · Steven Rostedt

Quoted 3 lines · On Wed, 4 Apr 2018 08:23:40 +0200 Michal Hocko <[email protected]> wrote:

If you are afraid of that then you can have a look at {set,clear}_current_oom_origin() which will automatically select the current process as an oom victim and kill it.

Would it even receive the signal? Does alloc_pages_node() even respond to signals? Because the OOM happens while the allocation loop is running.

Well, you would need to do something like:

Quoted 19 lines

I tried it out, I did the following:

	set_current_oom_origin();
	for (i = 0; i < nr_pages; i++) {
		struct page *page;
		/*
		 * __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
		 * gracefully without invoking oom-killer and the system is not
		 * destabilized.
		 */
		bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
				    GFP_KERNEL | __GFP_RETRY_MAYFAIL,
				    cpu_to_node(cpu));
		if (!bpage)
			goto free_pages;

		list_add(&bpage->list, pages);

Quoted 4 lines

		page = alloc_pages_node(cpu_to_node(cpu),
					GFP_KERNEL | __GFP_RETRY_MAYFAIL, 0);
		if (!page)
			goto free_pages;  

		if (fatal_signal_pending())
			fgoto free_pages;

Quoted 5 lines

		bpage->page = page_address(page);
		rb_init_page(bpage->page);
	}
	clear_current_oom_origin();  

If you use __GFP_RETRY_MAYFAIL it would have to be somedy else to trigger the OOM killer and this user context would get killed. If you drop __GFP_RETRY_MAYFAIL it would be this context to trigger the OOM but it would still be the selected victim.

Quoted 38 lines · On Wed, 4 Apr 2018 16:23:29 +0200 Michal Hocko <[email protected]> wrote:

Quoted 10 lines · Steven Rostedt

Quoted 3 lines · On Wed, 4 Apr 2018 08:23:40 +0200 Michal Hocko <[email protected]> wrote:

If you are afraid of that then you can have a look at {set,clear}_current_oom_origin() which will automatically select the current process as an oom victim and kill it.

Would it even receive the signal? Does alloc_pages_node() even respond to signals? Because the OOM happens while the allocation loop is running.

Well, you would need to do something like:

Quoted 23 lines

I tried it out, I did the following:

	set_current_oom_origin();
	for (i = 0; i < nr_pages; i++) {
		struct page *page;
		/*
		 * __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
		 * gracefully without invoking oom-killer and the system is not
		 * destabilized.
		 */
		bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
				    GFP_KERNEL | __GFP_RETRY_MAYFAIL,
				    cpu_to_node(cpu));
		if (!bpage)
			goto free_pages;

		list_add(&bpage->list, pages);

		page = alloc_pages_node(cpu_to_node(cpu),
					GFP_KERNEL | __GFP_RETRY_MAYFAIL, 0);
		if (!page)
			goto free_pages;  

Quoted 2 lines

		if (fatal_signal_pending())
			fgoto free_pages;

But wouldn't page be NULL in this case?

Quoted 5 lines

Quoted 4 lines

		bpage->page = page_address(page);
		rb_init_page(bpage->page);
	}
	clear_current_oom_origin();  

Quoted 4 lines

If you use __GFP_RETRY_MAYFAIL it would have to be somedy else to trigger the OOM killer and this user context would get killed. If you drop __GFP_RETRY_MAYFAIL it would be this context to trigger the OOM but it would still be the selected victim.

Then we guarantee to kill the process instead of just sending a -ENOMEM, which would change user space ABI, and is a NO NO.

Ideally, we want to avoid an OOM. I could add the above as well, when si_mem_avaiable() returns something that is greater than what is available, and at least this is the process that will get the OOM if it fails to allocate.

Would that work for you?

Quoted 19 lines

I tried it out, I did the following:

	set_current_oom_origin();
	for (i = 0; i < nr_pages; i++) {
		struct page *page;
		/*
		 * __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
		 * gracefully without invoking oom-killer and the system is not
		 * destabilized.
		 */
		bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
				    GFP_KERNEL | __GFP_RETRY_MAYFAIL,
				    cpu_to_node(cpu));
		if (!bpage)
			goto free_pages;

		list_add(&bpage->list, pages);

Quoted 4 lines

		page = alloc_pages_node(cpu_to_node(cpu),
					GFP_KERNEL | __GFP_RETRY_MAYFAIL, 0);
		if (!page)
			goto free_pages;  

		if (fatal_signal_pending())
			fgoto free_pages;

Quoted 5 lines

		bpage->page = page_address(page);
		rb_init_page(bpage->page);
	}
	clear_current_oom_origin();  

I originally was going to remove the RETRY_MAYFAIL, but adding this check (at the end of the loop though) appears to have OOM consistently kill this task.

I still like to keep RETRY_MAYFAIL, because it wont trigger OOM if nothing comes in and tries to do an allocation, but instead will fail nicely with -ENOMEM.

Quoted 4 lines · Steven Rostedt

I originally was going to remove the RETRY_MAYFAIL, but adding this check (at the end of the loop though) appears to have OOM consistently kill this task.

Quoted 3 lines

I still like to keep RETRY_MAYFAIL, because it wont trigger OOM if nothing comes in and tries to do an allocation, but instead will fail nicely with -ENOMEM.

I still don't get why you want RETRY_MAYFAIL. You know that tries *harder* to allocate memory than plain GFP_KERNEL does, right? And that seems like the exact opposite of what you want.

Quoted 9 lines · Matthew Wilcox

Quoted 7 lines · Steven Rostedt

I originally was going to remove the RETRY_MAYFAIL, but adding this check (at the end of the loop though) appears to have OOM consistently kill this task.

I still like to keep RETRY_MAYFAIL, because it wont trigger OOM if nothing comes in and tries to do an allocation, but instead will fail nicely with -ENOMEM.

Quoted 3 lines

I still don't get why you want RETRY_MAYFAIL. You know that tries *harder* to allocate memory than plain GFP_KERNEL does, right? And that seems like the exact opposite of what you want.

No. We do want it to try harder but not if its already setup for failure.

Quoted 10 lines · Joel Fernandes

Quoted 9 lines · Matthew Wilcox

Quoted 7 lines · Steven Rostedt

I originally was going to remove the RETRY_MAYFAIL, but adding this check (at the end of the loop though) appears to have OOM consistently kill this task.

I still like to keep RETRY_MAYFAIL, because it wont trigger OOM if nothing comes in and tries to do an allocation, but instead will fail nicely with -ENOMEM.

Quoted 5 lines

Quoted 3 lines

I still don't get why you want RETRY_MAYFAIL. You know that tries *harder* to allocate memory than plain GFP_KERNEL does, right? And that seems like the exact opposite of what you want.

No. We do want it to try harder but not if its already setup for failure.

I understand you don't want GFP_NORETRY. But why is it more important for this allocation to succeed than other normal GFP_KERNEL allocations?

Quoted 2 lines

I understand you don't want GFP_NORETRY. But why is it more important for this allocation to succeed than other normal GFP_KERNEL allocations?

I guess they simply want a failure rather than OOM even when they can shoot themselves into head by using oom_origin. It is still quite ugly to see OOM report...

Quoted 5 lines · Matthew Wilcox

Quoted 4 lines · Joel Fernandes

Quoted 3 lines · Matthew Wilcox

I still don't get why you want RETRY_MAYFAIL. You know that tries *harder* to allocate memory than plain GFP_KERNEL does, right? And that seems like the exact opposite of what you want.

Quoted 6 lines · Michal Hocko

Quoted 5 lines · Matthew Wilcox

Quoted 4 lines · Joel Fernandes

Quoted 3 lines · Matthew Wilcox

I still don't get why you want RETRY_MAYFAIL. You know that tries *harder* to allocate memory than plain GFP_KERNEL does, right? And that seems like the exact opposite of what you want.

Argh. The comment confused me. OK, now I've read the source and understand that GFP_KERNEL | __GFP_RETRY_MAYFAIL tries exactly as hard as GFP_KERNEL *except* that it won't cause OOM itself. But any other simultaneous GFP_KERNEL allocation without __GFP_RETRY_MAYFAIL will cause an OOM. (And that's why we're having a conversation)

That's a problem because we have places in the kernel that call kv[zm]alloc(very_large_size, GFP_KERNEL), and that will turn into vmalloc, which will do the exact same thing, only it will trigger OOM all by itself (assuming the largest free chunk of address space in the vmalloc area is larger than the amount of free memory).

I considered an alloc_page_array(), but that doesn't fit well with the design of the ring buffer code. We could have:

struct page *alloc_page_list_node(int nid, gfp_t gfp_mask, unsigned long nr);

and link the allocated pages together through page->lru.

We could also have a GFP flag that says to only succeed if we're further above the existing watermark than normal. __GFP_LOW (==ALLOC_LOW), if you like. That would give us the desired behaviour of trying all of the reclaim methods that GFP_KERNEL would, but not being able to exhaust all the memory that GFP_KERNEL allocations would take.

Quoted 5 lines · Matthew Wilcox

Argh. The comment confused me. OK, now I've read the source and understand that GFP_KERNEL | __GFP_RETRY_MAYFAIL tries exactly as hard as GFP_KERNEL *except* that it won't cause OOM itself. But any other simultaneous GFP_KERNEL allocation without __GFP_RETRY_MAYFAIL will cause an OOM. (And that's why we're having a conversation)

Well, I can udnerstand how this can be confusing. The all confusion boils down to the small-never-fails semantic we have. So all reclaim modificators (__GFP_NOFAIL, __GFP_RETRY_MAYFAIL, __GFP_NORETRY) only modify the _default_ behavior.

Quoted 5 lines

That's a problem because we have places in the kernel that call kv[zm]alloc(very_large_size, GFP_KERNEL), and that will turn into vmalloc, which will do the exact same thing, only it will trigger OOM all by itself (assuming the largest free chunk of address space in the vmalloc area is larger than the amount of free memory).

well, hardcoded GFP_KERNEL from vmalloc guts is yet another, ehm, herritage that you are not so proud of.

Quoted 5 lines

We could also have a GFP flag that says to only succeed if we're further above the existing watermark than normal. __GFP_LOW (==ALLOC_LOW), if you like. That would give us the desired behaviour of trying all of the reclaim methods that GFP_KERNEL would, but not being able to exhaust all the memory that GFP_KERNEL allocations would take.

Well, I would be really careful with yet another gfp mask. They are so incredibly hard to define properly and then people kinda tend to screw your best intentions with their usecases ;)
Failing on low wmark is very close to __GFP_NORETRY or even __GFP_NOWAIT, btw. So let's try to not overthink this...

Quoted 7 lines · Michal Hocko

Quoted 5 lines · Matthew Wilcox

Argh. The comment confused me. OK, now I've read the source and understand that GFP_KERNEL | __GFP_RETRY_MAYFAIL tries exactly as hard as GFP_KERNEL *except* that it won't cause OOM itself. But any other simultaneous GFP_KERNEL allocation without __GFP_RETRY_MAYFAIL will cause an OOM. (And that's why we're having a conversation)

Quoted 4 lines

Well, I can udnerstand how this can be confusing. The all confusion boils down to the small-never-fails semantic we have. So all reclaim modificators (__GFP_NOFAIL, __GFP_RETRY_MAYFAIL, __GFP_NORETRY) only modify the _default_ behavior.

Now that I understand the flag, I'll try to write a more clear explanation.

Quoted 6 lines

Quoted 5 lines

That's a problem because we have places in the kernel that call kv[zm]alloc(very_large_size, GFP_KERNEL), and that will turn into vmalloc, which will do the exact same thing, only it will trigger OOM all by itself (assuming the largest free chunk of address space in the vmalloc area is larger than the amount of free memory).

Quoted 2 lines

well, hardcoded GFP_KERNEL from vmalloc guts is yet another, ehm, herritage that you are not so proud of.

Certainly not, but that's not what I'm concerned about; I'm concerned about the allocation of the pages, not the allocation of the array containing the page pointers.

Quoted 11 lines

Quoted 5 lines

We could also have a GFP flag that says to only succeed if we're further above the existing watermark than normal. __GFP_LOW (==ALLOC_LOW), if you like. That would give us the desired behaviour of trying all of the reclaim methods that GFP_KERNEL would, but not being able to exhaust all the memory that GFP_KERNEL allocations would take.

Well, I would be really careful with yet another gfp mask. They are so incredibly hard to define properly and then people kinda tend to screw your best intentions with their usecases ;)
Failing on low wmark is very close to __GFP_NORETRY or even __GFP_NOWAIT, btw. So let's try to not overthink this...

Oh, indeed. We must be able to clearly communicate to users when they should use this flag. I have in mind something like this:

* __GFP_HIGH indicates that the caller is high-priority and that granting
* the request is necessary before the system can make forward progress.
* For example, creating an IO context to clean pages.
*
* __GFP_LOW indicates that the caller is low-priority and that it should

 *   not be allocated pages that would cause the system to get into an
 *   out-of-memory situation.  For example, allocating multiple individual
 *   pages in order to satisfy a larger request.

Quoted 6 lines · Matthew Wilcox

Quoted 2 lines

well, hardcoded GFP_KERNEL from vmalloc guts is yet another, ehm, herritage that you are not so proud of.

Certainly not, but that's not what I'm concerned about; I'm concerned about the allocation of the pages, not the allocation of the array containing the page pointers.

Those pages will use the gfp flag you give to vmalloc IIRC. It is page tables that are GFP_KERNEL unconditionally.

Quoted 15 lines

Quoted 11 lines

Quoted 5 lines

We could also have a GFP flag that says to only succeed if we're further above the existing watermark than normal. __GFP_LOW (==ALLOC_LOW), if you like. That would give us the desired behaviour of trying all of the reclaim methods that GFP_KERNEL would, but not being able to exhaust all the memory that GFP_KERNEL allocations would take.

Well, I would be really careful with yet another gfp mask. They are so incredibly hard to define properly and then people kinda tend to screw your best intentions with their usecases ;)
Failing on low wmark is very close to __GFP_NORETRY or even __GFP_NOWAIT, btw. So let's try to not overthink this...

Oh, indeed. We must be able to clearly communicate to users when they should use this flag. I have in mind something like this:

Quoted 8 lines

* __GFP_HIGH indicates that the caller is high-priority and that granting
* the request is necessary before the system can make forward progress.
* For example, creating an IO context to clean pages.
*
* __GFP_LOW indicates that the caller is low-priority and that it should

 *   not be allocated pages that would cause the system to get into an
 *   out-of-memory situation.  For example, allocating multiple individual
 *   pages in order to satisfy a larger request.

So how exactly the low fits into GFP_NOWAIT, GFP_NORETRY and GFP_RETRY_MAFAIL? We _do_have several levels of how hard to try and we have users relying on that. And do not forget about costly vs. non-costly sizes.

That being said, we should not hijack this thread more and further enhancements should be discussed separatelly. I am all for making the whole allocation api less obscure but keep in mind that we have really hard historical restrictions.

From a "user guide" perspective:

On Thu, Apr 05, 2018 at 08:54:44PM +0200, Michal Hocko wrote:

Quoted 1 line

From a "user guide" perspective:

When allocating memory, you can choose:

- What kind of memory to allocate (DMA, NORMAL, HIGHMEM)
- Where to get the pages from
- Local node only (THISNODE)
- Only in compliance with cpuset policy (HARDWALL)
- Spread the pages between zones (WRITE)
- The movable zone (MOVABLE)
- The reclaimable zone (RECLAIMABLE)

- What you are willing to do if no free memory is available:
- Nothing at all (NOWAIT)
- Use my own time to free memory (DIRECT_RECLAIM)
- But only try once (NORETRY)
- Can call into filesystems (FS)
- Can start I/O (IO)
- Can sleep (!ATOMIC)
- Steal time from other processes to free memory (KSWAPD_RECLAIM)

- Kill other processes to get their memory (!RETRY_MAYFAIL)

- All of the above, and wait forever (NOFAIL)
- Take from emergency reserves (HIGH)
- ... but not the last parts of the regular reserves (LOW)

OK, we already split the documentation into these categories. So we got at least the structure right ;)

Quoted 7 lines

- What kind of memory to allocate (DMA, NORMAL, HIGHMEM)
- Where to get the pages from
- Local node only (THISNODE)
- Only in compliance with cpuset policy (HARDWALL)
- Spread the pages between zones (WRITE)
- The movable zone (MOVABLE)
- The reclaimable zone (RECLAIMABLE)

Quoted 8 lines

- What you are willing to do if no free memory is available:
- Nothing at all (NOWAIT)
- Use my own time to free memory (DIRECT_RECLAIM)
- But only try once (NORETRY)
- Can call into filesystems (FS)
- Can start I/O (IO)
- Can sleep (!ATOMIC)
- Steal time from other processes to free memory (KSWAPD_RECLAIM)

What does that mean? If I drop the flag, do not steal? Well I do because they will hit direct reclaim sooner...

Quoted 1 line

- Kill other processes to get their memory (!RETRY_MAYFAIL)

Not really for costly orders.

Quoted 3 lines

- All of the above, and wait forever (NOFAIL)
- Take from emergency reserves (HIGH)
- ... but not the last parts of the regular reserves (LOW)

What does that mean and how it is different from NOWAIT? Is this about the low watermark and if yes do we want to teach users about this and make the whole thing even more complicated? Does it wake kswapd? What is the eagerness ordering? LOW, NOWAIT, NORETRY, RETRY_MAYFAIL, NOFAIL?

Quoted 2 lines · Michal Hocko

OK, we already split the documentation into these categories. So we got at least the structure right ;)

Yes, this part of the documentation makes sense to me :-)

Quoted 16 lines

Quoted 15 lines

- What kind of memory to allocate (DMA, NORMAL, HIGHMEM)
- Where to get the pages from
- Local node only (THISNODE)
- Only in compliance with cpuset policy (HARDWALL)
- Spread the pages between zones (WRITE)
- The movable zone (MOVABLE)
- The reclaimable zone (RECLAIMABLE)
- What you are willing to do if no free memory is available:
- Nothing at all (NOWAIT)
- Use my own time to free memory (DIRECT_RECLAIM)
- But only try once (NORETRY)
- Can call into filesystems (FS)
- Can start I/O (IO)
- Can sleep (!ATOMIC)
- Steal time from other processes to free memory (KSWAPD_RECLAIM)

Quoted 2 lines

What does that mean? If I drop the flag, do not steal? Well I do because they will hit direct reclaim sooner...

If they allocate memory, sure. A process which stays in its working set won't, unless it's preempted by kswapd.

Quoted 3 lines

Quoted 1 line

- Kill other processes to get their memory (!RETRY_MAYFAIL)

Not really for costly orders.

Yes, need to be more precise there.

Quoted 9 lines

Quoted 3 lines

- All of the above, and wait forever (NOFAIL)
- Take from emergency reserves (HIGH)
- ... but not the last parts of the regular reserves (LOW)

What does that mean and how it is different from NOWAIT? Is this about the low watermark and if yes do we want to teach users about this and make the whole thing even more complicated? Does it wake kswapd? What is the eagerness ordering? LOW, NOWAIT, NORETRY, RETRY_MAYFAIL, NOFAIL?

LOW doesn't quite fit into the eagerness scale with the other flags; instead it's composable with them. So you can specify NOWAIT | LOW, NORETRY | LOW, NOFAIL | LOW, etc. All I have in mind is something like this:

if (alloc_flags & ALLOC_HIGH)
min -= min / 2;

(quoted diff) +2 -0

+	if (alloc_flags & ALLOC_LOW)
+		min += min / 2;

The idea is that a GFP_KERNEL | __GFP_LOW allocation cannot force a GFP_KERNEL allocation into an OOM situation because it cannot take the last pages of memory before the watermark.

It can still make a
GFP_KERNEL allocation *more likely* to hit OOM (just like any other kind of allocation can), but it can't do it by itself.

---

I've been wondering about combining the DIRECT_RECLAIM, NORETRY, RETRY_MAYFAIL and NOFAIL flags together into a single field:
0 => RECLAIM_NEVER, /* !DIRECT_RECLAIM */
1 => RECLAIM_ONCE, /* NORETRY */
2 => RECLAIM_PROGRESS, /* RETRY_MAYFAIL */
3 => RECLAIM_FOREVER, /* NOFAIL */

The existance of __GFP_RECLAIM makes this a bit tricky. I honestly don't know what this code is asking for:

kernel/power/swap.c: __get_free_page(__GFP_RECLAIM | __GFP_HIGH);
but I suspect I'll have to find out. There's about 60 places to look at.

I also want to add __GFP_KILL (to be part of the GFP_KERNEL definition).

That way, each bit that you set in the GFP mask increases the things the page allocator can do to get memory for you. At the moment, RETRY_MAYFAIL subtracts the ability to kill other tasks, which is unusual.

For example,
this test in kvmalloc_node:

        WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL);

doesn't catch RETRY_MAYFAIL being set.

Quoted 5 lines · Matthew Wilcox

Quoted 4 lines

Quoted 1 line

- Steal time from other processes to free memory (KSWAPD_RECLAIM)

What does that mean? If I drop the flag, do not steal? Well I do because they will hit direct reclaim sooner...

Quoted 2 lines

If they allocate memory, sure. A process which stays in its working set won't, unless it's preempted by kswapd.

Well, I was probably not clear here. KSWAPD_RECLAIM is not something you want to drop because this is a cooperative flag. If you do not use it then you are effectivelly pushing others to the direct reclaim because the kswapd won't be woken up and won't do the background work. Your working make it sound as a good thing to drop.

Quoted 16 lines

Quoted 5 lines

What does that mean and how it is different from NOWAIT? Is this about the low watermark and if yes do we want to teach users about this and make the whole thing even more complicated? Does it wake kswapd? What is the eagerness ordering? LOW, NOWAIT, NORETRY, RETRY_MAYFAIL, NOFAIL?

LOW doesn't quite fit into the eagerness scale with the other flags; instead it's composable with them. So you can specify NOWAIT | LOW, NORETRY | LOW, NOFAIL | LOW, etc. All I have in mind is something like this:

if (alloc_flags & ALLOC_HIGH)
min -= min / 2;

(quoted diff) +2 -0

+	if (alloc_flags & ALLOC_LOW)
+		min += min / 2;

Quoted 3 lines

The idea is that a GFP_KERNEL | __GFP_LOW allocation cannot force a GFP_KERNEL allocation into an OOM situation because it cannot take the last pages of memory before the watermark.

So what are we going to do if the LOW watermark cannot succeed?

Quoted 3 lines

It can still make a
GFP_KERNEL allocation *more likely* to hit OOM (just like any other kind of allocation can), but it can't do it by itself.

So who would be a user of __GFP_LOW?

Quoted 7 lines

I've been wondering about combining the DIRECT_RECLAIM, NORETRY, RETRY_MAYFAIL and NOFAIL flags together into a single field:
0 => RECLAIM_NEVER, /* !DIRECT_RECLAIM */
1 => RECLAIM_ONCE, /* NORETRY */
2 => RECLAIM_PROGRESS, /* RETRY_MAYFAIL */
3 => RECLAIM_FOREVER, /* NOFAIL */

Quoted 2 lines

The existance of __GFP_RECLAIM makes this a bit tricky. I honestly don't know what this code is asking for:

I am not sure I follow here. Is the RECLAIM_ an internal thing to the allocator?

Quoted 2 lines

kernel/power/swap.c: __get_free_page(__GFP_RECLAIM | __GFP_HIGH);
but I suspect I'll have to find out. There's about 60 places to look at.

Well, it would be more understandable if this was written as (GFP_KERNEL | __GFP_HIGH) & ~(__GFP_FS|__GFP_IO)

Quoted 1 line

I also want to add __GFP_KILL (to be part of the GFP_KERNEL definition).

What does __GFP_KILL means?

Quoted 3 lines

That way, each bit that you set in the GFP mask increases the things the page allocator can do to get memory for you. At the moment, RETRY_MAYFAIL subtracts the ability to kill other tasks, which is unusual.

Well, it is not all that great because some flags add capabilities while some remove them but, well, life is hard when you try to extend an interface which was not all that great from the very beginning.

Quoted 6 lines · Michal Hocko

Quoted 5 lines · Matthew Wilcox

Quoted 4 lines

Quoted 1 line

- Steal time from other processes to free memory (KSWAPD_RECLAIM)

What does that mean? If I drop the flag, do not steal? Well I do because they will hit direct reclaim sooner...

Quoted 8 lines

Quoted 2 lines

If they allocate memory, sure. A process which stays in its working set won't, unless it's preempted by kswapd.

Well, I was probably not clear here. KSWAPD_RECLAIM is not something you want to drop because this is a cooperative flag. If you do not use it then you are effectivelly pushing others to the direct reclaim because the kswapd won't be woken up and won't do the background work. Your working make it sound as a good thing to drop.

If memory is low, *somebody* has to reclaim. As I understand it, kswapd was originally introduced because networking might do many allocations from interrupt context, and so was unable to do its own reclaiming. On a machine which was used only for routing, there was no userspace process to do the reclaiming, so it ran out of memory. But if you're an HPC person who's expecting their long-running tasks to be synchronised and not be unnecessarily disturbed, having kswapd preempting your task is awful.

I'm not arguing in favour of removing kswapd or anything like that, but if you're not willing/able to reclaim memory yourself, then you're necessarily stealing time from other tasks in order to have reclaim happen.

Quoted 16 lines

Quoted 16 lines

Quoted 5 lines

What does that mean and how it is different from NOWAIT? Is this about the low watermark and if yes do we want to teach users about this and make the whole thing even more complicated? Does it wake kswapd? What is the eagerness ordering? LOW, NOWAIT, NORETRY, RETRY_MAYFAIL, NOFAIL?

LOW doesn't quite fit into the eagerness scale with the other flags; instead it's composable with them. So you can specify NOWAIT | LOW, NORETRY | LOW, NOFAIL | LOW, etc. All I have in mind is something like this:

if (alloc_flags & ALLOC_HIGH)
min -= min / 2;

(quoted diff) +2 -0

+	if (alloc_flags & ALLOC_LOW)
+		min += min / 2;

Quoted 5 lines

Quoted 3 lines

The idea is that a GFP_KERNEL | __GFP_LOW allocation cannot force a GFP_KERNEL allocation into an OOM situation because it cannot take the last pages of memory before the watermark.

So what are we going to do if the LOW watermark cannot succeed?

Depends on the other flags. GFP_NOWAIT | GFP_LOW will just return NULL (somewhat more readily than a plain GFP_NOWAIT would). GFP_NORETRY | GFP_LOW will do one pass through reclaim. If it gets enough pages to drag the zone above the watermark, then it'll succeed, otherwise return NULL. NOFAIL | LOW will keep retrying forever. GFP_KERNEL | GFP_LOW ... hmm, that'll OOM-kill another process more eagerly that

a regular GFP_KERNEL allocation would. We'll need a little tweak so GFP_LOW implies __GFP_RETRY_MAYFAIL.

Quoted 5 lines

Quoted 3 lines

It can still make a
GFP_KERNEL allocation *more likely* to hit OOM (just like any other kind of allocation can), but it can't do it by itself.

So who would be a user of __GFP_LOW?

vmalloc and Steven's ringbuffer. If I write a kernel module that tries to vmalloc 1TB of space, it'll OOM-kill everything on the machine trying to get enough memory to fill the page array.

Probably everyone using
__GFP_RETRY_MAYFAIL today, to be honest. It's more likely to accomplish what they want -- trying slightly less hard to get memory than GFP_KERNEL allocations would.

Quoted 7 lines

Quoted 7 lines

I've been wondering about combining the DIRECT_RECLAIM, NORETRY, RETRY_MAYFAIL and NOFAIL flags together into a single field:
0 => RECLAIM_NEVER, /* !DIRECT_RECLAIM */
1 => RECLAIM_ONCE, /* NORETRY */
2 => RECLAIM_PROGRESS, /* RETRY_MAYFAIL */
3 => RECLAIM_FOREVER, /* NOFAIL */

Quoted 5 lines

Quoted 2 lines

The existance of __GFP_RECLAIM makes this a bit tricky. I honestly don't know what this code is asking for:

I am not sure I follow here. Is the RECLAIM_ an internal thing to the allocator?

No, I'm talking about changing the __GFP flags like this:

(quoted diff) +1 -3

@@ -24,10 +24,8 @@ struct vm_area_struct;
 #define ___GFP_HIGH            0x20u
 #define ___GFP_IO              0x40u
 #define ___GFP_FS              0x80u
+#define ___GFP_ACCOUNT         0x100u
 #define ___GFP_NOWARN          0x200u
-#define ___GFP_RETRY_MAYFAIL   0x400u
-#define ___GFP_NOFAIL          0x800u
-#define ___GFP_NORETRY         0x1000u
 #define ___GFP_MEMALLOC                0x2000u
 #define ___GFP_COMP            0x4000u
 #define ___GFP_ZERO            0x8000u

@@ -35,8 +33,10 @@ struct vm_area_struct;
 #define ___GFP_HARDWALL                0x20000u
 #define ___GFP_THISNODE                0x40000u

Quoted 5 lines

Quoted 2 lines

kernel/power/swap.c: __get_free_page(__GFP_RECLAIM | __GFP_HIGH);
but I suspect I'll have to find out. There's about 60 places to look at.

Well, it would be more understandable if this was written as (GFP_KERNEL | __GFP_HIGH) & ~(__GFP_FS|__GFP_IO)

Yeah, I think it's really (GFP_NOIO | __GFP_HIGH)

Quoted 3 lines

Quoted 1 line

I also want to add __GFP_KILL (to be part of the GFP_KERNEL definition).

What does __GFP_KILL means?

Allows OOM killing. So it's the inverse of the GFP_RETRY_MAYFAIL bit.

As I understand it, kswapd
was originally introduced because networking might do many allocations from interrupt context, and so was unable to do its own reclaiming

First, I've noticed the network drivers were allocating memory in interrupt handlers. That sounds strange to me, because as far as I know, this behaviour is discouraged and may lead to DDOS attack.

I understand you don't want GFP_NORETRY. But why is it more important for this allocation to succeed than other normal GFP_KERNEL allocations?