Skip to content

Commit

Permalink
usdt: Use new usdt_addsem_probe* helpers
Browse files Browse the repository at this point in the history
While debugging a high memory consumption issue, I noticed that a
USDT::Context object can take ~10M per instance [0]. Along with the new
--usdt-file-activation, bpftrace can potentially hold onto many dozens
of USDT:Context instances, causing memory issues.

This patch makes use of the new bcc usdt semaphore helpers (
iovisor/bcc#2953 ) to avoid holding onto
USDT::Context instances for the lifetime of the tracing session.

The new algorithm is:

1. create a USDT::Context
2. increment the semaphore count for the probe we care about
3. destroy the USDT::Context
4. repeat 1-3 for all probes we want to attach to
5. do our tracing
6. create a USDT::Context for the probe we care about
7. decrement the semaphore count
8. destroy the USDT::Context
9. repeat 6-8 for all the probes we're attached to

This approach also has the benefit of 1 USDT::Context instance being
alive at a time which can help keep memory high watermark low.

[0]: Through gdb single stepping and /proc/pid/status. Exact process is
not described here b/c memory usage probably varies based on tracee
binary.
  • Loading branch information
danobi committed Jun 15, 2020
1 parent 825b6ff commit 6be5a71
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 5 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ and this project adheres to
- [#1356](https://github.com/iovisor/bpftrace/pull/1356)
- Decrement usdt semaphore count after bpftrace execution
- [#1370](https://github.com/iovisor/bpftrace/pull/1370)
- Reduce high memory consumption when using usdt semaphore
- [#1374](https://github.com/iovisor/bpftrace/pull/1374)

#### Tools

Expand Down
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ check_symbol_exists(bcc_prog_load "${LIBBCC_INCLUDE_DIRS}/bcc/libbpf.h" HAVE_BCC
check_symbol_exists(bcc_create_map "${LIBBCC_INCLUDE_DIRS}/bcc/libbpf.h" HAVE_BCC_CREATE_MAP)
check_symbol_exists(bcc_elf_foreach_sym "${LIBBCC_INCLUDE_DIRS}/bcc/bcc_elf.h" HAVE_BCC_ELF_FOREACH_SYM)
check_symbol_exists(bpf_attach_kfunc "${LIBBCC_INCLUDE_DIRS}/bcc/libbpf.h" HAVE_BCC_KFUNC)
check_symbol_exists(bcc_usdt_addsem_probe "${LIBBCC_INCLUDE_DIRS}/bcc/bcc_usdt.h" HAVE_BCC_USDT_ADDSEM)
set(CMAKE_REQUIRED_LIBRARIES)
set(CMAKE_REQUIRED_LINK_OPTIONS)

Expand Down
3 changes: 3 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ endif(HAVE_BCC_CREATE_MAP)
if(HAVE_BCC_ELF_FOREACH_SYM)
target_compile_definitions(bpftrace PRIVATE HAVE_BCC_ELF_FOREACH_SYM)
endif(HAVE_BCC_ELF_FOREACH_SYM)
if(HAVE_BCC_USDT_ADDSEM)
target_compile_definitions(bpftrace PRIVATE HAVE_BCC_USDT_ADDSEM)
endif(HAVE_BCC_USDT_ADDSEM)
if (LIBBPF_BTF_DUMP_FOUND)
target_compile_definitions(bpftrace PRIVATE HAVE_LIBBPF_BTF_DUMP)
target_include_directories(bpftrace PUBLIC ${LIBBPF_INCLUDE_DIRS})
Expand Down
54 changes: 49 additions & 5 deletions src/attached_probe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -796,13 +796,44 @@ void AttachedProbe::attach_usdt(int pid)
struct bcc_usdt_location loc = {};
int err;
void *ctx;
// TODO: fn_name may need a unique suffix for each attachment on the same
// probe:
std::string fn_name = "probe_" + probe_.attach_point + "_1";

#ifdef HAVE_BCC_USDT_ADDSEM
// NB: we are careful to capture by value here everything that will not
// be available in AttachedProbe destructor.
auto addsem = [this, fn_name](void *c, int16_t val) -> int {
if (this->probe_.ns == "")
return bcc_usdt_addsem_probe(
c, this->probe_.attach_point.c_str(), fn_name.c_str(), val);
else
return bcc_usdt_addsem_fully_specified_probe(
c,
this->probe_.ns.c_str(),
this->probe_.attach_point.c_str(),
fn_name.c_str(),
val);
};
#endif // HAVE_BCC_USDT_ADDSEM

if (pid)
{
// FIXME when iovisor/bcc#2064 is merged, optionally pass probe_.path
ctx = bcc_usdt_new_frompid(pid, nullptr);
if (!ctx)
throw std::runtime_error("Error initializing context for probe: " + probe_.name + ", for PID: " + std::to_string(pid));

#ifdef HAVE_BCC_USDT_ADDSEM
usdt_destructor_ = [pid, addsem]() {
void *c = bcc_usdt_new_frompid(pid, nullptr);
if (!c)
return;

addsem(c, -1);
bcc_usdt_close(c);
};
#endif // HAVE_BCC_USDT_ADDSEM
}
else
{
Expand All @@ -811,14 +842,21 @@ void AttachedProbe::attach_usdt(int pid)
throw std::runtime_error("Error initializing context for probe: " + probe_.name);
}

#ifndef HAVE_BCC_USDT_ADDSEM
// Defer context destruction until probes are detached b/c context
// destruction will decrement usdt semaphore count.
usdt_destructor_ = [ctx]() { bcc_usdt_close(ctx); };

// TODO: fn_name may need a unique suffix for each attachment on the same probe:
std::string fn_name = "probe_" + probe_.attach_point + "_1";
// see https://github.com/iovisor/bcc/pull/2294 for BCC_USDT_HAS_FULLY_SPECIFIED_PROBE
#ifdef BCC_USDT_HAS_FULLY_SPECIFIED_PROBE
#endif // HAVE_BCC_USDT_ADDSEM

#ifdef HAVE_BCC_USDT_ADDSEM
// Use semaphore increment API to avoid having to hold onto the usdt context
// for the entire tracing session. Reason we do it this way instead of
// holding onto usdt context is b/c each usdt context can take lots of memory
// (~10MB). This, coupled with --usdt-file-activation and tracees that have a
// forking model can cause bpftrace to use huge amounts of memory if we hold
// onto the contexts.
err = addsem(ctx, +1);
#elif defined(BCC_USDT_HAS_FULLY_SPECIFIED_PROBE)
if (probe_.ns == "")
err = bcc_usdt_enable_probe(ctx, probe_.attach_point.c_str(), fn_name.c_str());
else
Expand Down Expand Up @@ -846,6 +884,12 @@ void AttachedProbe::attach_usdt(int pid)
throw std::runtime_error("Error finding location for probe: " + probe_.name);
probe_.loc = loc.address;

#ifdef HAVE_BCC_USDT_ADDSEM
// If we use the bcc_usdt_addsem*() API, bcc won't decrement semaphore count
// in bcc_usdt_close(). So we are free to close context here.
bcc_usdt_close(ctx);
#endif // HAVE_BCC_USDT_ADDSEM

offset_ = resolve_offset(probe_.path, probe_.attach_point, probe_.loc);

int perf_event_fd = bpf_attach_uprobe(progfd_, attachtype(probe_.type),
Expand Down
6 changes: 6 additions & 0 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ static int info()
<< "yes" << std::endl;
#else
<< "no" << std::endl;
#endif
std::cerr << " bcc_usdt_addsem: "
#ifdef HAVE_BCC_USDT_ADDSEM
<< "yes" << std::endl;
#else
<< "no" << std::endl;
#endif
std::cerr << " libbpf: "
#ifdef HAVE_LIBBPF
Expand Down
3 changes: 3 additions & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ endif(LIBBPF_BTF_DUMP_FOUND)
if (HAVE_BCC_KFUNC)
target_compile_definitions(bpftrace_test PRIVATE HAVE_BCC_KFUNC)
endif(HAVE_BCC_KFUNC)
if(HAVE_BCC_USDT_ADDSEM)
target_compile_definitions(bpftrace PRIVATE HAVE_BCC_USDT_ADDSEM)
endif(HAVE_BCC_USDT_ADDSEM)
target_compile_definitions(bpftrace_test PRIVATE TEST_CODEGEN_LOCATION="${CMAKE_SOURCE_DIR}/tests/codegen/llvm/")
if(HAVE_BFD_DISASM)
target_compile_definitions(bpftrace_test PRIVATE HAVE_BFD_DISASM)
Expand Down

0 comments on commit 6be5a71

Please sign in to comment.