From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
stable@vger.kernel.org, linux-mm@kvack.org,
Zhaoyang Huang <huangzhaoyang@gmail.com>,
Joel Fernandes <joelaf@google.com>
Subject: [for-next][PATCH 13/18] ring-buffer: Check if memory is available before allocation
Date: Fri, 06 Apr 2018 09:00:48 -0400 [thread overview]
Message-ID: <20180406130113.687518198@goodmis.org> (raw)
In-Reply-To: 20180406130035.400292196@goodmis.org
[-- Attachment #1: 0013-ring-buffer-Check-if-memory-is-available-before-allo.patch --]
[-- Type: text/plain, Size: 3020 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The ring buffer is made up of a link list of pages. When making the ring
buffer bigger, it will allocate all the pages it needs before adding to the
ring buffer, and if it fails, it frees them and returns an error. This makes
increasing the ring buffer size an all or nothing action. When this was
first created, the pages were allocated with "NORETRY". This was to not
cause any Out-Of-Memory (OOM) actions from allocating the ring buffer. But
NORETRY was too strict, as the ring buffer would fail to expand even when
there's memory available, but was taken up in the page cache.
Commit 848618857d253 ("tracing/ring_buffer: Try harder to allocate") changed
the allocating from NORETRY to RETRY_MAYFAIL. The RETRY_MAYFAIL would
allocate from the page cache, but if there was no memory available, it would
simple fail the allocation and not trigger an OOM.
This worked fine, but had one problem. As the ring buffer would allocate one
page at a time, it could take up all memory in the system before it failed
to allocate and free that memory. If the allocation is happening and the
ring buffer allocates all memory and then tries to take more than available,
its allocation will not trigger an OOM, but if there's any allocation that
happens someplace else, that could trigger an OOM, even though once the ring
buffer's allocation fails, it would free up all the previous memory it tried
to allocate, and allow other memory allocations to succeed.
Commit d02bd27bd33dd ("mm/page_alloc.c: calculate 'available' memory in a
separate function") separated out si_mem_availble() as a separate function
that could be used to see how much memory is available in the system. Using
this function to make sure that the ring buffer could be allocated before it
tries to allocate pages we can avoid allocating all memory in the system and
making it vulnerable to OOMs if other allocations are taking place.
Link: http://lkml.kernel.org/r/1522320104-6573-1-git-send-email-zhaoyang.huang@spreadtrum.com
CC: stable@vger.kernel.org
Cc: linux-mm@kvack.org
Fixes: 848618857d253 ("tracing/ring_buffer: Try harder to allocate")
Requires: d02bd27bd33dd ("mm/page_alloc.c: calculate 'available' memory in a separate function")
Reported-by: Zhaoyang Huang <huangzhaoyang@gmail.com>
Tested-by: Joel Fernandes <joelaf@google.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/ring_buffer.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 515be03e3009..966128f02121 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -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;
+ /* Check if the available memory is there first */
+ i = si_mem_available();
+ if (i < nr_pages)
+ return -ENOMEM;
+
for (i = 0; i < nr_pages; i++) {
struct page *page;
/*
--
2.15.1
next prev parent reply other threads:[~2018-04-06 13:02 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 01/18] tracing: Fix a potential NULL dereference Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 02/18] init: Fix initcall0 name as it is "pure" not "early" Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 03/18] tracing: Default to using trace_global_clock if sched_clock is unstable Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 04/18] tracing: Mention trace_clock=global when warning about unstable clocks Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 05/18] ftrace: Drop a VLA in module_exists() Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 06/18] tracing: Fix display of hist trigger expressions containing timestamps Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 07/18] tracing: Dont add flag strings when displaying variable references Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 08/18] tracing: Add action comparisons when testing matching hist triggers Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 09/18] tracing: Make sure variable string fields are NULL-terminated Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 10/18] tracing: Uninitialized variable in create_tracing_map_fields() Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 11/18] vsprintf: Do not preprocess non-dereferenced pointers for bprintf (%px and %pK) Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 12/18] lockdep: Add print_irqtrace_events() to __warn Steven Rostedt
2018-04-06 13:00 ` Steven Rostedt [this message]
2018-04-06 13:00 ` [for-next][PATCH 14/18] ring-buffer: Add set/clear_current_oom_origin() during allocations Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 15/18] tracing: Hide global trace clock from lockdep Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 16/18] tracing: Fixup logic inversion on setting trace_global_clock defaults Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 17/18] tracing: Add rcu dereference annotation for filter->prog Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 18/18] tracing: Add rcu dereference annotation for test func that touches filter->prog Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180406130113.687518198@goodmis.org \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=huangzhaoyang@gmail.com \
--cc=joelaf@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mingo@kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox