linux: assign CPU temperatures by package/core or CCD #1352
Conversation
BenBE
added
enhancement
Closes htop-dev#806. Closes htop-dev#1048. Closes htop-dev#1176. Closes htop-dev#1335. Addresses htop-dev#879 (on Linux).
leahneukirchen
force-pushed
the
cpu-topology
branch
from
f61c2d0
to
d4cb5d1
Compare
|
Fixed build without libsensors. Fixed assignment on RPi. |
|
Note that the cpu are only parsed once, this takes <0.1ms on my machine. |
|
I think this is ready to go, should I squash it together sensibly? |
As you like. Just took a quick peek at the list of commits and some suggest that they mostly rectify things changed in previous commits of the PR: Those are the prime subjects to squash in the PR right now. So there's not really much to squash together right now AFAICS. |
|
@leahneukirchen Anything left to do from your side? If not please mark the PR ready for review. TIA. |
| @@ -603,6 +603,100 @@ static void scanCPUFrequencyFromCPUinfo(LinuxMachine* this) { | |||
| } | |||
| } | |||
|
|
|||
| #ifdef HAVE_SENSORS_SENSORS_H | |||
| static void LinuxMachine_fetchCPUTopologyFromCPUinfo(LinuxMachine* this) { | |||
There was a problem hiding this comment.
This function does similar to scanCPUFrequencyFromCPUinfo() scan /proc/cpuinfo.
Parsing the frequency here should not make a performance difference.
Then scanCPUFrequencyFromSysCPUFreq() can be changed to write its values first to a local array and on success override the frequencies from that array.
Thus /proc/cpuinfo is not scanned multiple times.
There was a problem hiding this comment.
Then we need to move the ifdef HAVE_SENSORS_SENSORS_H into that function, which makes it harder to read I think.
| @@ -674,6 +772,13 @@ Machine* Machine_new(UsersTable* usersTable, uid_t userId) { | |||
| // Initialize CPU count | |||
| LinuxMachine_updateCPUcount(this); | |||
|
|
|||
| #ifdef HAVE_SENSORS_SENSORS_H | |||
| // Fetch CPU topology | |||
| LinuxMachine_fetchCPUTopologyFromCPUinfo(this); | |||
There was a problem hiding this comment.
maybe LinuxMachine_fetchCPUTopologyFromCPUinfo() can return whether the topology changed and then LibSensors_countCCDs() and LinuxMachine_assignCCDs() get only called on change?
There was a problem hiding this comment.
I don't know what this means, this code runs only once already?
|
@leahneukirchen Do you want to implement the three review comments from @cgzones ? |
|
I somehow dislike that the sensors are iterated twice: once in |
Seconded, libsensors is slow |
|
Tested on my notebook with Alder Lake CPU... It shows temperatures for all cores at least. |
Closes #806.
Closes #1048.
Closes #1176.
Closes #1335.
Addresses #879 (on Linux).