-
Notifications
You must be signed in to change notification settings - Fork 178
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
assume phys mem == usable mem on ARM #337
Conversation
totUsable, err := memoryTotalUsableBytesFromPath(filepath.Join(path, "meminfo")) | ||
if err != nil { | ||
return nil, err | ||
blockSizeBytes, err = memoryBlockSizeBytes(paths.SysDevicesSystemMemory) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jaypipes , I think should use memTotalPhysicalBytes function here, memTotalPhysicalBytes will scan syslog to get physical size, and check return value > 0 to decide whether to use totUsable memory.
ghw/pkg/memory/memory_linux.go
Lines 105 to 113 in 227643e
func memTotalPhysicalBytes(paths *linuxpath.Paths) (total int64) { | |
defer func() { | |
// fallback to the syslog file approach in case of error | |
if total < 0 { | |
total = memTotalPhysicalBytesFromSyslog(paths) | |
} | |
}() | |
// detect physical memory from /sys/devices/system/memory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wanyaoqi good point. done!
`/sys/devices/system/memory` does not exist on linux/arm systems that do not support memory hotplug. This means that the `/sys/devices/system/memory/block_size_bytes` file also does not exist and that is the file that we use to determine the physical RAM associated with a RAM module affined to a NUMA node. This commit updates the logic in `pkg/memory.AreaForNode` on linux platforms to tolerate the error returned by `memoryBlockSizeBytes()` when `/sys/devices/system/memory/block_size_bytes` does not exist and set a memory `Area`'s `TotalPhysicalBytes` value to the value of either the syslog-determined physical memory total or `TotalUsableBytes`. Issue #336 Signed-off-by: Jay Pipes <jaypipes@gmail.com>
@ffromani any chance you have some time to review this one? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
done and merged! |
/sys/devices/system/memory
does not exist on linux/arm systems that do not support memory hotplug. This means that the/sys/devices/system/memory/block_size_bytes
file also does not exist and that is the file that we use to determine the physical RAM associated with a RAM module affined to a NUMA node.This commit updates the logic in
pkg/memory.AreaForNode
on linux platforms to tolerate the error returned bymemoryBlockSizeBytes()
when/sys/devices/system/memory/block_size_bytes
does not exist and set a memoryArea
'sTotalPhysicalBytes
value to the value ofTotalUsableBytes
.Issue #336