Skip to content

Commit 273bd9f

Browse files
authored
Improve smaps parsing support (#942)
* Remove size hint, read_to_string should try_reserve in its impl * Correctly handles smap region addresses with varying lengths * Adds changelog entry * Use more specific VMA header matching regex
1 parent df0ae51 commit 273bd9f

File tree

2 files changed

+115
-7
lines changed

2 files changed

+115
-7
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77
## Unreleased
88
### Added
99
- Retrieve memory, CPU information from cgroup controller for every pid observed on Linux.
10+
### Fixed
11+
- Fixes bugs in `smaps` parsing code that can result in under-counting RSS in
12+
the smaps view of the data.
1013

1114
## [0.22.0-rc1]
1215
### Fixed

lading/src/observer/memory.rs

Lines changed: 112 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use regex::Regex;
22
use std::{collections::HashMap, io::Read};
33

4-
const SMAP_SIZE_HINT: usize = 128 * 1024; // 128 kB
54
const BYTES_PER_KIBIBYTE: u64 = 1024;
65

76
#[derive(thiserror::Error, Debug)]
@@ -56,7 +55,7 @@ impl Rollup {
5655
pub(crate) fn from_file(path: &str) -> Result<Self, Error> {
5756
let mut file: std::fs::File = std::fs::OpenOptions::new().read(true).open(path)?;
5857

59-
let mut contents = String::with_capacity(SMAP_SIZE_HINT);
58+
let mut contents = String::new();
6059
file.read_to_string(&mut contents)?;
6160

6261
Self::from_str(&contents)
@@ -203,8 +202,11 @@ impl Region {
203202
};
204203

205204
if let (None, None) = (start, end) {
206-
let start_str = &token[0..12];
207-
let end_str = &token[13..25];
205+
let dash_loc = token.find('-').ok_or(Error::Parsing(format!(
206+
"Could not find dash in addr: {token}"
207+
)))?;
208+
let start_str = &token[0..dash_loc];
209+
let end_str = &token[dash_loc+1..];
208210
start = Some(u64::from_str_radix(start_str, 16)?);
209211
end = Some(u64::from_str_radix(end_str, 16)?);
210212
} else if perms.is_none() {
@@ -354,19 +356,19 @@ impl Regions {
354356
fn from_file(path: &str) -> Result<Self, Error> {
355357
let mut file: std::fs::File = std::fs::OpenOptions::new().read(true).open(path)?;
356358

357-
let mut contents = String::with_capacity(SMAP_SIZE_HINT);
359+
let mut contents = String::new();
358360
file.read_to_string(&mut contents)?;
359361

360362
Self::from_str(&contents)
361363
}
362364

363-
fn from_str(contents: &str) -> Result<Self, Error> {
365+
fn into_region_strs(contents: &str) -> Vec<&str> {
364366
let mut str_regions = Vec::new();
365367
// split this smaps file into regions
366368
// regions are separated by a line like this:
367369
// 7fffa9f39000-7fffa9f3b000 r-xp 00000000 00:00 0 [vdso]
368370
let region_start_regex =
369-
Regex::new("[[:xdigit:]]{12}-[[:xdigit:]]{12}").expect("Regex to be valid");
371+
Regex::new("(?m)^[[:xdigit:]]+-[[:xdigit:]]+").expect("Regex to be valid");
370372
let mut start_indices = region_start_regex.find_iter(contents).map(|m| m.start());
371373

372374
if let Some(mut start_index) = start_indices.next() {
@@ -378,6 +380,11 @@ impl Regions {
378380
str_regions.push(&contents[start_index..]);
379381
};
380382

383+
str_regions
384+
}
385+
386+
fn from_str(contents: &str) -> Result<Self, Error> {
387+
let str_regions = Self::into_region_strs(contents);
381388
let regions = str_regions
382389
.iter()
383390
.map(|s| Region::from_str(s))
@@ -688,4 +695,102 @@ VmFlags: rd ex mr mw me de sd";
688695
assert_eq!(rollup.pss_file, None);
689696
assert_eq!(rollup.pss_shmem, None);
690697
}
698+
699+
#[test]
700+
fn test_varying_hex_len_mappings() {
701+
let region =
702+
" 7fffa9f39000-7fffa9f3b000 r-xp 00000000 00:00 0 [vdso]
703+
Size: 8 kB
704+
KernelPageSize: 4 kB
705+
MMUPageSize: 4 kB
706+
Rss: 8 kB
707+
Pss: 2 kB
708+
Pss_Dirty: 0 kB
709+
Shared_Clean: 8 kB
710+
Shared_Dirty: 0 kB
711+
Private_Clean: 0 kB
712+
Private_Dirty: 0 kB
713+
Referenced: 8 kB
714+
Anonymous: 0 kB
715+
LazyFree: 0 kB
716+
AnonHugePages: 0 kB
717+
ShmemPmdMapped: 0 kB
718+
FilePmdMapped: 0 kB
719+
Shared_Hugetlb: 0 kB
720+
Private_Hugetlb: 0 kB
721+
Swap: 0 kB
722+
SwapPss: 0 kB
723+
Locked: 0 kB
724+
THPeligible: 0
725+
ProtectionKey: 0
726+
VmFlags: rd ex mr mw me de sd";
727+
let region = Region::from_str(region).expect("Parsing failed");
728+
729+
assert_eq!(region.start, 0x7fffa9f39000);
730+
assert_eq!(region.end, 0x7fffa9f3b000);
731+
732+
let region =
733+
"00400000-0e8dd000 r-xp 00000000 00:00 0 [vdso]
734+
Size: 8 kB
735+
KernelPageSize: 4 kB
736+
MMUPageSize: 4 kB
737+
Rss: 8 kB
738+
Pss: 2 kB
739+
Pss_Dirty: 0 kB
740+
Shared_Clean: 8 kB
741+
Shared_Dirty: 0 kB
742+
Private_Clean: 0 kB
743+
Private_Dirty: 0 kB
744+
Referenced: 8 kB
745+
Anonymous: 0 kB
746+
LazyFree: 0 kB
747+
AnonHugePages: 0 kB
748+
ShmemPmdMapped: 0 kB
749+
FilePmdMapped: 0 kB
750+
Shared_Hugetlb: 0 kB
751+
Private_Hugetlb: 0 kB
752+
Swap: 0 kB
753+
SwapPss: 0 kB
754+
Locked: 0 kB
755+
THPeligible: 0
756+
ProtectionKey: 0
757+
VmFlags: rd ex mr mw me de sd";
758+
759+
let region = Region::from_str(region).expect("Parsing failed");
760+
761+
assert_eq!(region.start, 0x00400000);
762+
assert_eq!(region.end, 0x0e8dd000);
763+
764+
let region =
765+
"ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vdso]
766+
Size: 8 kB
767+
KernelPageSize: 4 kB
768+
MMUPageSize: 4 kB
769+
Rss: 8 kB
770+
Pss: 2 kB
771+
Pss_Dirty: 0 kB
772+
Shared_Clean: 8 kB
773+
Shared_Dirty: 0 kB
774+
Private_Clean: 0 kB
775+
Private_Dirty: 0 kB
776+
Referenced: 8 kB
777+
Anonymous: 0 kB
778+
LazyFree: 0 kB
779+
AnonHugePages: 0 kB
780+
ShmemPmdMapped: 0 kB
781+
FilePmdMapped: 0 kB
782+
Shared_Hugetlb: 0 kB
783+
Private_Hugetlb: 0 kB
784+
Swap: 0 kB
785+
SwapPss: 0 kB
786+
Locked: 0 kB
787+
THPeligible: 0
788+
ProtectionKey: 0
789+
VmFlags: rd ex mr mw me de sd";
790+
791+
let region = Region::from_str(region).expect("Parsing failed");
792+
793+
assert_eq!(region.start, 0xffffffffff600000);
794+
assert_eq!(region.end, 0xffffffffff601000);
795+
}
691796
}

0 commit comments

Comments
 (0)