Skip to content

Commit

Permalink
Add a limit on maximum number of profiled PIDs
Browse files Browse the repository at this point in the history
  • Loading branch information
r1viollet committed Jan 19, 2024
1 parent 37106ee commit e625878
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 17 deletions.
1 change: 1 addition & 0 deletions include/common_symbol_errors.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ enum SymbolErrors {
unknown_dso,
dwfl_frame,
incomplete_stack,
max_pids,
lost_event,
};

Expand Down
4 changes: 4 additions & 0 deletions include/ddprof_defs.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ inline constexpr auto k_min_number_samples_per_ring_buffer = 8;

inline constexpr int k_size_api_key = 32;

// Maximum number of profiled pids
// Exceeding a number of PIDs overloads file descriptors and memory
inline constexpr unsigned kMaxProfiledPids{100};

// Linux Inode type
using inode_t = uint64_t;

Expand Down
1 change: 1 addition & 0 deletions include/ddres_list.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ enum { DD_COMMON_START_RANGE = 1000, DD_NATIVE_START_RANGE = 2000 };
X(DWFL_LIB_ERROR, "error withing dwfl library") \
X(UW_CACHE_ERROR, "error from unwinding cache") \
X(UW_ERROR, "error from unwinding code") \
X(UW_MAX_PIDS, "Maximum number of PIDs reached") \
X(UW_MAX_DEPTH, "max depth reached in unwinding") \
X(CAPLIB, "error when reading capabilities") \
X(USERID, "error in user ID manipulations") \
Expand Down
3 changes: 2 additions & 1 deletion include/dwfl_hdr.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ struct DwflWrapper {

class DwflHdr {
public:
DwflWrapper &get_or_insert(pid_t pid);
// checks against maximum number of PIDs
DwflWrapper *get_or_insert(pid_t pid);
std::vector<pid_t> get_unvisited() const;
void reset_unvisited();
void clear_pid(pid_t pid);
Expand Down
2 changes: 2 additions & 0 deletions src/common_symbol_lookup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ Symbol symbol_from_common(SymbolErrors lookup_case) {
return {std::string(), std::string("[incomplete]"), 0, std::string()};
case SymbolErrors::lost_event:
return {std::string(), std::string("[lost]"), 0, std::string()};
case SymbolErrors::max_pids:
return {std::string(), std::string("[maximum pids]"), 0, std::string()};
default:
break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/ddprof_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ DDRes ddprof_unwind_sample(DDProfContext &ctx, perf_event_sample *sample,
ddprof_stats_add(STATS_UNWIND_TRUNCATED_INPUT, 1, nullptr);
}

if (us->_dwfl_wrapper->_inconsistent) {
if (us->_dwfl_wrapper && us->_dwfl_wrapper->_inconsistent) {
// Loaded modules were inconsistent, assume we should flush everything.
LG_WRN("(Inconsistent DWFL/DSOs)%d - Free associated objects", us->pid);
DDRES_CHECK_FWD(worker_pid_free(ctx, us->pid));
Expand Down
9 changes: 6 additions & 3 deletions src/dwfl_hdr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,19 @@ DDRes DwflWrapper::attach(pid_t pid, const Dwfl_Thread_Callbacks *callbacks,
return {};
}

DwflWrapper &DwflHdr::get_or_insert(pid_t pid) {
DwflWrapper *DwflHdr::get_or_insert(pid_t pid) {
_visited_pid.insert(pid);
auto it = _dwfl_map.find(pid);
if (it == _dwfl_map.end()) {
if (_dwfl_map.size() >= kMaxProfiledPids) {
return nullptr;
}
// insert new dwfl for this pid
auto pair = _dwfl_map.emplace(pid, DwflWrapper());
assert(pair.second); // expect insertion to be OK
return pair.first->second;
return &pair.first->second;
}
return it->second;
return &it->second;
}

DDProfMod *DwflWrapper::unsafe_get(FileInfoId_t file_info_id) {
Expand Down
21 changes: 11 additions & 10 deletions src/unwind.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,15 @@ bool is_stack_complete(UnwindState *us) {
root_func) != s_expected_root_frames.end();
}

void find_dso_add_error_frame(UnwindState *us) {
DsoHdr::DsoFindRes const find_res =
us->dso_hdr.dso_find_closest(us->pid, us->current_ip);
add_error_frame(find_res.second ? &(find_res.first->second) : nullptr, us,
us->current_ip);
void find_dso_add_error_frame(DDRes ddres, UnwindState *us) {
if (ddres._what == DD_WHAT_UW_MAX_PIDS) {
add_common_frame(us, SymbolErrors::max_pids);
} else {
DsoHdr::DsoFindRes const find_res =
us->dso_hdr.dso_find_closest(us->pid, us->current_ip);
add_error_frame(find_res.second ? &(find_res.first->second) : nullptr, us,
us->current_ip);
}
}

void add_container_id(UnwindState *us) {
Expand Down Expand Up @@ -99,17 +103,14 @@ DDRes unwindstate_unwind(UnwindState *us) {
res = unwind_dwfl(us);
}
if (IsDDResNotOK(res)) {
find_dso_add_error_frame(us);
}

if (!is_stack_complete(us)) {
find_dso_add_error_frame(res, us);
} else if (!is_stack_complete(us)) {
us->output.is_incomplete = true;
ddprof_stats_add(STATS_UNWIND_INCOMPLETE_STACK, 1, nullptr);
// Only add [incomplete] virtual frame if stack is not already truncated !
if (!is_max_stack_depth_reached(*us)) {
add_common_frame(us, SymbolErrors::incomplete_stack);
}

} else {
us->output.is_incomplete = false;
}
Expand Down
7 changes: 5 additions & 2 deletions src/unwind_dwfl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,10 @@ DDRes add_runtime_symbol_frame(UnwindState *us, const Dso &dso, ElfAddress_t pc,

DDRes unwind_init_dwfl(UnwindState *us) {
// Create or get the dwfl object associated to cache
us->_dwfl_wrapper = &(us->dwfl_hdr.get_or_insert(us->pid));
us->_dwfl_wrapper = us->dwfl_hdr.get_or_insert(us->pid);
if (!us->_dwfl_wrapper) {
return ddres_warn(DD_WHAT_UW_MAX_PIDS);
}
if (!us->_dwfl_wrapper->_attached) {
// we need to add at least one module to figure out the architecture (to
// create the unwinding backend)
Expand All @@ -267,7 +270,7 @@ DDRes unwind_init_dwfl(UnwindState *us) {
// Find an elf file we can load for this PID
for (auto it = map.cbegin(); it != map.cend(); ++it) {
const Dso &dso = it->second;
if (dso.is_executable()) {
if (dso.is_executable() && dso._type == DsoType::kStandard) {
FileInfoId_t const file_info_id =
us->dso_hdr.get_or_insert_file_info(dso);
if (file_info_id <= k_file_info_error) {
Expand Down

0 comments on commit e625878

Please sign in to comment.