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

assume phys mem == usable mem on ARM #337

Merged
merged 1 commit into from
May 2, 2023
Merged

assume phys mem == usable mem on ARM #337

merged 1 commit into from
May 2, 2023

Conversation

jaypipes
Copy link
Owner

/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 TotalUsableBytes.

Issue #336

@jaypipes jaypipes requested a review from ffromani April 21, 2023 18:28
totUsable, err := memoryTotalUsableBytesFromPath(filepath.Join(path, "meminfo"))
if err != nil {
return nil, err
blockSizeBytes, err = memoryBlockSizeBytes(paths.SysDevicesSystemMemory)

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.

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

Copy link
Owner Author

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>
@jaypipes
Copy link
Owner Author

jaypipes commented May 2, 2023

@ffromani any chance you have some time to review this one? :)

Copy link
Collaborator

@ffromani ffromani left a comment

Choose a reason for hiding this comment

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

LGTM!

@ffromani ffromani merged commit 6aad076 into main May 2, 2023
@ffromani
Copy link
Collaborator

ffromani commented May 2, 2023

@ffromani any chance you have some time to review this one? :)

done and merged!

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.

3 participants