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

Add cpufreq support on Darwin for Apple Silicon machines #1272

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ darwin_platform_headers = \
darwin/DarwinMachine.h \
darwin/DarwinProcess.h \
darwin/DarwinProcessList.h \
darwin/CpuFreq.h \
darwin/Platform.h \
darwin/PlatformHelpers.h \
darwin/ProcessField.h \
Expand All @@ -353,6 +354,7 @@ darwin_platform_sources = \
darwin/DarwinMachine.c \
darwin/DarwinProcess.c \
darwin/DarwinProcessList.c \
darwin/CpuFreq.c \
generic/fdstat_sysctl.c \
generic/gettime.c \
generic/hostname.c \
Expand Down
38 changes: 38 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ AC_USE_SYSTEM_EXTENSIONS

# ----------------------------------------------------------------------

# ----------------------------------------------------------------------
# Checks for cpu.
# ----------------------------------------------------------------------

case "$host_cpu" in
aarch64*)
my_htop_cpu=aarch64
AC_DEFINE([HTOP_AARCH64], [], [Building for aarch64.])
;;
esac

# ----------------------------------------------------------------------

# ----------------------------------------------------------------------
# Checks for compiler.
Expand Down Expand Up @@ -678,6 +690,30 @@ fi
# ----------------------------------------------------------------------


# ----------------------------------------------------------------------
# Checks for Darwin features and flags.
# ----------------------------------------------------------------------

if test "$my_htop_cpu" = aarch64; then
AC_ARG_ENABLE([libioreport],
[AS_HELP_STRING([--enable-libioreport],
[enable libIOReport support for getting cpu frequency on Apple Silicon machines @<:@default=yes@:>@])],
[],
[enable_libioreport=yes])
case "$enable_libioreport" in
no)
;;
yes)
AC_CHECK_LIB([IOReport], [IOReportCreateSubscription])
;;
*)
AC_MSG_ERROR([bad value '$enable_libioreport' for --enable_libioreport])
;;
esac
fi

# ----------------------------------------------------------------------

Comment on lines +693 to +716
Copy link
Member

Choose a reason for hiding this comment

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

This should probably follow the pattern used for other libraries and include some auto-detection of library availability.

# ----------------------------------------------------------------------
# Checks for compiler warnings.
# ----------------------------------------------------------------------
Expand Down Expand Up @@ -769,6 +805,8 @@ AM_CONDITIONAL([HTOP_SOLARIS], [test "$my_htop_platform" = solaris])
AM_CONDITIONAL([HTOP_PCP], [test "$my_htop_platform" = pcp])
AM_CONDITIONAL([HTOP_UNSUPPORTED], [test "$my_htop_platform" = unsupported])

AM_CONDITIONAL([HTOP_AARCH64], [test "$my_htop_cpu" = aarch64])

Comment on lines +808 to +809
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this is handled consistently, as we currently don't handle different CPU architectures any different while on the some underlying OS/platform. There's no difference between Linux on ARM vs. Linux on i386 or x64.

Thus I'm not really in on adding this additional define. Thoughts @ @fasterit @natoscott @cgzones?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just use the predefined macro __aarch64__ to detect the CPU architecture, and combine with HTOP_DARWIN defined by the configure script to get what we need?

AC_SUBST(my_htop_platform)
AC_CONFIG_FILES([Makefile htop.1])
AC_OUTPUT
Expand Down
176 changes: 176 additions & 0 deletions darwin/CpuFreq.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
#include "darwin/CpuFreq.h"

#ifdef CPUFREQ_SUPPORT

#include <stdlib.h>
#include <string.h>
#include <stdint.h>
Comment on lines +5 to +7
Copy link
Member

Choose a reason for hiding this comment

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

Heder order, second blank after headers. Cf. styleguide.


