Skip to contentSkip to conversation

cpupower: Add cpuidle library

PATCH In review Thomas Renninger 3 participants 7 messages Apr 25, 2016 – Apr 29, 2016 CPU power monitoring

lore
Share & tools
lore

Thomas Renninger

author
#0 lore raw

cover cpupower: Add cpuidle library

Hi,

this is a cleanup I wanted to do for a long time.

I already added the change to our build service and things still build against the same archs like arm, ppc, x86:
https://build.opensuse.org/package/show/hardware/cpupower

It would be nice to get the one or other review.

Rafael: If you could queue this up for mainline inclusion at some point of time, I would very much appreciate it!

Thanks,

Thomas

author
#1 lore raw

Re: [PATCH 1/1] cpupower: Add cpuidle parts into library

This more or less is a renaming and moving of functions and should not introduce any functional change.

cpupower was built from cpufrequtils (which had a C library providing easy access to cpu frequency platform info). In the meantime it got enhanced by quite some neat cpuidle userspace tools.

Now the cpu idle functions have been separated and added to the cpupower.so library.
So beside an already existing public header file:
cpufreq.h
cpupower now also exports these cpu idle functions in:
cpuidle.h

Here again pasted for better review of the interfaces:

======================================
int cpuidle_is_state_disabled(unsigned int cpu,

                                       unsigned int idlestate);
int cpuidle_state_disable(unsigned int cpu, unsigned int idlestate,
                                   unsigned int disable);
unsigned long cpuidle_state_latency(unsigned int cpu,
                                                unsigned int idlestate);
unsigned long cpuidle_state_usage(unsigned int cpu,
                                        unsigned int idlestate);
unsigned long long cpuidle_state_time(unsigned int cpu,
                                                unsigned int idlestate);
char *cpuidle_state_name(unsigned int cpu,
                                unsigned int idlestate);
char *cpuidle_state_desc(unsigned int cpu,
                                unsigned int idlestate);

unsigned int cpuidle_state_count(unsigned int cpu);

char *cpuidle_get_governor(void);
char *cpuidle_get_driver(void);

======================================

Signed-off-by Thomas Renninger <[email protected]>
tools/power/cpupower/Makefile +7 -5
index 2e2ba2e..1ca525d 100644
@@ -58,7 +58,7 @@ DESTDIR ?=
 # and _should_ modify the PACKAGE_BUGREPORT definition
 
 VERSION=			$(shell ./utils/version-gen.sh)
-LIB_MAJ=			0.0.0
+LIB_MAJ=			0.0.1
 LIB_MIN=			0
 
 PACKAGE =			cpupower

@@ -124,7 +124,7 @@ WARNINGS += -Wshadow
 CFLAGS += -DVERSION=\"$(VERSION)\" -DPACKAGE=\"$(PACKAGE)\" \
 		-DPACKAGE_BUGREPORT=\"$(PACKAGE_BUGREPORT)\" -D_GNU_SOURCE
 
-UTIL_OBJS =  utils/helpers/amd.o utils/helpers/topology.o utils/helpers/msr.o \
+UTIL_OBJS =  utils/helpers/amd.o utils/helpers/msr.o \
 	utils/helpers/sysfs.o utils/helpers/misc.o utils/helpers/cpuid.o \
 	utils/helpers/pci.o utils/helpers/bitmask.o \
 	utils/idle_monitor/nhm_idle.o utils/idle_monitor/snb_idle.o \

@@ -143,9 +143,9 @@ UTIL_HEADERS = utils/helpers/helpers.h utils/idle_monitor/cpupower-monitor.h \
 	utils/helpers/bitmask.h \
 	utils/idle_monitor/idle_monitors.h utils/idle_monitor/idle_monitors.def
 
-LIB_HEADERS = 	lib/cpufreq.h lib/sysfs.h
-LIB_SRC = 	lib/cpufreq.c lib/sysfs.c
-LIB_OBJS = 	lib/cpufreq.o lib/sysfs.o
+LIB_HEADERS = 	lib/cpufreq.h lib/cpupower.h lib/cpuidle.h
+LIB_SRC = 	lib/cpufreq.c lib/cpupower.c lib/cpuidle.c
+LIB_OBJS = 	lib/cpufreq.o lib/cpupower.o lib/cpuidle.o
 LIB_OBJS :=	$(addprefix $(OUTPUT),$(LIB_OBJS))
 
 CFLAGS +=	-pipe

