-
-
Notifications
You must be signed in to change notification settings - Fork 442
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
|
@@ -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 | ||
|
||
# ---------------------------------------------------------------------- | ||
|
||
# ---------------------------------------------------------------------- | ||
# Checks for compiler warnings. | ||
# ---------------------------------------------------------------------- | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we just use the predefined macro |
||
AC_SUBST(my_htop_platform) | ||
AC_CONFIG_FILES([Makefile htop.1]) | ||
AC_OUTPUT | ||
|
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you are just returning success, prefer |
||||||||||||||||||||||||||||||||
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]; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer Also, should be |
||||||||||||||||||||||||||||||||
io_registry_entry_t cpu_io_registry_entry = IORegistryEntryFromPath(kIOMainPortDefault, buf); | ||||||||||||||||||||||||||||||||
if (cpu_io_registry_entry == MACH_PORT_NULL) { | ||||||||||||||||||||||||||||||||
return -1; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This leaks memory … There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error handling is one of the rare cases where |
||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
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; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is that 8 coming from? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that the
Suggested change
|
||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
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) { | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
would be also fine … |
||||||||||||||||||||||||||||||||
return -1; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||
return -1; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Memory leak. |
||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
data->prev_samples = NULL; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If no |
||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
if (data->prev_samples == NULL) { | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||
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) { | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Normally you would even drop the NULL check, but given that There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this missing a follow-up call of |
||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
#endif |
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)) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some of it is needed in the |
||||||
|
||||||
/* Definitions */ | ||||||
typedef struct { | ||||||
uint32_t num_frequencies; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should include |
||||||
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; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Also include There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer |
||||||
|
||||||
/* 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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this check for |
||
#endif | ||
} | ||
|
||
Machine* Machine_new(UsersTable* usersTable, uid_t userId) { | ||
|
@@ -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; | ||
} | ||
|
||
|
@@ -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); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By refactoring |
||
#endif | ||
|
||
ZfsArcStats zfs; | ||
} DarwinMachine; | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
#else | ||||||||||||||
mtr->values[CPU_METER_FREQUENCY] = NAN; | ||||||||||||||
#endif | ||||||||||||||
mtr->values[CPU_METER_TEMPERATURE] = NAN; | ||||||||||||||
|
||||||||||||||
return CLAMP(total, 0.0, 100.0); | ||||||||||||||
|
There was a problem hiding this comment.
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.