[RFC PATCH] tools/power turbostat: fix parameter passing for forked command

[RFC PATCH] tools/power turbostat: fix parameter passing for forked command

[Chen Yu]

turbostat supports forked command when sampling cpu state. However,
the forked command is not allowed to be executed with options, otherwise
turbostat might regard these options as invalid turbostat options.
For example:

./turbostat stress -c 4 -t 10
./turbostat: unrecognized option '-t'

This patch adds -e option for turbostat, followed by a command
string quoted by " or ':
./turbostat -e "stress -c 4 -t 10"
stress: info: [26429] dispatching hogs: 4 cpu, 0 io, 0 vm, 0 hdd
stress: info: [26429] successful run completed in 10s
     CPU Avg_MHz   %Busy Bzy_MHz TSC_MHz
       -    1688   50.06    3372    3392
       0       9    0.28    3370    3392
       4    3369   99.92    3372    3392
       1    2853   84.10    3392    3392
       5       1    0.03    2818    3392
       2     516   15.77    3270    3392
       6    3370   99.95    3371    3392
       3      19    0.56    3382    3392
       7    3369   99.89    3372    3392
10.002188 sec

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 57 +++++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 323b65e..6eb5f65 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -71,6 +71,7 @@ unsigned int extra_msr_offset32;
 unsigned int extra_msr_offset64;
 unsigned int extra_delta_offset32;
 unsigned int extra_delta_offset64;
+unsigned int command_need_fork;
 int do_smi;
 double bclk;
 unsigned int show_pkg;
@@ -124,10 +125,51 @@ int base_cpu;
 
 #define MAX(a, b) ((a) > (b) ? (a) : (b))
 
+#define MAX_COMMAND_PARAM_NUM		20
+#define MAX_COMMAND_LEN			50
 int aperf_mperf_unstable;
 int backwards_count;
 char *progname;
 
+char command_to_fork_string[MAX_COMMAND_LEN];
+char *command_to_fork_argv[MAX_COMMAND_PARAM_NUM];
+
+int convert_string_to_argv(void)
+{
+	int i = 0;
+	char *cur_pointer = command_to_fork_string;
+	char *copy_string = NULL;
+	/*trim space ahead*/
+	while (isspace(*cur_pointer))
+		*cur_pointer++ = '\0';
+	/*start parsing*/
+	copy_string = cur_pointer;
+	/*no available command*/
+	if (*cur_pointer == '\0')
+		return -EINVAL;
+	while (*cur_pointer != '\0') {
+		if (isspace(*cur_pointer)) {
+			/*
+			 * save to argv[i]. The last argv[i] for
+			 * execvp must be NULL, so the max valid
+			 * index is MAX_COMMAND_PARAM_NUM-2.
+			 */
+			*cur_pointer++ = '\0';
+			if (i > (MAX_COMMAND_PARAM_NUM-2))
+				return -EINVAL;
+			command_to_fork_argv[i++] = copy_string;
+			while (isspace(*cur_pointer))
+				cur_pointer++;
+			copy_string = cur_pointer;
+		} else
+			cur_pointer++;
+	}
+	command_to_fork_argv[i++] = copy_string;
+	/*the last parameter for execvp must be NULL*/
+	command_to_fork_argv[i] = NULL;
+	return 0;
+}
+
 cpu_set_t *cpu_present_set, *cpu_affinity_set;
 size_t cpu_present_setsize, cpu_affinity_setsize;
 
@@ -2699,6 +2741,7 @@ void help()
 	"If no COMMAND is specified, turbostat wakes every 5-seconds\n"
 	"to print statistics, until interrupted.\n"
 	"--debug	run in \"debug\" mode\n"
+	"--execute	forks COMMAND for executing, COMMAND must be quoted by \" or \'\n"
 	"--interval sec	Override default 5-second measurement interval\n"
 	"--help		print this help message\n"
 	"--counter msr	print 32-bit counter at address \"msr\"\n"
