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

Linux/LibSensors: adjust data for k10temp #1249

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 42 additions & 28 deletions linux/LibSensors.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ in the source distribution for its full text.

#ifdef BUILD_STATIC

#define sym_sensors_init sensors_init
#define sym_sensors_cleanup sensors_cleanup
#define sym_sensors_init sensors_init
#define sym_sensors_cleanup sensors_cleanup
#define sym_sensors_get_detected_chips sensors_get_detected_chips
#define sym_sensors_get_features sensors_get_features
#define sym_sensors_get_subfeature sensors_get_subfeature
#define sym_sensors_get_value sensors_get_value
#define sym_sensors_get_features sensors_get_features
#define sym_sensors_get_subfeature sensors_get_subfeature
#define sym_sensors_get_value sensors_get_value

#else

Expand Down Expand Up @@ -128,32 +128,34 @@ int LibSensors_reload(void) {
return sym_sensors_init(NULL);
}

static int tempDriverPriority(const sensors_chip_name* chip) {
static const struct TempDriverDefs {
typedef enum TempDriver_ {
TD_CORETEMP,
TD_CPUTEMP,
TD_CPUTHERMAL,
TD_K10TEMP,
TD_ZENPOWER,
TD_ACPITZ,
TD_UNKNOWN,
} TempDriver;

static const struct TempDriverDefs {
const char* prefix;
int priority;
} tempDrivers[] = {
{ "coretemp", 0 },
{ "via_cputemp", 0 },
{ "cpu_thermal", 0 },
{ "k10temp", 0 },
{ "zenpower", 0 },
/* Low priority drivers */
{ "acpitz", 1 },
};

for (size_t i = 0; i < ARRAYSIZE(tempDrivers); i++)
if (String_eq(chip->prefix, tempDrivers[i].prefix))
return tempDrivers[i].priority;

return -1;
}
} tempDrivers[TD_UNKNOWN] = {
[TD_CORETEMP] = { "coretemp", 0 },
[TD_CPUTEMP] = { "via_cputemp", 0 },
[TD_CPUTHERMAL] = { "cpu_thermal", 0 },
[TD_K10TEMP] = { "k10temp", 0 },
[TD_ZENPOWER] = { "zenpower", 0 },
/* Low priority drivers */
[TD_ACPITZ] = { "acpitz", 1 },
};

