Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

context.memTotalPhysicalBytes(): Use a Syscall #171

Closed
wants to merge 1 commit into from

Conversation

alrs
Copy link
Contributor

@alrs alrs commented Mar 10, 2020

This looks to be either:

  1. Me not understanding a corner case that requires getting memory information out of logfiles.

  2. A good place to use syscall.

If you're open to using system calls instead of parsing files, there are probably a few more commits like this in me.

@alrs
Copy link
Contributor Author

alrs commented Mar 10, 2020

Thinking about this, it's more likely you're looking for something like this, right?

https://github.com/digitalocean/go-smbios/blob/master/cmd/lsdimms/main.go

@jaypipes
Copy link
Owner

Thinking about this, it's more likely you're looking for something like this, right?

https://github.com/digitalocean/go-smbios/blob/master/cmd/lsdimms/main.go

Interesting code there, thanks for linking it here! I think the above may be a good approach for adding the Linux implementation of @KingRial's Windows-specific patch that adds memory-module/DIMM information: #162

Thoughts?

Copy link
Owner

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this won't return the physical amount of memory :) See explanation inline...

func (ctx *context) memTotalPhysicalBytes() (uint64, error) {
var info syscall.Sysinfo_t
err := syscall.Sysinfo(&info)
return info.Totalram, err
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, sysinfo_t.totalram is the usable memory amount, not the physical memory amount :)

http://man7.org/linux/man-pages/man2/sysinfo.2.html

The results of sysinfo(2) are replicated in /proc/meminfo, which is what is used to get the total usable bytes below. See the comment on (original) line 124 below that explains why we look up the physical memory size using syslog.

That said, I think using the DMI approach (that the digitalocean library uses) would allow us to simply add up the physical size of all DIMMs attached to the host. The only problem with that is, of course, DMI access is a privileged operation so we're back in the same situation as before that caused me to originally write this code... (accessing syslog sometimes requires lower privileges than accessing DMI information, thus the fallback to using syslog instead of DMI)

See the note

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants