[<prev] [next>] [day] [month] [year] [list]
Message-ID: <94c3faee898b5436cc0b837c6778011a060b8468.1749480264.git.rohanlambture13@gmail.com>
Date: Mon, 9 Jun 2025 20:19:12 +0530
From: Rohan Lambture <rohanlambture13@...il.com>
To: trenn@...e.com,
shuah@...nel.org,
jwyatt@...hat.com,
jkacur@...hat.com
Cc: linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org,
Rohan Lambture <rohanlambture13@...il.com>
Subject: [PATCH] cpupower: Add cpu_start()/cpu_stop() callbacks for monitors
Move per-CPU logic from inside individual monitors to the main
monitoring framework by adding cpu_start() and cpu_stop() callback
functions to the cpuidle_monitor structure.
This refactoring allows the framework to handle per-CPU scheduling
and gives higher priority to fork_it operations as mentioned in
the TODO. Individual monitors now only need to implement per-CPU
initialization and cleanup logic without managing the CPU iteration
themselves.
Changes made:
- Add cpu_start()/cpu_stop() function pointers to cpuidle_monitor struct
- Update monitoring framework to call per-CPU callbacks for each CPU
- Refactor cpuidle_sysfs and mperf monitors to use new callback pattern
- Maintain backward compatibility for monitors without per-CPU callbacks
This addresses the TODO item: "Add cpu_start()/cpu_stop() callbacks
for monitor -> This is to move the per_cpu logic from inside the
monitor to outside it."
Signed-off-by: Rohan Lambture <rohanlambture13@...il.com>
---
.../utils/idle_monitor/cpuidle_sysfs.c | 44 ++++++-----
.../utils/idle_monitor/cpupower-monitor.c | 78 +++++++++++++++++--
.../utils/idle_monitor/cpupower-monitor.h | 2 +
.../utils/idle_monitor/mperf_monitor.c | 30 +++----
4 files changed, 114 insertions(+), 40 deletions(-)
diff --git a/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c b/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c
index 8b42c2f0a5b0..01b1de04e03b 100644
--- a/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c
+++ b/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c
@@ -43,35 +43,39 @@ static int cpuidle_get_count_percent(unsigned int id, double *percent,
static int cpuidle_start(void)
{
- int cpu, state;
clock_gettime(CLOCK_REALTIME, &start_time);
- for (cpu = 0; cpu < cpu_count; cpu++) {
- for (state = 0; state < cpuidle_sysfs_monitor.hw_states_num;
- state++) {
- previous_count[cpu][state] =
- cpuidle_state_time(cpu, state);
- dprint("CPU %d - State: %d - Val: %llu\n",
- cpu, state, previous_count[cpu][state]);
- }
- }
return 0;
}
static int cpuidle_stop(void)
{
- int cpu, state;
struct timespec end_time;
+
clock_gettime(CLOCK_REALTIME, &end_time);
timediff = timespec_diff_us(start_time, end_time);
+ return 0;
+}
- for (cpu = 0; cpu < cpu_count; cpu++) {
- for (state = 0; state < cpuidle_sysfs_monitor.hw_states_num;
- state++) {
- current_count[cpu][state] =
- cpuidle_state_time(cpu, state);
- dprint("CPU %d - State: %d - Val: %llu\n",
- cpu, state, previous_count[cpu][state]);
- }
+static int cpuidle_cpu_start(unsigned int cpu)
+{
+ int state;
+
+ for (state = 0; state < cpuidle_sysfs_monitor.hw_states_num; state++) {
+ previous_count[cpu][state] = cpuidle_state_time(cpu, state);
+ dprint("CPU %d - State: %d - Val: %llu\n",
+ cpu, state, previous_count[cpu][state]);
+ }
+ return 0;
+}
+
+static int cpuidle_cpu_stop(unsigned int cpu)
+{
+ int state;
+
+ for (state = 0; state < cpuidle_sysfs_monitor.hw_states_num; state++) {
+ current_count[cpu][state] = cpuidle_state_time(cpu, state);
+ dprint("CPU %d - State: %d - Val: %llu\n",
+ cpu, state, current_count[cpu][state]);
}
return 0;
}
@@ -205,6 +209,8 @@ struct cpuidle_monitor cpuidle_sysfs_monitor = {
.hw_states = cpuidle_cstates,
.start = cpuidle_start,
.stop = cpuidle_stop,
+ .cpu_start = cpuidle_cpu_start,
+ .cpu_stop = cpuidle_cpu_stop,
.do_register = cpuidle_register,
.unregister = cpuidle_unregister,
.flags.needs_root = 0,
diff --git a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
index ad493157f826..096e3cf35eb3 100644
--- a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
+++ b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
@@ -304,12 +304,29 @@ int fork_it(char **argv)
unsigned long long timediff;
pid_t child_pid;
struct timespec start, end;
+ int cpu;
child_pid = fork();
clock_gettime(CLOCK_REALTIME, &start);
- for (num = 0; num < avail_monitors; num++)
- monitors[num]->start();
+ /* Call global start callbacks first */
+ for (num = 0; num < avail_monitors; num++) {
+ if (monitors[num]->start)
+ monitors[num]->start();
+ }
+
+ /* Call per-CPU start callbacks */
+ for (num = 0; num < avail_monitors; num++) {
+ if (monitors[num]->cpu_start) {
+ for (cpu = 0; cpu < cpu_count; cpu++) {
+ if (monitors[num]->flags.per_cpu_schedule) {
+ if (bind_cpu(cpu))
+ continue;
+ }
+ monitors[num]->cpu_start(cpu);
+ }
+ }
+ }
if (!child_pid) {
/* child */
@@ -332,8 +349,25 @@ int fork_it(char **argv)
}
}
clock_gettime(CLOCK_REALTIME, &end);
- for (num = 0; num < avail_monitors; num++)
- monitors[num]->stop();
+
+ /* Call per-CPU stop callbacks */
+ for (num = 0; num < avail_monitors; num++) {
+ if (monitors[num]->cpu_stop) {
+ for (cpu = 0; cpu < cpu_count; cpu++) {
+ if (monitors[num]->flags.per_cpu_schedule) {
+ if (bind_cpu(cpu))
+ continue;
+ }
+ monitors[num]->cpu_stop(cpu);
+ }
+ }
+ }
+
+ /* Call global stop callbacks */
+ for (num = 0; num < avail_monitors; num++) {
+ if (monitors[num]->stop)
+ monitors[num]->stop();
+ }
timediff = timespec_diff_us(start, end);
if (WIFEXITED(status))
@@ -352,10 +386,25 @@ int do_interval_measure(int i)
for (cpu = 0; cpu < cpu_count; cpu++)
bind_cpu(cpu);
+ /* Call global start callbacks first */
for (num = 0; num < avail_monitors; num++) {
dprint("HW C-state residency monitor: %s - States: %d\n",
monitors[num]->name, monitors[num]->hw_states_num);
- monitors[num]->start();
+ if (monitors[num]->start)
+ monitors[num]->start();
+ }
+
+ /* Call per-CPU start callbacks */
+ for (num = 0; num < avail_monitors; num++) {
+ if (monitors[num]->cpu_start) {
+ for (cpu = 0; cpu < cpu_count; cpu++) {
+ if (monitors[num]->flags.per_cpu_schedule) {
+ if (bind_cpu(cpu))
+ continue;
+ }
+ monitors[num]->cpu_start(cpu);
+ }
+ }
}
sleep(i);
@@ -364,9 +413,24 @@ int do_interval_measure(int i)
for (cpu = 0; cpu < cpu_count; cpu++)
bind_cpu(cpu);
- for (num = 0; num < avail_monitors; num++)
- monitors[num]->stop();
+ /* Call per-CPU stop callbacks */
+ for (num = 0; num < avail_monitors; num++) {
+ if (monitors[num]->cpu_stop) {
+ for (cpu = 0; cpu < cpu_count; cpu++) {
+ if (monitors[num]->flags.per_cpu_schedule) {
+ if (bind_cpu(cpu))
+ continue;
+ }
+ monitors[num]->cpu_stop(cpu);
+ }
+ }
+ }
+ /* Call global stop callbacks */
+ for (num = 0; num < avail_monitors; num++) {
+ if (monitors[num]->stop)
+ monitors[num]->stop();
+ }
return 0;
}
diff --git a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h
index c559d3115330..830ad5ee68d6 100644
--- a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h
+++ b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h
@@ -57,6 +57,8 @@ struct cpuidle_monitor {
cstate_t *hw_states;
int (*start) (void);
int (*stop) (void);
+ int (*cpu_start) (unsigned int cpu);
+ int (*cpu_stop) (unsigned int cpu);
struct cpuidle_monitor* (*do_register) (void);
void (*unregister)(void);
unsigned int overflow_s;
diff --git a/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c b/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c
index 73b6b10cbdd2..6340f5d771b6 100644
--- a/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c
+++ b/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c
@@ -224,27 +224,27 @@ static int mperf_get_count_freq(unsigned int id, unsigned long long *count,
static int mperf_start(void)
{
- int cpu;
-
- for (cpu = 0; cpu < cpu_count; cpu++) {
- clock_gettime(CLOCK_REALTIME, &time_start[cpu]);
- mperf_get_tsc(&tsc_at_measure_start[cpu]);
- mperf_init_stats(cpu);
- }
-
return 0;
}
static int mperf_stop(void)
{
- int cpu;
+ return 0;
+}
- for (cpu = 0; cpu < cpu_count; cpu++) {
- mperf_measure_stats(cpu);
- mperf_get_tsc(&tsc_at_measure_end[cpu]);
- clock_gettime(CLOCK_REALTIME, &time_end[cpu]);
- }
+static int mperf_cpu_start(unsigned int cpu)
+{
+ clock_gettime(CLOCK_REALTIME, &time_start[cpu]);
+ mperf_get_tsc(&tsc_at_measure_start[cpu]);
+ mperf_init_stats(cpu);
+ return 0;
+}
+static int mperf_cpu_stop(unsigned int cpu)
+{
+ mperf_measure_stats(cpu);
+ mperf_get_tsc(&tsc_at_measure_end[cpu]);
+ clock_gettime(CLOCK_REALTIME, &time_end[cpu]);
return 0;
}
@@ -373,6 +373,8 @@ struct cpuidle_monitor mperf_monitor = {
.hw_states = mperf_cstates,
.start = mperf_start,
.stop = mperf_stop,
+ .cpu_start = mperf_cpu_start,
+ .cpu_stop = mperf_cpu_stop,
.do_register = mperf_register,
.unregister = mperf_unregister,
.flags.needs_root = 1,
--
2.49.0
Powered by blists - more mailing lists