void LibSensors_getCPUTemperatures(CPUData* cpus, unsigned int existingCPUs, unsigned int activeCPUs) {
assert(existingCPUs > 0 && existingCPUs < 16384);

double* data = xMallocArray(existingCPUs + 1, sizeof(double));
for (size_t i = 0; i < existingCPUs + 1; i++)
float* data = xMallocArray(existingCPUs + 1, sizeof(float));
for (size_t i = 0; i <= existingCPUs; i++)
data[i] = NAN;

#ifndef BUILD_STATIC
Expand All @@ -163,11 +165,22 @@ void LibSensors_getCPUTemperatures(CPUData* cpus, unsigned int existingCPUs, uns

unsigned int coreTempCount = 0;
int topPriority = 99;
TempDriver topDriver = TD_UNKNOWN;

int n = 0;
for (const sensors_chip_name* chip = sym_sensors_get_detected_chips(NULL, &n); chip; chip = sym_sensors_get_detected_chips(NULL, &n)) {
const int priority = tempDriverPriority(chip);
if (priority < 0)
int priority = -1;
TempDriver driver = TD_UNKNOWN;

for (size_t i = 0; i < ARRAYSIZE(tempDrivers); i++) {
if (chip->prefix && String_eq(chip->prefix, tempDrivers[i].prefix)) {
priority = tempDrivers[i].priority;
driver = i;
break;
}
}

if (driver == TD_UNKNOWN)
continue;

if (priority > topPriority)
Expand All @@ -180,6 +193,7 @@ void LibSensors_getCPUTemperatures(CPUData* cpus, unsigned int existingCPUs, uns
}

topPriority = priority;
topDriver = driver;

int m = 0;
for (const sensors_feature* feature = sym_sensors_get_features(chip, &m); feature; feature = sym_sensors_get_features(chip, &m)) {
Expand Down Expand Up @@ -239,7 +253,7 @@ void LibSensors_getCPUTemperatures(CPUData* cpus, unsigned int existingCPUs, uns

/* No package temperature - set to max core temperature */
if (coreTempCount > 0 && isNaN(data[0])) {
double maxTemp = -HUGE_VAL;
float maxTemp = -HUGE_VALF;
for (size_t i = 1; i <= existingCPUs; i++) {
if (isgreater(data[i], maxTemp)) {
maxTemp = data[i];
Expand Down
16 changes: 15 additions & 1 deletion linux/LinuxMachine.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,15 @@ static void LinuxMachine_updateCPUcount(LinuxMachine* this) {
this->cpuData[1].online = true;
super->activeCPUs = 1;
super->existingCPUs = 1;
this->maxCoreId = 0;
}

DIR* dir = opendir("/sys/devices/system/cpu");
if (!dir)
return;

unsigned int currExisting = super->existingCPUs;
unsigned short maxCoreId = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we bother with the unsigned short data type here (rather than unsigned int)? There is no space constraint here (it's not an array) and short data types typically process slower in most CPU architectures.

Copy link
Member Author

Choose a reason for hiding this comment

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

To save two bytes in the CPUData struct.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cgzones Not worth it when your other code becomes larger due to the instructions processing 16-bit registers. (At least in IA-32 & x86-64, instructions processing 16-bit registers need a prefix and each becomes one byte larger than 32-bit counterparts.)


const struct dirent* entry;
while ((entry = readdir(dir)) != NULL) {
Expand All @@ -73,7 +75,7 @@ static void LinuxMachine_updateCPUcount(LinuxMachine* this) {
continue;

char* endp;
unsigned long int id = strtoul(entry->d_name + 3, &endp, 10);
const unsigned long int id = strtoul(entry->d_name + 3, &endp, 10);
if (id == ULONG_MAX || endp == entry->d_name + 3 || *endp != '\0')
continue;

Expand Down Expand Up @@ -106,6 +108,17 @@ static void LinuxMachine_updateCPUcount(LinuxMachine* this) {
this->cpuData[id + 1].online = false;
}

res = xReadfileat(cpuDirFd, "topology/core_id", buffer, sizeof(buffer));
if (res > 0) {
unsigned long int parsedCoreId = strtoul(buffer, &endp, 10);
if (parsedCoreId < USHRT_MAX && endp != buffer && (*endp == '\0' || *endp == '\n')) {
unsigned short coreId = (unsigned short) parsedCoreId;
this->cpuData[id + 1].coreId = coreId;

maxCoreId = MAXIMUM(maxCoreId, coreId);
}
}

Compat_openatArgClose(cpuDirFd);
}

Expand All @@ -125,6 +138,7 @@ static void LinuxMachine_updateCPUcount(LinuxMachine* this) {
super->activeCPUs = active;
assert(existing == currExisting);
super->existingCPUs = currExisting;
this->maxCoreId = maxCoreId;
}

static void LinuxMachine_scanMemoryInfo(LinuxMachine* this) {
Expand Down
2 changes: 2 additions & 0 deletions linux/LinuxMachine.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ typedef struct CPUData_ {
double temperature;
#endif

unsigned short coreId; /* Note: might be higer than core count, due to disabled ones */
bool online;
} CPUData;

Expand All @@ -73,6 +74,7 @@ typedef struct LinuxMachine_ {
double period;

CPUData* cpuData;
unsigned short maxCoreId; /* Note: might be higer than core count, due to disabled ones */

memory_t totalHugePageMem;
memory_t usedHugePageMem[HTOP_HUGEPAGE_COUNT];
Expand Down