/* Implementations */
int CpuFreq_init(const Machine* machine, CpuFreqData* data) {
Copy link
Member

Choose a reason for hiding this comment

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

If you are just returning success, prefer bool from stdbool.h

data->existingCPUs = machine->existingCPUs;
data->cluster_type_per_cpu = xCalloc(data->existingCPUs, sizeof(uint32_t));
data->frequencies = xCalloc(data->existingCPUs, sizeof(double));

/* Determine the cluster type for all CPUs */
char buf[128];
Copy link
Member

Choose a reason for hiding this comment

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

Limit the scope of this stack buffer to the body of the loop.


for (uint32_t num_cpus = 0U; num_cpus < data->existingCPUs; num_cpus++) {
snprintf(buf, sizeof(buf), "IODeviceTree:/cpus/cpu%u", num_cpus);
Copy link
Member

Choose a reason for hiding this comment

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

Prefer xSnprintf from XUtils.h due to overflow check.

Also, should be "…/cpu%"PRIu32 or "…/cpu%zu" (if using size_t for num_cpus.

io_registry_entry_t cpu_io_registry_entry = IORegistryEntryFromPath(kIOMainPortDefault, buf);
if (cpu_io_registry_entry == MACH_PORT_NULL) {
return -1;
Copy link
Member

Choose a reason for hiding this comment

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

This leaks memory …

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this and all similar subsequent comments: the intention was that even on failure, we should be in a consistent state, so CpuFreq_cleanup can (and should) be called (and it is, if you look at DarwinMachine.c). If that's not preferred, I can refactor, but it will introduce more boilerplate error handling.

Copy link
Member

Choose a reason for hiding this comment

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

Error handling is one of the rare cases where goto err; is actually a quite common pattern.

}

CFDataRef cluster_type_ref = (CFDataRef) IORegistryEntryCreateCFProperty(cpu_io_registry_entry, CFSTR("cluster-type"), kCFAllocatorDefault, 0U);
if (cluster_type_ref == NULL) {
IOObjectRelease(cpu_io_registry_entry);
return -1;
Copy link
Member

Choose a reason for hiding this comment

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

Memory leak.

}
if (CFDataGetLength(cluster_type_ref) != 2) {
CFRelease(cluster_type_ref);
IOObjectRelease(cpu_io_registry_entry);
return -1;
Copy link
Member

Choose a reason for hiding this comment

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

Memory leak.

}
UniChar cluster_type_char;
CFDataGetBytes(cluster_type_ref, CFRangeMake(0, 2), (uint8_t*) &cluster_type_char);
CFRelease(cluster_type_ref);

uint32_t cluster_type = 0U;
switch (cluster_type_char) {
case L'E':
cluster_type = 0U;
break;
case L'P':
cluster_type = 1U;
break;
default:
/* Unknown cluster type */
IOObjectRelease(cpu_io_registry_entry);
return -1;
Copy link
Member

Choose a reason for hiding this comment

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

Memory leak.

}

data->cluster_type_per_cpu[num_cpus] = cluster_type;

IOObjectRelease(cpu_io_registry_entry);
}

/*
* Determine frequencies for per-cluster-type performance states
* Frequencies for the "E" cluster type are stored in voltage-states1,
* frequencies for the "P" cluster type are stored in voltage-states5.
* This seems to be hardcoded.
*/
const CFStringRef voltage_states_key_per_cluster[CPUFREQ_NUM_CLUSTER_TYPES] = {CFSTR("voltage-states1"), CFSTR("voltage-states5")};

io_registry_entry_t pmgr_registry_entry = IORegistryEntryFromPath(kIOMainPortDefault, "IODeviceTree:/arm-io/pmgr");
if (pmgr_registry_entry == MACH_PORT_NULL) {
return -1;
Copy link
Member

Choose a reason for hiding this comment

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

Memory leak.

}
for (size_t i = 0U; i < CPUFREQ_NUM_CLUSTER_TYPES; i++) {
CFDataRef voltage_states_ref =
(CFDataRef) IORegistryEntryCreateCFProperty(pmgr_registry_entry, voltage_states_key_per_cluster[i], kCFAllocatorDefault, 0U);
if (voltage_states_ref == NULL) {
IOObjectRelease(pmgr_registry_entry);
return -1;
Copy link
Member

Choose a reason for hiding this comment

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

Memory leak.

}

CpuFreqPowerStateFrequencies* cluster_frequencies = &data->cpu_frequencies_per_cluster_type[i];
cluster_frequencies->num_frequencies = CFDataGetLength(voltage_states_ref) / 8;
cluster_frequencies->frequencies = xCalloc(cluster_frequencies->num_frequencies, sizeof(double));
const uint8_t* voltage_states_data = CFDataGetBytePtr(voltage_states_ref);
for (size_t j = 0U; j < cluster_frequencies->num_frequencies; j++) {
uint32_t freq_value;
memcpy(&freq_value, voltage_states_data + j * 8, 4);
Copy link
Member

Choose a reason for hiding this comment

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

Where is that 8 coming from?

Copy link
Contributor Author

@bakaid bakaid Jul 26, 2023

Choose a reason for hiding this comment

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

This is how these values are stored, in an unstructured byte array, every 8 bytes storing one 4 byte frequency value and one 4 byte voltage value. There is no documentation for this, it had to be determined based on trial and error (and matching it with the frequency values displayed by powermetrics).

Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to have some documentation of this somewhere. You could even do a packed structure with these values and reference those values that way.

cluster_frequencies->frequencies[j] = (65536000.0 / freq_value) * 1000000;
Copy link
Member

Choose a reason for hiding this comment

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

Given that the uint32_t from freq_value is promoted to double for this division anyways, let's shorten this expression:

Suggested change
cluster_frequencies->frequencies[j] = (65536000.0 / freq_value) * 1000000;
cluster_frequencies->frequencies[j] = 65536e9 / freq_value;

}
CFRelease(voltage_states_ref);
}
IOObjectRelease(pmgr_registry_entry);


/* Create subscription for CPU performance states */
CFMutableDictionaryRef channels = IOReportCopyChannelsInGroup(CFSTR("CPU Stats"), CFSTR("CPU Core Performance States"), NULL, NULL);
if (channels == NULL) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (channels == NULL) {
if (!channels) {

would be also fine …

return -1;
Copy link
Member

Choose a reason for hiding this comment

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

Memory leak.

}

data->subscribed_channels = NULL;
data->subscription = IOReportCreateSubscription(NULL, channels, &data->subscribed_channels, 0U, NULL);

CFRelease(channels);

if (data->subscription == NULL) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (data->subscription == NULL) {
if (!data->subscription) {

return -1;
Copy link
Member

Choose a reason for hiding this comment

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

Memory leak.

}

data->prev_samples = NULL;
Copy link
Member

Choose a reason for hiding this comment

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

IF this is for field initialization it shouldn't go that far to the end. Especially as it's not used anywhere else in this function.


return 0;
}

void CpuFreq_update(CpuFreqData* data) {
CFDictionaryRef samples = IOReportCreateSamples(data->subscription, data->subscribed_channels, NULL);
if (samples == NULL) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (samples == NULL) {
if (!samples) {

return;
Copy link
Member

Choose a reason for hiding this comment

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

If no samples could be gathered, technically we should also update the prev_samples

}

if (data->prev_samples == NULL) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (data->prev_samples == NULL) {
if (!data->prev_samples) {

data->prev_samples = samples;
return;
}

/* Residency is cumulative, we have to diff two samples to get a current view */
CFDictionaryRef samples_delta = IOReportCreateSamplesDelta(data->prev_samples, samples, NULL);

/* Iterate through performance state residencies. Index 0 is the idle residency, index 1-n is the per-performance state residency. */
__block uint32_t cpu_i = 0U;
IOReportIterate(samples_delta, ^(IOReportSampleRef ch) {
Copy link
Member

Choose a reason for hiding this comment

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

As that's not strictly C99 conforming code, I assume that's intermixed ObjC stuff thrown in for good measure?

Copy link
Contributor Author

@bakaid bakaid Jul 26, 2023

Choose a reason for hiding this comment

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

Not ObjC, rather an Apple extension for Blocks. We must use it, since that's what the API expects as a callback.

if (cpu_i >= data->existingCPUs) {
/* The report contains more CPUs than we know about. This should not happen. */
return kIOReportIterOk; // TODO: find way to possibly stop iteration early on error
}
const CpuFreqPowerStateFrequencies* cpu_frequencies = &data->cpu_frequencies_per_cluster_type[data->cluster_type_per_cpu[cpu_i]];
uint32_t state_count = IOReportStateGetCount(ch);
if (state_count != cpu_frequencies->num_frequencies + 1) {
/* The number of reported states does not match the number of previously queried frequencies. This should not happen. */
return kIOReportIterOk; // TODO: find way to possibly stop iteration early on error
}
/* Calculate average frequency based on residency and per-performance state frequency */
double average_freq = 0.0;
int64_t total_residency = 0U;
for (uint32_t i = 0U; i < state_count; i++) {
const int64_t residency = IOReportStateGetResidency(ch, i);
total_residency += residency;
/* We count idle as the smallest frequency */
average_freq += residency * cpu_frequencies->frequencies[(i == 0) ? 0 : (i - 1)];
}
average_freq /= total_residency;
data->frequencies[cpu_i] = average_freq / 1000000; // Convert to MHz

cpu_i++;
return kIOReportIterOk;
});
CFRelease(samples_delta);

CFRelease(data->prev_samples);
data->prev_samples = samples;
}

void CpuFreq_cleanup(CpuFreqData* data) {
if (data->subscription != NULL) {
CFRelease(data->subscription);
}
if (data->subscribed_channels != NULL) {
CFRelease(data->subscribed_channels);
}
if (data->prev_samples != NULL) {
CFRelease(data->prev_samples);
}
Comment on lines +161 to +169
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (data->subscription != NULL) {
CFRelease(data->subscription);
}
if (data->subscribed_channels != NULL) {
CFRelease(data->subscribed_channels);
}
if (data->prev_samples != NULL) {
CFRelease(data->prev_samples);
}
if (data->subscription)
CFRelease(data->subscription);
if (data->subscribed_channels)
CFRelease(data->subscribed_channels);
if (data->prev_samples)
CFRelease(data->prev_samples);

Normally you would even drop the NULL check, but given that CFRelease is kinda picky in that regard, these checks need to stay … :( But we can at least reduce the noise …

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's why they are there :) If NULL equivalency check is not the preferred style, I will refactor those along with the rest.

free(data->cluster_type_per_cpu);
free(data->frequencies);
for (size_t i = 0U; i < CPUFREQ_NUM_CLUSTER_TYPES; i++) {
free(data->cpu_frequencies_per_cluster_type[i].frequencies);
}
Comment on lines +172 to +174
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this missing a follow-up call of free(data->cpu_frequencies_per_cluster_type);???

}
#endif
66 changes: 66 additions & 0 deletions darwin/CpuFreq.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
#ifndef HEADER_CpuFreq
#define HEADER_CpuFreq

#include "Machine.h"

#if (defined(HAVE_LIBIOREPORT) && defined(HTOP_AARCH64))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#if (defined(HAVE_LIBIOREPORT) && defined(HTOP_AARCH64))
#if defined(HAVE_LIBIOREPORT) && defined(HTOP_AARCH64)

We're not writing Lisp …

#define CPUFREQ_SUPPORT

#include <IOKit/IOKitLib.h>
#include <CoreFoundation/CoreFoundation.h>

/* Private API definitions from libIOReport*/
enum {
kIOReportIterOk = 0,
};
typedef struct IOReportSubscriptionRef *IOReportSubscriptionRef;
typedef CFDictionaryRef IOReportSampleRef;
typedef CFDictionaryRef IOReportChannelRef;
typedef int (^io_report_iterate_callback_t)(IOReportSampleRef ch);
extern void IOReportIterate(CFDictionaryRef samples, io_report_iterate_callback_t callback);
extern CFMutableDictionaryRef IOReportCopyChannelsInGroup(CFStringRef, CFStringRef, void*, void*);
extern IOReportSubscriptionRef IOReportCreateSubscription(void *a, CFMutableDictionaryRef desiredChannels, CFMutableDictionaryRef *subbedChannels, uint64_t channel_id, CFTypeRef b);
extern CFDictionaryRef IOReportCreateSamples(IOReportSubscriptionRef iorsub, CFMutableDictionaryRef subbedChannels, CFTypeRef a);
extern uint32_t IOReportStateGetCount(IOReportChannelRef ch);
extern uint64_t IOReportStateGetResidency(IOReportChannelRef ch, uint32_t index);
extern CFDictionaryRef IOReportCreateSamplesDelta(CFDictionaryRef prev, CFDictionaryRef current, CFTypeRef a);
Comment on lines +6 to +26
Copy link
Member

Choose a reason for hiding this comment

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

This is probably all best kept to the implementation file instead of in the header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some of it is needed in the CpuFreqData struct, most of it I can move to the implementation.


/* Definitions */
typedef struct {
uint32_t num_frequencies;
Copy link
Member

Choose a reason for hiding this comment

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

Should include stdint.h in the header

double* frequencies;
} CpuFreqPowerStateFrequencies;

/*
* Seems to be hardcoded for now on all Apple Silicon platforms, no way to get it dynamically.
* Current cluster types are "E" for efficiency cores and "P" for performance cores.
*/
#define CPUFREQ_NUM_CLUSTER_TYPES 2

typedef struct {
/* Number of CPUs */
unsigned int existingCPUs;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
unsigned int existingCPUs;
size_t existingCPUs;

Also include stddef.h

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer size_t too, but it is unsigned int in Machine, where I copy it from.


/* existingCPUs records, containing which CPU belongs to which cluster type ("E": 0, "P": 1) */
uint32_t* cluster_type_per_cpu;

/* Frequencies for all power states per cluster type */
CpuFreqPowerStateFrequencies cpu_frequencies_per_cluster_type[CPUFREQ_NUM_CLUSTER_TYPES];

/* IOReport subscription handlers */
IOReportSubscriptionRef subscription;
CFMutableDictionaryRef subscribed_channels;

/* Last IOReport sample */
CFDictionaryRef prev_samples;

/* existingCPUs records, containing last determined frequency per CPU in MHz */
double* frequencies;
} CpuFreqData;

int CpuFreq_init(const Machine* machine, CpuFreqData* data);
void CpuFreq_update(CpuFreqData* data);
void CpuFreq_cleanup(CpuFreqData* data);
#endif

#endif
12 changes: 12 additions & 0 deletions darwin/DarwinMachine.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ void Machine_scan(Machine* super) {
DarwinMachine_allocateCPULoadInfo(&host->curr_load);
DarwinMachine_getVMStats(&host->vm_stats);
openzfs_sysctl_updateArcStats(&host->zfs);
#ifdef CPUFREQ_SUPPORT
CpuFreq_update(&host->cpu_freq);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this check for this->cpu_freq_ok if it's stored from init earlier?

#endif
}

Machine* Machine_new(UsersTable* usersTable, uid_t userId) {
Expand All @@ -97,6 +100,11 @@ Machine* Machine_new(UsersTable* usersTable, uid_t userId) {
openzfs_sysctl_init(&this->zfs);
openzfs_sysctl_updateArcStats(&this->zfs);

#ifdef CPUFREQ_SUPPORT
/* Initialize CPU frequency data */
this->cpu_freq_ok = CpuFreq_init(&this->super, &this->cpu_freq) == 0;
#endif

return super;
}

Expand All @@ -105,6 +113,10 @@ void Machine_delete(Machine* super) {

DarwinMachine_freeCPULoadInfo(&this->prev_load);

#ifdef CPUFREQ_SUPPORT
CpuFreq_cleanup(&this->cpu_freq);
#endif

Machine_done(super);
free(this);
}
Expand Down
6 changes: 5 additions & 1 deletion darwin/DarwinMachine.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ in the source distribution for its full text.

#include "Machine.h"
#include "zfs/ZfsArcStats.h"

#include "CpuFreq.h"

typedef struct DarwinMachine_ {
Machine super;
Expand All @@ -21,6 +21,10 @@ typedef struct DarwinMachine_ {
vm_statistics_data_t vm_stats;
processor_cpu_load_info_t prev_load;
processor_cpu_load_info_t curr_load;
#ifdef CPUFREQ_SUPPORT
CpuFreqData cpu_freq;
bool cpu_freq_ok;
Comment on lines +25 to +26
Copy link
Member

Choose a reason for hiding this comment

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

By refactoring CpuFreq_init to return CpuFreqData* on success and NULL on failure, the bool can be omitted.

#endif

ZfsArcStats zfs;
} DarwinMachine;
Expand Down
8 changes: 8 additions & 0 deletions darwin/Platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,15 @@ double Platform_setCPUValues(Meter* mtr, unsigned int cpu) {
/* Convert to percent and return */
total = mtr->values[CPU_METER_NICE] + mtr->values[CPU_METER_NORMAL] + mtr->values[CPU_METER_KERNEL];

#ifdef CPUFREQ_SUPPORT
if (dhost->cpu_freq_ok) {
mtr->values[CPU_METER_FREQUENCY] = dhost->cpu_freq.frequencies[cpu - 1];
} else {
mtr->values[CPU_METER_FREQUENCY] = NAN;
}
Comment on lines +283 to +287
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (dhost->cpu_freq_ok) {
mtr->values[CPU_METER_FREQUENCY] = dhost->cpu_freq.frequencies[cpu - 1];
} else {
mtr->values[CPU_METER_FREQUENCY] = NAN;
}
mtr->values[CPU_METER_FREQUENCY] = dhost->cpu_freq_ok ? dhost->cpu_freq.frequencies[cpu - 1] : NAN;

#else
mtr->values[CPU_METER_FREQUENCY] = NAN;
#endif
mtr->values[CPU_METER_TEMPERATURE] = NAN;

return CLAMP(total, 0.0, 100.0);
Expand Down