So, it is possible to reproduce the issue with a unit test, however it requires
specifying an explicit memory size to the run script.
The issue only occurs when the bitset size is an exact multiple of
mmu::huge_page_size. In that case, the first mmap that is greater than or
equal to mmu::huge_page_size will be allocated from the top most range.
When that allocation is munmapped, the system will hang.
Please see the patch below which adds a tst-page-range-allocator.cc unit test.
After applying and rebuilding the test image you should be able to trigger the
hang by calling:
timmons@rtp-skink osv[0]> scripts/run.py -m512.15M -c2 -e
tests/tst-page-range-allocator.so
warning: TCG doesn't support requested feature: CPUID.01H:ECX.vmx [bit 5]
warning: TCG doesn't support requested feature: CPUID.01H:ECX.vmx [bit 5]
OSv v0.24-474-g9cceaca
eth0: 192.168.122.15
_bitmap.size() = 131072, end = 131072
[ hangs here… ]
P.S. Since the unit test requires an explicit configuration, I’m not sure if
it’s useful to add it to the patch… Let me know if you think otherwise.
diff --git a/core/mempool.cc b/core/mempool.cc
index 569a0ac..437b54c 100644
--- a/core/mempool.cc
+++ b/core/mempool.cc
@@ -767,6 +767,10 @@ void page_range_allocator::free(page_range* pr)
pr2->size += pr->size;
pr = pr2;
}
+ auto end = get_bitmap_idx(*pr) + pr->size / page_size;
+ if (end >= _bitmap.size()) {
+ printf("_bitmap.size() = %zu, end = %zu\n", _bitmap.size(), end);
+ }
if (_bitmap[get_bitmap_idx(*pr) + pr->size / page_size]) {
auto pr2 = static_cast<page_range*>(static_cast<void*>(pr) + pr->size);
remove(*pr2);
diff --git a/modules/tests/Makefile b/modules/tests/Makefile
index b8348dd..715a7c1 100644
--- a/modules/tests/Makefile
+++ b/modules/tests/Makefile
@@ -87,7 +87,7 @@ tests := tst-pthread.so misc-ramdisk.so tst-vblk.so
tst-bsd-evh.so \
tst-ifaddrs.so tst-pthread-affinity-inherit.so tst-sem-timed-wait.so \
tst-ttyname.so tst-pthread-barrier.so tst-feexcept.so tst-math.so \
tst-sigaltstack.so tst-fread.so tst-tcp-cork.so tst-tcp-v6.so \
- tst-calloc.so
+ tst-calloc.so tst-page-range-allocator.so
# libstatic-thread-variable.so tst-static-thread-variable.so \
diff --git a/tests/tst-page-range-allocator.cc
b/tests/tst-page-range-allocator.cc
new file mode 100644
index 0000000..bf2a9aa
--- /dev/null
+++ b/tests/tst-page-range-allocator.cc
@@ -0,0 +1,28 @@
+#include <osv/mmu.hh>
+#include <sys/mman.h>
+#include <string.h>
+
+int tests = 0, fails = 0;
+
+constexpr int bufsize = mmu::huge_page_size;
+
+static void report(bool ok, const char *msg)
+{
+ ++tests;
+ fails += !ok;
+ debug("%s: %s\n", (ok ? "PASS" : "FAIL"), msg);
+}
+
+int main()
+{
+ void *buffer = mmap(NULL, bufsize, PROT_READ|PROT_WRITE,
MAP_SHARED|MAP_ANONYMOUS, -1, 0);
+ report(buffer != nullptr, "mmap");
+
+ // Force the buffer to get mapped.
+ memset(buffer, 0, bufsize);
+
+ int error = munmap(buffer, bufsize);
+ report(error == 0, "mumap");
+
+ debug("SUMMARY: %d tests, %d failures\n", tests, fails);
+}
From: OSv Gopher <[email protected]> on behalf of Timmons Player
<[email protected]>
Date: Monday, January 15, 2018 at 3:05 PM
To: Waldek Kozaczuk <[email protected]>, OSv Gopher
<[email protected]>
Subject: Re: [PATCH] memory: respect _bitset length bounds in
page_range_allocator
I don’t know. I could only ever reproduce it on 1 CPU setups on a specific
hypervisor. I’ll investigate reproducing it via a unit test.
Timmons
From: OSv Gopher <[email protected]> on behalf of Waldek Kozaczuk
<[email protected]>
Date: Monday, January 15, 2018 at 2:19 PM
To: OSv Gopher <[email protected]>
Subject: Re: [PATCH] memory: respect _bitset length bounds in
page_range_allocator
How difficult would it be to create a unit test around this bug to prove what
it fixes?
On Monday, January 15, 2018 at 12:52:24 PM UTC-5, Timmons C. Player wrote:
The page_range_allocator attempts to merge neighbor page ranges
when a page range is freed in order to minimize memory fragmentation.
However, the original implementation didn't validate indexes before
checking the _bitset to determine whether a page range had a neighbor
or not. This change make sure that indexes stay within bounds.
This is the correct fix for the fix attempted in
44f8324e3f2e53f55022a1d93c8b072514e7ac07
Signed-off-by: Timmons C. Player <[email protected]>
---
core/mempool.cc | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/core/mempool.cc b/core/mempool.cc
index e0d0867..1ac3f67 100644
--- a/core/mempool.cc
+++ b/core/mempool.cc
@@ -761,13 +761,15 @@ page_range* page_range_allocator::alloc_aligned(size_t
size, size_t offset,
void page_range_allocator::free(page_range* pr)
{
- if (_bitmap[get_bitmap_idx(*pr) - 1]) {
+ auto idx = get_bitmap_idx(*pr);
+ if (idx && _bitmap[idx - 1]) {
auto pr2 = *(reinterpret_cast<page_range**>(pr) - 1);
remove(*pr2);
pr2->size += pr->size;
pr = pr2;
}
- if (_bitmap[get_bitmap_idx(*pr) + pr->size / page_size - 1]) {
+ auto next_idx = get_bitmap_idx(*pr) + pr->size / page_size;
+ if (next_idx < _bitmap.size() && _bitmap[next_idx]) {
auto pr2 = static_cast<page_range*>(static_cast<void*>(pr) +
pr->size);
remove(*pr2);
pr->size += pr2->size;
--
2.7.4
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
Spirent Communications e-mail confidentiality.
------------------------------------------------------------------------
This e-mail contains confidential and / or privileged information belonging to
Spirent Communications plc, its affiliates and / or subsidiaries. If you are
not the intended recipient, you are hereby notified that any disclosure,
copying, distribution and / or the taking of any action based upon reliance on
the contents of this transmission is strictly forbidden. If you have received
this message in error please notify the sender by return e-mail and delete it
from your system.
Spirent Communications plc
Northwood Park, Gatwick Road, Crawley, West Sussex, RH10 9XN, United Kingdom.
Tel No. +44 (0) 1293 767676
Fax No. +44 (0) 1293 767677
Registered in England Number 470893
Registered at Northwood Park, Gatwick Road, Crawley, West Sussex, RH10 9XN,
United Kingdom.
Or if within the US,
Spirent Communications,
27349 Agoura Road, Calabasas, CA, 91301, USA.
Tel No. 1-818-676- 2300
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
