From: Waldemar Kozaczuk <jwkozac...@gmail.com>
Committer: Waldemar Kozaczuk <jwkozac...@gmail.com>
Branch: master
trace: do not use malloc/free when interrupts are disabled
This patch fixes a subtle bug found when working on lazy stack changes
and trying to establish some invariants in relevant places in the kernel code.
One of the findings was that the memory allocation logic or more
specifically the functions malloc() and free() require both interrupts and
preemption enabled to function correctly.
If one starts analysis with memory::pool::alloc(), he/she will notice
a DROP_LOCK(preempt_lock) executed under WITH_LOCK(preempt) if
the pool is empty. This means that the code assumes the preemption must
be enabled before calling alloc(), otherwise the code in DROP_LOCK would
run with preemption disabled which is contrary to the intention.
Also, after drilling down the add_page() calling path, one eventually
would find the call to reclaimers::wait() which calls thread::wait_until(). The
thread::do_wait_until() called by the former enforces that both interrupts
and preemption needs to be enabled with assertion.
The bottom line of the analysis above is that both interrupts and preemption
must be enabled before calling malloc() (same applies to free()). This
effectively
means that trying to call malloc() after disabling interrupts might not work
(which
would only happen if relevent pool was out of free pages AND l1 pool was empty
and
had to refill from l2 pool which probably is quite rare).
So this patch changes the code in trace::create_trace_dump() to avoid
calling new/malloc() (see copies.emplace_back()) and instead makes it
preallocate
the vector ahead of time and then simply copy the trace buffer to a relevant
spot in the vector.
Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com>
---
diff --git a/core/trace.cc b/core/trace.cc
--- a/core/trace.cc
+++ b/core/trace.cc
@@ -697,7 +697,7 @@ std::string
trace::create_trace_dump()
{
semaphore signal(0);
- std::vector<trace_buf> copies;
+ std::vector<trace_buf> copies(sched::cpus.size());
auto is_valid_tracepoint = [](const tracepoint_base * tp_test) {
for (auto & tp : tracepoint_base::tp_list) {
@@ -717,7 +717,7 @@ trace::create_trace_dump()
irq.save();
arch::irq_disable_notrace();
auto * tbp = percpu_trace_buffer.for_cpu(cpu);
- copies.emplace_back(*tbp);
+ copies[i] = trace_buf(*tbp);
irq.restore();
signal.post();
}, sched::thread::attr().pin(cpu)));
--
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 osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/0000000000000da02b05e7777708%40google.com.
