[Powertop] [PATCH] factor out powertop_init() function [was "Segmentation fault occurs with --calibrate"]

[Powertop] [PATCH] factor out powertop_init() function [was "Segmentation fault occurs with --calibrate"]

[Sergey Senozhatsky]

[-- Attachment #1: Type: text/plain, Size: 5002 bytes --]

[PATCH] factor out powertop_init() function

Factor out powertop_init() function to perform basic initialization on
powertop start up.

Commit 057d6126eb6329c86b29a2e0219c0d0e49a84191 moved init after
commandline options parsing. However, in some cases we need it before
parsing, e.g. -- calibration.

Reported-by: Lekensteyn <lekensteyn(a)gmail.com>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>

---

 src/main.cpp |  110 +++++++++++++++++++++++++++++++---------------------------
 1 file changed, 58 insertions(+), 52 deletions(-)

diff --git a/src/main.cpp b/src/main.cpp
index 433fea6..b5720f0 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -280,15 +280,68 @@ static void checkroot() {
 	}
 	
 }
-int main(int argc, char **argv)
+
+static void powertop_init(void)
 {
+	static char initialized = 0;
 	int ret;
+	struct statfs st_fs;
+
+	if (initialized)
+		return;
+
+	checkroot(); 
+	ret = system("/sbin/modprobe cpufreq_stats > /dev/null 2>&1");
+	ret = system("/sbin/modprobe msr > /dev/null 2>&1");
+	statfs("/sys/kernel/debug", &st_fs);
+
+	if (st_fs.f_type != (long) DEBUGFS_MAGIC) {
+		if (access("/bin/mount", X_OK) == 0) {
+			ret = system("/bin/mount -t debugfs debugfs /sys/kernel/debug > /dev/null 2>&1");
+		} else {
+			ret = system("mount -t debugfs debugfs /sys/kernel/debug > /dev/null 2>&1");
+		}
+		if (ret != 0) {
+			printf(_("Failed to mount debugfs!\n"));
+			printf(_("exiting...\n"));
+			exit(EXIT_FAILURE);
+		}
+	}
+
+	srand(time(NULL));
+
+	if (access("/var/cache/", W_OK) == 0)
+		mkdir("/var/cache/powertop", 0600);
+	else
+		mkdir("/data/local/powertop", 0600);
+
+	load_results("saved_results.powertop");
+	load_parameters("saved_parameters.powertop");
+
+	enumerate_cpus();
+	create_all_devices();
+	detect_power_meters();
+
+	register_parameter("base power", 100, 0.5);
+	register_parameter("cpu-wakeups", 39.5);
+	register_parameter("cpu-consumption", 1.56);
+	register_parameter("gpu-operations", 0.5576);
+	register_parameter("disk-operations-hard", 0.2);
+	register_parameter("disk-operations", 0.0);
+	register_parameter("xwakes", 0.1);
+
+	load_board_params();
+	initialized = 1;
+}
+
+
+int main(int argc, char **argv)
+{
 	int option_index;
 	int c;
 	bool wantreport = FALSE;
 	char filename[4096];
 	int  iterations = 1;
-	struct statfs st_fs;
 
 #ifndef DISABLE_TRYCATCH
 	set_new_handler(out_of_memory);
@@ -322,6 +375,7 @@ int main(int argc, char **argv)
 				break;
 
 			case 'c':
+				powertop_init();
 				calibrate();
 				break;
 
@@ -350,48 +404,8 @@ int main(int argc, char **argv)
 				break;
 		}
 	}
-	
-	checkroot(); 
-	ret = system("/sbin/modprobe cpufreq_stats > /dev/null 2>&1");
-	ret = system("/sbin/modprobe msr > /dev/null 2>&1");
-
-	statfs("/sys/kernel/debug", &st_fs);
-	if (st_fs.f_type != (long) DEBUGFS_MAGIC) {
-		if (access("/bin/mount", X_OK) == 0) {
-			ret = system("/bin/mount -t debugfs debugfs /sys/kernel/debug > /dev/null 2>&1");
-		} else {
-			ret = system("mount -t debugfs debugfs /sys/kernel/debug > /dev/null 2>&1");
-		}
-		if (ret != 0) {
-			printf(_("Failed to mount debugfs!\n"));
-			printf(_("exiting...\n"));
-			exit(EXIT_FAILURE);
-		}
-	}
-
-	srand(time(NULL));
-
-	if (access("/var/cache/", W_OK) == 0)
-		mkdir("/var/cache/powertop", 0600);
-	else
-		mkdir("/data/local/powertop", 0600);
 
