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

fix: CPU frequency detection of FreeBSD #12440

Merged
merged 2 commits into from
Mar 18, 2022

Conversation

ilyam8
Copy link
Member

@ilyam8 ilyam8 commented Mar 17, 2022

Summary

ssia

Test Plan

Tested on FreeBSD freebsd 12.3-RELEASE FreeBSD 12.3-RELEASE r371126 GENERIC amd64:

  • master branch
wget -q -O system-info.sh https://raw.githubusercontent.com/netdata/netdata/master/daemon/system-info.sh; sh system-info.sh | grep -i freq; rm system-info.sh
NETDATA_SYSTEM_CPU_FREQ=unknown
  • this PR
wget -q -O system-info.sh https://raw.githubusercontent.com/ilyam8/netdata/fix_cpu_freq_freebsd/daemon/system-info.sh; sh system-info.sh | grep -i freq; rm system-info.sh
NETDATA_SYSTEM_CPU_FREQ=2400069742
Additional Information

@thiagoftsm
Copy link
Contributor

@ilyam8 the PR is getting CPU frequency, but I also observed two important differences for my CPU. The CPU vendor is set on FreeBSD as unknown, and the frequency on Linux has a round value:

FREEBSD

NETDATA_SYSTEM_CPU_VENDOR=unknown
NETDATA_SYSTEM_CPU_MODEL=Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz
NETDATA_SYSTEM_CPU_FREQ=2904071733

Linux

NETDATA_SYSTEM_CPU_VENDOR=GenuineIntel
NETDATA_SYSTEM_CPU_MODEL=Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz
NETDATA_SYSTEM_CPU_FREQ=2700000000

For the CPU frequency it is easy to justify, but are we going to fix the vendor in another PR?
For these tests I used FreeBSD 13.0-RELEASE-p8 and Slackware 15.0, both were running on VirtualBox.

@ilyam8
Copy link
Member Author

ilyam8 commented Mar 17, 2022

For the CPU frequency it is easy to justify, but are we going to fix the vendor in another PR?

Yes. I am not sure if that is an important field.

Copy link
Contributor

@thiagoftsm thiagoftsm left a comment

Choose a reason for hiding this comment

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

After last answer I am approving based in tests described here.

@ilyam8 ilyam8 requested a review from surajnpn March 17, 2022 13:30
@Ferroin
Copy link
Member

Ferroin commented Mar 17, 2022

For the CPU frequency it is easy to justify, but are we going to fix the vendor in another PR?

Yes. I am not sure if that is an important field.

We should ideally be reporting correctly, but we need to confirm if this is an issue in our script or in FreeBSD itself (given that the test system in question was a VM, it would not surprise me if FreeBSD just doesn’t provide the info there).

@thiagoftsm
Copy link
Contributor

For the CPU frequency it is easy to justify, but are we going to fix the vendor in another PR?

Yes. I am not sure if that is an important field.

We should ideally be reporting correctly, but we need to confirm if this is an issue in our script or in FreeBSD itself (given that the test system in question was a VM, it would not surprise me if FreeBSD just doesn’t provide the info there).

I will update my FreeBSD on host, and I will retest this PR again, to complement my answer this is the complete output I received on VM:

