libmeminfo: Swap accounting bug fixes
This fixes a couple issues:
1. Swap offsets are 50-bit wide, so store them in uint64_t instead of
uint16_t.
2. Swap offset starts from 1 instead of 0. Add one extra slot in
swap offset count array to account for this.
Test: Run procrank and does not see incorrect PSwap numbers anymore.
Change-Id: I2471970d4a29909e8a0874f6c69af3f9a0b15b02
diff --git a/include/meminfo/procmeminfo.h b/include/meminfo/procmeminfo.h
index 8c1280f..4c4a27d 100644
--- a/include/meminfo/procmeminfo.h
+++ b/include/meminfo/procmeminfo.h
@@ -83,7 +83,7 @@
// Returns 'true' on success and the value of Pss in the out parameter.
bool SmapsOrRollupPss(uint64_t* pss) const;
- const std::vector<uint16_t>& SwapOffsets();
+ const std::vector<uint64_t>& SwapOffsets();
// Reads /proc/<pid>/pagemap for this process for each page within
// the 'vma' and stores that in 'pagemap'. It is assumed that the 'vma'
@@ -106,7 +106,7 @@
std::vector<Vma> maps_;
MemUsage usage_;
- std::vector<uint16_t> swap_offsets_;
+ std::vector<uint64_t> swap_offsets_;
};
// Makes callback for each 'vma' or 'map' found in file provided. The file is expected to be in the
diff --git a/libmeminfo_test.cpp b/libmeminfo_test.cpp
index d75357d..5d5f22e 100644
--- a/libmeminfo_test.cpp
+++ b/libmeminfo_test.cpp
@@ -202,7 +202,7 @@
// If we created the object for getting working set,
// the swap offsets must be empty
ProcMemInfo proc_mem(pid, true);
- const std::vector<uint16_t>& swap_offsets = proc_mem.SwapOffsets();
+ const std::vector<uint64_t>& swap_offsets = proc_mem.SwapOffsets();
EXPECT_EQ(swap_offsets.size(), 0);
}
diff --git a/procmeminfo.cpp b/procmeminfo.cpp
index 9e9a705..e755ff1 100644
--- a/procmeminfo.cpp
+++ b/procmeminfo.cpp
@@ -202,7 +202,7 @@
return SmapsOrRollupPssFromFile(path, pss);
}
-const std::vector<uint16_t>& ProcMemInfo::SwapOffsets() {
+const std::vector<uint64_t>& ProcMemInfo::SwapOffsets() {
if (get_wss_) {
LOG(WARNING) << "Trying to read process swap offsets for " << pid_
<< " using invalid object";
diff --git a/tools/procrank.cpp b/tools/procrank.cpp
index 1e44ff9..e7c1af9 100644
--- a/tools/procrank.cpp
+++ b/tools/procrank.cpp
@@ -85,7 +85,8 @@
bool valid() const { return pid_ != -1; }
- void CalculateSwap(const uint16_t* swap_offset_array, float zram_compression_ratio) {
+ void CalculateSwap(const std::vector<uint16_t>& swap_offset_array,
+ float zram_compression_ratio) {
for (auto& off : swap_offsets_) {
proportional_swap_ += getpagesize() / swap_offset_array[off];
unique_swap_ += swap_offset_array[off] == 1 ? getpagesize() : 0;
@@ -102,7 +103,7 @@
uint64_t zswap() const { return zswap_; }
// Wrappers to ProcMemInfo
- const std::vector<uint16_t>& SwapOffsets() const { return swap_offsets_; }
+ const std::vector<uint64_t>& SwapOffsets() const { return swap_offsets_; }
const MemUsage& Usage() const { return usage_or_wss_; }
const MemUsage& Wss() const { return usage_or_wss_; }
@@ -114,7 +115,7 @@
uint64_t unique_swap_;
uint64_t zswap_;
MemUsage usage_or_wss_;
- std::vector<uint16_t> swap_offsets_;
+ std::vector<uint64_t> swap_offsets_;
};
// Show working set instead of memory consumption
@@ -177,11 +178,11 @@
return true;
}
-static bool count_swap_offsets(const ProcessRecord& proc, uint16_t* swap_offset_array,
- uint32_t size) {
- const std::vector<uint16_t>& swp_offs = proc.SwapOffsets();
+static bool count_swap_offsets(const ProcessRecord& proc,
+ std::vector<uint16_t>& swap_offset_array) {
+ const std::vector<uint64_t>& swp_offs = proc.SwapOffsets();
for (auto& off : swp_offs) {
- if (off >= size) {
+ if (off >= swap_offset_array.size()) {
std::cerr << "swap offset " << off << " is out of bounds for process: " << proc.pid()
<< std::endl;
return false;
@@ -249,7 +250,7 @@
}
static void print_processes(std::stringstream& ss, std::vector<ProcessRecord>& procs,
- uint16_t* swap_offset_array) {
+ const std::vector<uint16_t>& swap_offset_array) {
for (auto& proc : procs) {
total_pss += show_wss ? proc.Wss().pss : proc.Usage().pss;
total_uss += show_wss ? proc.Wss().uss : proc.Usage().uss;
@@ -446,7 +447,7 @@
uint64_t swap_total = smi.mem_swap_kb() * 1024;
has_swap = swap_total > 0;
// Allocate the swap array
- auto swap_offset_array = std::make_unique<uint16_t[]>(swap_total / getpagesize());
+ std::vector<uint16_t> swap_offset_array(swap_total / getpagesize() + 1, 0);
if (has_swap) {
has_zram = smi.mem_zram_kb() > 0;
if (has_zram) {
@@ -476,7 +477,7 @@
// collect swap_offset counts from all processes in 1st pass
if (!show_wss && has_swap &&
- !count_swap_offsets(proc, swap_offset_array.get(), swap_total / getpagesize())) {
+ !count_swap_offsets(proc, swap_offset_array)) {
std::cerr << "Failed to count swap offsets for process: " << pid << std::endl;
return false;
}
@@ -515,7 +516,7 @@
ss << std::endl;
// 2nd pass to calculate and get per process stats to add them up
- print_processes(ss, procs, swap_offset_array.get());
+ print_processes(ss, procs, swap_offset_array);
// Add separator to output
print_separator(ss);
