Message-ID: <0634d990-d25c-49a9-a1ae-ad7cded35fce@amd.com>
Date:   Mon, 12 Nov 2018 21:13:38 +0000
From:   "Moger, Babu" <Babu.Moger@....com>
To:     Fenghua Yu <fenghua.yu@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, H Peter Anvin <hpa@...or.com>,
        Tony Luck <tony.luck@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Reinette Chatre <reinette.chatre@...el.com>,
        James Morse <james.morse@....com>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        Sai Praneeth Prakhya <sai.praneeth.prakhya@...el.com>,
        Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@...el.com>
CC:     linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 4/8] selftests/resctrl: Add callback to start a
 benchmark



On 10/31/18 4:02 PM, Fenghua Yu wrote:
> From: Sai Praneeth Prakhya <sai.praneeth.prakhya@...el.com>
> 
> 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 <sai.praneeth.prakhya@...el.com>
> Signed-off-by: Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@...el.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@...el.com>
> ---
>  tools/testing/selftests/resctrl/membw.c   | 272 ++++++++++++++++++++++++++++++
>  tools/testing/selftests/resctrl/resctrl.h |  27 +++
>  2 files changed, 299 insertions(+)
> 
> diff --git a/tools/testing/selftests/resctrl/membw.c b/tools/testing/selftests/resctrl/membw.c
> index 3146cf8b7468..75a20de0ff71 100644
> --- a/tools/testing/selftests/resctrl/membw.c
> +++ b/tools/testing/selftests/resctrl/membw.c
> @@ -11,6 +11,7 @@
>   */
>  #include "resctrl.h"
>  
> +#define MB			(1024 * 1024)
>  #define UNCORE_IMC		"uncore_imc"
>  #define READ_FILE_NAME		"events/cas_count_read"
>  #define WRITE_FILE_NAME		"events/cas_count_write"
> @@ -429,3 +430,274 @@ 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)
> +{
> +	unsigned long diff = labs(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: %lu\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 Mem_BW_resc: %lu \t Difference: %lu\n",
> +			    bm_pid, bw_imc, 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)

I think this condition should be "<=". If the value is zero, it should exit.

> +		return bw_imc;
> +
> +	bw_resc_end = get_mem_bw_resctrl();
> +	if (bw_resc_end < 0)

I am not very sure about this. Please check if it should be "<=" 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)
> +{
> +	int ret = 0, pipefd[2], pipe_message = 0;
> +	char *resctrl_val = param->resctrl_val;
> +	unsigned long bw_resc_start = 0;
> +	struct sigaction sigact;
> +	union sigval value;
> +	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) {
> +		perror("Failed to open sig file");
> +
> +		return -1;
> +	}
> +	if (fprintf(fp, "%d\n", 0) <= 0) {
> +		perror("Unable to establish sync bw parent & child");
> +		fclose(fp);
> +
> +		return -1;
> +	}
> +	fclose(fp);
> +
> +	if (pipe(pipefd)) {
> +		perror("Unable to create pipe");
> +
> +		return -1;
> +	}
> +
> +	/*
> +	 * 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 -1;
> +	}
> +
> +	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");
> +
> +		/* Tell parent that child is ready */
> +		close(pipefd[0]);
> +		pipe_message = 1;
> +		write(pipefd[1], &pipe_message, sizeof(pipe_message));
> +		close(pipefd[1]);
> +
> +		/* Suspend child until delivery of "SIGUSR1" from parent */
> +		sigsuspend(&sigact.sa_mask);
> +
> +		PARENT_EXIT("Child is done");
> +	}
> +
> +	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 control&monitoring 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 waits for child to be ready. */
> +	close(pipefd[1]);
> +	while (pipe_message != 1)
> +		read(pipefd[0], &pipe_message, sizeof(pipe_message));
> +	close(pipefd[0]);
> +
> +	/* 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);
> +			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 01822f6258af..9820af253d4d 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>
> @@ -28,9 +29,34 @@
>  		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
> + * @span:		Memory bytes accessed in each benchmark iteration
> + * @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;
> +	int	(*setup)(int num, ...);
> +};
> +
>  pid_t bm_pid, ppid;
>  
>  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);
> @@ -43,5 +69,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 */
> 

Powered by blists - more mailing lists