From 229542cd6c4b0b6b91c59ea0abd7a06a8217b06b Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 11 Oct 2021 00:10:34 +0000 Subject: [PATCH] Avoid negative values in malloc_utils metrics (#2692) ## Proposed Changes While investigating memory usage I noticed that the malloc metrics were going negative once they passed 2GiB. This is because the underlying `mallinfo` function returns a `i32`, and we were casting it straight to an `i64`, preserving the sign. The long-term fix will be to move to `mallinfo2`, but it's still not yet widely available. --- common/malloc_utils/src/glibc.rs | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 861deec4d..f65c933dd 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -85,23 +85,33 @@ pub fn scrape_mallinfo_metrics() { // The docs for this function say it is thread-unsafe since it may return inconsistent results. // Since these are just metrics it's not a concern to us if they're sometimes inconsistent. // - // There exists a `malloc2` function, however it was release in February 2021 and this seems too - // recent to rely on. + // There exists a `mallinfo2` function, however it was released in February 2021 and this seems + // too recent to rely on. // // Docs: // // https://man7.org/linux/man-pages/man3/mallinfo.3.html let mallinfo = mallinfo(); - set_gauge(&MALLINFO_ARENA, mallinfo.arena as i64); - set_gauge(&MALLINFO_ORDBLKS, mallinfo.ordblks as i64); - set_gauge(&MALLINFO_SMBLKS, mallinfo.smblks as i64); - set_gauge(&MALLINFO_HBLKS, mallinfo.hblks as i64); - set_gauge(&MALLINFO_HBLKHD, mallinfo.hblkhd as i64); - set_gauge(&MALLINFO_FSMBLKS, mallinfo.fsmblks as i64); - set_gauge(&MALLINFO_UORDBLKS, mallinfo.uordblks as i64); - set_gauge(&MALLINFO_FORDBLKS, mallinfo.fordblks as i64); - set_gauge(&MALLINFO_KEEPCOST, mallinfo.keepcost as i64); + /// Cast a C integer as returned by `mallinfo` to an unsigned i64. + /// + /// A cast from `i32` to `i64` preserves the sign bit, resulting in incorrect negative values. + /// Going via `u32` treats the sign bit as part of the number. + /// + /// Results are still wrong for memory usage over 4GiB due to limitations of mallinfo. + fn unsigned_i64(x: i32) -> i64 { + x as u32 as i64 + } + + set_gauge(&MALLINFO_ARENA, unsigned_i64(mallinfo.arena)); + set_gauge(&MALLINFO_ORDBLKS, unsigned_i64(mallinfo.ordblks)); + set_gauge(&MALLINFO_SMBLKS, unsigned_i64(mallinfo.smblks)); + set_gauge(&MALLINFO_HBLKS, unsigned_i64(mallinfo.hblks)); + set_gauge(&MALLINFO_HBLKHD, unsigned_i64(mallinfo.hblkhd)); + set_gauge(&MALLINFO_FSMBLKS, unsigned_i64(mallinfo.fsmblks)); + set_gauge(&MALLINFO_UORDBLKS, unsigned_i64(mallinfo.uordblks)); + set_gauge(&MALLINFO_FORDBLKS, unsigned_i64(mallinfo.fordblks)); + set_gauge(&MALLINFO_KEEPCOST, unsigned_i64(mallinfo.keepcost)); } /// Perform all configuration routines.