@@ -3022,6 +3065,7 @@ void cmdline(int argc, char **argv)
 {
 	int opt;
 	int option_index = 0;
+	command_need_fork = 0;
 	static struct option long_options[] = {
 		{"Counter",	required_argument,	0, 'C'},
 		{"counter",	required_argument,	0, 'c'},
@@ -3036,6 +3080,7 @@ void cmdline(int argc, char **argv)
 		{"processor",	no_argument,		0, 'p'},
 		{"Summary",	no_argument,		0, 'S'},
 		{"TCC",		required_argument,	0, 'T'},
+		{"execute",	required_argument,	0, 'e'},
 		{"version",	no_argument,		0, 'v' },
 		{0,		0,			0,  0 }
 	};
@@ -3057,6 +3102,14 @@ void cmdline(int argc, char **argv)
 		case 'd':
 			debug++;
 			break;
+		case 'e':
+			if (strlen(optarg) < MAX_COMMAND_LEN) {
+				strcpy(command_to_fork_string, optarg);
+				if (!convert_string_to_argv()) {
+					command_need_fork = 1;
+					break;
+				}
+			};
 		case 'h':
 		default:
 			help();
@@ -3109,8 +3162,8 @@ int main(int argc, char **argv)
 	/*
 	 * if any params left, it must be a command to fork
 	 */
-	if (argc - optind)
-		return fork_it(argv + optind);
+	if (command_need_fork)
+		return fork_it((char **)command_to_fork_argv);
 	else
 		turbostat_loop();
 
-- 
1.9.1

Re: [RFC PATCH] tools/power turbostat: fix parameter passing for forked command

[Len Brown]

Thanks for reporting this, Yu.

Does this pre-pending a '+' to getopt_long's optstring to prevent it
from permuting the arguments help?:


-       while ((opt = getopt_long_only(argc, argv, "C:c:Ddhi:JM:m:PpST:v",

+       while ((opt = getopt_long_only(argc, argv, "+C:c:Ddhi:JM:m:PpST:v",



Len Brown, Intel Open Source Technology Center

RE: [RFC PATCH] tools/power turbostat: fix parameter passing for forked command

[Chen, Yu C]

> -----Original Message-----
> From: lenb417@gmail.com [mailto:lenb417@gmail.com] On Behalf Of Len
> Brown
> Sent: Thursday, July 16, 2015 9:18 AM
> To: Chen, Yu C
> Cc: Brown, Len; Wysocki, Rafael J; Zhang, Rui; Linux PM list
> Subject: Re: [RFC PATCH] tools/power turbostat: fix parameter passing for
> forked command
> 
> Thanks for reporting this, Yu.
> 
> Does this pre-pending a '+' to getopt_long's optstring to prevent it from
> permuting the arguments help?:
> 
> 
> -       while ((opt = getopt_long_only(argc, argv, "C:c:Ddhi:JM:m:PpST:v",
> 
> +       while ((opt = getopt_long_only(argc, argv,
> + "+C:c:Ddhi:JM:m:PpST:v",
> 
Yeah, it works. 
Thanks!
> 
> 
> Len Brown, Intel Open Source Technology Center

Re: [RFC PATCH] tools/power turbostat: fix parameter passing for forked command

[chenyu5]

On 2015年07月16日 09:18, Len Brown wrote:
> Thanks for reporting this, Yu.
>
> Does this pre-pending a '+' to getopt_long's optstring to prevent it
> from permuting the arguments help?:
>
>
> -       while ((opt = getopt_long_only(argc, argv, "C:c:Ddhi:JM:m:PpST:v",
>
> +       while ((opt = getopt_long_only(argc, argv, "+C:c:Ddhi:JM:m:PpST:v",
>
Hi, Len,
this patch works, and because I did not see this patch in linux-pm
  branch or bleeding-edge branch, so I re-post this patch titled with
'v2' , is it suitable? thanks.

Yu
>
>
> Len Brown, Intel Open Source Technology Center
>