Hi Fenghua/Sai,

> -----Original Message-----
> From: Fenghua Yu <[email protected]>
> Sent: Thursday, October 25, 2018 6:07 PM
> To: Thomas Gleixner <[email protected]>; Ingo Molnar
> <[email protected]>; H Peter Anvin <[email protected]>; Tony Luck
> <[email protected]>; Peter Zijlstra <[email protected]>; Reinette
> Chatre <[email protected]>; Moger, Babu
> <[email protected]>; James Morse <[email protected]>; Ravi V
> Shankar <[email protected]>; Sai Praneeth Prakhya
> <[email protected]>; Arshiya Hayatkhan Pathan
> <[email protected]>
> Cc: linux-kernel <[email protected]>; Fenghua Yu
> <[email protected]>
> Subject: [PATCH v2 4/8] selftests/resctrl: Add callback to start a benchmark
> 
> From: Sai Praneeth Prakhya <[email protected]>
> 
> The callback starts a child process and puts the child pid in created
> resctrl group with specified memory bandwidth in schemata. The child
> starts running benchmark.
> 
> Signed-off-by: Sai Praneeth Prakhya <[email protected]>
> Signed-off-by: Arshiya Hayatkhan Pathan
> <[email protected]>
> Signed-off-by: Fenghua Yu <[email protected]>
> ---
>  tools/testing/selftests/resctrl/membw.c   | 273
> ++++++++++++++++++++++++++++++
>  tools/testing/selftests/resctrl/resctrl.h |  27 +++
>  2 files changed, 300 insertions(+)
> 
> diff --git a/tools/testing/selftests/resctrl/membw.c
> b/tools/testing/selftests/resctrl/membw.c
> index 1bced7e7f148..dfcd9c1244d8 100644
> --- a/tools/testing/selftests/resctrl/membw.c
> +++ b/tools/testing/selftests/resctrl/membw.c
> @@ -401,3 +401,276 @@ static unsigned long get_mem_bw_resctrl(void)
> 
>       return mbm_total;
>  }
> +
> +pid_t bm_pid, ppid;
> +
> +static void ctrlc_handler(int signum, siginfo_t *info, void *ptr)
> +{
> +     kill(bm_pid, SIGKILL);
> +     printf("Ending\n\n");
> +
> +     exit(EXIT_SUCCESS);
> +}
> +
> +/*
> + * print_results_bw: the memory bandwidth results are stored in a file
> + * @filename:                file that stores the results
> + * @bm_pid:          child pid that runs benchmark
> + * @bw_imc:          perf imc counter value
> + * @bw_resc:         memory bandwidth value
> + *
> + * Return:           0 on success. non-zero on failure.
> + */
> +static int print_results_bw(char *filename,  int bm_pid, float bw_imc,
> +                         unsigned long bw_resc)
> +{
> +     int diff = abs(bw_imc - bw_resc);
> +     FILE *fp;
> +
> +     if (strcmp(filename, "stdio") == 0 || strcmp(filename, "stderr") == 0)
> {
> +             printf("Pid: %d \t Mem_BW_iMC: %f \t ", bm_pid, bw_imc);
> +             printf("Mem_BW_resc: %lu \t Difference: %d\n", bw_resc,
> diff);
> +     } else {
> +             fp = fopen(filename, "a");
> +             if (!fp) {
> +                     perror("Cannot open results file");
> +
> +                     return errno;
> +             }
> +             if (fprintf(fp, "Pid: %d \t Mem_BW_iMC: %f \t ",
> +                         bm_pid, bw_imc) <= 0 ||
> +                 fprintf(fp, "Mem_BW_resc: %lu \t Difference: %d\n",
> +                         bw_resc, diff) <= 0) {
> +                     fclose(fp);
> +                     perror("Could not log results.");
> +
> +                     return errno;
> +             }
> +             fclose(fp);
> +     }
> +
> +     return 0;
> +}
> +
> +static int
> +measure_vals(struct resctrl_val_param *param, unsigned long
> *bw_resc_start)
> +{
> +     unsigned long bw_imc, bw_resc, bw_resc_end;
> +     int ret;
> +
> +     /*
> +      * Measure memory bandwidth from resctrl and from
> +      * another source which is perf imc value or could
> +      * be something else if perf imc event is not available.
> +      * Compare the two values to validate resctrl value.
> +      * It takes 1sec to measure the data.
> +      */
> +     bw_imc = get_mem_bw_imc(param->cpu_no, param->bw_report);
> +     if (bw_imc < 0)
> +             return bw_imc;
> +
> +     bw_resc_end = get_mem_bw_resctrl();
> +     if (bw_resc_end < 0)
> +             return bw_resc_end;
> +
> +     bw_resc = (bw_resc_end - *bw_resc_start) / MB;
> +     ret = print_results_bw(param->filename, bm_pid, bw_imc,
> bw_resc);
> +     if (ret)
> +             return ret;
> +
> +     *bw_resc_start = bw_resc_end;
> +
> +     return 0;
> +}
> +
> +/*
> + * membw_val:                execute benchmark and measure memory
> bandwidth on
> + *                   the benchmark
> + * @benchmark_cmd:   benchmark command and its arguments
> + * @param:           parameters passed to membw_val()
> + *
> + * Return:           0 on success. non-zero on failure.
> + */
> +int membw_val(char **benchmark_cmd, struct resctrl_val_param *param)
> +{
> +     char *resctrl_val = param->resctrl_val;
> +     unsigned long bw_resc_start = 0;
> +     struct sigaction sigact;
> +     union sigval value;
> +     int sig = 0, ret = 0;
> +     FILE *fp;
> +
> +     if (strcmp(param->filename, "") == 0)
> +             sprintf(param->filename, "stdio");
> +
> +     if (strcmp(param->bw_report, "") == 0)
> +             param->bw_report = "total";
> +
> +     ret = validate_resctrl_feature_request(resctrl_val);
> +     if (ret)
> +             return ret;
> +
> +     if ((strcmp(resctrl_val, "mba")) == 0 ||
> +         (strcmp(resctrl_val, "mbm")) == 0) {
> +             ret = validate_bw_report_request(param->bw_report);
> +             if (ret)
> +                     return ret;
> +     }
> +
> +     ret = remount_resctrlfs(param->mum_resctrlfs);
> +     if (ret)
> +             return ret;
> +
> +     /*
> +      * If benchmark wasn't successfully started by child, then child should
> +      * kill parent, so save parent's pid
> +      */
> +     ppid = getpid();
> +
> +     /* File based synchronization between parent and child */
> +     fp = fopen("sig", "w");
> +     if (!fp || (fprintf(fp, "%d\n", 0) <= 0) || (fclose(fp) == EOF)) {

Same comment as patch 2. If fprintf fails,  will it fclose the file? I suggest 
to separate these checks.

> +             perror("Unable to establish sync bw parent & child");
> +
> +             return errno;
> +     }
> +
> +     /*
> +      * Fork to start benchmark, save child's pid so that it can be killed
> +      * when needed
> +      */
> +     bm_pid = fork();
> +     if (bm_pid == -1) {
> +             perror("Unable to fork");
> +
> +             return errno;
> +     }
> +
> +     if (bm_pid == 0) {
> +             /*
> +              * Mask all signals except SIGUSR1, parent uses SIGUSR1 to
> +              * start benchmark
> +              */
> +             sigfillset(&sigact.sa_mask);
> +             sigdelset(&sigact.sa_mask, SIGUSR1);
> +
> +             sigact.sa_sigaction = run_benchmark;
> +             sigact.sa_flags = SA_SIGINFO;
> +
> +             /* Register for "SIGUSR1" signal from parent */
> +             if (sigaction(SIGUSR1, &sigact, NULL))
> +                     PARENT_EXIT("Can't register child for signal");
> +
> +             /* Signal parent that child is ready */
> +             fp = fopen("sig", "w");
> +             if (!fp || (fprintf(fp, "%d\n", 1) <= 0) ||
> +                 (fclose(fp) == EOF))

Same comment as above. If fprintf fails,  will it fclose the file? I suggest to 
separate these checks.

> +                     PARENT_EXIT("can't signal that child is ready");
> +
> +             /* Suspend child until delivery of "SIGUSR1" from parent */
> +             sigsuspend(&sigact.sa_mask);
> +     }
> +
> +     printf("Benchmark PID: %d\n", bm_pid);
> +
> +     /*
> +      * Register CTRL-C handler for parent, as it has to kill benchmark
> +      * before exiting
> +      */
> +     sigact.sa_sigaction = ctrlc_handler;
> +     sigemptyset(&sigact.sa_mask);
> +     sigact.sa_flags = SA_SIGINFO;
> +     if (sigaction(SIGINT, &sigact, NULL) ||
> +         sigaction(SIGHUP, &sigact, NULL)) {
> +             perror("Can't register parent for CTRL-C handler");
> +             ret = errno;
> +             goto out;
> +     }
> +
> +     value.sival_ptr = benchmark_cmd;
> +
> +     /* Taskset benchmark to specified cpu */
> +     ret = taskset_benchmark(bm_pid, param->cpu_no);
> +     if (ret)
> +             goto out;
> +
> +     /* Write benchmark to specified con_mon grp, mon_grp in resctrl
> FS*/
> +     ret = write_bm_pid_to_resctrl(bm_pid, param->ctrlgrp, param-
> >mongrp,
> +                                   resctrl_val);
> +     if (ret)
> +             goto out;
> +
> +     if ((strcmp(resctrl_val, "mbm") == 0) ||
> +         (strcmp(resctrl_val, "mba") == 0)) {
> +             ret = initialize_mem_bw_imc();
> +             if (ret)
> +                     goto out;
> +
> +             initialize_mem_bw_resctrl(param->ctrlgrp, param->mongrp,
> +                                       param->cpu_no, resctrl_val);
> +     }
> +
> +     /*
> +      * Parent should signal child to start executing benchmark only upon
> +      * receiving a signal from child saying that it's ready
> +      */
> +     while (sig == 0) {
> +             fp = fopen("sig", "r");
> +             if (!fp) {
> +                     perror("Unable to open 'sig' file");
> +                     ret = errno;
> +                     goto out;
> +             }
> +             fscanf(fp, "%d\n", &sig);
> +             if (fclose(fp) == EOF) {
> +                     perror("Unable to close 'sig' file");
> +                     ret = errno;
> +                     goto out;
> +             }
> +     }
> +     if (system(RM_SIG_FILE) != 0)
> +             perror("Unable to remove 'sig' file");
> +
> +     /* Signal child to start benchmark */
> +     if (sigqueue(bm_pid, SIGUSR1, value) == -1) {
> +             perror("Unable to signal child to start execution");
> +             ret = errno;
> +             goto out;
> +     }
> +
> +     /* Give benchmark enough time to fully run */
> +     sleep(1);
> +
> +     /* Test runs until the callback setup() tells the test to stop. */
> +     while (1) {
> +             if (strcmp(resctrl_val, "mbm") == 0) {
> +                     ret = param->setup(1, param);
> +                     if (ret) {
> +                             ret = 0;
> +                             break;
> +                     }
> +
> +                     ret = measure_vals(param, &bw_resc_start);
> +                     if (ret)
> +                             break;
> +             } else if ((strcmp(resctrl_val, "mba") == 0)) {
> +                     ret = param->setup(1, param->cpu_no);
> +                     if (ret) {
> +                             ret = 0;
> +                             break;
> +                     }
> +
> +                     ret = measure_vals(param, &bw_resc_start);
> +                     if (ret)
> +                             break;
> +             } else {
> +                     break;
> +             }
> +     }
> +
> +out:
> +     kill(bm_pid, SIGKILL);
> +     umount_resctrlfs();
> +
> +     return ret;
> +}
> diff --git a/tools/testing/selftests/resctrl/resctrl.h
> b/tools/testing/selftests/resctrl/resctrl.h
> index fe3c3434df97..f1de2dee8f50 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -3,6 +3,7 @@
>  #ifndef RESCTRL_H
>  #define RESCTRL_H
>  #include <stdio.h>
> +#include <stdarg.h>
>  #include <errno.h>
>  #include <sched.h>
>  #include <stdlib.h>
> @@ -33,10 +34,35 @@
>               exit(EXIT_FAILURE);             \
>       } while (0)
> 
> +/*
> + * resctrl_val_param:        resctrl test parameters
> + * @resctrl_val:     Resctrl feature (Eg: mbm, mba.. etc)
> + * @ctrlgrp:         Name of the control monitor group (con_mon grp)
> + * @mongrp:          Name of the monitor group (mon grp)
> + * @cpu_no:          CPU number to which the benchmark would be
> binded
> + * @mum_resctrlfs:   Should the resctrl FS be remounted?
> + * @filename:                Name of file to which the o/p should be written
> + * @bw_report:               Bandwidth report type (reads vs writes)
> + * @setup:           Call back function to setup test environment
> + */
> +struct resctrl_val_param {
> +     char    *resctrl_val;
> +     char    ctrlgrp[64];
> +     char    mongrp[64];
> +     int     cpu_no;
> +     int     span;
> +     int     mum_resctrlfs;
> +     char    filename[64];
> +     char    *bw_report;
> +     char    *bm_type;
> +     int     (*setup)(int num, ...);
> +};
> +
>  pid_t bm_pid, ppid;
>  int ben_count;
> 
>  int remount_resctrlfs(bool mum_resctrlfs);
> +int umount_resctrlfs(void);
>  char get_sock_num(int cpu_no);
>  int validate_bw_report_request(char *bw_report);
>  int validate_resctrl_feature_request(char *resctrl_val);
> @@ -49,5 +75,6 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char
> *ctrlgrp, char *mongrp,
>  int perf_event_open(struct perf_event_attr *hw_event, pid_t pid, int cpu,
>                   int group_fd, unsigned long flags);
>  int run_fill_buf(int span, int malloc_and_init_memory, int memflush, int op);
> +int membw_val(char **benchmark_cmd, struct resctrl_val_param *param);
> 
>  #endif /* RESCTRL_H */
> --
> 2.5.0

Reply via email to