-	load_results("saved_results.powertop");
-	load_parameters("saved_parameters.powertop");
-
-	enumerate_cpus();
-	create_all_devices();
-	detect_power_meters();
-
-	register_parameter("base power", 100, 0.5);
-	register_parameter("cpu-wakeups", 39.5);
-	register_parameter("cpu-consumption", 1.56);
-	register_parameter("gpu-operations", 0.5576);
-	register_parameter("disk-operations-hard", 0.2);
-	register_parameter("disk-operations", 0.0);
-	register_parameter("xwakes", 0.1);
-
-	load_board_params();
+	powertop_init();
 
 	if (wantreport)
 		 report(time_out, iterations, filename);
@@ -399,8 +413,6 @@ int main(int argc, char **argv)
 	if (debug_learning)
 		printf("Learning debugging enabled\n");
 
-
-
 	learn_parameters(250, 0);
 	save_parameters("saved_parameters.powertop");
 
@@ -412,7 +424,6 @@ int main(int argc, char **argv)
 		exit(0);
 	}
 
-
 	/* first one is short to not let the user wait too long */
 	init_display();
 	one_measurement(1);
@@ -420,11 +431,9 @@ int main(int argc, char **argv)
 	tuning_update_display();
 	show_tab(0);
 
-
-
 	while (!leave_powertop) {
-		one_measurement(time_out);
 		show_cur_tab();
+		one_measurement(time_out);
 		learn_parameters(15, 0);
 	}
 #ifndef DISABLE_NCURSES
@@ -432,7 +441,6 @@ int main(int argc, char **argv)
 #endif
 	printf(_("Leaving PowerTOP\n"));
 
-
 	end_process_data();
 	clear_process_data();
 	end_cpu_data();
@@ -450,6 +458,4 @@ int main(int argc, char **argv)
 	clear_all_cpus();
 
 	return 0;
-
-
 }

Re: [Powertop] [PATCH] factor out powertop_init() function [was "Segmentation fault occurs with --calibrate"]

[Sergey Senozhatsky]