@@ -265,6 +265,7 @@ install-lib:
 	$(CP) $(OUTPUT)libcpupower.so* $(DESTDIR)${libdir}/
 	$(INSTALL) -d $(DESTDIR)${includedir}
 	$(INSTALL_DATA) lib/cpufreq.h $(DESTDIR)${includedir}/cpufreq.h
+	$(INSTALL_DATA) lib/cpuidle.h $(DESTDIR)${includedir}/cpuidle.h
 
 install-tools:
 	$(INSTALL) -d $(DESTDIR)${bindir}

@@ -296,6 +297,7 @@ install: all install-lib install-tools install-man $(INSTALL_NLS) $(INSTALL_BENC
 uninstall:
 	- rm -f $(DESTDIR)${libdir}/libcpupower.*
 	- rm -f $(DESTDIR)${includedir}/cpufreq.h
+	- rm -f $(DESTDIR)${includedir}/cpuidle.h
 	- rm -f $(DESTDIR)${bindir}/utils/cpupower
 	- rm -f $(DESTDIR)${mandir}/man1/cpupower.1
 	- rm -f $(DESTDIR)${mandir}/man1/cpupower-frequency-set.1
tools/power/cpupower/bench/system.c +2 -1
index f01e3f4..c25a74a 100644
@@ -26,6 +26,7 @@
 #include <sched.h>
 
 #include <cpufreq.h>
+#include <cpupower.h>
 
 #include "config.h"
 #include "system.h"

@@ -60,7 +61,7 @@ int set_cpufreq_governor(char *governor, unsigned int cpu)
 
 	dprintf("set %s as cpufreq governor\n", governor);
 
-	if (cpufreq_cpu_exists(cpu) != 0) {
+	if (cpupower_is_cpu_online(cpu) != 0) {
 		perror("cpufreq_cpu_exists");
 		fprintf(stderr, "error: cpu %u does not exist\n", cpu);
 		return -1;
tools/power/cpupower/lib/cpufreq.c +529 -21

657 lines not on this page · raw message

tools/power/cpupower/lib/cpufreq.h +26 -33

163 lines not on this page · raw message

tools/power/cpupower/lib/cpuidle.c new file +380 -0

380 lines not on this page · raw message

tools/power/cpupower/lib/cpuidle.h new file +23 -0
new file mode 100644
index 0000000..04eb3cf
@@ -0,0 +1,23 @@
+#ifndef __CPUPOWER_CPUIDLE_H__
+#define __CPUPOWER_CPUIDLE_H__
+
+int cpuidle_is_state_disabled(unsigned int cpu,
+				       unsigned int idlestate);
+int cpuidle_state_disable(unsigned int cpu, unsigned int idlestate,
+				   unsigned int disable);
+unsigned long cpuidle_state_latency(unsigned int cpu,
+						unsigned int idlestate);
+unsigned long cpuidle_state_usage(unsigned int cpu,
+					unsigned int idlestate);
+unsigned long long cpuidle_state_time(unsigned int cpu,
+						unsigned int idlestate);
+char *cpuidle_state_name(unsigned int cpu,
+				unsigned int idlestate);
+char *cpuidle_state_desc(unsigned int cpu,
+				unsigned int idlestate);
+unsigned int cpuidle_state_count(unsigned int cpu);
+
+char *cpuidle_get_governor(void);
+char *cpuidle_get_driver(void);
+
+#endif /* __CPUPOWER_HELPERS_SYSFS_H__ */
tools/power/cpupower/lib/cpupower.c new file +192 -0

192 lines not on this page · raw message

tools/power/cpupower/lib/cpupower.h new file +35 -0
new file mode 100644
index 0000000..fa031fc
@@ -0,0 +1,35 @@
+#ifndef __CPUPOWER_CPUPOWER_H__
+#define __CPUPOWER_CPUPOWER_H__
+
+struct cpupower_topology {
+	/* Amount of CPU cores, packages and threads per core in the system */
+	unsigned int cores;
+	unsigned int pkgs;
+	unsigned int threads; /* per core */
+
+	/* Array gets mallocated with cores entries, holding per core info */
+	struct cpuid_core_info *core_info;
+};
+
+struct cpuid_core_info {
+	int pkg;
+	int core;
+	int cpu;
+
+	/* flags */
+	unsigned int is_online:1;
+};
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+int get_cpu_topology(struct cpupower_topology *cpu_top);
+void cpu_topology_release(struct cpupower_topology cpu_top);
+int cpupower_is_cpu_online(unsigned int cpu);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
tools/power/cpupower/lib/cpupower_intern.h new file +5 -0
new file mode 100644
index 0000000..f8ec400
@@ -0,0 +1,5 @@
+#define PATH_TO_CPU "/sys/devices/system/cpu/"
+#define MAX_LINE_LEN 4096
+#define SYSFS_PATH_MAX 255
+
+unsigned int sysfs_read_file(const char *path, char *buf, size_t buflen);
tools/power/cpupower/lib/sysfs.c deleted +0 -672

672 lines not on this page · raw message

tools/power/cpupower/lib/sysfs.h deleted +0 -31
deleted file mode 100644
index c76a5e0..0000000
@@ -1,31 +0,0 @@
-/* General */
-extern unsigned int sysfs_cpu_exists(unsigned int cpu);
-
-/* CPUfreq */
-extern unsigned long sysfs_get_freq_kernel(unsigned int cpu);
-extern unsigned long sysfs_get_freq_hardware(unsigned int cpu);
-extern unsigned long sysfs_get_freq_transition_latency(unsigned int cpu);
-extern int sysfs_get_freq_hardware_limits(unsigned int cpu,
-					unsigned long *min, unsigned long *max);
-extern char *sysfs_get_freq_driver(unsigned int cpu);
-extern struct cpufreq_policy *sysfs_get_freq_policy(unsigned int cpu);
-extern struct cpufreq_available_governors *sysfs_get_freq_available_governors(
-	unsigned int cpu);
-extern struct cpufreq_available_frequencies *sysfs_get_available_frequencies(
-	unsigned int cpu);
-extern struct cpufreq_affected_cpus *sysfs_get_freq_affected_cpus(
-	unsigned int cpu);
-extern struct cpufreq_affected_cpus *sysfs_get_freq_related_cpus(
-	unsigned int cpu);
-extern struct cpufreq_stats *sysfs_get_freq_stats(unsigned int cpu,
-						unsigned long long *total_time);
-extern unsigned long sysfs_get_freq_transitions(unsigned int cpu);
-extern int sysfs_set_freq_policy(unsigned int cpu,
-				struct cpufreq_policy *policy);
-extern int sysfs_modify_freq_policy_min(unsigned int cpu,
-					unsigned long min_freq);
-extern int sysfs_modify_freq_policy_max(unsigned int cpu,
-					unsigned long max_freq);
-extern int sysfs_modify_freq_policy_governor(unsigned int cpu, char *governor);
-extern int sysfs_set_frequency(unsigned int cpu,
-			unsigned long target_frequency);
tools/power/cpupower/utils/cpufreq-info.c +2 -2
index 0e67643..e9679f4 100644
@@ -256,7 +256,7 @@ static void debug_output_one(unsigned int cpu)
 	struct cpufreq_available_governors *governors;
 	struct cpufreq_stats *stats;
 
-	if (cpufreq_cpu_exists(cpu))
+	if (cpupower_is_cpu_online(cpu))
 		return;
 
 	freq_kernel = cpufreq_get_freq_kernel(cpu);

@@ -647,7 +647,7 @@ int cmd_freq_info(int argc, char **argv)
 
 		if (!bitmask_isbitset(cpus_chosen, cpu))
 			continue;
-		if (cpufreq_cpu_exists(cpu)) {
+		if (cpupower_is_cpu_online(cpu)) {
 			printf(_("couldn't analyze CPU %d as it doesn't seem to be present\n"), cpu);
 			continue;
 		}
tools/power/cpupower/utils/cpufreq-set.c +4 -4
index 0fbd1a2..b4bf769 100644
@@ -16,8 +16,8 @@
 #include <getopt.h>
 
 #include "cpufreq.h"
+#include "cpuidle.h"
 #include "helpers/helpers.h"
-#include "helpers/sysfs.h"
 
 #define NORM_FREQ_LEN 32
 

@@ -296,7 +296,7 @@ int cmd_freq_set(int argc, char **argv)
 			struct cpufreq_affected_cpus *cpus;
 
 			if (!bitmask_isbitset(cpus_chosen, cpu) ||
-			    cpufreq_cpu_exists(cpu))
+			    cpupower_is_cpu_online(cpu))
 				continue;
 
 			cpus = cpufreq_get_related_cpus(cpu);

@@ -316,10 +316,10 @@ int cmd_freq_set(int argc, char **argv)
 	     cpu <= bitmask_last(cpus_chosen); cpu++) {
 
 		if (!bitmask_isbitset(cpus_chosen, cpu) ||
-		    cpufreq_cpu_exists(cpu))
+		    cpupower_is_cpu_online(cpu))
 			continue;
 
-		if (sysfs_is_cpu_online(cpu) != 1)
+		if (cpupower_is_cpu_online(cpu) != 1)
 			continue;
 
 		printf(_("Setting cpu: %d\n"), cpu);
tools/power/cpupower/utils/cpuidle-info.c +16 -16
index 750c1d8..8473ec4 100644
@@ -13,9 +13,9 @@
 #include <string.h>
 #include <getopt.h>
 #include <cpufreq.h>
+#include <cpuidle.h>
 
 #include "helpers/helpers.h"
-#include "helpers/sysfs.h"
 #include "helpers/bitmask.h"
 
 #define LINE_LEN 10

@@ -27,7 +27,7 @@ static void cpuidle_cpu_output(unsigned int cpu, int verbose)
 
 	printf(_ ("Analyzing CPU %d:\n"), cpu);
 
-	idlestates = sysfs_get_idlestate_count(cpu);
+	idlestates = cpuidle_state_count(cpu);
 	if (idlestates == 0) {
 		printf(_("CPU %u: No idle states\n"), cpu);
 		return;

@@ -36,7 +36,7 @@ static void cpuidle_cpu_output(unsigned int cpu, int verbose)
 	printf(_("Number of idle states: %d\n"), idlestates);
 	printf(_("Available idle states:"));
 	for (idlestate = 0; idlestate < idlestates; idlestate++) {
-		tmp = sysfs_get_idlestate_name(cpu, idlestate);
+		tmp = cpuidle_state_name(cpu, idlestate);
 		if (!tmp)
 			continue;
 		printf(" %s", tmp);

@@ -48,28 +48,28 @@ static void cpuidle_cpu_output(unsigned int cpu, int verbose)
 		return;
 
 	for (idlestate = 0; idlestate < idlestates; idlestate++) {
-		int disabled = sysfs_is_idlestate_disabled(cpu, idlestate);
+		int disabled = cpuidle_is_state_disabled(cpu, idlestate);
 		/* Disabled interface not supported on older kernels */
 		if (disabled < 0)
 			disabled = 0;
-		tmp = sysfs_get_idlestate_name(cpu, idlestate);
+		tmp = cpuidle_state_name(cpu, idlestate);
 		if (!tmp)
 			continue;
 		printf("%s%s:\n", tmp, (disabled) ? " (DISABLED) " : "");
 		free(tmp);
 
-		tmp = sysfs_get_idlestate_desc(cpu, idlestate);
+		tmp = cpuidle_state_desc(cpu, idlestate);
 		if (!tmp)
 			continue;
 		printf(_("Flags/Description: %s\n"), tmp);
 		free(tmp);
 
 		printf(_("Latency: %lu\n"),
-		       sysfs_get_idlestate_latency(cpu, idlestate));
+		       cpuidle_state_latency(cpu, idlestate));
 		printf(_("Usage: %lu\n"),
-		       sysfs_get_idlestate_usage(cpu, idlestate));
+		       cpuidle_state_usage(cpu, idlestate));
 		printf(_("Duration: %llu\n"),
-		       sysfs_get_idlestate_time(cpu, idlestate));
+		       cpuidle_state_time(cpu, idlestate));
 	}
 	printf("\n");
 }

