On 05/11/2015 07:27 PM, Tuukka Tikkanen wrote:
On Mon, 11 May 2015, Daniel Lezcano wrote:
On 05/06/2015 02:29 PM, Zhaoyang Huang wrote:
This commit add regression test for idlestat.Please find bellowing message
for how to use.
eg: import the mytrace file and compare it with previous mytrace_old file to
see if they match each other
./idlestat --regression -f ./mytrace -t 10 -p -c -w -b ./mytrace_old -T
eg: import mytrace file and export the report file as myreport_new. compare the
myreport_new with myreport_old
to see if they match with each other ./idlestat --regression -f ./mytrace -t 10 -p -c -w -o ./myreport_new -b
./myreport_old -O
eg: set the store format of ftrace as bin and mmap it to userspace ./idlestat --trace -f ./mytrace -t 10 -m
Signed-off-by: Zhaoyang Huang zhaoyang.huang@linaro.org
This is overkill IMO.
I suggested another way in the Jira. Is there any reason to not use the proposed approach ?
I've been meaning to reply to this patch, but as there is so much I'd comment on I've been postponing it because "I don't have THAT much time on hand right now."
Briefly my points would be
- This patch does not add any Makefile target for regression testing. I
believe that is the intended addition.
- Why implement a reduced functionality "diff" into the program, when
diff is included even within busybox?
Exactly.
- Instead of "restore stdout" simply do what the output framework
strongly already hints at: Add a output stream variable to output handler private data (defaulting to stdout) and send output there. I've thought about doing it myself, but there has not been any compelling use case until now. All the open/close/whatever handler hooks already exist and any handler doing any output is getting the private data pointer.
- Several code lines are erroneous, but I think those comments become
irrelevant if the above points are addressed instead.
e.g.
Obtaining file size in bytes but allocating that many chars (most of the time same result, but the added *sizeof(char) can only add extra memory consumption.
restore_stdout() happily passes uninitialized value to dup2 when output has not been redirected.
Almost all return values are ignored.
etc
Tuukka
idlestat.c | 233
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
idlestat.h | 7 +- topology.c | 19 +++++ topology.h | 1 + trace.h | 1 + utils.c | 10 +++ utils.h | 1 + 7 files changed, 270 insertions(+), 2 deletions(-)
diff --git a/idlestat.c b/idlestat.c index 4d773f4..cb481c9 100644 --- a/idlestat.c +++ b/idlestat.c @@ -41,6 +41,13 @@ #ifdef ANDROID #include <libgen.h> #endif +#include <sys/mman.h> /* for mmap and munmap */ +#include <sys/types.h> /* for open */ +#include <sys/stat.h> /* for open */ +#include <fcntl.h> /* for open */ +#include <unistd.h> /* for lseek and write */ +#include <stdio.h> +#include <string.h> /* for memcpy */
#include "idlestat.h" #include "utils.h" @@ -1074,6 +1081,7 @@ int getoptions(int argc, char *argv[], struct
program_options *options)
struct option long_options[] = { { "trace", no_argument, &options->mode, TRACE }, { "import", no_argument, &options->mode, IMPORT },
{ "regression", no_argument, &options->mode,REGRESSION },
{ "baseline-trace", required_argument, NULL, 'b' }, { "idle", no_argument, NULL, 'c' }, { "energy-model-file", required_argument, NULL, 'e' },@@ -1090,6 +1098,9 @@ int getoptions(int argc, char *argv[], struct
program_options *options)
{ "poll-interval", required_argument, NULL, 'I' }, { "buffer-size", required_argument, NULL, 'S' }, { "version", no_argument, NULL, 'V' },
{ "mmap", optional_argument, NULL, 'm' },{ "trace file compare", no_argument, NULL, 'T' },{ "output file compare", no_argument, NULL, 'O' }, { 0, 0, 0, 0 } }; int c;@@ -1103,7 +1114,7 @@ int getoptions(int argc, char *argv[], struct
program_options *options)
int optindex = 0;
c = getopt_long(argc, argv, ":b:ce:f:ho:pr:t:vwBCI:S:V",
c = getopt_long(argc, argv, ":b:ce:f:ho:pr:t:vwBCI:S:VmTO", long_options, &optindex); if (c == -1) break;@@ -1174,6 +1185,15 @@ int getoptions(int argc, char *argv[], struct
program_options *options)
case 'S': options->tbs.percpu_buffer_size = atoi(optarg); break;
case 'm':options->mmap_flag = true;break;case 'T':options->tracefile_comp_flag = true;break;case 'O':options->output_comp_flag = true;break; case 0: /* getopt_long() set a variable, just keep going*/
break; case ':': /* missing option argument */@@ -1197,6 +1217,21 @@ int getoptions(int argc, char *argv[], struct
program_options *options)
return -1; }
- if (options->mode == REGRESSION) {
if ((options->filename == NULL)&&(options->outfilename == NULL)) {fprintf(stderr, "expected file name for regressiontest\n");
return -1;}if (options->filename && bad_filename(options->filename))return -1;if (options->outfilename &&bad_filename(options->outfilename))
return -1;return optind;- }
if (NULL == options->filename) { fprintf(stderr, "expected -f <trace filename>\n"); return -1;@@ -1401,6 +1436,158 @@ static int execute(int argc, char *argv[], char
*const envp[],
return -1;}
+int binary_compare(struct program_options * options) +{
- FILE * fp;
- FILE * fp_baseline;
- char * file;
- char * file_baseline;
- int file_size;
- int ret;
- printf("running binary_compare\n");
- fp = fopen(options->filename,"r");
- fseek( fp , 0 , SEEK_END );
- file_size = ftell( fp );
- printf( "%s %d\n" ,options->filename, file_size );
- fseek( fp , 0 , SEEK_SET);
- file = (char *)malloc( file_size * sizeof( char ) );
- fread( file , file_size , sizeof(char) , fp);
- fp_baseline = fopen(options->baseline_filename,"r");
- fseek( fp_baseline , 0 , SEEK_END );
- file_size = ftell(fp_baseline);
- printf( "%s %d\n" ,options->baseline_filename, file_size );
- fseek( fp_baseline , 0 , SEEK_SET);
- file_baseline = (char *)malloc( file_size * sizeof( char ) );
- fread( file_baseline , file_size , sizeof(char) , fp_baseline);
- ret = strcmp(file,file_baseline);
- if(ret)
fprintf(stderr, "[IDLE_STAT REGRESSION TEST] <binary_comparefailed>\n");
- else
printf( "[IDLE_STAT REGRESSION TEST] <binary_comparesuccess>\n");
- fclose(fp);
- fclose(fp_baseline);
- free(file);
- free(file_baseline);
- return ret;
+}
+int output_compare(struct program_options * options) +{
- struct report_ops *output_handler = NULL;
- void *report_data = NULL;
- struct cpu_topology *cpu_topo = NULL;
- struct cpuidle_datas *datas;
- char * tmp_filename;
- int ret = 0;
- /* Load the idle states information */
- datas = idlestat_load(options->filename);
- if (is_err(datas))
return 1;- cpu_topo = datas->topo;
- output_handler = get_report_ops(options->report_type_name);
- if (is_err(output_handler))
return 1;- if (output_handler->open_report_file(options->outfilename,
report_data))
return 1;- if (options->display & IDLE_DISPLAY) {
output_handler->cstate_table_header(report_data);dump_cpu_topo_info(output_handler, report_data,display_cstates, cpu_topo, 1);output_handler->cstate_table_footer(report_data);- }
- if (options->display & FREQUENCY_DISPLAY) {
output_handler->pstate_table_header(report_data);dump_cpu_topo_info(output_handler, report_data,display_pstates, cpu_topo, 0);output_handler->pstate_table_footer(report_data);- }
- if (options->display & WAKEUP_DISPLAY) {
output_handler->wakeup_table_header(report_data);dump_cpu_topo_info(output_handler, report_data,display_wakeup, cpu_topo, 1);output_handler->wakeup_table_footer(report_data);- }
- restore_stdout();
- /*backup the filename*/
- tmp_filename = options->filename;
- /*compare the new output file with the old one*/
- options->filename = options->outfilename;
- ret = binary_compare(options);
- if(ret)
printf("output file comparsion of %s and %sfailed\n",options->filename,options->baseline_filename);
- else
printf("[IDLE_STAT REGRESSION TEST] <output_comparesuccess>\n");
- /*restore the file name*/
- options->filename = tmp_filename;
- return ret;
+}
+int regression(struct program_options * options) +{
- struct cpuidle_datas *datas;
- struct cpu_topology *cpu_topo = NULL;
- struct report_ops *output_handler = NULL;
- int ret = 0;
- char topo_info[512];
- output_handler = get_report_ops(options->report_type_name);
- if (is_err(output_handler))
return 1;- if (output_handler->check_options &&
output_handler->check_options(options) < 0)return 1;- /* Load the idle states information */
- datas = idlestat_load(options->filename);
- if (is_err(datas)){
fprintf(stderr, "[IDLE_STAT REGRESSION TEST] <trace fileimport failed>\n");
ret = -1;- }
- else
printf("[IDLE_STAT REGRESSION TEST] <trace file importsuccess>\n");
- cpu_topo = read_sysfs_cpu_topo();
- if (is_err(cpu_topo)) {
fprintf(stderr, "Failed to read CPU topology info from"" sysfs.\n");ret = -1;- }
- else{
outstr_topo_info(topo_info,cpu_topo);printf("%s",topo_info);printf("[IDLE_STAT REGRESSION TEST] <CPU topology info readsuccess>\n");
- }
- if(options->tracefile_comp_flag)
ret = binary_compare(options);- if(options->output_comp_flag)
ret = output_compare(options);- return ret;
+}
- int main(int argc, char *argv[], char *const envp[]) { struct cpuidle_datas *datas;
@@ -1414,10 +1601,24 @@ int main(int argc, char *argv[], char *const
envp[])
struct trace_options *saved_trace_options = NULL; void *report_data = NULL;
int fd;
int flength;
struct stat statbuff;
args = getoptions(argc, argv, &options); if (args <= 0) return 1;if (options.mode == REGRESSION){
if (regression(&options)){printf("regression failed\n");}else{printf("regression success\n");}return 0;}
/* Tracing requires manipulation of some files only accessible * to root */ if ((options.mode == TRACE) && getuid()) {@@ -1497,6 +1698,16 @@ int main(int argc, char *argv[], char *const envp[])
initp = build_init_pstates(cpu_topo);
/*set the trace file format to bin*/if(options.mmap_flag){FILE * file;file = fopen(TRACE_OPTIONS, "r+");perror(__func__);printf("%s open %p\n",TRACE_OPTIONS,file);store_line("bin",(void *)file);} /* Start the recording */ if (idlestat_trace_enable(true)) goto err_restore_trace_options;@@ -1532,6 +1743,26 @@ int main(int argc, char *argv[], char *const envp[]) initp, cpu_topo)) goto err_restore_trace_options;
if(options.mmap_flag){fd = open(options.filename, O_RDONLY);if((fd == -1) || fstat(fd, &statbuff)){perror(__func__);printf("[idlestat] file length getfailed\n");
close(fd);return 1;}flength = statbuff.st_size;printf("[idlestat]: mmaped file is %s fd %x length%d\n",options.filename,fd,flength);
options.mapped_mem = mmap(NULL, flength,PROT_READ,MAP_PRIVATE,fd,0);
if((int)(void *)options.mapped_mem == (int)-1){perror(__func__);close(fd);return 1;}//printf("%s\n", options.mapped_mem);close(fd);}/* Restore original kernel ftrace options */ if (idlestat_restore_trace_options(saved_trace_options)) return 1;diff --git a/idlestat.h b/idlestat.h index e030f6a..1ca9748 100644 --- a/idlestat.h +++ b/idlestat.h @@ -121,7 +121,8 @@ struct cpuidle_datas {
enum modes { TRACE = 0,
- IMPORT
IMPORT,
REGRESSION };
struct trace_buffer_settings {
@@ -140,6 +141,10 @@ struct program_options { int verbose; char *energy_model_filename; char *report_type_name;
bool mmap_flag;
bool tracefile_comp_flag;
bool output_comp_flag;
char * mapped_mem; };
#define IDLE_DISPLAY 0x1
diff --git a/topology.c b/topology.c index 39b07bd..ee00bfe 100644 --- a/topology.c +++ b/topology.c @@ -280,6 +280,25 @@ int outfile_topo_info(FILE *f, struct cpu_topology
*topo_list)
return 0;}
+int outstr_topo_info(char *s, struct cpu_topology *topo_list) +{
- struct cpu_physical *s_phy;
- struct cpu_core *s_core;
- struct cpu_cpu *s_cpu;
- list_for_each_entry(s_phy, &topo_list->physical_head,
list_physical)
{
sprintf(s, "cluster%c:\n", s_phy->physical_id + 'A');list_for_each_entry(s_core, &s_phy->core_head, list_core) {sprintf(s, "\tcore%d\n", s_core->core_id);list_for_each_entry(s_cpu, &s_core->cpu_head,list_cpu){
sprintf(s, "\t\tcpu%d\n", s_cpu->cpu_id);}}- }
- return 0;
+}
- struct cpu_cpu *find_cpu_point(struct cpu_topology *topo_list, int
cpuid) { struct cpu_physical *s_phy; diff --git a/topology.h b/topology.h index 84e5bc1..4c7964e 100644 --- a/topology.h +++ b/topology.h @@ -115,5 +115,6 @@ extern int core_get_highest_freq(struct cpu_core
*core);
core_get_highest_freq(cpu_to_core(cpuid, topo))extern int setup_topo_states(struct cpuidle_datas *datas); +extern int outstr_topo_info(char *s, struct cpu_topology *topo_list);
#endif diff --git a/trace.h b/trace.h index a2f5867..29b4d93 100644 --- a/trace.h +++ b/trace.h @@ -34,6 +34,7 @@ #define TRACE_FREE TRACE_PATH "/free_buffer" #define TRACE_FILE TRACE_PATH "/trace" #define TRACE_STAT_FILE TRACE_PATH "/per_cpu/cpu0/stats" +#define TRACE_OPTIONS TRACE_PATH "/trace_options" #define TRACE_IDLE_NRHITS_PER_SEC 10000 #define TRACE_IDLE_LENGTH 196 #define TRACE_CPUFREQ_NRHITS_PER_SEC 100 diff --git a/utils.c b/utils.c index 48be965..1c6bc96 100644 --- a/utils.c +++ b/utils.c @@ -210,14 +210,17 @@ out_free: return ret; }
+int std_fd; int redirect_stdout_to_file(const char *path) { int ret = 0; int fd;
if (path) {
std_fd = dup(STDOUT_FILENO); fd = open(path, O_RDWR | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR | S_IRGRP|S_IROTH);
if (fd < 0) { fprintf(stderr, "%s: failed to open '%s'\n",__func__, path);
return -1;@@ -237,6 +240,13 @@ int redirect_stdout_to_file(const char *path) return 0; }
+int restore_stdout() +{
- int ret = 0;
- ret = dup2(std_fd,STDOUT_FILENO);
- printf("%s %d \n",__func__,ret);
+}
- void display_factored_time(double time, int align) { char buffer[128];
diff --git a/utils.h b/utils.h index ee7a9a2..6b8808c 100644 --- a/utils.h +++ b/utils.h @@ -49,5 +49,6 @@ extern int check_window_size(void); extern int error(const char *str); extern void *ptrerror(const char *str); extern int is_err(const void *ptr); +extern int restore_stdout();
#endif
-- http://www.linaro.org/ Linaro.org Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro Facebook | http://twitter.com/#!/linaroorg Twitter | http://www.linaro.org/linaro-blog/ Blog
Sched-tools mailing list Sched-tools@lists.linaro.org https://lists.linaro.org/mailman/listinfo/sched-tools