[-- Attachment #1: Type: text/plain, Size: 5175 bytes --]

On (05/18/12 09:49), Lekensteyn wrote:
> 
> On Thursday 17 May 2012 19:44:58 Sergey Senozhatsky wrote:
> > [PATCH] factor out powertop_init() function
> > 
> > Factor out powertop_init() function to perform basic initialization on
> > powertop start up.
> > 
> > Commit 057d6126eb6329c86b29a2e0219c0d0e49a84191 moved init after
> > commandline options parsing. However, in some cases we need it before
> > parsing, e.g. -- calibration.
> > 
> > Reported-by: Lekensteyn <lekensteyn(a)gmail.com>
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>


Please Cc the list next time.

>
>
> Tested-by: Lekensteyn <lekensteyn(a)gmail.com>
> 

Thank you for testing.

> Now this problem is solved, I hit another problem with --calibrate. The 
> program throws an exception because my 
> /sys/class/power_supply/BAT0/current_now returns -ENODEV when the cable is 
> removed from the laptop (and thus when the battery is being used). This issue 
> is at least in kernel 3.2, but probably occurs in 3.0 too. 2.6.38 is half 
> unaffected (only shortly after disconnecting the power cable). I saw somewhere 
> that this device is replaced by power_current (cannot remember exact name), 
> but I have no power... entries in the BAT0 directory.
> 
> With the 1.97 version from Debian, I do not see ACPI power estimate, but it 
> does not crash either. The old powertop 1 (1.13?) showed the ACPI power 
> estimate correctly.
> 
> The backtrace for the error:
> Loaded 0 prior measurements
> Cannot load from file /var/cache/powertop/saved_parameters.powertop
> Starting PowerTOP power estimate calibration 
> Calibrating idle
> terminate called after throwing an instance of 'std::ios_base::failure'
>   what():  basic_filebuf::underflow error reading the file
>

well, from the top of my head, the only sane solution is to catch fstream 
(as fstream cast) exceptions. 

I'll send a patch in a minute.

	-ss

 
> #10 0x000000000041b0e3 in read_sysfs (filename=..., ok=0x7fffffffc45d) at 
> lib.cpp:190
> #11 0x0000000000441f84 in sysfs_power_meter::get_sysfs_attr (this=<optimized 
> out>, attribute=<optimized out>, value=0x7fffffffd4ac)
>     at measurement/sysfs.cpp:44
> #12 0x00000000004420ff in sysfs_power_meter::set_rate_from_current 
> (this=0x83faf0, voltage=11.994) at measurement/sysfs.cpp:86
> #13 0x00000000004422c9 in sysfs_power_meter::measure (this=0x83faf0) at 
> measurement/sysfs.cpp:143
> #14 0x000000000043ff6d in end_power_measurement () at 
> measurement/measurement.cpp:86
> #15 0x0000000000442e3a in one_measurement (seconds=<optimized out>) at 
> main.cpp:190
> #16 0x000000000043f762 in idle_calibration () at calibrate/calibrate.cpp:423
> #17 calibrate () at calibrate/calibrate.cpp:466
> #18 0x00000000004066b5 in main (argc=2, argv=0x7fffffffe728) at main.cpp:379
> ... (gdb) up ...
> #10 0x000000000041b0e3 in read_sysfs (filename=..., ok=0x7fffffffc45d) at 
> lib.cpp:190
> 190             file >> i;
> (gdb) print filename.c_str()
> $2 = 0x844d78 "/sys/class/power_supply/BAT0/current_now"
> 
> Below is are the contents of the BAT0 directory with connected power cable:
> > alarm:0
> > charge_full:4231000
> > charge_full_design:5600000
> > charge_now:4231000
> > current_now:0
> > cycle_count:0
> > manufacturer:NOTEBOOK
> > model_name:BAT
> > present:1
> > serial_number:0001
> > status:Full
> > technology:Li-ion
> > type:Battery
> > uevent:POWER_SUPPLY_NAME=BAT0
> > uevent:POWER_SUPPLY_STATUS=Full
> > uevent:POWER_SUPPLY_PRESENT=1
> > uevent:POWER_SUPPLY_TECHNOLOGY=Li-ion
> > uevent:POWER_SUPPLY_CYCLE_COUNT=0
> > uevent:POWER_SUPPLY_VOLTAGE_MIN_DESIGN=11100000
> > uevent:POWER_SUPPLY_VOLTAGE_NOW=12720000
> > uevent:POWER_SUPPLY_CURRENT_NOW=0
> > uevent:POWER_SUPPLY_CHARGE_FULL_DESIGN=5600000
> > uevent:POWER_SUPPLY_CHARGE_FULL=4231000
> > uevent:POWER_SUPPLY_CHARGE_NOW=4231000
> > uevent:POWER_SUPPLY_MODEL_NAME=BAT
> > uevent:POWER_SUPPLY_MANUFACTURER=NOTEBOOK
> > uevent:POWER_SUPPLY_SERIAL_NUMBER=0001
> > voltage_min_design:11100000
> > voltage_now:12720000
> 
> And here with cable disconnected:
> > alarm:0
> > charge_full:4231000
> > charge_full_design:5600000
> > charge_now:4205000
> > grep: current_now: No such device
> > cycle_count:0
> > manufacturer:NOTEBOOK
> > model_name:BAT
> > present:1
> > serial_number:0001
> > status:Discharging
> > technology:Li-ion
> > type:Battery
> > uevent:POWER_SUPPLY_NAME=BAT0
> > uevent:POWER_SUPPLY_STATUS=Discharging
> > uevent:POWER_SUPPLY_PRESENT=1
> > uevent:POWER_SUPPLY_TECHNOLOGY=Li-ion
> > uevent:POWER_SUPPLY_CYCLE_COUNT=0
> > uevent:POWER_SUPPLY_VOLTAGE_MIN_DESIGN=11100000
> > uevent:POWER_SUPPLY_VOLTAGE_NOW=12263000
> > uevent:POWER_SUPPLY_CHARGE_FULL_DESIGN=5600000
> > uevent:POWER_SUPPLY_CHARGE_FULL=4231000
> > uevent:POWER_SUPPLY_CHARGE_NOW=4205000
> > uevent:POWER_SUPPLY_MODEL_NAME=BAT
> > uevent:POWER_SUPPLY_MANUFACTURER=NOTEBOOK
> > uevent:POWER_SUPPLY_SERIAL_NUMBER=0001
> > voltage_min_design:11100000
> > voltage_now:12263000
> 
> I am trying to find out which commit caused it.
> 
> Regards,
> Peter
>