@@ -78,7 +78,7 @@ static void cpuidle_general_output(void)
 {
 	char *tmp;
 
-	tmp = sysfs_get_cpuidle_driver();
+	tmp = cpuidle_get_driver();
 	if (!tmp) {
 		printf(_("Could not determine cpuidle driver\n"));
 		return;

@@ -87,7 +87,7 @@ static void cpuidle_general_output(void)
 	printf(_("CPUidle driver: %s\n"), tmp);
 	free(tmp);
 
-	tmp = sysfs_get_cpuidle_governor();
+	tmp = cpuidle_get_governor();
 	if (!tmp) {
 		printf(_("Could not determine cpuidle governor\n"));
 		return;

@@ -102,7 +102,7 @@ static void proc_cpuidle_cpu_output(unsigned int cpu)
 	long max_allowed_cstate = 2000000000;
 	unsigned int cstate, cstates;
 
-	cstates = sysfs_get_idlestate_count(cpu);
+	cstates = cpuidle_state_count(cpu);
 	if (cstates == 0) {
 		printf(_("CPU %u: No C-states info\n"), cpu);
 		return;

@@ -117,11 +117,11 @@ static void proc_cpuidle_cpu_output(unsigned int cpu)
 			 "type[C%d] "), cstate, cstate);
 		printf(_("promotion[--] demotion[--] "));
 		printf(_("latency[%03lu] "),
-		       sysfs_get_idlestate_latency(cpu, cstate));
+		       cpuidle_state_latency(cpu, cstate));
 		printf(_("usage[%08lu] "),
-		       sysfs_get_idlestate_usage(cpu, cstate));
+		       cpuidle_state_usage(cpu, cstate));
 		printf(_("duration[%020Lu] \n"),
-		       sysfs_get_idlestate_time(cpu, cstate));
+		       cpuidle_state_time(cpu, cstate));
 	}
 }
 

@@ -190,7 +190,7 @@ int cmd_idle_info(int argc, char **argv)
 	     cpu <= bitmask_last(cpus_chosen); cpu++) {
 
 		if (!bitmask_isbitset(cpus_chosen, cpu) ||
-		    cpufreq_cpu_exists(cpu))
+		    cpupower_is_cpu_online(cpu))
 			continue;
 
 		switch (output_param) {
tools/power/cpupower/utils/cpuidle-set.c +13 -13
index d6b6ae4..691c24d 100644
@@ -5,12 +5,12 @@
 #include <limits.h>
 #include <string.h>
 #include <ctype.h>
-
 #include <getopt.h>
 
-#include "cpufreq.h"
+#include <cpufreq.h>
+#include <cpuidle.h>
+
 #include "helpers/helpers.h"
-#include "helpers/sysfs.h"
 
 static struct option info_opts[] = {
      {"disable",	required_argument,		NULL, 'd'},

@@ -104,16 +104,16 @@ int cmd_idle_set(int argc, char **argv)
 		if (!bitmask_isbitset(cpus_chosen, cpu))
 			continue;
 
-		if (sysfs_is_cpu_online(cpu) != 1)
+		if (cpupower_is_cpu_online(cpu) != 1)
 			continue;
 
-		idlestates = sysfs_get_idlestate_count(cpu);
+		idlestates = cpuidle_state_count(cpu);
 		if (idlestates <= 0)
 			continue;
 
 		switch (param) {
 		case 'd':
-			ret = sysfs_idlestate_disable(cpu, idlestate, 1);
+			ret = cpuidle_state_disable(cpu, idlestate, 1);
 			if (ret == 0)
 		printf(_("Idlestate %u disabled on CPU %u\n"),  idlestate, cpu);
 			else if (ret == -1)

@@ -126,7 +126,7 @@ int cmd_idle_set(int argc, char **argv)
 		       idlestate, cpu);
 			break;
 		case 'e':
-			ret = sysfs_idlestate_disable(cpu, idlestate, 0);
+			ret = cpuidle_state_disable(cpu, idlestate, 0);
 			if (ret == 0)
 		printf(_("Idlestate %u enabled on CPU %u\n"),  idlestate, cpu);
 			else if (ret == -1)

@@ -140,13 +140,13 @@ int cmd_idle_set(int argc, char **argv)
 			break;
 		case 'D':
 			for (idlestate = 0; idlestate < idlestates; idlestate++) {
-				disabled = sysfs_is_idlestate_disabled
+				disabled = cpuidle_is_state_disabled
 					(cpu, idlestate);
-				state_latency = sysfs_get_idlestate_latency
+				state_latency = cpuidle_state_latency
 					(cpu, idlestate);
 				if (disabled == 1) {
 					if (latency > state_latency){
-						ret = sysfs_idlestate_disable
+						ret = cpuidle_state_disable
 							(cpu, idlestate, 0);
 						if (ret == 0)
 		printf(_("Idlestate %u enabled on CPU %u\n"),  idlestate, cpu);

@@ -154,7 +154,7 @@ int cmd_idle_set(int argc, char **argv)
 					continue;
 				}
 				if (latency <= state_latency){
-					ret = sysfs_idlestate_disable
+					ret = cpuidle_state_disable
 						(cpu, idlestate, 1);
 					if (ret == 0)
 		printf(_("Idlestate %u disabled on CPU %u\n"), idlestate, cpu);

@@ -163,10 +163,10 @@ int cmd_idle_set(int argc, char **argv)
 			break;
 		case 'E':
 			for (idlestate = 0; idlestate < idlestates; idlestate++) {
-				disabled = sysfs_is_idlestate_disabled
+				disabled = cpuidle_is_state_disabled
 					(cpu, idlestate);
 				if (disabled == 1) {
-					ret = sysfs_idlestate_disable
+					ret = cpuidle_state_disable
 						(cpu, idlestate, 0);
 					if (ret == 0)
 		printf(_("Idlestate %u enabled on CPU %u\n"), idlestate, cpu);
tools/power/cpupower/utils/cpupower-info.c +3 -2
index 10299f2..0a02485 100644
@@ -13,8 +13,9 @@
 #include <getopt.h>
 
 #include <cpufreq.h>
+#include <cpuidle.h>
+
 #include "helpers/helpers.h"
-#include "helpers/sysfs.h"
 
 static struct option set_opts[] = {
      {"perf-bias", optional_argument, NULL, 'b'},

@@ -84,7 +85,7 @@ int cmd_info(int argc, char **argv)
 	     cpu <= bitmask_last(cpus_chosen); cpu++) {
 
 		if (!bitmask_isbitset(cpus_chosen, cpu) ||
-		    cpufreq_cpu_exists(cpu))
+		    cpupower_is_cpu_online(cpu))
 			continue;
 
 		printf(_("analyzing CPU %d:\n"), cpu);
tools/power/cpupower/utils/cpupower-set.c +3 -3
index 3e6f374..cf24c09 100644
@@ -12,9 +12,9 @@
 #include <string.h>
 #include <getopt.h>
 
-#include <cpufreq.h>
+#include <cpuidle.h>
+
 #include "helpers/helpers.h"
-#include "helpers/sysfs.h"
 #include "helpers/bitmask.h"
 
 static struct option set_opts[] = {

@@ -79,7 +79,7 @@ int cmd_set(int argc, char **argv)
 	     cpu <= bitmask_last(cpus_chosen); cpu++) {
 
 		if (!bitmask_isbitset(cpus_chosen, cpu) ||
-		    cpufreq_cpu_exists(cpu))
+		    cpupower_is_cpu_online(cpu))
 			continue;
 
 		if (params.perf_bias) {
tools/power/cpupower/utils/helpers/helpers.h +1 -25
index aa9e954..afb66f8 100644
@@ -14,6 +14,7 @@
 #include <locale.h>
 
 #include "helpers/bitmask.h"
+#include <cpupower.h>
 
 /* Internationalization ****************************/
 #ifdef NLS

@@ -92,31 +93,6 @@ extern int get_cpu_info(unsigned int cpu, struct cpupower_cpu_info *cpu_info);
 extern struct cpupower_cpu_info cpupower_cpu_info;
 /* cpuid and cpuinfo helpers  **************************/
 
-struct cpuid_core_info {
-	int pkg;
-	int core;
-	int cpu;
-
-	/* flags */
-	unsigned int is_online:1;
-};
-
-/* CPU topology/hierarchy parsing ******************/
-struct cpupower_topology {
-	/* Amount of CPU cores, packages and threads per core in the system */
-	unsigned int cores;
-	unsigned int pkgs;
-	unsigned int threads; /* per core */
-
-	/* Array gets mallocated with cores entries, holding per core info */
-	struct cpuid_core_info *core_info;
-};
-
-extern int get_cpu_topology(struct cpupower_topology *cpu_top);
-extern void cpu_topology_release(struct cpupower_topology cpu_top);
-
-/* CPU topology/hierarchy parsing ******************/
-
 /* X86 ONLY ****************************************/
 #if defined(__i386__) || defined(__x86_64__)
tools/power/cpupower/utils/helpers/topology.c +2 -105

112 lines not on this page · raw message

tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c +6 -6
index bcd22a1..1b5da00 100644
@@ -10,8 +10,8 @@
 #include <stdint.h>
 #include <string.h>
 #include <limits.h>
+#include <cpuidle.h>
 
-#include "helpers/sysfs.h"
 #include "helpers/helpers.h"
 #include "idle_monitor/cpupower-monitor.h"
 

@@ -51,7 +51,7 @@ static int cpuidle_start(void)
 		for (state = 0; state < cpuidle_sysfs_monitor.hw_states_num;
 		     state++) {
 			previous_count[cpu][state] =
-				sysfs_get_idlestate_time(cpu, state);
+				cpuidle_state_time(cpu, state);
 			dprint("CPU %d - State: %d - Val: %llu\n",
 			       cpu, state, previous_count[cpu][state]);
 		}

@@ -70,7 +70,7 @@ static int cpuidle_stop(void)
 		for (state = 0; state < cpuidle_sysfs_monitor.hw_states_num;
 		     state++) {
 			current_count[cpu][state] =
-				sysfs_get_idlestate_time(cpu, state);
+				cpuidle_state_time(cpu, state);
 			dprint("CPU %d - State: %d - Val: %llu\n",
 			       cpu, state, previous_count[cpu][state]);
 		}

@@ -132,13 +132,13 @@ static struct cpuidle_monitor *cpuidle_register(void)
 	char *tmp;
 
 	/* Assume idle state count is the same for all CPUs */
-	cpuidle_sysfs_monitor.hw_states_num = sysfs_get_idlestate_count(0);
+	cpuidle_sysfs_monitor.hw_states_num = cpuidle_state_count(0);
 
 	if (cpuidle_sysfs_monitor.hw_states_num <= 0)
 		return NULL;
 
 	for (num = 0; num < cpuidle_sysfs_monitor.hw_states_num; num++) {
-		tmp = sysfs_get_idlestate_name(0, num);
+		tmp = cpuidle_state_name(0, num);
 		if (tmp == NULL)
 			continue;
 

@@ -146,7 +146,7 @@ static struct cpuidle_monitor *cpuidle_register(void)
 		strncpy(cpuidle_cstates[num].name, tmp, CSTATE_NAME_LEN - 1);
 		free(tmp);
 
-		tmp = sysfs_get_idlestate_desc(0, num);
+		tmp = cpuidle_state_desc(0, num);
 		if (tmp == NULL)
 			continue;
 		strncpy(cpuidle_cstates[num].desc, tmp,	CSTATE_DESC_LEN - 1);
#2 lore raw

Re: [PATCH 1/1] cpupower: Add cpuidle parts into library

Quoted 33 lines · Thomas Renninger

This more or less is a renaming and moving of functions and should not introduce any functional change.

cpupower was built from cpufrequtils (which had a C library providing easy access to cpu frequency platform info). In the meantime it got enhanced by quite some neat cpuidle userspace tools.

Now the cpu idle functions have been separated and added to the cpupower.so library.
So beside an already existing public header file:
cpufreq.h
cpupower now also exports these cpu idle functions in:
cpuidle.h

Here again pasted for better review of the interfaces:

======================================
int cpuidle_is_state_disabled(unsigned int cpu,

                                       unsigned int idlestate);
int cpuidle_state_disable(unsigned int cpu, unsigned int idlestate,
                                   unsigned int disable);
unsigned long cpuidle_state_latency(unsigned int cpu,
                                                unsigned int idlestate);
unsigned long cpuidle_state_usage(unsigned int cpu,
                                        unsigned int idlestate);
unsigned long long cpuidle_state_time(unsigned int cpu,
                                                unsigned int idlestate);
char *cpuidle_state_name(unsigned int cpu,
                                unsigned int idlestate);
char *cpuidle_state_desc(unsigned int cpu,
                                unsigned int idlestate);

unsigned int cpuidle_state_count(unsigned int cpu);

Quoted 7 lines

char *cpuidle_get_governor(void);
char *cpuidle_get_driver(void);

======================================

Signed-off-by Thomas Renninger <[email protected]>

It doesn't apply on top of 4.6-rc5 for me. Can you please rebase on top of the current Linus' tree?

Thanks,
Rafael

#3 lore raw
In reply to Thomas Renninger
Quoted 4 lines · Thomas Renninger

Hi,

this is a cleanup I wanted to do for a long time.

Quoted 8 lines

I already added the change to our build service and things still build against the same archs like arm, ppc, x86:
https://build.opensuse.org/package/show/hardware/cpupower

It would be nice to get the one or other review.

Rafael: If you could queue this up for mainline inclusion at some point of time, I would very much appreciate it!

I can do it right now, but there are a few cpupower patches in Patchwork that I'm not sure what to do with.

Do you want me to drop those on the floor?

Thanks,
Rafael

author
#4 lore raw
Quoted 9 lines · Rafael J. Wysocki
Quoted 8 lines · Thomas Renninger

Hi,

this is a cleanup I wanted to do for a long time.

I already added the change to our build service and things still build against the same archs like arm, ppc, x86:
https://build.opensuse.org/package/show/hardware/cpupower

Quoted 7 lines
Quoted 4 lines

It would be nice to get the one or other review.

Rafael: If you could queue this up for mainline inclusion at some point of time, I would very much appreciate it!

I can do it right now, but there are a few cpupower patches in Patchwork that I'm not sure what to do with.

Where do I find the patchwork stuff?

Quoted 1 line

Do you want me to drop those on the floor?

No, please not.

I went through the list and found some patches. Not critical, but valid fixes.
The one from Mattia already was in, in another form.

I will send them right now.

This should now more or less match with what patchwork gives you?

Thanks,

Thomas

#5 lore raw
Quoted 24 lines · Thomas Renninger
Quoted 9 lines · Rafael J. Wysocki
Quoted 8 lines · Thomas Renninger

Hi,

this is a cleanup I wanted to do for a long time.

I already added the change to our build service and things still build against the same archs like arm, ppc, x86:
https://build.opensuse.org/package/show/hardware/cpupower

Quoted 7 lines
Quoted 4 lines

It would be nice to get the one or other review.

Rafael: If you could queue this up for mainline inclusion at some point of time, I would very much appreciate it!

I can do it right now, but there are a few cpupower patches in Patchwork that I'm not sure what to do with.

Where do I find the patchwork stuff?

Quoted 1 line

Do you want me to drop those on the floor?

No, please not.

Quoted 8 lines

I went through the list and found some patches. Not critical, but valid fixes.
The one from Mattia already was in, in another form.

I will send them right now.

This should now more or less match with what patchwork gives you?

That's about right. There is one more patch in Patchwork that is not present in your series:

https://patchwork.kernel.org/patch/8301391/

I've queued up your series for 4.7, but I'm still unsure about the above one.

Quoted 8 lines

Thanks,

Thomas
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#6 lore raw
Quoted 2 lines · Rafael J. Wysocki
Quoted 1 line · Thomas Renninger

On Tuesday, April 26, 2

...

Quoted 7 lines
Quoted 2 lines

This should now more or less match with what patchwork gives you?

That's about right. There is one more patch in Patchwork that is not present in your series:

https://patchwork.kernel.org/patch/8301391/

Thanks, I have overseen this one.

Quoted 2 lines

I've queued up your series for 4.7, but I'm still unsure about the above one.

Ah, I also wanted to fix this and then decided it's not worth it. But now that we have a patch (compile tested from my side, looks correct).

This fixes cppcheck error:

Checking bench/parse.c...
[bench/parse.c:91]: (error) Common realloc mistake: 'filename' nulled but not freed upon failure

I had to merge this one on top of the others, I'll send the adjusted version in a minute.

Thanks!

Thomas