search

eminence / procfs

Rust library for reading the Linux procfs filesystem
Other
362 stars 106 forks source link

Add /proc/iomem #216

Closed tatref closed 1 year ago

tatref commented 1 year ago

Hi,

This PR adds support for parsing /proc/iomem

The content looks like:

00100000-dffeffff : System RAM
  49200000-4a202387 : Kernel code
  4a400000-4b00bfff : Kernel rodata
  4b200000-4b7f503f : Kernel data
  4bfdb000-4c5fffff : Kernel bss
  c3000000-deffffff : Crash kernel
dfff0000-dfffffff : ACPI Tables
e0000000-fdffffff : PCI Bus 0000:00
  e0000000-e0ffffff : 0000:00:02.0

Some values are constant like System RAM, Kernel code... but others depend on the hardware and kernel modules: vmwgfx probe, PCI Bus xxxx...

Should we try to enumerate all of these in an enum like MMapPath, with a special variant holding a String? Or we can keep it like this, with a simple String?

Another point is that this requires root or sudo, otherwise all addresses are 0. At first I added a panic! in the example, but I guess this would break CI?

What do you think?

eminence commented 1 year ago

Should we try to enumerate all of these in an enum like MMapPath, with a special variant holding a String? Or we can keep it like this, with a simple String?

From first glance, it looks like there might be too many variants to try to enumerate all of them. So my vote is to just store them as Strings.

Another point is that this requires root or sudo, otherwise all addresses are 0. At first I added a panic! in the example, but I guess this would break CI?

Let's make sure the code can handle the case when all addresses are zero. In the test, the assertions can do things like: if i am root, make sure the parsed addresses are non-zero.

Also, I have no idea what's up with the CI failures. I'll look at that later

tatref commented 1 year ago

The code already handle the 0 addresses, so it's fine

I fixed CI, I was missing a colon in a docstring

eminence commented 1 year ago

One last question: the data in /proc/iomem looks hierarchical, based on the indentation. Is that structure something we should capture?

tatref commented 1 year ago

One last question: the data in /proc/iomem looks hierarchical, based on the indentation. Is that structure something we should capture?

Yes we could add indentation.

Should we construct a real tree, or just record the indentation index?

eminence commented 1 year ago

I think having a tree would be best. But maybe we can leave that for a followup pull request. I don't think tracking just the indentation is the right thing to do.

Something strange is going on with one of your commits: 9e7aff95965648413e6a9308ba70fb9f4993d114. This commit somehow includes some of the smaps_rollup changes from #214. I'm not quite sure how this happened, but can you take a look? Rebasing on to the latest master branch might be one way to resolve this.

tatref commented 1 year ago

I rebased from master, the commits look good.

The present version includes the indentation. Do you want me to remove it before we settle on some kind of tree structure?

eminence commented 1 year ago

Do you want me to remove it before we settle on some kind of tree structure?

No, we can leave it for now. But let's find something better before the next procfs release.