NETDATA_CONTAINER_OS_NAME=none
NETDATA_CONTAINER_OS_ID=none
NETDATA_CONTAINER_OS_ID_LIKE=none
NETDATA_CONTAINER_OS_VERSION=none
NETDATA_CONTAINER_OS_VERSION_ID=none
NETDATA_CONTAINER_OS_DETECTION=none
NETDATA_CONTAINER_IS_OFFICIAL_IMAGE=false
NETDATA_HOST_OS_NAME=FreeBSD
NETDATA_HOST_OS_ID=FreeBSD
NETDATA_HOST_OS_ID_LIKE=FreeBSD
NETDATA_HOST_OS_VERSION=13.0-RELEASE-p8
NETDATA_HOST_OS_VERSION_ID=unknown
NETDATA_HOST_OS_DETECTION=uname
NETDATA_HOST_IS_K8S_NODE=false
NETDATA_SYSTEM_KERNEL_NAME=FreeBSD
NETDATA_SYSTEM_KERNEL_VERSION=1300139
NETDATA_SYSTEM_ARCHITECTURE=amd64
NETDATA_SYSTEM_VIRTUALIZATION=none
NETDATA_SYSTEM_VIRT_DETECTION=none
NETDATA_SYSTEM_CONTAINER=unknown
NETDATA_SYSTEM_CONTAINER_DETECTION=none
NETDATA_SYSTEM_CPU_LOGICAL_CPU_COUNT=2
NETDATA_SYSTEM_CPU_VENDOR=unknown
NETDATA_SYSTEM_CPU_MODEL=Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz
NETDATA_SYSTEM_CPU_FREQ=2904071733
NETDATA_SYSTEM_CPU_DETECTION=sysctl
NETDATA_SYSTEM_TOTAL_RAM=1038655488
NETDATA_SYSTEM_RAM_DETECTION=sysctl
NETDATA_SYSTEM_TOTAL_DISK_SIZE=15591567360
NETDATA_SYSTEM_DISK_DETECTION=df

@ilyam8
Copy link
Member Author

ilyam8 commented Mar 17, 2022

I will retest this PR again

This PR fixes only CPU freq detection.

@thiagoftsm
Copy link
Contributor

@Ferroin when I run direct on host, the Processor is correctly identified:

NETDATA_CONTAINER_OS_NAME=none
NETDATA_CONTAINER_OS_ID=none
NETDATA_CONTAINER_OS_ID_LIKE=none
NETDATA_CONTAINER_OS_VERSION=none
NETDATA_CONTAINER_OS_VERSION_ID=none
NETDATA_CONTAINER_OS_DETECTION=none
NETDATA_CONTAINER_IS_OFFICIAL_IMAGE=false
NETDATA_HOST_OS_NAME=FreeBSD
NETDATA_HOST_OS_ID=FreeBSD
NETDATA_HOST_OS_ID_LIKE=FreeBSD
NETDATA_HOST_OS_VERSION=13.0-RELEASE-p8
NETDATA_HOST_OS_VERSION_ID=unknown
NETDATA_HOST_OS_DETECTION=uname
NETDATA_HOST_IS_K8S_NODE=false
NETDATA_SYSTEM_KERNEL_NAME=FreeBSD
NETDATA_SYSTEM_KERNEL_VERSION=1300139
NETDATA_SYSTEM_ARCHITECTURE=amd64
NETDATA_SYSTEM_VIRTUALIZATION=none
NETDATA_SYSTEM_VIRT_DETECTION=none
NETDATA_SYSTEM_CONTAINER=unknown
NETDATA_SYSTEM_CONTAINER_DETECTION=none
NETDATA_SYSTEM_CPU_LOGICAL_CPU_COUNT=4
NETDATA_SYSTEM_CPU_VENDOR=Intel(R) Corporation
NETDATA_SYSTEM_CPU_MODEL=Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz
NETDATA_SYSTEM_CPU_FREQ=2600000000
NETDATA_SYSTEM_CPU_DETECTION=dmidecode
NETDATA_SYSTEM_TOTAL_RAM=17051623424
NETDATA_SYSTEM_RAM_DETECTION=sysctl
NETDATA_SYSTEM_TOTAL_DISK_SIZE=127906705408
NETDATA_SYSTEM_DISK_DETECTION=df

@ilyam8 ignore the previous message, because looks like something related with virtual machine.

@ilyam8 ilyam8 merged commit c1532d5 into netdata:master Mar 18, 2022
@ilyam8 ilyam8 deleted the fix_cpu_freq_freebsd branch March 18, 2022 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants