linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest
@ 2018-10-25 23:06 Fenghua Yu
  2018-10-25 23:06 ` [PATCH v2 1/8] selftests/resctrl: Add README for resctrl tests Fenghua Yu
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Fenghua Yu @ 2018-10-25 23:06 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Tony Luck,
	Peter Zijlstra, Reinette Chatre, Babu Moger, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel, Fenghua Yu

With more and more resctrl features are being added by Intel, AMD
and ARM, a test tool is becoming more and more useful to validate
that both hardware and software functionalities work as expected.

We introduce resctrl selftest to cover resctrl features on both
X86 and ARM architectures. It first implements MBM (Memory Bandwidth
Monitoring) and MBA (Memory Bandwidth Allocation) tests. We can enhance
the selftest tool to include more functionality tests in future.

There is an existing resctrl test suit 'intel_cmt_cat'. But the major
purpose of the tool is to test Intel(R) RDT hardware via writing and
reading MSR registers. It does access resctrl file system; but the
functionalities are very limited. And it doesn't support automatic test
and a lot of manual verifications are involved.

So the selftest tool we are introducing here provides a convenient
tool which does automatic resctrl testing, is easily available in kernel
tree, and will be extended to AMD QoS and ARM MPAM.

The selftest tool is in tools/testing/selftests/resctrl in order to have
generic test code for all architectures.

Changelog:
v2:
- Change code based on comments from Babu Moger
- Clean up other places.

Arshiya Hayatkhan Pathan (2):
  selftests/resctrl: Add mbm test
  selftests/resctrl: Add mba test

Fenghua Yu (2):
  selftests/resctrl: Add README for resctrl tests
  selftests/resctrl: Add the test in MAINTAINERS

Sai Praneeth Prakhya (4):
  selftests/resctrl: Add basic resctrl file system operations and data
  selftests/resctrl: Read memory bandwidth from perf IMC counter and
    from resctrl file system
  selftests/resctrl: Add callback to start a benchmark
  selftests/resctrl: Add built in benchmark

 MAINTAINERS                                     |   1 +
 tools/testing/selftests/resctrl/Makefile        |  16 +
 tools/testing/selftests/resctrl/README          |  53 ++
 tools/testing/selftests/resctrl/fill_buf.c      | 175 ++++++
 tools/testing/selftests/resctrl/mba_test.c      | 175 ++++++
 tools/testing/selftests/resctrl/mbm_test.c      | 143 +++++
 tools/testing/selftests/resctrl/membw.c         | 678 ++++++++++++++++++++++++
 tools/testing/selftests/resctrl/resctrl.h       |  88 +++
 tools/testing/selftests/resctrl/resctrl_tests.c | 138 +++++
 tools/testing/selftests/resctrl/resctrlfs.c     | 465 ++++++++++++++++
 10 files changed, 1932 insertions(+)
 create mode 100644 tools/testing/selftests/resctrl/Makefile
 create mode 100644 tools/testing/selftests/resctrl/README
 create mode 100644 tools/testing/selftests/resctrl/fill_buf.c
 create mode 100644 tools/testing/selftests/resctrl/mba_test.c
 create mode 100644 tools/testing/selftests/resctrl/mbm_test.c
 create mode 100644 tools/testing/selftests/resctrl/membw.c
 create mode 100644 tools/testing/selftests/resctrl/resctrl.h
 create mode 100644 tools/testing/selftests/resctrl/resctrl_tests.c
 create mode 100644 tools/testing/selftests/resctrl/resctrlfs.c

-- 
2.5.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/8] selftests/resctrl: Add README for resctrl tests
  2018-10-25 23:06 [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Fenghua Yu
@ 2018-10-25 23:06 ` Fenghua Yu
  2018-10-25 23:07 ` [PATCH v2 2/8] selftests/resctrl: Add basic resctrl file system operations and data Fenghua Yu
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Fenghua Yu @ 2018-10-25 23:06 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Tony Luck,
	Peter Zijlstra, Reinette Chatre, Babu Moger, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel, Fenghua Yu

resctrl tests will be implemented. README is added for the tool first.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 tools/testing/selftests/resctrl/README | 53 ++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 tools/testing/selftests/resctrl/README

diff --git a/tools/testing/selftests/resctrl/README b/tools/testing/selftests/resctrl/README
new file mode 100644
index 000000000000..09851a619547
--- /dev/null
+++ b/tools/testing/selftests/resctrl/README
@@ -0,0 +1,53 @@
+resctrl_tests - resctrl file system test suit
+
+Authors:
+	Fenghua Yu <fenghua.yu@intel.com>
+	Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
+	Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
+
+resctrl_tests tests various resctrl functionalities and interfaces including
+both software and hardware.
+
+Currently it supports Memory Bandwidth Monitoring test and Memory Bandwidth
+Allocation test on Intel RDT hardware. More tests will be added in the future.
+And the test suit can be extended to cover AMD QoS and ARM MPAM hardware
+as well.
+
+BUILD
+-----
+
+Run "make" to build executable file "resctrl_tests".
+
+RUN
+---
+
+To use resctrl_tests, root or sudoer privileges are required. This is because
+the test needs to mount resctrl file system and change contents in the file
+system.
+
+Executing the test without any parameter will run all supported tests:
+
+	sudo ./resctrl_tests
+
+OVERVIEW OF EXECUTION
+---------------------
+
+A test case has four stages:
+
+  - setup: mount resctrl file system, create group, setup schemata, move test
+    process pids to tasks, start benchmark.
+  - execute: let benchmark run
+  - verify: get resctrl data and verify the data with another source, e.g.
+    perf event.
+  - teardown: umount resctrl and clear temporary files.
+
+ARGUMENTS
+---------
+
+Parameter '-h' shows usage information.
+
+usage: resctrl_tests [-h] [-b benchmark_cmd [options]] [-t test list]
+        -b benchmark_cmd [options]: run specified benchmark
+         default benchmark is builtin fill_buf
+        -t test list: run tests specified in the test list, e.g. -t mbm,mba
+        -h: help
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/8] selftests/resctrl: Add basic resctrl file system operations and data
  2018-10-25 23:06 [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Fenghua Yu
  2018-10-25 23:06 ` [PATCH v2 1/8] selftests/resctrl: Add README for resctrl tests Fenghua Yu
@ 2018-10-25 23:07 ` Fenghua Yu
  2018-10-29 21:51   ` Moger, Babu
  2018-10-25 23:07 ` [PATCH v2 3/8] selftests/resctrl: Read memory bandwidth from perf IMC counter and from resctrl file system Fenghua Yu
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Fenghua Yu @ 2018-10-25 23:07 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Tony Luck,
	Peter Zijlstra, Reinette Chatre, Babu Moger, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel, Fenghua Yu

From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>

The basic resctrl file system operations and data are added for future
usage by resctrl selftest tool.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 tools/testing/selftests/resctrl/Makefile    |  10 +
 tools/testing/selftests/resctrl/resctrl.h   |  53 ++++
 tools/testing/selftests/resctrl/resctrlfs.c | 465 ++++++++++++++++++++++++++++
 3 files changed, 528 insertions(+)
 create mode 100644 tools/testing/selftests/resctrl/Makefile
 create mode 100644 tools/testing/selftests/resctrl/resctrl.h
 create mode 100644 tools/testing/selftests/resctrl/resctrlfs.c

diff --git a/tools/testing/selftests/resctrl/Makefile b/tools/testing/selftests/resctrl/Makefile
new file mode 100644
index 000000000000..bd5c5418961e
--- /dev/null
+++ b/tools/testing/selftests/resctrl/Makefile
@@ -0,0 +1,10 @@
+CC = gcc
+CFLAGS = -g -Wall
+
+*.o: *.c
+	$(CC) $(CFLAGS) -c *.c
+
+.PHONY: clean
+
+clean:
+	$(RM) *.o *~
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
new file mode 100644
index 000000000000..fe3c3434df97
--- /dev/null
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#define _GNU_SOURCE
+#ifndef RESCTRL_H
+#define RESCTRL_H
+#include <stdio.h>
+#include <errno.h>
+#include <sched.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <signal.h>
+#include <dirent.h>
+#include <stdbool.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <sys/mount.h>
+#include <sys/types.h>
+#include <asm/unistd.h>
+#include <linux/perf_event.h>
+
+#define MB			(1024 * 1024)
+#define RESCTRL_PATH		"/sys/fs/resctrl"
+#define PHYS_ID_PATH		"/sys/devices/system/cpu/cpu"
+#define RESCTRL_MBM		"L3 monitoring detected"
+#define RESCTRL_MBA		"MB allocation detected"
+#define MAX_RESCTRL_FEATURES	2
+#define RM_SIG_FILE		"rm -rf sig"
+
+#define PARENT_EXIT(err_msg)			\
+	do {					\
+		perror(err_msg);		\
+		kill(ppid, SIGKILL);		\
+		exit(EXIT_FAILURE);		\
+	} while (0)
+
+pid_t bm_pid, ppid;
+int ben_count;
+
+int remount_resctrlfs(bool mum_resctrlfs);
+char get_sock_num(int cpu_no);
+int validate_bw_report_request(char *bw_report);
+int validate_resctrl_feature_request(char *resctrl_val);
+int taskset_benchmark(pid_t bm_pid, int cpu_no);
+void run_benchmark(int signum, siginfo_t *info, void *ucontext);
+int write_schemata(char *ctrlgrp, char *schemata, int cpu_no,
+		   char *resctrl_val);
+int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
+			    char *resctrl_val);
+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);
+
+#endif /* RESCTRL_H */
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
new file mode 100644
index 000000000000..d73726ef2002
--- /dev/null
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -0,0 +1,465 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Basic resctrl file system operations
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Authors:
+ *    Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
+ *    Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
+ *    Fenghua Yu <fenghua.yu@intel.com>
+ */
+#include "resctrl.h"
+
+/*
+ * remount_resctrlfs:	Remount resctrl FS at /sys/fs/resctrl
+ * @mum_resctrlfs:	Should the resctrl FS be remounted?
+ *
+ * If not mounted, mount it.
+ * If mounted and mum_resctrlfs then remount resctrl FS.
+ * If mounted and !mum_resctrlfs then noop
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+int remount_resctrlfs(bool mum_resctrlfs)
+{
+	DIR *dp;
+	struct dirent *ep;
+	unsigned int count = 0;
+
+	/*
+	 * If kernel is built with CONFIG_RESCTRL, then /sys/fs/resctrl should
+	 * be present by default
+	 */
+	dp = opendir(RESCTRL_PATH);
+	if (dp) {
+		while ((ep = readdir(dp)) != NULL)
+			count++;
+		closedir(dp);
+	} else {
+		perror("Unable to read /sys/fs/resctrl");
+
+		return errno;
+	}
+
+	/*
+	 * If resctrl FS has more than two entries, it means that resctrl FS has
+	 * already been mounted. The two default entries are "." and "..", these
+	 * are present even when resctrl FS is not mounted
+	 */
+	if (count > 2) {
+		if (mum_resctrlfs) {
+			if (umount(RESCTRL_PATH) != 0) {
+				perror("Unable to umount resctrl");
+
+				return errno;
+			}
+			printf("Remount: done!\n");
+		} else {
+			printf("Mounted already. Not remounting!\n");
+
+			return 0;
+		}
+	}
+
+	if (mount("resctrl", RESCTRL_PATH, "resctrl", 0, NULL) != 0) {
+		perror("Unable to mount resctrl FS at /sys/fs/resctrl");
+
+		return errno;
+	}
+
+	return 0;
+}
+
+int umount_resctrlfs(void)
+{
+	if (umount(RESCTRL_PATH)) {
+		perror("Unable to umount resctrl");
+
+		return errno;
+	}
+
+	return 0;
+}
+
+char get_sock_num(int cpu_no)
+{
+	char sock_num, phys_pkg_path[1024];
+	FILE *fp;
+
+	sprintf(phys_pkg_path, "%s%d/topology/physical_package_id",
+		PHYS_ID_PATH, cpu_no);
+	fp = fopen(phys_pkg_path, "r");
+	if (!fp || fscanf(fp, "%c", &sock_num) <= 0 || fclose(fp) == EOF) {
+		perror("Could not get socket number");
+
+		return -1;
+	}
+
+	return sock_num;
+}
+
+/*
+ * taskset_benchmark:	Taskset PID (i.e. benchmark) to a specified cpu
+ * @bm_pid:		PID that should be binded
+ * @cpu_no:		CPU number at which the PID would be binded
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+int taskset_benchmark(pid_t bm_pid, int cpu_no)
+{
+	cpu_set_t my_set;
+
+	CPU_ZERO(&my_set);
+	CPU_SET(cpu_no, &my_set);
+
+	if (sched_setaffinity(bm_pid, sizeof(cpu_set_t), &my_set)) {
+		perror("Unable to taskset benchmark");
+
+		return -1;
+	}
+
+	printf("Taskset benchmark: done!\n");
+
+	return 0;
+}
+
+/*
+ * Run a specified benchmark or fill_buf (default benchmark). Direct
+ * benchmark stdio to /dev/null
+ */
+void run_benchmark(int signum, siginfo_t *info, void *ucontext)
+{
+	char **benchmark_cmd;
+	int span, operation, ret;
+
+	benchmark_cmd = info->si_ptr;
+
+	/*
+	 * Direct stdio of child to /dev/null, so that only parent writes to
+	 * stdio (console)
+	 */
+	if (!freopen("/dev/null", "w", stdout))
+		PARENT_EXIT("Unable to direct BM op to /dev/null");
+
+	if (strcmp(benchmark_cmd[0], "fill_buf") == 0) {
+		/* Execute default fill_buf benchmark */
+		span = atoi(benchmark_cmd[1]);
+		operation = atoi(benchmark_cmd[4]);
+		if (run_fill_buf(span, 1, 1, operation))
+			fprintf(stderr, "Error in running fill buffer\n");
+	} else {
+		/* Execute specified benchmark */
+		ret = execvp(benchmark_cmd[0], benchmark_cmd);
+		if (ret)
+			perror("wrong\n");
+	}
+
+	PARENT_EXIT("Unable to run specified benchmark");
+}
+
+/*
+ * create_con_mon_grp:	Create a con_mon group *only* if one doesn't exist
+ * @ctrlgrp:		Name of the con_mon group
+ * @controlgroup:	Path at which it should be created
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+static int create_con_mon_grp(const char *ctrlgrp, const char *controlgroup)
+{
+	int found_ctrl_grp = 0;
+	struct dirent *ep;
+	DIR *dp;
+
+	/*
+	 * At this point, we are guaranteed to have resctrl FS mounted and if
+	 * ctrlgrp == NULL, it means, user wants to use root con_mon grp, so do
+	 * nothing
+	 */
+	if (!ctrlgrp)
+		return 0;
+
+	/* Check if requested con_mon grp exists or not */
+	dp = opendir(RESCTRL_PATH);
+	if (dp) {
+		while ((ep = readdir(dp)) != NULL) {
+			if (strcmp(ep->d_name, ctrlgrp) == 0)
+				found_ctrl_grp = 1;
+		}
+		closedir(dp);
+	} else {
+		perror("Unable to open resctrlfs for con_mon grp");
+
+		return errno;
+	}
+
+	/* Requested con_mon grp doesn't exist, hence create it */
+	if (found_ctrl_grp == 0) {
+		if (mkdir(controlgroup, 0) == -1) {
+			perror("Unable to create con_mon group");
+
+			return errno;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * create_mon_grp:	Create a monitor group *only* if one doesn't exist
+ * @mongrp:		Name of the monitor group
+ * @controlgroup:	Path of con_mon grp at which the mon grp will be created
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+static int create_mon_grp(const char *mongrp, const char *controlgroup)
+{
+	char monitorgroup[1024];
+	int found_mon_grp = 0;
+	struct dirent *ep;
+	DIR *dp;
+
+	/* Check if requested mon grp exists or not */
+	sprintf(monitorgroup, "%s/mon_groups", controlgroup);
+	dp = opendir(monitorgroup);
+	if (dp) {
+		while ((ep = readdir(dp)) != NULL) {
+			if (strcmp(ep->d_name, mongrp) == 0)
+				found_mon_grp = 1;
+		}
+		closedir(dp);
+	} else {
+		perror("Unable to open resctrl FS for mon group");
+
+		return -1;
+	}
+
+	/* Requested mon grp doesn't exist, hence create it */
+	sprintf(monitorgroup, "%s/mon_groups/%s", controlgroup, mongrp);
+	if (found_mon_grp == 0) {
+		if (mkdir(monitorgroup, 0) == -1) {
+			perror("Unable to create mon group");
+
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * write_bm_pid_to_resctrl:	Write a PID (i.e. benchmark) to resctrl FS
+ * @bm_pid:			PID that should be written
+ * @ctrlgrp:			Name of the control monitor group (con_mon grp)
+ * @mongrp:			Name of the monitor group (mon grp)
+ * @resctrl_val:		Resctrl feature (Eg: mbm, mba.. etc)
+ *
+ * If a con_mon grp is requested, create it and write pid to it, otherwise
+ * write pid to root con_mon grp.
+ * If a mon grp is requested, create it and write pid to it, otherwise
+ * pid is not written, this means that pid is in con_mon grp and hence
+ * should consult con_mon grp's mon_data directory for results.
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
+			    char *resctrl_val)
+{
+	char controlgroup[1024], monitorgroup[1024];
+	FILE *fp;
+	int ret;
+
+	if (ctrlgrp)
+		sprintf(controlgroup, "%s/%s", RESCTRL_PATH, ctrlgrp);
+	else
+		sprintf(controlgroup, "%s", RESCTRL_PATH);
+
+	ret = create_con_mon_grp(ctrlgrp, controlgroup);
+	if (ret)
+		return ret;
+
+	/* Create mon grp, only for monitoring features like "mbm" */
+	if ((strcmp(resctrl_val, "mbm") == 0)) {
+		if (mongrp) {
+			ret = create_mon_grp(mongrp, controlgroup);
+			if (ret)
+				return ret;
+
+			sprintf(monitorgroup, "%s/mon_groups/%s/tasks",
+				controlgroup, mongrp);
+		}
+	}
+
+	strcat(controlgroup, "/tasks");
+
+	/* Write child pid to con_mon grp */
+	fp = fopen(controlgroup, "w");
+	if (!fp || fprintf(fp, "%d\n", bm_pid) <= 0 || fclose(fp) == EOF) {
+		perror("Failed to write child to con_mon grp");
+
+		return errno;
+	}
+
+	/* Write child pid to mon grp, only for "mbm" */
+	if ((strcmp(resctrl_val, "mbm") == 0)) {
+		if (mongrp) {
+			fp = fopen(monitorgroup, "w");
+			if (!fp || fprintf(fp, "%d\n", bm_pid) <= 0 ||
+			    fclose(fp) == EOF) {
+				perror("Failed to write child to mon grp");
+
+				return errno;
+			}
+		}
+	}
+
+	printf("Write benchmark to resctrl FS: done!\n");
+
+	return 0;
+}
+
+/*
+ * write_schemata:	Update schemata of a con_mon grp
+ * @ctrlgrp:		Name of the con_mon grp
+ * @schemata:		Schemata that should be updated to
+ * @cpu_no:		CPU number that the benchmark PID is binded to
+ * @resctrl_val:	Resctrl feature (Eg: mbm, mba.. etc)
+ *
+ * Update schemata of a con_mon grp *only* if requested resctrl feature is
+ * allocation type
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
+{
+	char sock_num, controlgroup[1024], schema[1024];
+	FILE *fp;
+
+	if (strcmp(resctrl_val, "mba") == 0) {
+		if (!schemata) {
+			printf("Schemata empty, so not updating\n");
+
+			return 0;
+		}
+		sock_num = get_sock_num(cpu_no);
+		if (sock_num < 0)
+			return -1;
+
+		if (ctrlgrp)
+			sprintf(controlgroup, "%s/%s/schemata", RESCTRL_PATH,
+				ctrlgrp);
+		else
+			sprintf(controlgroup, "%s/schemata", RESCTRL_PATH);
+		sprintf(schema, "%s%c%c%s", "MB:", sock_num, '=', schemata);
+
+		fp = fopen(controlgroup, "w");
+		if (!fp || fprintf(fp, "%s\n", schema) <= 0 ||
+		    fclose(fp) == EOF) {
+			perror("Unable to write schemata to con_mon grp");
+
+			return errno;
+		}
+		printf("Write schemata to resctrl FS: done!\n");
+	}
+
+	return 0;
+}
+
+/*
+ * Check if the requested feature is a valid resctrl feature or not.
+ * If yes, check if it's supported by this platform or not.
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+int validate_resctrl_feature_request(char *resctrl_val)
+{
+	const char *resctrl_features_list[MAX_RESCTRL_FEATURES] = {
+			"mbm", "mba"};
+	int resctrl_features_supported[MAX_RESCTRL_FEATURES] = {0, 0};
+	int i, valid_resctrl_feature = -1;
+	char line[1024];
+	FILE *fp;
+
+	if (!resctrl_val) {
+		fprintf(stderr, "resctrl feature cannot be NULL\n");
+
+		return -1;
+	}
+
+	/* Is the resctrl feature request valid? */
+	for (i = 0; i < MAX_RESCTRL_FEATURES; i++) {
+		if (strcmp(resctrl_features_list[i], resctrl_val) == 0)
+			valid_resctrl_feature = i;
+	}
+	if (valid_resctrl_feature == -1) {
+		fprintf(stderr, "Not a valid resctrl feature request\n");
+
+		return -1;
+	}
+
+	/* Enumerate resctrl features supported by this platform */
+	if (system("dmesg > dmesg") != 0) {
+		perror("Could not create custom dmesg file");
+
+		return errno;
+	}
+
+	fp = fopen("dmesg", "r");
+	if (!fp) {
+		perror("Could not read custom created dmesg");
+
+		return errno;
+	}
+
+	while (fgets(line, 1024, fp)) {
+		if ((strstr(line, RESCTRL_MBM)) != NULL)
+			resctrl_features_supported[0] = 1;
+		if ((strstr(line, RESCTRL_MBA)) != NULL)
+			resctrl_features_supported[1] = 1;
+	}
+	if (fclose(fp) == EOF) {
+		perror("Error in closing file");
+
+		return errno;
+	}
+
+	if (system("rm -rf dmesg") != 0)
+		perror("Unable to remove 'dmesg' file");
+
+	/* Is the resctrl feature request supported? */
+	if (!resctrl_features_supported[valid_resctrl_feature]) {
+		fprintf(stderr, "resctrl feature not supported!");
+
+		return -1;
+	}
+
+	return 0;
+}
+
+int validate_bw_report_request(char *bw_report)
+{
+	if (strcmp(bw_report, "reads") == 0)
+		return 0;
+	if (strcmp(bw_report, "writes") == 0)
+		return 0;
+	if (strcmp(bw_report, "nt-writes") == 0) {
+		strcpy(bw_report, "writes");
+		return 0;
+	}
+	if (strcmp(bw_report, "total") == 0)
+		return 0;
+
+	fprintf(stderr, "Requested iMC B/W report type unavailable\n");
+
+	return -1;
+}
+
+int perf_event_open(struct perf_event_attr *hw_event, pid_t pid, int cpu,
+		    int group_fd, unsigned long flags)
+{
+	int ret;
+
+	ret = syscall(__NR_perf_event_open, hw_event, pid, cpu,
+		      group_fd, flags);
+	return ret;
+}
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/8] selftests/resctrl: Read memory bandwidth from perf IMC counter and from resctrl file system
  2018-10-25 23:06 [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Fenghua Yu
  2018-10-25 23:06 ` [PATCH v2 1/8] selftests/resctrl: Add README for resctrl tests Fenghua Yu
  2018-10-25 23:07 ` [PATCH v2 2/8] selftests/resctrl: Add basic resctrl file system operations and data Fenghua Yu
@ 2018-10-25 23:07 ` Fenghua Yu
  2018-10-29 22:00   ` Moger, Babu
  2018-10-29 22:14   ` Moger, Babu
  2018-10-25 23:07 ` [PATCH v2 4/8] selftests/resctrl: Add callback to start a benchmark Fenghua Yu
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 16+ messages in thread
From: Fenghua Yu @ 2018-10-25 23:07 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Tony Luck,
	Peter Zijlstra, Reinette Chatre, Babu Moger, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel, Fenghua Yu

From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>

Total memory bandwidth can be monitored from perf IMC counter and from
resctrl file system. Later the two will be compared to verify the total
memory bandwidth read from resctrl is correct.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 tools/testing/selftests/resctrl/membw.c | 403 ++++++++++++++++++++++++++++++++
 1 file changed, 403 insertions(+)
 create mode 100644 tools/testing/selftests/resctrl/membw.c

diff --git a/tools/testing/selftests/resctrl/membw.c b/tools/testing/selftests/resctrl/membw.c
new file mode 100644
index 000000000000..1bced7e7f148
--- /dev/null
+++ b/tools/testing/selftests/resctrl/membw.c
@@ -0,0 +1,403 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Memory bandwidth monitoring and allocation library
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Authors:
+ *    Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
+ *    Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
+ *    Fenghua Yu <fenghua.yu@intel.com>
+ */
+#include "resctrl.h"
+
+#define UNCORE_IMC		"uncore_imc"
+#define READ_FILE_NAME		"events/cas_count_read"
+#define WRITE_FILE_NAME		"events/cas_count_write"
+#define DYN_PMU_PATH		"/sys/bus/event_source/devices"
+#define SCALE			0.00006103515625
+#define MAX_IMCS		20
+#define MAX_TOKENS		5
+#define READ			0
+#define WRITE			1
+#define CON_MON_MBM_LOCAL_BYTES_PATH				\
+	"%s/%s/mon_groups/%s/mon_data/mon_L3_0%c/mbm_local_bytes"
+
+#define CON_MBM_LOCAL_BYTES_PATH		\
+	"%s/%s/mon_data/mon_L3_0%c/mbm_local_bytes"
+
+#define MON_MBM_LOCAL_BYTES_PATH		\
+	"%s/mon_groups/%s/mon_data/mon_L3_0%c/mbm_local_bytes"
+
+#define MBM_LOCAL_BYTES_PATH			\
+	"%s/mon_data/mon_L3_0%c/mbm_local_bytes"
+
+struct membw_read_format {
+	__u64 value;         /* The value of the event */
+	__u64 time_enabled;  /* if PERF_FORMAT_TOTAL_TIME_ENABLED */
+	__u64 time_running;  /* if PERF_FORMAT_TOTAL_TIME_RUNNING */
+	__u64 id;            /* if PERF_FORMAT_ID */
+};
+
+struct imc_counter_config {
+	__u32 type;
+	__u64 event;
+	__u64 umask;
+	struct perf_event_attr pe;
+	struct membw_read_format return_value;
+	int fd;
+};
+
+static struct imc_counter_config imc_counters_config[MAX_IMCS][2];
+static char mbm_total_path[1024];
+static int imcs;
+
+void membw_initialize_perf_event_attr(int i, int j)
+{
+	memset(&imc_counters_config[i][j].pe, 0,
+	       sizeof(struct perf_event_attr));
+	imc_counters_config[i][j].pe.type = imc_counters_config[i][j].type;
+	imc_counters_config[i][j].pe.size = sizeof(struct perf_event_attr);
+	imc_counters_config[i][j].pe.disabled = 1;
+	imc_counters_config[i][j].pe.inherit = 1;
+	imc_counters_config[i][j].pe.exclude_guest = 1;
+	imc_counters_config[i][j].pe.config =
+		imc_counters_config[i][j].umask << 8 |
+		imc_counters_config[i][j].event;
+	imc_counters_config[i][j].pe.sample_type = PERF_SAMPLE_IDENTIFIER;
+	imc_counters_config[i][j].pe.read_format =
+		PERF_FORMAT_TOTAL_TIME_ENABLED | PERF_FORMAT_TOTAL_TIME_RUNNING;
+}
+
+static int open_perf_event(int i, int cpu_no, int j)
+{
+	imc_counters_config[i][j].fd =
+		perf_event_open(&imc_counters_config[i][j].pe, -1, cpu_no, -1,
+				PERF_FLAG_FD_CLOEXEC);
+
+	if (imc_counters_config[i][j].fd == -1) {
+		fprintf(stderr, "Error opening leader %llx\n",
+			imc_counters_config[i][j].pe.config);
+
+		return -1;
+	}
+
+	return 0;
+}
+
+void membw_ioctl_perf_event_ioc_reset_enable(int i, int j)
+{
+	ioctl(imc_counters_config[i][j].fd, PERF_EVENT_IOC_RESET, 0);
+	ioctl(imc_counters_config[i][j].fd, PERF_EVENT_IOC_ENABLE, 0);
+}
+
+void membw_ioctl_perf_event_ioc_disable(int i, int j)
+{
+	ioctl(imc_counters_config[i][j].fd, PERF_EVENT_IOC_DISABLE, 0);
+}
+
+/*
+ * get_event_and_umask:	Parse config into event and umask
+ * @cas_count_cfg:	Config
+ * @count:		iMC number
+ * @op:			Operation (read/write)
+ */
+void get_event_and_umask(char *cas_count_cfg, int count, bool op)
+{
+	char *token[MAX_TOKENS];
+	int i = 0;
+
+	strcat(cas_count_cfg, ",");
+	token[0] = strtok(cas_count_cfg, "=,");
+
+	for (i = 1; i < MAX_TOKENS; i++)
+		token[i] = strtok(NULL, "=,");
+
+	for (i = 0; i < MAX_TOKENS; i++) {
+		if (!token[i])
+			break;
+		if (strcmp(token[i], "event") == 0) {
+			if (op == READ)
+				imc_counters_config[count][READ].event =
+				strtol(token[i + 1], NULL, 16);
+			else
+				imc_counters_config[count][WRITE].event =
+				strtol(token[i + 1], NULL, 16);
+		}
+		if (strcmp(token[i], "umask") == 0) {
+			if (op == READ)
+				imc_counters_config[count][READ].umask =
+				strtol(token[i + 1], NULL, 16);
+			else
+				imc_counters_config[count][WRITE].umask =
+				strtol(token[i + 1], NULL, 16);
+		}
+	}
+}
+
+/* Get type and config (read and write) of an iMC counter */
+static int read_from_imc_dir(char *imc_dir, int count)
+{
+	char cas_count_cfg[1024], imc_counter_cfg[1024], imc_counter_type[1024];
+	FILE *fp;
+
+	/* Get type of iMC counter */
+	sprintf(imc_counter_type, "%s%s", imc_dir, "type");
+	fp = fopen(imc_counter_type, "r");
+	if (!fp ||
+	    fscanf(fp, "%u", &imc_counters_config[count][READ].type) <= 0 ||
+	    fclose(fp) == EOF) {
+		perror("Could not get imc type");
+
+		return errno;
+	}
+
+	imc_counters_config[count][WRITE].type =
+				imc_counters_config[count][READ].type;
+
+	/* Get read config */
+	sprintf(imc_counter_cfg, "%s%s", imc_dir, READ_FILE_NAME);
+	fp = fopen(imc_counter_cfg, "r");
+	if (!fp || fscanf(fp, "%s", cas_count_cfg) <= 0 || fclose(fp) == EOF) {
+		perror("Could not get imc cas count read");
+
+		return errno;
+	}
+
+	get_event_and_umask(cas_count_cfg, count, READ);
+
+	/* Get write config */
+	sprintf(imc_counter_cfg, "%s%s", imc_dir, WRITE_FILE_NAME);
+	fp = fopen(imc_counter_cfg, "r");
+	if (!fp || fscanf(fp, "%s", cas_count_cfg) <= 0 || fclose(fp) == EOF) {
+		perror("Could not get imc cas count write");
+
+		return errno;
+	}
+
+	get_event_and_umask(cas_count_cfg, count, WRITE);
+
+	return 0;
+}
+
+/*
+ * A system can have 'n' number of iMC (Integrated Memory Controller)
+ * counters, get that 'n'. For each iMC counter get it's type and config.
+ * Also, each counter has two configs, one for read and the other for write.
+ * A config again has two parts, event and umask.
+ * Enumerate all these details into an array of structures.
+ *
+ * Return: >= 0 on success. < 0 on failure.
+ */
+static int num_of_imcs(void)
+{
+	unsigned int count = 0;
+	char imc_dir[1024];
+	struct dirent *ep;
+	int ret;
+	DIR *dp;
+
+	dp = opendir(DYN_PMU_PATH);
+	if (dp) {
+		while ((ep = readdir(dp))) {
+			if (strstr(ep->d_name, UNCORE_IMC)) {
+				sprintf(imc_dir, "%s/%s/", DYN_PMU_PATH,
+					ep->d_name);
+				ret = read_from_imc_dir(imc_dir, count);
+				if (ret)
+					return ret;
+
+				count++;
+			}
+		}
+		closedir(dp);
+		if (count == 0) {
+			perror("Unable find iMC counters!\n");
+
+			return -1;
+		}
+	} else {
+		perror("Unable to open PMU directory!\n");
+
+		return -1;
+	}
+
+	return count;
+}
+
+static int initialize_mem_bw_imc(void)
+{
+	int imc, j;
+
+	imcs = num_of_imcs();
+	if (imcs < 0)
+		return imcs;
+
+	/* Initialize perf_event_attr structures for all iMC's */
+	for (imc = 0; imc < imcs; imc++) {
+		for (j = 0; j < 2; j++)
+			membw_initialize_perf_event_attr(imc, j);
+	}
+
+	return 0;
+}
+
+/*
+ * get_mem_bw_imc:	Memory band width as reported by iMC counters
+ * @cpu_no:		CPU number that the benchmark PID is binded to
+ * @bw_report:		Bandwidth report type (reads, writes)
+ *
+ * Memory B/W utilized by a process on a socket can be calculated using
+ * iMC counters. Perf events are used to read these counters.
+ *
+ * Return: >= 0 on success. < 0 on failure.
+ */
+static float get_mem_bw_imc(int cpu_no, char *bw_report)
+{
+	float reads, writes, of_mul_read, of_mul_write;
+	int imc, j, ret;
+
+	/* Start all iMC counters to log values (both read and write) */
+	reads = 0, writes = 0, of_mul_read = 1, of_mul_write = 1;
+	for (imc = 0; imc < imcs; imc++) {
+		for (j = 0; j < 2; j++) {
+			ret = open_perf_event(imc, cpu_no, j);
+			if (ret)
+				return -1;
+		}
+		for (j = 0; j < 2; j++)
+			membw_ioctl_perf_event_ioc_reset_enable(imc, j);
+	}
+
+	sleep(1);
+
+	/* Stop counters after a second to get results (both read and write) */
+	for (imc = 0; imc < imcs; imc++) {
+		for (j = 0; j < 2; j++)
+			membw_ioctl_perf_event_ioc_disable(imc, j);
+	}
+
+	/*
+	 * Get results which are stored in struct type imc_counter_config
+	 * Take over flow into consideration before calculating total b/w
+	 */
+	for (imc = 0; imc < imcs; imc++) {
+		struct imc_counter_config *r =
+			&imc_counters_config[imc][READ];
+		struct imc_counter_config *w =
+			&imc_counters_config[imc][WRITE];
+
+		if (read(r->fd, &r->return_value,
+			 sizeof(struct membw_read_format)) == -1) {
+			perror("Couldn't get read b/w through iMC");
+
+			return -1;
+		}
+
+		if (read(w->fd, &w->return_value,
+			 sizeof(struct membw_read_format)) == -1) {
+			perror("Couldn't get write bw through iMC");
+
+			return -1;
+		}
+
+		__u64 r_time_enabled = r->return_value.time_enabled;
+		__u64 r_time_running = r->return_value.time_running;
+
+		if (r_time_enabled != r_time_running)
+			of_mul_read = (float)r_time_enabled /
+					(float)r_time_running;
+
+		__u64 w_time_enabled = w->return_value.time_enabled;
+		__u64 w_time_running = w->return_value.time_running;
+
+		if (w_time_enabled != w_time_running)
+			of_mul_write = (float)w_time_enabled /
+					(float)w_time_running;
+		reads += r->return_value.value * of_mul_read * SCALE;
+		writes += w->return_value.value * of_mul_write * SCALE;
+	}
+
+	for (imc = 0; imc < imcs; imc++) {
+		close(imc_counters_config[imc][READ].fd);
+		close(imc_counters_config[imc][WRITE].fd);
+	}
+
+	if (strcmp(bw_report, "reads") == 0)
+		return reads;
+
+	if (strcmp(bw_report, "writes") == 0)
+		return writes;
+
+	return (reads + writes);
+}
+
+void set_mbm_path(const char *ctrlgrp, const char *mongrp, char sock_num)
+{
+	if (ctrlgrp && mongrp)
+		sprintf(mbm_total_path, CON_MON_MBM_LOCAL_BYTES_PATH,
+			RESCTRL_PATH, ctrlgrp, mongrp, sock_num);
+	else if (!ctrlgrp && mongrp)
+		sprintf(mbm_total_path, MON_MBM_LOCAL_BYTES_PATH, RESCTRL_PATH,
+			mongrp, sock_num);
+	else if (ctrlgrp && !mongrp)
+		sprintf(mbm_total_path, CON_MBM_LOCAL_BYTES_PATH, RESCTRL_PATH,
+			ctrlgrp, sock_num);
+	else if (!ctrlgrp && !mongrp)
+		sprintf(mbm_total_path, MBM_LOCAL_BYTES_PATH, RESCTRL_PATH,
+			sock_num);
+}
+
+/*
+ * initialize_mem_bw_resctrl:	Appropriately populate "mbm_total_path"
+ * @ctrlgrp:			Name of the control monitor group (con_mon grp)
+ * @mongrp:			Name of the monitor group (mon grp)
+ * @cpu_no:			CPU number that the benchmark PID is binded to
+ * @resctrl_val:		Resctrl feature (Eg: mbm, mba.. etc)
+ */
+static void initialize_mem_bw_resctrl(const char *ctrlgrp, const char *mongrp,
+				      int cpu_no, char *resctrl_val)
+{
+	char sock_num;
+
+	sock_num = get_sock_num(cpu_no);
+	if (sock_num < 0)
+		return;
+
+	if (strcmp(resctrl_val, "mbm") == 0)
+		set_mbm_path(ctrlgrp, mongrp, sock_num);
+
+	if ((strcmp(resctrl_val, "mba") == 0)) {
+		if (ctrlgrp)
+			sprintf(mbm_total_path, CON_MBM_LOCAL_BYTES_PATH,
+				RESCTRL_PATH, ctrlgrp, sock_num);
+		else
+			sprintf(mbm_total_path, MBM_LOCAL_BYTES_PATH,
+				RESCTRL_PATH, sock_num);
+	}
+}
+
+/*
+ * Get MBM Local bytes as reported by resctrl FS
+ * For MBM,
+ * 1. If con_mon grp and mon grp are given, then read from con_mon grp's mon grp
+ * 2. If only con_mon grp is given, then read from con_mon grp
+ * 3. If both are not given, then read from root con_mon grp
+ * For MBA,
+ * 1. If con_mon grp is given, then read from it
+ * 2. If con_mon grp is not given, then read from root con_mon grp
+ */
+static unsigned long get_mem_bw_resctrl(void)
+{
+	unsigned long mbm_total = 0;
+	FILE *fp;
+
+	fp = fopen(mbm_total_path, "r");
+	if (!fp || fscanf(fp, "%lu", &mbm_total) <= 0 || fclose(fp) == EOF) {
+		perror("Could not get mbm local bytes");
+
+		return -1;
+	}
+
+	return mbm_total;
+}
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/8] selftests/resctrl: Add callback to start a benchmark
  2018-10-25 23:06 [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Fenghua Yu
                   ` (2 preceding siblings ...)
  2018-10-25 23:07 ` [PATCH v2 3/8] selftests/resctrl: Read memory bandwidth from perf IMC counter and from resctrl file system Fenghua Yu
@ 2018-10-25 23:07 ` Fenghua Yu
  2018-10-29 22:03   ` Moger, Babu
  2018-10-25 23:07 ` [PATCH v2 5/8] selftests/resctrl: Add built in benchmark Fenghua Yu
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Fenghua Yu @ 2018-10-25 23:07 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Tony Luck,
	Peter Zijlstra, Reinette Chatre, Babu Moger, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel, Fenghua Yu

From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.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@intel.com>
Signed-off-by: Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 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)) {
+		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))
+			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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/8] selftests/resctrl: Add built in benchmark
  2018-10-25 23:06 [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Fenghua Yu
                   ` (3 preceding siblings ...)
  2018-10-25 23:07 ` [PATCH v2 4/8] selftests/resctrl: Add callback to start a benchmark Fenghua Yu
@ 2018-10-25 23:07 ` Fenghua Yu
  2018-10-25 23:07 ` [PATCH v2 6/8] selftests/resctrl: Add mbm test Fenghua Yu
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Fenghua Yu @ 2018-10-25 23:07 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Tony Luck,
	Peter Zijlstra, Reinette Chatre, Babu Moger, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel, Fenghua Yu

From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>

Built-in benchmark fill_buf generates stressful memory bandwidth
and cache traffic.

Later it will be used as a default benchmark by various resctrl tests
such as MBA (Memory Bandwidth Allocation) and MBM (Memory Bandwidth
Monitoring) tests.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 tools/testing/selftests/resctrl/fill_buf.c | 175 +++++++++++++++++++++++++++++
 1 file changed, 175 insertions(+)
 create mode 100644 tools/testing/selftests/resctrl/fill_buf.c

diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c
new file mode 100644
index 000000000000..d9950b5d068d
--- /dev/null
+++ b/tools/testing/selftests/resctrl/fill_buf.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * fill_buf benchmark
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Authors:
+ *    Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
+ *    Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
+ *    Fenghua Yu <fenghua.yu@intel.com>
+ */
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <inttypes.h>
+#include <malloc.h>
+#include <string.h>
+
+#include "resctrl.h"
+
+#define CL_SIZE			(64)
+#define PAGE_SIZE		(4 * 1024)
+#define MB			(1024 * 1024)
+
+static unsigned char *startptr;
+
+static void sb(void)
+{
+	asm volatile("sfence\n\t"
+		     : : : "memory");
+}
+
+static void ctrl_handler(int signo)
+{
+	free(startptr);
+	printf("\nEnding\n");
+	sb();
+	exit(EXIT_SUCCESS);
+}
+
+static void cl_flush(void *p)
+{
+	asm volatile("clflush (%0)\n\t"
+		     : : "r"(p) : "memory");
+}
+
+static void mem_flush(void *p, size_t s)
+{
+	char *cp = (char *)p;
+	size_t i = 0;
+
+	s = s / CL_SIZE; /* mem size in cache llines */
+
+	for (i = 0; i < s; i++)
+		cl_flush(&cp[i * CL_SIZE]);
+
+	sb();
+}
+
+static void *malloc_and_init_memory(size_t s)
+{
+	uint64_t *p64;
+	size_t s64;
+
+	void *p = memalign(PAGE_SIZE, s);
+
+	p64 = (uint64_t *)p;
+	s64 = s / sizeof(uint64_t);
+
+	while (s64 > 0) {
+		*p64 = (uint64_t)rand();
+		p64 += (CL_SIZE / sizeof(uint64_t));
+		s64 -= (CL_SIZE / sizeof(uint64_t));
+	}
+
+	return p;
+}
+
+static void fill_cache_read(unsigned char *start_ptr, unsigned char *end_ptr)
+{
+	while (1) {
+		unsigned char sum, *p;
+
+		p = start_ptr;
+		/* Read two chars in each cache line to stress cache */
+		while (p < (end_ptr - 1024)) {
+			sum += p[0] + p[32] + p[64] + p[96] + p[128] +
+			       p[160] + p[192] + p[224] + p[256] + p[288] +
+			       p[320] + p[352] + p[384] + p[416] + p[448] +
+			       p[480] + p[512] + p[544] + p[576] + p[608] +
+			       p[640] + p[672] + p[704] + p[736] + p[768] +
+			       p[800] + p[832] + p[864] + p[896] + p[928] +
+			       p[960] + p[992];
+			p += 1024;
+		}
+	}
+}
+
+static void fill_cache_write(unsigned char *start_ptr, unsigned char *end_ptr)
+{
+	while (1) {
+		while (start_ptr < end_ptr) {
+			*start_ptr = '1';
+			start_ptr += (CL_SIZE / 2);
+		}
+		start_ptr = startptr;
+	}
+}
+
+static void
+fill_cache(unsigned long long buf_size, int malloc_and_init,
+	   int memflush, int op)
+{
+	unsigned char *start_ptr, *end_ptr;
+	unsigned long long i;
+
+	if (malloc_and_init) {
+		start_ptr = malloc_and_init_memory(buf_size);
+		printf("Started benchmark with memalign\n");
+	} else {
+		start_ptr = malloc(buf_size);
+		printf("Started benchmark with malloc\n");
+	}
+
+	if (!start_ptr)
+		return;
+
+	startptr = start_ptr;
+	end_ptr = start_ptr + buf_size;
+
+	/*
+	 * It's better to touch the memory once to avoid any compiler
+	 * optimizations
+	 */
+	if (!malloc_and_init) {
+		for (i = 0; i < buf_size; i++)
+			*start_ptr++ = (unsigned char)rand();
+	}
+
+	start_ptr = startptr;
+
+	/* Flush the memory before using to avoid "cache hot pages" effect */
+	if (memflush) {
+		mem_flush(start_ptr, buf_size);
+		printf("Started benchmark with memflush\n");
+	} else {
+		printf("Started benchmark *without* memflush\n");
+	}
+
+	if (op == 0)
+		fill_cache_read(start_ptr, end_ptr);
+	else
+		fill_cache_write(start_ptr, end_ptr);
+
+	free(startptr);
+}
+
+int run_fill_buf(int span, int malloc_and_init_memory, int memflush, int op)
+{
+	unsigned long long cache_size = span * MB;
+
+	/* set up ctrl-c handler */
+	if (signal(SIGINT, ctrl_handler) == SIG_ERR)
+		printf("Failed to catch SIGINT!\n");
+	if (signal(SIGHUP, ctrl_handler) == SIG_ERR)
+		printf("Failed to catch SIGHUP!\n");
+
+	printf("Cache size in Bytes = %llu\n", cache_size);
+
+	fill_cache(cache_size, malloc_and_init_memory, memflush, op);
+
+	return -1;
+}
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/8] selftests/resctrl: Add mbm test
  2018-10-25 23:06 [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Fenghua Yu
                   ` (4 preceding siblings ...)
  2018-10-25 23:07 ` [PATCH v2 5/8] selftests/resctrl: Add built in benchmark Fenghua Yu
@ 2018-10-25 23:07 ` Fenghua Yu
  2018-10-25 23:07 ` [PATCH v2 7/8] selftests/resctrl: Add mba test Fenghua Yu
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Fenghua Yu @ 2018-10-25 23:07 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Tony Luck,
	Peter Zijlstra, Reinette Chatre, Babu Moger, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel, Fenghua Yu

From: Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>

mbm test is the first implemented selftest. It starts a stressful
memory bandwidth benchmark and assigns the bandwidth pid in a
resctrl monitoring group. Read and compare perf IMC counter and mbm
total bytes for the benchmark. The numbers should be close enough
to pass the test.

Default benchmark is built-in fill_buf. But users can specify their
own benchmark by option "-b".

We can add memory bandwidth monitoring for multiple processes in the
future.

Signed-off-by: Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 tools/testing/selftests/resctrl/Makefile        |   8 +-
 tools/testing/selftests/resctrl/mbm_test.c      | 143 ++++++++++++++++++++++++
 tools/testing/selftests/resctrl/membw.c         |   2 +
 tools/testing/selftests/resctrl/resctrl.h       |   4 +
 tools/testing/selftests/resctrl/resctrl_tests.c | 124 ++++++++++++++++++++
 5 files changed, 280 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/resctrl/mbm_test.c
 create mode 100644 tools/testing/selftests/resctrl/resctrl_tests.c

diff --git a/tools/testing/selftests/resctrl/Makefile b/tools/testing/selftests/resctrl/Makefile
index bd5c5418961e..51165f9f8a9d 100644
--- a/tools/testing/selftests/resctrl/Makefile
+++ b/tools/testing/selftests/resctrl/Makefile
@@ -1,10 +1,16 @@
 CC = gcc
 CFLAGS = -g -Wall
 
+all: resctrl_tests
+
 *.o: *.c
 	$(CC) $(CFLAGS) -c *.c
 
+resctrl_tests: *.o
+	$(CC) $(CFLAGS) -o resctrl_tests resctrl_tests.o resctrlfs.o \
+		 membw.o fill_buf.o mbm_test.o
+
 .PHONY: clean
 
 clean:
-	$(RM) *.o *~
+	$(RM) *.o *~ resctrl_tests
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
new file mode 100644
index 000000000000..f98f4e20a021
--- /dev/null
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -0,0 +1,143 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Memory Bandwidth Monitoring (MBM) test
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Authors:
+ *    Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
+ *    Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
+ *    Fenghua Yu <fenghua.yu@intel.com>
+ */
+#include "resctrl.h"
+
+#define RESULT_FILE_NAME	"result_mbm"
+#define MAX_DIFF		300
+#define NUM_OF_RUNS		5
+
+static void
+show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, int span)
+{
+	unsigned long avg_bw_imc = 0, avg_bw_resc = 0, avg_diff = 0;
+	int runs, sum_bw_imc = 0, sum_bw_resc = 0;
+
+	/*
+	 * Discard the first value which is inaccurate due to monitoring setup
+	 * transition phase.
+	 */
+	for (runs = 1; runs < NUM_OF_RUNS ; runs++) {
+		sum_bw_imc += bw_imc[runs];
+		sum_bw_resc += bw_resc[runs];
+	}
+
+	avg_bw_imc = sum_bw_imc / 4;
+	avg_bw_resc = sum_bw_resc / 4;
+	avg_diff = avg_bw_resc - avg_bw_imc;
+
+	printf("\nSpan (MB): %d \t", span);
+	printf("avg_bw_imc: %ld\t", avg_bw_imc);
+	printf("avg_bw_resc: %ld\t", avg_bw_resc);
+	printf("avg_diff: %d \t", abs(avg_diff));
+
+	if (abs(avg_diff) > MAX_DIFF)
+		printf(" failed\n");
+	else
+		printf(" passed\n");
+}
+
+static int check_results(int span)
+{
+	unsigned long bw_imc[NUM_OF_RUNS], bw_resc[NUM_OF_RUNS];
+	char temp[1024], *token_array[8];
+	char output[] = RESULT_FILE_NAME;
+	int runs;
+	FILE *fp;
+
+	printf("\nChecking for pass/fail\n");
+
+	fp = fopen(output, "r");
+	if (!fp) {
+		perror("Error in opening file\n");
+
+		return errno;
+	}
+
+	runs = 0;
+	while (fgets(temp, 1024, fp)) {
+		char *token = strtok(temp, ":\t");
+		int i = 0;
+
+		while (token) {
+			token_array[i++] = token;
+			token = strtok(NULL, ":\t");
+		}
+
+		bw_resc[runs] = atoll(token_array[5]);
+		bw_imc[runs] = atoll(token_array[3]);
+		runs++;
+	}
+
+	show_bw_info(bw_imc, bw_resc, span);
+
+	fclose(fp);
+
+	return 0;
+}
+
+static int mbm_setup(int num, ...)
+{
+	struct resctrl_val_param *p;
+	static int num_of_runs;
+	va_list param;
+	int ret = 0;
+
+	/* Run NUM_OF_RUNS times */
+	if (num_of_runs++ >= NUM_OF_RUNS)
+		return -1;
+
+	va_start(param, num);
+	p = va_arg(param, struct resctrl_val_param *);
+	va_end(param);
+
+	/* Set up shemata with 100% allocation on the first run. */
+	if (num_of_runs == 0)
+		ret = write_schemata(p->ctrlgrp, "100", p->cpu_no, "mbm");
+
+	return ret;
+}
+
+void mbm_test_cleanup(void)
+{
+	remove(RESULT_FILE_NAME);
+}
+
+int mbm_bw_change(char *resctrl_val_type, int core_id, int span,
+		  char *bw_report, char *bm_type, char **benchmark_cmd)
+{
+	struct resctrl_val_param param = {
+		.resctrl_val	= resctrl_val_type,
+		.ctrlgrp	= "c1",
+		.mongrp		= "m1",
+		.cpu_no		= core_id,
+		.mum_resctrlfs	= 1,
+		.filename	= RESULT_FILE_NAME,
+		.bw_report	=  bw_report,
+		.bm_type	= bm_type,
+		.setup		= mbm_setup
+	};
+	int ret;
+
+	remove(RESULT_FILE_NAME);
+
+	ret = membw_val(benchmark_cmd, &param);
+	if (ret)
+		return ret;
+
+	ret = check_results(span);
+	if (ret)
+		return ret;
+
+	mbm_test_cleanup();
+
+	return 0;
+}
diff --git a/tools/testing/selftests/resctrl/membw.c b/tools/testing/selftests/resctrl/membw.c
index dfcd9c1244d8..78d0689022a1 100644
--- a/tools/testing/selftests/resctrl/membw.c
+++ b/tools/testing/selftests/resctrl/membw.c
@@ -407,6 +407,8 @@ pid_t bm_pid, ppid;
 static void ctrlc_handler(int signum, siginfo_t *info, void *ptr)
 {
 	kill(bm_pid, SIGKILL);
+	umount_resctrlfs();
+	tests_cleanup();
 	printf("Ending\n\n");
 
 	exit(EXIT_SUCCESS);
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index f1de2dee8f50..8d3623062ce0 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -76,5 +76,9 @@ 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);
+int mbm_bw_change(char *resctrl_val_type, int core_id, int span,
+		  char *bw_report, char *bm_type, char **benchmark_cmd);
+void tests_cleanup(void);
+void mbm_test_cleanup(void);
 
 #endif /* RESCTRL_H */
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
new file mode 100644
index 000000000000..2044b6b79f5b
--- /dev/null
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Resctrl tests
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Authors:
+ *    Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
+ *    Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
+ *    Fenghua Yu <fenghua.yu@intel.com>
+ */
+#include "resctrl.h"
+
+#define BENCHMARK_ARGS		64
+#define BENCHMARK_ARG_SIZE	64
+
+int ben_count;
+
+static void cmd_help(void)
+{
+	printf("usage: resctrl_tests [-h] [-b \"benchmark_cmd [options]\"] [-t test list]\n");
+	printf("\t-b benchmark_cmd [options]: run specified benchmark\n");
+	printf("\t default benchmark is builtin fill_buf\n");
+	printf("\t-t test list: run tests specified in the test list, ");
+	printf("e.g. -t mbm,mba\n");
+	printf("\t-h: help\n");
+}
+
+void tests_cleanup(void)
+{
+	mbm_test_cleanup();
+}
+
+int main(int argc, char **argv)
+{
+	char benchmark_cmd_area[BENCHMARK_ARGS][BENCHMARK_ARG_SIZE];
+	int res, c, core_id = 0, span = 250, argc_new = argc, i;
+	bool has_ben = false, mbm_test = true;
+	char *benchmark_cmd[BENCHMARK_ARGS];
+	char bw_report[64], bm_type[64];
+
+	while ((c = getopt(argc_new, argv, "ht:b:")) != -1) {
+		char *token;
+
+		switch (c) {
+		case 't':
+			token = strtok(optarg, ",");
+
+			mbm_test = false;
+			while (token) {
+				if (!strcmp(token, "mbm")) {
+					mbm_test = true;
+				} else {
+					printf("invalid argument\n");
+
+					return -1;
+				}
+				token = strtok(NULL, ":\t");
+			}
+			break;
+		case 'b':
+			/* Extract benchmark command from command line. */
+			token = strtok(optarg, " ");
+			i = 0;
+			while (token) {
+				benchmark_cmd[i] = benchmark_cmd_area[i];
+				strcpy(benchmark_cmd[i++], token);
+				if (i >= BENCHMARK_ARGS) {
+					printf("Too many benchmark args\n");
+
+					return -1;
+				}
+				token = strtok(NULL, " ");
+			}
+			benchmark_cmd[i] = NULL;
+			has_ben = true;
+			break;
+		case 'h':
+			cmd_help();
+
+			return 0;
+		default:
+			printf("invalid argument\n");
+
+			return -1;
+		}
+	}
+
+	/*
+	 * We need root privileges to run because
+	 * 1. We write to resctrl FS
+	 * 2. We execute perf commands
+	 */
+	if (geteuid() != 0) {
+		perror("Please run this program as root\n");
+
+		return errno;
+	}
+
+	if (!has_ben) {
+		/* If no benchmark is given by "-b" argument, use fill_buf. */
+		for (i = 0; i < 5; i++)
+			benchmark_cmd[i] = benchmark_cmd_area[i];
+		strcpy(benchmark_cmd[0], "fill_buf");
+		sprintf(benchmark_cmd[1], "%d", span);
+		strcpy(benchmark_cmd[2], "1");
+		strcpy(benchmark_cmd[3], "1");
+		strcpy(benchmark_cmd[4], "0");
+		benchmark_cmd[5] = NULL;
+	}
+
+	sprintf(bw_report, "reads");
+	sprintf(bm_type, "fill_buf");
+
+	if (mbm_test) {
+		printf("\nMBM BW Change Starting..\n");
+		res = mbm_bw_change("mbm", core_id, span, bw_report, bm_type,
+				    benchmark_cmd);
+		if (res)
+			printf("Error in running tests for mbm bw change!\n");
+	}
+
+	return 0;
+}
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 7/8] selftests/resctrl: Add mba test
  2018-10-25 23:06 [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Fenghua Yu
                   ` (5 preceding siblings ...)
  2018-10-25 23:07 ` [PATCH v2 6/8] selftests/resctrl: Add mbm test Fenghua Yu
@ 2018-10-25 23:07 ` Fenghua Yu
  2018-10-25 23:07 ` [PATCH v2 8/8] selftests/resctrl: Add the test in MAINTAINERS Fenghua Yu
  2018-10-29 22:56 ` [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Moger, Babu
  8 siblings, 0 replies; 16+ messages in thread
From: Fenghua Yu @ 2018-10-25 23:07 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Tony Luck,
	Peter Zijlstra, Reinette Chatre, Babu Moger, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel, Fenghua Yu

From: Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>

mba test starts a stressful memory bandwidth benchmark and allocates
memory bandwidth from 100% down to 10% for the benchmark process. For
each allocation, compare perf IMC counter and mbm total bytes from
resctrl. The difference between the two values should be within a
threshold to pass the test.

Default benchmark is built-in fill_buf. But users can specify their
own benchmark by option "-b".

We can add memory bandwidth allocation for multiple processes in the
future.

Signed-off-by: Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 tools/testing/selftests/resctrl/Makefile        |   2 +-
 tools/testing/selftests/resctrl/mba_test.c      | 175 ++++++++++++++++++++++++
 tools/testing/selftests/resctrl/resctrl.h       |   4 +
 tools/testing/selftests/resctrl/resctrl_tests.c |  16 ++-
 4 files changed, 195 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/resctrl/mba_test.c

diff --git a/tools/testing/selftests/resctrl/Makefile b/tools/testing/selftests/resctrl/Makefile
index 51165f9f8a9d..bf9f55e71d0c 100644
--- a/tools/testing/selftests/resctrl/Makefile
+++ b/tools/testing/selftests/resctrl/Makefile
@@ -8,7 +8,7 @@ all: resctrl_tests
 
 resctrl_tests: *.o
 	$(CC) $(CFLAGS) -o resctrl_tests resctrl_tests.o resctrlfs.o \
-		 membw.o fill_buf.o mbm_test.o
+		 membw.o fill_buf.o mbm_test.o mba_test.o
 
 .PHONY: clean
 
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
new file mode 100644
index 000000000000..f78ebc53a0ca
--- /dev/null
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Memory Bandwidth Allocation (MBA) test
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Authors:
+ *    Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
+ *    Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
+ *    Fenghua Yu <fenghua.yu@intel.com>
+ */
+#include "resctrl.h"
+
+#define RESULT_FILE_NAME	"result_mba"
+#define NUM_OF_RUNS		5
+#define MAX_DIFF		300
+#define ALLOCATION_MAX		100
+#define ALLOCATION_MIN		10
+#define ALLOCATION_STEP		10
+
+/*
+ * Change schemata percentage from 100 to 10%. Write schemata to specified
+ * con_mon grp, mon_grp in resctrl FS.
+ * For each allocation, run 5 times in order to get average values.
+ */
+static int mba_setup(int num, ...)
+{
+	static int runs_per_allocation, allocation = 100;
+	char allocation_str[64];
+	va_list param;
+	int cpu_no;
+
+	va_start(param, num);
+	cpu_no = va_arg(param, int);
+	va_end(param);
+
+	if (runs_per_allocation >= NUM_OF_RUNS)
+		runs_per_allocation = 0;
+
+	/* Only set up schemata once every NUM_OF_RUNS of allocations */
+	if (runs_per_allocation++ != 0)
+		return 0;
+
+	if (allocation < ALLOCATION_MIN || allocation > ALLOCATION_MAX)
+		return -1;
+
+	sprintf(allocation_str, "%d", allocation);
+
+	write_schemata("c1", allocation_str, cpu_no, "mba");
+	printf("changed schemata to : %d\n", allocation);
+	allocation -= ALLOCATION_STEP;
+
+	return 0;
+}
+
+static void show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc)
+{
+	int allocation, failed, runs;
+
+	/* Memory bandwidth from 100% down to 10% */
+	for (allocation = 0; allocation < ALLOCATION_MAX / ALLOCATION_STEP;
+	     allocation++) {
+		unsigned long avg_bw_imc = 0, avg_bw_resc = 0, avg_diff = 0;
+		int sum_bw_imc = 0, sum_bw_resc = 0;
+
+		/*
+		 * The first run is discarded due to inaccurate value from
+		 * phase transition.
+		 */
+		for (runs = NUM_OF_RUNS * allocation + 1;
+		     runs < NUM_OF_RUNS * allocation + NUM_OF_RUNS ; runs++) {
+			sum_bw_imc += bw_imc[runs];
+			sum_bw_resc += bw_resc[runs];
+		}
+
+		avg_bw_imc = sum_bw_imc / (NUM_OF_RUNS - 1);
+		avg_bw_resc = sum_bw_resc / (NUM_OF_RUNS - 1);
+		avg_diff = avg_bw_resc - avg_bw_imc;
+
+		printf("\nschemata percentage: %d \t",
+		       ALLOCATION_MAX - ALLOCATION_STEP * allocation);
+		printf("avg_bw_imc: %ld\t", avg_bw_imc);
+		printf("avg_bw_resc: %ld\t", avg_bw_resc);
+		printf("avg_diff: %d \t", abs(avg_diff));
+		if (abs(avg_diff) > MAX_DIFF) {
+			printf("failed\n");
+			failed = 1;
+		} else {
+			printf("passed\n");
+		}
+	}
+
+	if (failed) {
+		printf("\nTest for schemata change using MBA failed");
+		printf("as atleast one test failed!\n");
+	} else {
+		printf("\nTests for changing schemata using MBA passed!\n\n");
+	}
+}
+
+static int check_results(void)
+{
+	char *token_array[8], output[] = RESULT_FILE_NAME, temp[512];
+	unsigned long bw_imc[1024], bw_resc[1024];
+	int runs;
+	FILE *fp;
+
+	printf("\nchecking for pass/fail\n");
+	fp = fopen(output, "r");
+	if (!fp) {
+		perror("Error in opening file\n");
+
+		return errno;
+	}
+
+	runs = 0;
+	while (fgets(temp, 1024, fp)) {
+		char *token = strtok(temp, ":\t");
+		int fields = 0;
+
+		while (token) {
+			token_array[fields++] = token;
+			token = strtok(NULL, ":\t");
+		}
+
+		/* Field 3 is perf imc value */
+		bw_imc[runs] = atoll(token_array[3]);
+		/* Field 5 is resctrl value */
+		bw_resc[runs] = atoll(token_array[5]);
+		runs++;
+	}
+
+	fclose(fp);
+
+	show_mba_info(bw_imc, bw_resc);
+
+	return 0;
+}
+
+void mba_test_cleanup(void)
+{
+	remove(RESULT_FILE_NAME);
+}
+
+int mba_schemata_change(char *resctrl_val_type, int core_id, int span,
+			char *bw_report, char *bm_type,
+			char **benchmark_cmd)
+{
+	struct resctrl_val_param param = {
+		.resctrl_val	= resctrl_val_type,
+		.ctrlgrp	= "c1",
+		.mongrp		= "m1",
+		.cpu_no		= core_id,
+		.mum_resctrlfs	= 1,
+		.filename	= RESULT_FILE_NAME,
+		.bw_report	=  bw_report,
+		.bm_type	= bm_type,
+		.setup		= mba_setup
+	};
+	int ret;
+
+	remove(RESULT_FILE_NAME);
+
+	ret = membw_val(benchmark_cmd, &param);
+	if (ret)
+		return ret;
+
+	ret = check_results();
+	if (ret)
+		return ret;
+
+	mba_test_cleanup();
+
+	return 0;
+}
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 8d3623062ce0..5021349be5cd 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -80,5 +80,9 @@ int mbm_bw_change(char *resctrl_val_type, int core_id, int span,
 		  char *bw_report, char *bm_type, char **benchmark_cmd);
 void tests_cleanup(void);
 void mbm_test_cleanup(void);
+int mba_schemata_change(char *resctrl_val_type, int core_id, int span,
+			char *bw_report, char *bm_type,
+			char **benchmark_cmd);
+void mba_test_cleanup(void);
 
 #endif /* RESCTRL_H */
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
index 2044b6b79f5b..5fa07a00ffeb 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -29,13 +29,14 @@ static void cmd_help(void)
 void tests_cleanup(void)
 {
 	mbm_test_cleanup();
+	mba_test_cleanup();
 }
 
 int main(int argc, char **argv)
 {
 	char benchmark_cmd_area[BENCHMARK_ARGS][BENCHMARK_ARG_SIZE];
 	int res, c, core_id = 0, span = 250, argc_new = argc, i;
-	bool has_ben = false, mbm_test = true;
+	bool has_ben = false, mbm_test = true, mba_test = true;
 	char *benchmark_cmd[BENCHMARK_ARGS];
 	char bw_report[64], bm_type[64];
 
@@ -47,9 +48,12 @@ int main(int argc, char **argv)
 			token = strtok(optarg, ",");
 
 			mbm_test = false;
+			mba_test = false;
 			while (token) {
 				if (!strcmp(token, "mbm")) {
 					mbm_test = true;
+				} else if (!strcmp(token, "mba")) {
+					mba_test = true;
 				} else {
 					printf("invalid argument\n");
 
@@ -120,5 +124,15 @@ int main(int argc, char **argv)
 			printf("Error in running tests for mbm bw change!\n");
 	}
 
+	if (mba_test) {
+		printf("\nMBA Schemata Change Starting..\n");
+		if (!has_ben)
+			sprintf(benchmark_cmd[1], "%d", span);
+		res = mba_schemata_change("mba", core_id, span, bw_report,
+					  bm_type, benchmark_cmd);
+		if (res)
+			printf("Error in tests for mba-change-schemata!\n");
+	}
+
 	return 0;
 }
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 8/8] selftests/resctrl: Add the test in MAINTAINERS
  2018-10-25 23:06 [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Fenghua Yu
                   ` (6 preceding siblings ...)
  2018-10-25 23:07 ` [PATCH v2 7/8] selftests/resctrl: Add mba test Fenghua Yu
@ 2018-10-25 23:07 ` Fenghua Yu
  2018-10-29 22:56 ` [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Moger, Babu
  8 siblings, 0 replies; 16+ messages in thread
From: Fenghua Yu @ 2018-10-25 23:07 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Tony Luck,
	Peter Zijlstra, Reinette Chatre, Babu Moger, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel, Fenghua Yu

The resctrl selftests will be maintained by RDT maintainers.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 48a65c3a4189..ecd1369ef761 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12270,6 +12270,7 @@ S:	Supported
 F:	arch/x86/kernel/cpu/intel_rdt*
 F:	arch/x86/include/asm/intel_rdt_sched.h
 F:	Documentation/x86/intel_rdt*
+F:	tools/testing/selftests/resctrl
 
 READ-COPY UPDATE (RCU)
 M:	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* RE: [PATCH v2 2/8] selftests/resctrl: Add basic resctrl file system operations and data
  2018-10-25 23:07 ` [PATCH v2 2/8] selftests/resctrl: Add basic resctrl file system operations and data Fenghua Yu
@ 2018-10-29 21:51   ` Moger, Babu
       [not found]     ` <CALBSrqDRKbth2NAUFM+emBHVv7eZPX=O9Ki-=vUxi-QQc1wVnQ@mail.gmail.com>
  0 siblings, 1 reply; 16+ messages in thread
From: Moger, Babu @ 2018-10-29 21:51 UTC (permalink / raw)
  To: Fenghua Yu, Thomas Gleixner, Ingo Molnar, H Peter Anvin,
	Tony Luck, Peter Zijlstra, Reinette Chatre, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel

Hi Fenghua, Sai,

> -----Original Message-----
> From: Fenghua Yu <fenghua.yu@intel.com>
> Sent: Thursday, October 25, 2018 6:07 PM
> To: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> <mingo@redhat.com>; H Peter Anvin <hpa@zytor.com>; Tony Luck
> <tony.luck@intel.com>; Peter Zijlstra <peterz@infradead.org>; Reinette
> Chatre <reinette.chatre@intel.com>; Moger, Babu
> <Babu.Moger@amd.com>; James Morse <james.morse@arm.com>; Ravi V
> Shankar <ravi.v.shankar@intel.com>; Sai Praneeth Prakhya
> <sai.praneeth.prakhya@intel.com>; Arshiya Hayatkhan Pathan
> <arshiya.hayatkhan.pathan@intel.com>
> Cc: linux-kernel <linux-kernel@vger.kernel.org>; Fenghua Yu
> <fenghua.yu@intel.com>
> Subject: [PATCH v2 2/8] selftests/resctrl: Add basic resctrl file system
> operations and data
> 
> From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> 
> The basic resctrl file system operations and data are added for future
> usage by resctrl selftest tool.
> 
> Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> Signed-off-by: Arshiya Hayatkhan Pathan
> <arshiya.hayatkhan.pathan@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  tools/testing/selftests/resctrl/Makefile    |  10 +
>  tools/testing/selftests/resctrl/resctrl.h   |  53 ++++
>  tools/testing/selftests/resctrl/resctrlfs.c | 465
> ++++++++++++++++++++++++++++
>  3 files changed, 528 insertions(+)
>  create mode 100644 tools/testing/selftests/resctrl/Makefile
>  create mode 100644 tools/testing/selftests/resctrl/resctrl.h
>  create mode 100644 tools/testing/selftests/resctrl/resctrlfs.c
> 
> diff --git a/tools/testing/selftests/resctrl/Makefile
> b/tools/testing/selftests/resctrl/Makefile
> new file mode 100644
> index 000000000000..bd5c5418961e
> --- /dev/null
> +++ b/tools/testing/selftests/resctrl/Makefile
> @@ -0,0 +1,10 @@
> +CC = gcc
> +CFLAGS = -g -Wall
> +
> +*.o: *.c
> +	$(CC) $(CFLAGS) -c *.c
> +
> +.PHONY: clean
> +
> +clean:
> +	$(RM) *.o *~
> diff --git a/tools/testing/selftests/resctrl/resctrl.h
> b/tools/testing/selftests/resctrl/resctrl.h
> new file mode 100644
> index 000000000000..fe3c3434df97
> --- /dev/null
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#define _GNU_SOURCE
> +#ifndef RESCTRL_H
> +#define RESCTRL_H
> +#include <stdio.h>
> +#include <errno.h>
> +#include <sched.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <signal.h>
> +#include <dirent.h>
> +#include <stdbool.h>
> +#include <sys/stat.h>
> +#include <sys/ioctl.h>
> +#include <sys/mount.h>
> +#include <sys/types.h>
> +#include <asm/unistd.h>
> +#include <linux/perf_event.h>
> +
> +#define MB			(1024 * 1024)
> +#define RESCTRL_PATH		"/sys/fs/resctrl"
> +#define PHYS_ID_PATH		"/sys/devices/system/cpu/cpu"
> +#define RESCTRL_MBM		"L3 monitoring detected"
> +#define RESCTRL_MBA		"MB allocation detected"
> +#define MAX_RESCTRL_FEATURES	2
> +#define RM_SIG_FILE		"rm -rf sig"
> +
> +#define PARENT_EXIT(err_msg)			\
> +	do {					\
> +		perror(err_msg);		\
> +		kill(ppid, SIGKILL);		\
> +		exit(EXIT_FAILURE);		\
> +	} while (0)
> +
> +pid_t bm_pid, ppid;
> +int ben_count;
> +
> +int remount_resctrlfs(bool mum_resctrlfs);
> +char get_sock_num(int cpu_no);
> +int validate_bw_report_request(char *bw_report);
> +int validate_resctrl_feature_request(char *resctrl_val);
> +int taskset_benchmark(pid_t bm_pid, int cpu_no);
> +void run_benchmark(int signum, siginfo_t *info, void *ucontext);
> +int write_schemata(char *ctrlgrp, char *schemata, int cpu_no,
> +		   char *resctrl_val);
> +int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
> +			    char *resctrl_val);
> +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);
> +
> +#endif /* RESCTRL_H */
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
> b/tools/testing/selftests/resctrl/resctrlfs.c
> new file mode 100644
> index 000000000000..d73726ef2002
> --- /dev/null
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -0,0 +1,465 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Basic resctrl file system operations
> + *
> + * Copyright (C) 2018 Intel Corporation
> + *
> + * Authors:
> + *    Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
> + *    Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
> + *    Fenghua Yu <fenghua.yu@intel.com>
> + */
> +#include "resctrl.h"
> +
> +/*
> + * remount_resctrlfs:	Remount resctrl FS at /sys/fs/resctrl
> + * @mum_resctrlfs:	Should the resctrl FS be remounted?
> + *
> + * If not mounted, mount it.
> + * If mounted and mum_resctrlfs then remount resctrl FS.
> + * If mounted and !mum_resctrlfs then noop
> + *
> + * Return: 0 on success, non-zero on failure
> + */
> +int remount_resctrlfs(bool mum_resctrlfs)
> +{
> +	DIR *dp;
> +	struct dirent *ep;
> +	unsigned int count = 0;
> +
> +	/*
> +	 * If kernel is built with CONFIG_RESCTRL, then /sys/fs/resctrl should
> +	 * be present by default
> +	 */
> +	dp = opendir(RESCTRL_PATH);
> +	if (dp) {
> +		while ((ep = readdir(dp)) != NULL)
> +			count++;
> +		closedir(dp);
> +	} else {
> +		perror("Unable to read /sys/fs/resctrl");
> +
> +		return errno;
> +	}
> +
> +	/*
> +	 * If resctrl FS has more than two entries, it means that resctrl FS has
> +	 * already been mounted. The two default entries are "." and "..",
> these
> +	 * are present even when resctrl FS is not mounted
> +	 */
> +	if (count > 2) {
> +		if (mum_resctrlfs) {
> +			if (umount(RESCTRL_PATH) != 0) {
> +				perror("Unable to umount resctrl");
> +
> +				return errno;
> +			}
> +			printf("Remount: done!\n");
> +		} else {
> +			printf("Mounted already. Not remounting!\n");
> +
> +			return 0;
> +		}
> +	}
> +
> +	if (mount("resctrl", RESCTRL_PATH, "resctrl", 0, NULL) != 0) {
> +		perror("Unable to mount resctrl FS at /sys/fs/resctrl");
> +
> +		return errno;
> +	}
> +
> +	return 0;
> +}
> +
> +int umount_resctrlfs(void)
> +{
> +	if (umount(RESCTRL_PATH)) {
> +		perror("Unable to umount resctrl");
> +
> +		return errno;
> +	}
> +
> +	return 0;
> +}
> +
> +char get_sock_num(int cpu_no)
> +{
> +	char sock_num, phys_pkg_path[1024];
> +	FILE *fp;
> +
> +	sprintf(phys_pkg_path, "%s%d/topology/physical_package_id",
> +		PHYS_ID_PATH, cpu_no);
> +	fp = fopen(phys_pkg_path, "r");

There should corresponding fclose for this. In general, I would check all the fopens in this series. I found few of the files not closed while returning.
More comments below.

> +	if (!fp || fscanf(fp, "%c", &sock_num) <= 0 || fclose(fp) == EOF) {
> +		perror("Could not get socket number");
> +
> +		return -1;
> +	}
> +
> +	return sock_num;
> +}
> +
> +/*
> + * taskset_benchmark:	Taskset PID (i.e. benchmark) to a specified
> cpu
> + * @bm_pid:		PID that should be binded
> + * @cpu_no:		CPU number at which the PID would be binded
> + *
> + * Return: 0 on success, non-zero on failure
> + */
> +int taskset_benchmark(pid_t bm_pid, int cpu_no)
> +{
> +	cpu_set_t my_set;
> +
> +	CPU_ZERO(&my_set);
> +	CPU_SET(cpu_no, &my_set);
> +
> +	if (sched_setaffinity(bm_pid, sizeof(cpu_set_t), &my_set)) {
> +		perror("Unable to taskset benchmark");
> +
> +		return -1;
> +	}
> +
> +	printf("Taskset benchmark: done!\n");
> +
> +	return 0;
> +}
> +
> +/*
> + * Run a specified benchmark or fill_buf (default benchmark). Direct
> + * benchmark stdio to /dev/null
> + */
> +void run_benchmark(int signum, siginfo_t *info, void *ucontext)
> +{
> +	char **benchmark_cmd;
> +	int span, operation, ret;
> +
> +	benchmark_cmd = info->si_ptr;
> +
> +	/*
> +	 * Direct stdio of child to /dev/null, so that only parent writes to
> +	 * stdio (console)
> +	 */
> +	if (!freopen("/dev/null", "w", stdout))
> +		PARENT_EXIT("Unable to direct BM op to /dev/null");

Do you need fclose for this before returning from this function?

> +
> +	if (strcmp(benchmark_cmd[0], "fill_buf") == 0) {
> +		/* Execute default fill_buf benchmark */
> +		span = atoi(benchmark_cmd[1]);
> +		operation = atoi(benchmark_cmd[4]);
> +		if (run_fill_buf(span, 1, 1, operation))
> +			fprintf(stderr, "Error in running fill buffer\n");
> +	} else {
> +		/* Execute specified benchmark */
> +		ret = execvp(benchmark_cmd[0], benchmark_cmd);
> +		if (ret)
> +			perror("wrong\n");
> +	}
> +
> +	PARENT_EXIT("Unable to run specified benchmark");
> +}
> +
> +/*
> + * create_con_mon_grp:	Create a con_mon group *only* if one
> doesn't exist
> + * @ctrlgrp:		Name of the con_mon group
> + * @controlgroup:	Path at which it should be created
> + *
> + * Return: 0 on success, non-zero on failure
> + */
> +static int create_con_mon_grp(const char *ctrlgrp, const char
> *controlgroup)
> +{
> +	int found_ctrl_grp = 0;
> +	struct dirent *ep;
> +	DIR *dp;
> +
> +	/*
> +	 * At this point, we are guaranteed to have resctrl FS mounted and if
> +	 * ctrlgrp == NULL, it means, user wants to use root con_mon grp, so
> do
> +	 * nothing
> +	 */
> +	if (!ctrlgrp)
> +		return 0;
> +
> +	/* Check if requested con_mon grp exists or not */
> +	dp = opendir(RESCTRL_PATH);
> +	if (dp) {
> +		while ((ep = readdir(dp)) != NULL) {
> +			if (strcmp(ep->d_name, ctrlgrp) == 0)
> +				found_ctrl_grp = 1;
> +		}
> +		closedir(dp);
> +	} else {
> +		perror("Unable to open resctrlfs for con_mon grp");
> +
> +		return errno;
> +	}
> +
> +	/* Requested con_mon grp doesn't exist, hence create it */
> +	if (found_ctrl_grp == 0) {
> +		if (mkdir(controlgroup, 0) == -1) {
> +			perror("Unable to create con_mon group");
> +
> +			return errno;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * create_mon_grp:	Create a monitor group *only* if one doesn't exist
> + * @mongrp:		Name of the monitor group
> + * @controlgroup:	Path of con_mon grp at which the mon grp will be
> created
> + *
> + * Return: 0 on success, non-zero on failure
> + */
> +static int create_mon_grp(const char *mongrp, const char *controlgroup)
> +{
> +	char monitorgroup[1024];
> +	int found_mon_grp = 0;
> +	struct dirent *ep;
> +	DIR *dp;
> +
> +	/* Check if requested mon grp exists or not */
> +	sprintf(monitorgroup, "%s/mon_groups", controlgroup);
> +	dp = opendir(monitorgroup);
> +	if (dp) {
> +		while ((ep = readdir(dp)) != NULL) {
> +			if (strcmp(ep->d_name, mongrp) == 0)
> +				found_mon_grp = 1;
> +		}
> +		closedir(dp);
> +	} else {
> +		perror("Unable to open resctrl FS for mon group");
> +
> +		return -1;
> +	}
> +
> +	/* Requested mon grp doesn't exist, hence create it */
> +	sprintf(monitorgroup, "%s/mon_groups/%s", controlgroup,
> mongrp);
> +	if (found_mon_grp == 0) {
> +		if (mkdir(monitorgroup, 0) == -1) {
> +			perror("Unable to create mon group");
> +
> +			return -1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * write_bm_pid_to_resctrl:	Write a PID (i.e. benchmark) to resctrl FS
> + * @bm_pid:			PID that should be written
> + * @ctrlgrp:			Name of the control monitor group
> (con_mon grp)
> + * @mongrp:			Name of the monitor group (mon grp)
> + * @resctrl_val:		Resctrl feature (Eg: mbm, mba.. etc)
> + *
> + * If a con_mon grp is requested, create it and write pid to it, otherwise
> + * write pid to root con_mon grp.
> + * If a mon grp is requested, create it and write pid to it, otherwise
> + * pid is not written, this means that pid is in con_mon grp and hence
> + * should consult con_mon grp's mon_data directory for results.
> + *
> + * Return: 0 on success, non-zero on failure
> + */
> +int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
> +			    char *resctrl_val)
> +{
> +	char controlgroup[1024], monitorgroup[1024];
> +	FILE *fp;
> +	int ret;
> +
> +	if (ctrlgrp)
> +		sprintf(controlgroup, "%s/%s", RESCTRL_PATH, ctrlgrp);
> +	else
> +		sprintf(controlgroup, "%s", RESCTRL_PATH);
> +
> +	ret = create_con_mon_grp(ctrlgrp, controlgroup);
> +	if (ret)
> +		return ret;
> +
> +	/* Create mon grp, only for monitoring features like "mbm" */
> +	if ((strcmp(resctrl_val, "mbm") == 0)) {
> +		if (mongrp) {
> +			ret = create_mon_grp(mongrp, controlgroup);
> +			if (ret)
> +				return ret;
> +
> +			sprintf(monitorgroup, "%s/mon_groups/%s/tasks",
> +				controlgroup, mongrp);
> +		}
> +	}
> +
> +	strcat(controlgroup, "/tasks");
> +
> +	/* Write child pid to con_mon grp */
> +	fp = fopen(controlgroup, "w");

I don't see corresponding fclose.

> +	if (!fp || fprintf(fp, "%d\n", bm_pid) <= 0 || fclose(fp) == EOF) {
> +		perror("Failed to write child to con_mon grp");
> +
> +		return errno;
> +	}
> +
> +	/* Write child pid to mon grp, only for "mbm" */
> +	if ((strcmp(resctrl_val, "mbm") == 0)) {
> +		if (mongrp) {
> +			fp = fopen(monitorgroup, "w");
> +			if (!fp || fprintf(fp, "%d\n", bm_pid) <= 0 ||
> +			    fclose(fp) == EOF) {


I feel too many checks at one place.  If fprintf fails,  will it fclose the file? I suggest to separate these checks.

> +				perror("Failed to write child to mon grp");
> +
> +				return errno;
> +			}
> +		}
> +	}
> +
> +	printf("Write benchmark to resctrl FS: done!\n");
> +
> +	return 0;
> +}
> +
> +/*
> + * write_schemata:	Update schemata of a con_mon grp
> + * @ctrlgrp:		Name of the con_mon grp
> + * @schemata:		Schemata that should be updated to
> + * @cpu_no:		CPU number that the benchmark PID is binded to
> + * @resctrl_val:	Resctrl feature (Eg: mbm, mba.. etc)
> + *
> + * Update schemata of a con_mon grp *only* if requested resctrl feature is
> + * allocation type
> + *
> + * Return: 0 on success, non-zero on failure
> + */
> +int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char
> *resctrl_val)
> +{
> +	char sock_num, controlgroup[1024], schema[1024];
> +	FILE *fp;
> +
> +	if (strcmp(resctrl_val, "mba") == 0) {
> +		if (!schemata) {
> +			printf("Schemata empty, so not updating\n");
> +
> +			return 0;
> +		}
> +		sock_num = get_sock_num(cpu_no);
> +		if (sock_num < 0)
> +			return -1;
> +
> +		if (ctrlgrp)
> +			sprintf(controlgroup, "%s/%s/schemata",
> RESCTRL_PATH,
> +				ctrlgrp);
> +		else
> +			sprintf(controlgroup, "%s/schemata",
> RESCTRL_PATH);
> +		sprintf(schema, "%s%c%c%s", "MB:", sock_num, '=',
> schemata);
> +
> +		fp = fopen(controlgroup, "w");
> +		if (!fp || fprintf(fp, "%s\n", schema) <= 0 ||
> +		    fclose(fp) == EOF) {

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

> +			perror("Unable to write schemata to con_mon grp");
> +
> +			return errno;
> +		}
> +		printf("Write schemata to resctrl FS: done!\n");
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Check if the requested feature is a valid resctrl feature or not.
> + * If yes, check if it's supported by this platform or not.
> + *
> + * Return: 0 on success, non-zero on failure
> + */
> +int validate_resctrl_feature_request(char *resctrl_val)
> +{
> +	const char *resctrl_features_list[MAX_RESCTRL_FEATURES] = {
> +			"mbm", "mba"};
> +	int resctrl_features_supported[MAX_RESCTRL_FEATURES] = {0, 0};
> +	int i, valid_resctrl_feature = -1;
> +	char line[1024];
> +	FILE *fp;
> +
> +	if (!resctrl_val) {
> +		fprintf(stderr, "resctrl feature cannot be NULL\n");
> +
> +		return -1;
> +	}
> +
> +	/* Is the resctrl feature request valid? */
> +	for (i = 0; i < MAX_RESCTRL_FEATURES; i++) {
> +		if (strcmp(resctrl_features_list[i], resctrl_val) == 0)
> +			valid_resctrl_feature = i;
> +	}
> +	if (valid_resctrl_feature == -1) {
> +		fprintf(stderr, "Not a valid resctrl feature request\n");
> +
> +		return -1;
> +	}
> +
> +	/* Enumerate resctrl features supported by this platform */
> +	if (system("dmesg > dmesg") != 0) {
> +		perror("Could not create custom dmesg file");
> +
> +		return errno;
> +	}
> +
> +	fp = fopen("dmesg", "r");
> +	if (!fp) {
> +		perror("Could not read custom created dmesg");
> +
> +		return errno;
> +	}
> +
> +	while (fgets(line, 1024, fp)) {
> +		if ((strstr(line, RESCTRL_MBM)) != NULL)
> +			resctrl_features_supported[0] = 1;
> +		if ((strstr(line, RESCTRL_MBA)) != NULL)
> +			resctrl_features_supported[1] = 1;
> +	}
> +	if (fclose(fp) == EOF) {
> +		perror("Error in closing file");
> +
> +		return errno;
> +	}
> +
> +	if (system("rm -rf dmesg") != 0)
> +		perror("Unable to remove 'dmesg' file");
> +
> +	/* Is the resctrl feature request supported? */
> +	if (!resctrl_features_supported[valid_resctrl_feature]) {
> +		fprintf(stderr, "resctrl feature not supported!");
> +
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +int validate_bw_report_request(char *bw_report)
> +{
> +	if (strcmp(bw_report, "reads") == 0)
> +		return 0;
> +	if (strcmp(bw_report, "writes") == 0)
> +		return 0;
> +	if (strcmp(bw_report, "nt-writes") == 0) {
> +		strcpy(bw_report, "writes");
> +		return 0;
> +	}
> +	if (strcmp(bw_report, "total") == 0)
> +		return 0;
> +
> +	fprintf(stderr, "Requested iMC B/W report type unavailable\n");
> +
> +	return -1;
> +}
> +
> +int perf_event_open(struct perf_event_attr *hw_event, pid_t pid, int cpu,
> +		    int group_fd, unsigned long flags)
> +{
> +	int ret;
> +
> +	ret = syscall(__NR_perf_event_open, hw_event, pid, cpu,
> +		      group_fd, flags);
> +	return ret;
> +}
> --
> 2.5.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v2 3/8] selftests/resctrl: Read memory bandwidth from perf IMC counter and from resctrl file system
  2018-10-25 23:07 ` [PATCH v2 3/8] selftests/resctrl: Read memory bandwidth from perf IMC counter and from resctrl file system Fenghua Yu
@ 2018-10-29 22:00   ` Moger, Babu
  2018-10-29 22:14   ` Moger, Babu
  1 sibling, 0 replies; 16+ messages in thread
From: Moger, Babu @ 2018-10-29 22:00 UTC (permalink / raw)
  To: Fenghua Yu, Thomas Gleixner, Ingo Molnar, H Peter Anvin,
	Tony Luck, Peter Zijlstra, Reinette Chatre, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel

Fenghua, Sai

> -----Original Message-----
> From: Fenghua Yu <fenghua.yu@intel.com>
> Sent: Thursday, October 25, 2018 6:07 PM
> To: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> <mingo@redhat.com>; H Peter Anvin <hpa@zytor.com>; Tony Luck
> <tony.luck@intel.com>; Peter Zijlstra <peterz@infradead.org>; Reinette
> Chatre <reinette.chatre@intel.com>; Moger, Babu
> <Babu.Moger@amd.com>; James Morse <james.morse@arm.com>; Ravi V
> Shankar <ravi.v.shankar@intel.com>; Sai Praneeth Prakhya
> <sai.praneeth.prakhya@intel.com>; Arshiya Hayatkhan Pathan
> <arshiya.hayatkhan.pathan@intel.com>
> Cc: linux-kernel <linux-kernel@vger.kernel.org>; Fenghua Yu
> <fenghua.yu@intel.com>
> Subject: [PATCH v2 3/8] selftests/resctrl: Read memory bandwidth from perf
> IMC counter and from resctrl file system
> 
> From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> 
> Total memory bandwidth can be monitored from perf IMC counter and from
> resctrl file system. Later the two will be compared to verify the total
> memory bandwidth read from resctrl is correct.
> 
> Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> Signed-off-by: Arshiya Hayatkhan Pathan
> <arshiya.hayatkhan.pathan@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  tools/testing/selftests/resctrl/membw.c | 403
> ++++++++++++++++++++++++++++++++
>  1 file changed, 403 insertions(+)
>  create mode 100644 tools/testing/selftests/resctrl/membw.c
> 
> diff --git a/tools/testing/selftests/resctrl/membw.c
> b/tools/testing/selftests/resctrl/membw.c
> new file mode 100644
> index 000000000000..1bced7e7f148
> --- /dev/null
> +++ b/tools/testing/selftests/resctrl/membw.c
> @@ -0,0 +1,403 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Memory bandwidth monitoring and allocation library
> + *
> + * Copyright (C) 2018 Intel Corporation
> + *
> + * Authors:
> + *    Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
> + *    Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
> + *    Fenghua Yu <fenghua.yu@intel.com>
> + */
> +#include "resctrl.h"
> +
> +#define UNCORE_IMC		"uncore_imc"
> +#define READ_FILE_NAME		"events/cas_count_read"
> +#define WRITE_FILE_NAME		"events/cas_count_write"
> +#define DYN_PMU_PATH		"/sys/bus/event_source/devices"
> +#define SCALE			0.00006103515625
> +#define MAX_IMCS		20
> +#define MAX_TOKENS		5
> +#define READ			0
> +#define WRITE			1
> +#define CON_MON_MBM_LOCAL_BYTES_PATH
> 	\
> +
> 	"%s/%s/mon_groups/%s/mon_data/mon_L3_0%c/mbm_local_byte
> s"
> +
> +#define CON_MBM_LOCAL_BYTES_PATH		\
> +	"%s/%s/mon_data/mon_L3_0%c/mbm_local_bytes"
> +
> +#define MON_MBM_LOCAL_BYTES_PATH		\
> +	"%s/mon_groups/%s/mon_data/mon_L3_0%c/mbm_local_bytes"
> +
> +#define MBM_LOCAL_BYTES_PATH			\
> +	"%s/mon_data/mon_L3_0%c/mbm_local_bytes"
> +
> +struct membw_read_format {
> +	__u64 value;         /* The value of the event */
> +	__u64 time_enabled;  /* if PERF_FORMAT_TOTAL_TIME_ENABLED
> */
> +	__u64 time_running;  /* if PERF_FORMAT_TOTAL_TIME_RUNNING
> */
> +	__u64 id;            /* if PERF_FORMAT_ID */
> +};
> +
> +struct imc_counter_config {
> +	__u32 type;
> +	__u64 event;
> +	__u64 umask;
> +	struct perf_event_attr pe;
> +	struct membw_read_format return_value;
> +	int fd;
> +};
> +
> +static struct imc_counter_config imc_counters_config[MAX_IMCS][2];
> +static char mbm_total_path[1024];
> +static int imcs;
> +
> +void membw_initialize_perf_event_attr(int i, int j)
> +{
> +	memset(&imc_counters_config[i][j].pe, 0,
> +	       sizeof(struct perf_event_attr));
> +	imc_counters_config[i][j].pe.type = imc_counters_config[i][j].type;
> +	imc_counters_config[i][j].pe.size = sizeof(struct perf_event_attr);
> +	imc_counters_config[i][j].pe.disabled = 1;
> +	imc_counters_config[i][j].pe.inherit = 1;
> +	imc_counters_config[i][j].pe.exclude_guest = 1;
> +	imc_counters_config[i][j].pe.config =
> +		imc_counters_config[i][j].umask << 8 |
> +		imc_counters_config[i][j].event;
> +	imc_counters_config[i][j].pe.sample_type =
> PERF_SAMPLE_IDENTIFIER;
> +	imc_counters_config[i][j].pe.read_format =
> +		PERF_FORMAT_TOTAL_TIME_ENABLED |
> PERF_FORMAT_TOTAL_TIME_RUNNING;
> +}
> +
> +static int open_perf_event(int i, int cpu_no, int j)
> +{
> +	imc_counters_config[i][j].fd =
> +		perf_event_open(&imc_counters_config[i][j].pe, -1,
> cpu_no, -1,
> +				PERF_FLAG_FD_CLOEXEC);
> +
> +	if (imc_counters_config[i][j].fd == -1) {
> +		fprintf(stderr, "Error opening leader %llx\n",
> +			imc_counters_config[i][j].pe.config);
> +
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +void membw_ioctl_perf_event_ioc_reset_enable(int i, int j)
> +{
> +	ioctl(imc_counters_config[i][j].fd, PERF_EVENT_IOC_RESET, 0);
> +	ioctl(imc_counters_config[i][j].fd, PERF_EVENT_IOC_ENABLE, 0);
> +}
> +
> +void membw_ioctl_perf_event_ioc_disable(int i, int j)
> +{
> +	ioctl(imc_counters_config[i][j].fd, PERF_EVENT_IOC_DISABLE, 0);
> +}
> +
> +/*
> + * get_event_and_umask:	Parse config into event and umask
> + * @cas_count_cfg:	Config
> + * @count:		iMC number
> + * @op:			Operation (read/write)
> + */
> +void get_event_and_umask(char *cas_count_cfg, int count, bool op)
> +{
> +	char *token[MAX_TOKENS];
> +	int i = 0;
> +
> +	strcat(cas_count_cfg, ",");
> +	token[0] = strtok(cas_count_cfg, "=,");
> +
> +	for (i = 1; i < MAX_TOKENS; i++)
> +		token[i] = strtok(NULL, "=,");
> +
> +	for (i = 0; i < MAX_TOKENS; i++) {
> +		if (!token[i])
> +			break;
> +		if (strcmp(token[i], "event") == 0) {
> +			if (op == READ)
> +				imc_counters_config[count][READ].event =
> +				strtol(token[i + 1], NULL, 16);
> +			else
> +				imc_counters_config[count][WRITE].event =
> +				strtol(token[i + 1], NULL, 16);
> +		}
> +		if (strcmp(token[i], "umask") == 0) {
> +			if (op == READ)
> +				imc_counters_config[count][READ].umask =
> +				strtol(token[i + 1], NULL, 16);
> +			else
> +				imc_counters_config[count][WRITE].umask =
> +				strtol(token[i + 1], NULL, 16);
> +		}
> +	}
> +}
> +
> +/* Get type and config (read and write) of an iMC counter */
> +static int read_from_imc_dir(char *imc_dir, int count)
> +{
> +	char cas_count_cfg[1024], imc_counter_cfg[1024],
> imc_counter_type[1024];
> +	FILE *fp;
> +
> +	/* Get type of iMC counter */
> +	sprintf(imc_counter_type, "%s%s", imc_dir, "type");
> +	fp = fopen(imc_counter_type, "r");
> +	if (!fp ||
> +	    fscanf(fp, "%u", &imc_counters_config[count][READ].type) <= 0 ||
> +	    fclose(fp) == EOF) {

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

> +		perror("Could not get imc type");
> +
> +		return errno;
> +	}
> +
> +	imc_counters_config[count][WRITE].type =
> +				imc_counters_config[count][READ].type;
> +
> +	/* Get read config */
> +	sprintf(imc_counter_cfg, "%s%s", imc_dir, READ_FILE_NAME);
> +	fp = fopen(imc_counter_cfg, "r");
> +	if (!fp || fscanf(fp, "%s", cas_count_cfg) <= 0 || fclose(fp) == EOF) {

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

> +		perror("Could not get imc cas count read");
> +
> +		return errno;
> +	}
> +
> +	get_event_and_umask(cas_count_cfg, count, READ);
> +
> +	/* Get write config */
> +	sprintf(imc_counter_cfg, "%s%s", imc_dir, WRITE_FILE_NAME);
> +	fp = fopen(imc_counter_cfg, "r");
> +	if (!fp || fscanf(fp, "%s", cas_count_cfg) <= 0 || fclose(fp) == EOF) {

Same comment as above.

> +		perror("Could not get imc cas count write");
> +
> +		return errno;
> +	}
> +
> +	get_event_and_umask(cas_count_cfg, count, WRITE);
> +
> +	return 0;
> +}
> +
> +/*
> + * A system can have 'n' number of iMC (Integrated Memory Controller)
> + * counters, get that 'n'. For each iMC counter get it's type and config.
> + * Also, each counter has two configs, one for read and the other for write.
> + * A config again has two parts, event and umask.
> + * Enumerate all these details into an array of structures.
> + *
> + * Return: >= 0 on success. < 0 on failure.
> + */
> +static int num_of_imcs(void)
> +{
> +	unsigned int count = 0;
> +	char imc_dir[1024];
> +	struct dirent *ep;
> +	int ret;
> +	DIR *dp;
> +
> +	dp = opendir(DYN_PMU_PATH);
> +	if (dp) {
> +		while ((ep = readdir(dp))) {
> +			if (strstr(ep->d_name, UNCORE_IMC)) {
> +				sprintf(imc_dir, "%s/%s/", DYN_PMU_PATH,
> +					ep->d_name);
> +				ret = read_from_imc_dir(imc_dir, count);
> +				if (ret)
> +					return ret;
> +
> +				count++;
> +			}
> +		}
> +		closedir(dp);
> +		if (count == 0) {
> +			perror("Unable find iMC counters!\n");
> +
> +			return -1;
> +		}
> +	} else {
> +		perror("Unable to open PMU directory!\n");
> +
> +		return -1;
> +	}
> +
> +	return count;
> +}
> +
> +static int initialize_mem_bw_imc(void)
> +{
> +	int imc, j;
> +
> +	imcs = num_of_imcs();
> +	if (imcs < 0)
> +		return imcs;
> +
> +	/* Initialize perf_event_attr structures for all iMC's */
> +	for (imc = 0; imc < imcs; imc++) {
> +		for (j = 0; j < 2; j++)
> +			membw_initialize_perf_event_attr(imc, j);
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * get_mem_bw_imc:	Memory band width as reported by iMC counters
> + * @cpu_no:		CPU number that the benchmark PID is binded to
> + * @bw_report:		Bandwidth report type (reads, writes)
> + *
> + * Memory B/W utilized by a process on a socket can be calculated using
> + * iMC counters. Perf events are used to read these counters.
> + *
> + * Return: >= 0 on success. < 0 on failure.
> + */
> +static float get_mem_bw_imc(int cpu_no, char *bw_report)
> +{
> +	float reads, writes, of_mul_read, of_mul_write;
> +	int imc, j, ret;
> +
> +	/* Start all iMC counters to log values (both read and write) */
> +	reads = 0, writes = 0, of_mul_read = 1, of_mul_write = 1;
> +	for (imc = 0; imc < imcs; imc++) {
> +		for (j = 0; j < 2; j++) {
> +			ret = open_perf_event(imc, cpu_no, j);
> +			if (ret)
> +				return -1;
> +		}
> +		for (j = 0; j < 2; j++)
> +			membw_ioctl_perf_event_ioc_reset_enable(imc, j);
> +	}
> +
> +	sleep(1);
> +
> +	/* Stop counters after a second to get results (both read and write)
> */
> +	for (imc = 0; imc < imcs; imc++) {
> +		for (j = 0; j < 2; j++)
> +			membw_ioctl_perf_event_ioc_disable(imc, j);
> +	}
> +
> +	/*
> +	 * Get results which are stored in struct type imc_counter_config
> +	 * Take over flow into consideration before calculating total b/w
> +	 */
> +	for (imc = 0; imc < imcs; imc++) {
> +		struct imc_counter_config *r =
> +			&imc_counters_config[imc][READ];
> +		struct imc_counter_config *w =
> +			&imc_counters_config[imc][WRITE];
> +
> +		if (read(r->fd, &r->return_value,
> +			 sizeof(struct membw_read_format)) == -1) {
> +			perror("Couldn't get read b/w through iMC");
> +
> +			return -1;
> +		}
> +
> +		if (read(w->fd, &w->return_value,
> +			 sizeof(struct membw_read_format)) == -1) {
> +			perror("Couldn't get write bw through iMC");
> +
> +			return -1;
> +		}
> +
> +		__u64 r_time_enabled = r->return_value.time_enabled;
> +		__u64 r_time_running = r->return_value.time_running;
> +
> +		if (r_time_enabled != r_time_running)
> +			of_mul_read = (float)r_time_enabled /
> +					(float)r_time_running;
> +
> +		__u64 w_time_enabled = w->return_value.time_enabled;
> +		__u64 w_time_running = w->return_value.time_running;
> +
> +		if (w_time_enabled != w_time_running)
> +			of_mul_write = (float)w_time_enabled /
> +					(float)w_time_running;
> +		reads += r->return_value.value * of_mul_read * SCALE;
> +		writes += w->return_value.value * of_mul_write * SCALE;
> +	}
> +
> +	for (imc = 0; imc < imcs; imc++) {
> +		close(imc_counters_config[imc][READ].fd);
> +		close(imc_counters_config[imc][WRITE].fd);
> +	}
> +
> +	if (strcmp(bw_report, "reads") == 0)
> +		return reads;
> +
> +	if (strcmp(bw_report, "writes") == 0)
> +		return writes;
> +
> +	return (reads + writes);
> +}
> +
> +void set_mbm_path(const char *ctrlgrp, const char *mongrp, char
> sock_num)
> +{
> +	if (ctrlgrp && mongrp)
> +		sprintf(mbm_total_path,
> CON_MON_MBM_LOCAL_BYTES_PATH,
> +			RESCTRL_PATH, ctrlgrp, mongrp, sock_num);
> +	else if (!ctrlgrp && mongrp)
> +		sprintf(mbm_total_path, MON_MBM_LOCAL_BYTES_PATH,
> RESCTRL_PATH,
> +			mongrp, sock_num);
> +	else if (ctrlgrp && !mongrp)
> +		sprintf(mbm_total_path, CON_MBM_LOCAL_BYTES_PATH,
> RESCTRL_PATH,
> +			ctrlgrp, sock_num);
> +	else if (!ctrlgrp && !mongrp)
> +		sprintf(mbm_total_path, MBM_LOCAL_BYTES_PATH,
> RESCTRL_PATH,
> +			sock_num);
> +}
> +
> +/*
> + * initialize_mem_bw_resctrl:	Appropriately populate "mbm_total_path"
> + * @ctrlgrp:			Name of the control monitor group
> (con_mon grp)
> + * @mongrp:			Name of the monitor group (mon grp)
> + * @cpu_no:			CPU number that the benchmark PID is
> binded to
> + * @resctrl_val:		Resctrl feature (Eg: mbm, mba.. etc)
> + */
> +static void initialize_mem_bw_resctrl(const char *ctrlgrp, const char
> *mongrp,
> +				      int cpu_no, char *resctrl_val)
> +{
> +	char sock_num;
> +
> +	sock_num = get_sock_num(cpu_no);
> +	if (sock_num < 0)
> +		return;
> +
> +	if (strcmp(resctrl_val, "mbm") == 0)
> +		set_mbm_path(ctrlgrp, mongrp, sock_num);
> +
> +	if ((strcmp(resctrl_val, "mba") == 0)) {
> +		if (ctrlgrp)
> +			sprintf(mbm_total_path,
> CON_MBM_LOCAL_BYTES_PATH,
> +				RESCTRL_PATH, ctrlgrp, sock_num);
> +		else
> +			sprintf(mbm_total_path, MBM_LOCAL_BYTES_PATH,
> +				RESCTRL_PATH, sock_num);
> +	}
> +}
> +
> +/*
> + * Get MBM Local bytes as reported by resctrl FS
> + * For MBM,
> + * 1. If con_mon grp and mon grp are given, then read from con_mon grp's
> mon grp
> + * 2. If only con_mon grp is given, then read from con_mon grp
> + * 3. If both are not given, then read from root con_mon grp
> + * For MBA,
> + * 1. If con_mon grp is given, then read from it
> + * 2. If con_mon grp is not given, then read from root con_mon grp
> + */
> +static unsigned long get_mem_bw_resctrl(void)
> +{
> +	unsigned long mbm_total = 0;
> +	FILE *fp;
> +
> +	fp = fopen(mbm_total_path, "r");
> +	if (!fp || fscanf(fp, "%lu", &mbm_total) <= 0 || fclose(fp) == EOF) {

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

> +		perror("Could not get mbm local bytes");
> +
> +		return -1;
> +	}
> +
> +	return mbm_total;
> +}
> --
> 2.5.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v2 4/8] selftests/resctrl: Add callback to start a benchmark
  2018-10-25 23:07 ` [PATCH v2 4/8] selftests/resctrl: Add callback to start a benchmark Fenghua Yu
@ 2018-10-29 22:03   ` Moger, Babu
  0 siblings, 0 replies; 16+ messages in thread
From: Moger, Babu @ 2018-10-29 22:03 UTC (permalink / raw)
  To: Fenghua Yu, Thomas Gleixner, Ingo Molnar, H Peter Anvin,
	Tony Luck, Peter Zijlstra, Reinette Chatre, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel

Hi Fenghua/Sai,


> -----Original Message-----
> From: Fenghua Yu <fenghua.yu@intel.com>
> Sent: Thursday, October 25, 2018 6:07 PM
> To: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> <mingo@redhat.com>; H Peter Anvin <hpa@zytor.com>; Tony Luck
> <tony.luck@intel.com>; Peter Zijlstra <peterz@infradead.org>; Reinette
> Chatre <reinette.chatre@intel.com>; Moger, Babu
> <Babu.Moger@amd.com>; James Morse <james.morse@arm.com>; Ravi V
> Shankar <ravi.v.shankar@intel.com>; Sai Praneeth Prakhya
> <sai.praneeth.prakhya@intel.com>; Arshiya Hayatkhan Pathan
> <arshiya.hayatkhan.pathan@intel.com>
> Cc: linux-kernel <linux-kernel@vger.kernel.org>; Fenghua Yu
> <fenghua.yu@intel.com>
> Subject: [PATCH v2 4/8] selftests/resctrl: Add callback to start a benchmark
> 
> From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.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@intel.com>
> Signed-off-by: Arshiya Hayatkhan Pathan
> <arshiya.hayatkhan.pathan@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v2 3/8] selftests/resctrl: Read memory bandwidth from perf IMC counter and from resctrl file system
  2018-10-25 23:07 ` [PATCH v2 3/8] selftests/resctrl: Read memory bandwidth from perf IMC counter and from resctrl file system Fenghua Yu
  2018-10-29 22:00   ` Moger, Babu
@ 2018-10-29 22:14   ` Moger, Babu
  1 sibling, 0 replies; 16+ messages in thread
From: Moger, Babu @ 2018-10-29 22:14 UTC (permalink / raw)
  To: Fenghua Yu, Thomas Gleixner, Ingo Molnar, H Peter Anvin,
	Tony Luck, Peter Zijlstra, Reinette Chatre, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel

Fenghua/Sai,

> -----Original Message-----
> From: Fenghua Yu <fenghua.yu@intel.com>
> Sent: Thursday, October 25, 2018 6:07 PM
> To: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> <mingo@redhat.com>; H Peter Anvin <hpa@zytor.com>; Tony Luck
> <tony.luck@intel.com>; Peter Zijlstra <peterz@infradead.org>; Reinette
> Chatre <reinette.chatre@intel.com>; Moger, Babu
> <Babu.Moger@amd.com>; James Morse <james.morse@arm.com>; Ravi V
> Shankar <ravi.v.shankar@intel.com>; Sai Praneeth Prakhya
> <sai.praneeth.prakhya@intel.com>; Arshiya Hayatkhan Pathan
> <arshiya.hayatkhan.pathan@intel.com>
> Cc: linux-kernel <linux-kernel@vger.kernel.org>; Fenghua Yu
> <fenghua.yu@intel.com>
> Subject: [PATCH v2 3/8] selftests/resctrl: Read memory bandwidth from perf
> IMC counter and from resctrl file system
> 
> From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> 
> Total memory bandwidth can be monitored from perf IMC counter and from
> resctrl file system. Later the two will be compared to verify the total
> memory bandwidth read from resctrl is correct.
> 
> Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> Signed-off-by: Arshiya Hayatkhan Pathan
> <arshiya.hayatkhan.pathan@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  tools/testing/selftests/resctrl/membw.c | 403
> ++++++++++++++++++++++++++++++++
>  1 file changed, 403 insertions(+)
>  create mode 100644 tools/testing/selftests/resctrl/membw.c
> 
> diff --git a/tools/testing/selftests/resctrl/membw.c
> b/tools/testing/selftests/resctrl/membw.c
> new file mode 100644
> index 000000000000..1bced7e7f148
> --- /dev/null
> +++ b/tools/testing/selftests/resctrl/membw.c
> @@ -0,0 +1,403 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Memory bandwidth monitoring and allocation library
> + *
> + * Copyright (C) 2018 Intel Corporation
> + *
> + * Authors:
> + *    Arshiya Hayatkhan Pathan <arshiya.hayatkhan.pathan@intel.com>
> + *    Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
> + *    Fenghua Yu <fenghua.yu@intel.com>
> + */
> +#include "resctrl.h"
> +
> +#define UNCORE_IMC		"uncore_imc"
> +#define READ_FILE_NAME		"events/cas_count_read"
> +#define WRITE_FILE_NAME		"events/cas_count_write"
> +#define DYN_PMU_PATH		"/sys/bus/event_source/devices"
> +#define SCALE			0.00006103515625
> +#define MAX_IMCS		20
> +#define MAX_TOKENS		5
> +#define READ			0
> +#define WRITE			1
> +#define CON_MON_MBM_LOCAL_BYTES_PATH
> 	\
> +
> 	"%s/%s/mon_groups/%s/mon_data/mon_L3_0%c/mbm_local_byte
> s"
> +
> +#define CON_MBM_LOCAL_BYTES_PATH		\
> +	"%s/%s/mon_data/mon_L3_0%c/mbm_local_bytes"
> +
> +#define MON_MBM_LOCAL_BYTES_PATH		\
> +	"%s/mon_groups/%s/mon_data/mon_L3_0%c/mbm_local_bytes"
> +
> +#define MBM_LOCAL_BYTES_PATH			\
> +	"%s/mon_data/mon_L3_0%c/mbm_local_bytes"
> +
> +struct membw_read_format {
> +	__u64 value;         /* The value of the event */
> +	__u64 time_enabled;  /* if PERF_FORMAT_TOTAL_TIME_ENABLED
> */
> +	__u64 time_running;  /* if PERF_FORMAT_TOTAL_TIME_RUNNING
> */
> +	__u64 id;            /* if PERF_FORMAT_ID */
> +};
> +
> +struct imc_counter_config {
> +	__u32 type;
> +	__u64 event;
> +	__u64 umask;
> +	struct perf_event_attr pe;
> +	struct membw_read_format return_value;
> +	int fd;
> +};
> +
> +static struct imc_counter_config imc_counters_config[MAX_IMCS][2];
> +static char mbm_total_path[1024];
> +static int imcs;
> +
> +void membw_initialize_perf_event_attr(int i, int j)
> +{
> +	memset(&imc_counters_config[i][j].pe, 0,
> +	       sizeof(struct perf_event_attr));
> +	imc_counters_config[i][j].pe.type = imc_counters_config[i][j].type;
> +	imc_counters_config[i][j].pe.size = sizeof(struct perf_event_attr);
> +	imc_counters_config[i][j].pe.disabled = 1;
> +	imc_counters_config[i][j].pe.inherit = 1;
> +	imc_counters_config[i][j].pe.exclude_guest = 1;
> +	imc_counters_config[i][j].pe.config =
> +		imc_counters_config[i][j].umask << 8 |
> +		imc_counters_config[i][j].event;
> +	imc_counters_config[i][j].pe.sample_type =
> PERF_SAMPLE_IDENTIFIER;
> +	imc_counters_config[i][j].pe.read_format =
> +		PERF_FORMAT_TOTAL_TIME_ENABLED |
> PERF_FORMAT_TOTAL_TIME_RUNNING;
> +}
> +
> +static int open_perf_event(int i, int cpu_no, int j)
> +{
> +	imc_counters_config[i][j].fd =
> +		perf_event_open(&imc_counters_config[i][j].pe, -1,
> cpu_no, -1,
> +				PERF_FLAG_FD_CLOEXEC);
> +
> +	if (imc_counters_config[i][j].fd == -1) {
> +		fprintf(stderr, "Error opening leader %llx\n",
> +			imc_counters_config[i][j].pe.config);
> +
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +void membw_ioctl_perf_event_ioc_reset_enable(int i, int j)
> +{
> +	ioctl(imc_counters_config[i][j].fd, PERF_EVENT_IOC_RESET, 0);
> +	ioctl(imc_counters_config[i][j].fd, PERF_EVENT_IOC_ENABLE, 0);
> +}
> +
> +void membw_ioctl_perf_event_ioc_disable(int i, int j)
> +{
> +	ioctl(imc_counters_config[i][j].fd, PERF_EVENT_IOC_DISABLE, 0);
> +}
> +
> +/*
> + * get_event_and_umask:	Parse config into event and umask
> + * @cas_count_cfg:	Config
> + * @count:		iMC number
> + * @op:			Operation (read/write)
> + */
> +void get_event_and_umask(char *cas_count_cfg, int count, bool op)
> +{
> +	char *token[MAX_TOKENS];
> +	int i = 0;
> +
> +	strcat(cas_count_cfg, ",");
> +	token[0] = strtok(cas_count_cfg, "=,");
> +
> +	for (i = 1; i < MAX_TOKENS; i++)
> +		token[i] = strtok(NULL, "=,");
> +
> +	for (i = 0; i < MAX_TOKENS; i++) {
> +		if (!token[i])
> +			break;
> +		if (strcmp(token[i], "event") == 0) {
> +			if (op == READ)
> +				imc_counters_config[count][READ].event =
> +				strtol(token[i + 1], NULL, 16);
> +			else
> +				imc_counters_config[count][WRITE].event =
> +				strtol(token[i + 1], NULL, 16);
> +		}
> +		if (strcmp(token[i], "umask") == 0) {
> +			if (op == READ)
> +				imc_counters_config[count][READ].umask =
> +				strtol(token[i + 1], NULL, 16);
> +			else
> +				imc_counters_config[count][WRITE].umask =
> +				strtol(token[i + 1], NULL, 16);
> +		}
> +	}
> +}
> +
> +/* Get type and config (read and write) of an iMC counter */
> +static int read_from_imc_dir(char *imc_dir, int count)
> +{
> +	char cas_count_cfg[1024], imc_counter_cfg[1024],
> imc_counter_type[1024];
> +	FILE *fp;
> +
> +	/* Get type of iMC counter */
> +	sprintf(imc_counter_type, "%s%s", imc_dir, "type");
> +	fp = fopen(imc_counter_type, "r");
> +	if (!fp ||
> +	    fscanf(fp, "%u", &imc_counters_config[count][READ].type) <= 0 ||
> +	    fclose(fp) == EOF) {
> +		perror("Could not get imc type");
> +
> +		return errno;
> +	}
> +
> +	imc_counters_config[count][WRITE].type =
> +				imc_counters_config[count][READ].type;
> +
> +	/* Get read config */
> +	sprintf(imc_counter_cfg, "%s%s", imc_dir, READ_FILE_NAME);
> +	fp = fopen(imc_counter_cfg, "r");
> +	if (!fp || fscanf(fp, "%s", cas_count_cfg) <= 0 || fclose(fp) == EOF) {
> +		perror("Could not get imc cas count read");
> +
> +		return errno;
> +	}
> +
> +	get_event_and_umask(cas_count_cfg, count, READ);
> +
> +	/* Get write config */
> +	sprintf(imc_counter_cfg, "%s%s", imc_dir, WRITE_FILE_NAME);
> +	fp = fopen(imc_counter_cfg, "r");
> +	if (!fp || fscanf(fp, "%s", cas_count_cfg) <= 0 || fclose(fp) == EOF) {
> +		perror("Could not get imc cas count write");
> +
> +		return errno;
> +	}
> +
> +	get_event_and_umask(cas_count_cfg, count, WRITE);
> +
> +	return 0;
> +}
> +
> +/*
> + * A system can have 'n' number of iMC (Integrated Memory Controller)
> + * counters, get that 'n'. For each iMC counter get it's type and config.
> + * Also, each counter has two configs, one for read and the other for write.
> + * A config again has two parts, event and umask.
> + * Enumerate all these details into an array of structures.
> + *
> + * Return: >= 0 on success. < 0 on failure.
> + */
> +static int num_of_imcs(void)
> +{
> +	unsigned int count = 0;
> +	char imc_dir[1024];
> +	struct dirent *ep;
> +	int ret;
> +	DIR *dp;
> +
> +	dp = opendir(DYN_PMU_PATH);
> +	if (dp) {
> +		while ((ep = readdir(dp))) {
> +			if (strstr(ep->d_name, UNCORE_IMC)) {
> +				sprintf(imc_dir, "%s/%s/", DYN_PMU_PATH,
> +					ep->d_name);
> +				ret = read_from_imc_dir(imc_dir, count);
> +				if (ret)
> +					return ret;

Missed this earlier. If read_from_imc_dir fails, then this could return without doing "closedir"

> +
> +				count++;
> +			}
> +		}
> +		closedir(dp);
> +		if (count == 0) {
> +			perror("Unable find iMC counters!\n");
> +
> +			return -1;
> +		}
> +	} else {
> +		perror("Unable to open PMU directory!\n");
> +
> +		return -1;
> +	}
> +
> +	return count;
> +}
> +
> +static int initialize_mem_bw_imc(void)
> +{
> +	int imc, j;
> +
> +	imcs = num_of_imcs();
> +	if (imcs < 0)
> +		return imcs;
> +
> +	/* Initialize perf_event_attr structures for all iMC's */
> +	for (imc = 0; imc < imcs; imc++) {
> +		for (j = 0; j < 2; j++)
> +			membw_initialize_perf_event_attr(imc, j);
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * get_mem_bw_imc:	Memory band width as reported by iMC counters
> + * @cpu_no:		CPU number that the benchmark PID is binded to
> + * @bw_report:		Bandwidth report type (reads, writes)
> + *
> + * Memory B/W utilized by a process on a socket can be calculated using
> + * iMC counters. Perf events are used to read these counters.
> + *
> + * Return: >= 0 on success. < 0 on failure.
> + */
> +static float get_mem_bw_imc(int cpu_no, char *bw_report)
> +{
> +	float reads, writes, of_mul_read, of_mul_write;
> +	int imc, j, ret;
> +
> +	/* Start all iMC counters to log values (both read and write) */
> +	reads = 0, writes = 0, of_mul_read = 1, of_mul_write = 1;
> +	for (imc = 0; imc < imcs; imc++) {
> +		for (j = 0; j < 2; j++) {
> +			ret = open_perf_event(imc, cpu_no, j);
> +			if (ret)
> +				return -1;
> +		}
> +		for (j = 0; j < 2; j++)
> +			membw_ioctl_perf_event_ioc_reset_enable(imc, j);
> +	}
> +
> +	sleep(1);
> +
> +	/* Stop counters after a second to get results (both read and write)
> */
> +	for (imc = 0; imc < imcs; imc++) {
> +		for (j = 0; j < 2; j++)
> +			membw_ioctl_perf_event_ioc_disable(imc, j);
> +	}
> +
> +	/*
> +	 * Get results which are stored in struct type imc_counter_config
> +	 * Take over flow into consideration before calculating total b/w
> +	 */
> +	for (imc = 0; imc < imcs; imc++) {
> +		struct imc_counter_config *r =
> +			&imc_counters_config[imc][READ];
> +		struct imc_counter_config *w =
> +			&imc_counters_config[imc][WRITE];
> +
> +		if (read(r->fd, &r->return_value,
> +			 sizeof(struct membw_read_format)) == -1) {
> +			perror("Couldn't get read b/w through iMC");
> +
> +			return -1;
> +		}
> +
> +		if (read(w->fd, &w->return_value,
> +			 sizeof(struct membw_read_format)) == -1) {
> +			perror("Couldn't get write bw through iMC");
> +
> +			return -1;
> +		}
> +
> +		__u64 r_time_enabled = r->return_value.time_enabled;
> +		__u64 r_time_running = r->return_value.time_running;
> +
> +		if (r_time_enabled != r_time_running)
> +			of_mul_read = (float)r_time_enabled /
> +					(float)r_time_running;
> +
> +		__u64 w_time_enabled = w->return_value.time_enabled;
> +		__u64 w_time_running = w->return_value.time_running;
> +
> +		if (w_time_enabled != w_time_running)
> +			of_mul_write = (float)w_time_enabled /
> +					(float)w_time_running;
> +		reads += r->return_value.value * of_mul_read * SCALE;
> +		writes += w->return_value.value * of_mul_write * SCALE;
> +	}
> +
> +	for (imc = 0; imc < imcs; imc++) {
> +		close(imc_counters_config[imc][READ].fd);
> +		close(imc_counters_config[imc][WRITE].fd);
> +	}
> +
> +	if (strcmp(bw_report, "reads") == 0)
> +		return reads;
> +
> +	if (strcmp(bw_report, "writes") == 0)
> +		return writes;
> +
> +	return (reads + writes);
> +}
> +
> +void set_mbm_path(const char *ctrlgrp, const char *mongrp, char
> sock_num)
> +{
> +	if (ctrlgrp && mongrp)
> +		sprintf(mbm_total_path,
> CON_MON_MBM_LOCAL_BYTES_PATH,
> +			RESCTRL_PATH, ctrlgrp, mongrp, sock_num);
> +	else if (!ctrlgrp && mongrp)
> +		sprintf(mbm_total_path, MON_MBM_LOCAL_BYTES_PATH,
> RESCTRL_PATH,
> +			mongrp, sock_num);
> +	else if (ctrlgrp && !mongrp)
> +		sprintf(mbm_total_path, CON_MBM_LOCAL_BYTES_PATH,
> RESCTRL_PATH,
> +			ctrlgrp, sock_num);
> +	else if (!ctrlgrp && !mongrp)
> +		sprintf(mbm_total_path, MBM_LOCAL_BYTES_PATH,
> RESCTRL_PATH,
> +			sock_num);
> +}
> +
> +/*
> + * initialize_mem_bw_resctrl:	Appropriately populate "mbm_total_path"
> + * @ctrlgrp:			Name of the control monitor group
> (con_mon grp)
> + * @mongrp:			Name of the monitor group (mon grp)
> + * @cpu_no:			CPU number that the benchmark PID is
> binded to
> + * @resctrl_val:		Resctrl feature (Eg: mbm, mba.. etc)
> + */
> +static void initialize_mem_bw_resctrl(const char *ctrlgrp, const char
> *mongrp,
> +				      int cpu_no, char *resctrl_val)
> +{
> +	char sock_num;
> +
> +	sock_num = get_sock_num(cpu_no);
> +	if (sock_num < 0)
> +		return;
> +
> +	if (strcmp(resctrl_val, "mbm") == 0)
> +		set_mbm_path(ctrlgrp, mongrp, sock_num);
> +
> +	if ((strcmp(resctrl_val, "mba") == 0)) {
> +		if (ctrlgrp)
> +			sprintf(mbm_total_path,
> CON_MBM_LOCAL_BYTES_PATH,
> +				RESCTRL_PATH, ctrlgrp, sock_num);
> +		else
> +			sprintf(mbm_total_path, MBM_LOCAL_BYTES_PATH,
> +				RESCTRL_PATH, sock_num);
> +	}
> +}
> +
> +/*
> + * Get MBM Local bytes as reported by resctrl FS
> + * For MBM,
> + * 1. If con_mon grp and mon grp are given, then read from con_mon grp's
> mon grp
> + * 2. If only con_mon grp is given, then read from con_mon grp
> + * 3. If both are not given, then read from root con_mon grp
> + * For MBA,
> + * 1. If con_mon grp is given, then read from it
> + * 2. If con_mon grp is not given, then read from root con_mon grp
> + */
> +static unsigned long get_mem_bw_resctrl(void)
> +{
> +	unsigned long mbm_total = 0;
> +	FILE *fp;
> +
> +	fp = fopen(mbm_total_path, "r");
> +	if (!fp || fscanf(fp, "%lu", &mbm_total) <= 0 || fclose(fp) == EOF) {
> +		perror("Could not get mbm local bytes");
> +
> +		return -1;
> +	}
> +
> +	return mbm_total;
> +}
> --
> 2.5.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest
  2018-10-25 23:06 [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Fenghua Yu
                   ` (7 preceding siblings ...)
  2018-10-25 23:07 ` [PATCH v2 8/8] selftests/resctrl: Add the test in MAINTAINERS Fenghua Yu
@ 2018-10-29 22:56 ` Moger, Babu
  8 siblings, 0 replies; 16+ messages in thread
From: Moger, Babu @ 2018-10-29 22:56 UTC (permalink / raw)
  To: Fenghua Yu, Thomas Gleixner, Ingo Molnar, H Peter Anvin,
	Tony Luck, Peter Zijlstra, Reinette Chatre, James Morse,
	Ravi V Shankar, Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan
  Cc: linux-kernel

Hi Fenghua, 

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org <linux-kernel-
> owner@vger.kernel.org> On Behalf Of Fenghua Yu
> Sent: Thursday, October 25, 2018 6:07 PM
> To: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> <mingo@redhat.com>; H Peter Anvin <hpa@zytor.com>; Tony Luck
> <tony.luck@intel.com>; Peter Zijlstra <peterz@infradead.org>; Reinette
> Chatre <reinette.chatre@intel.com>; Moger, Babu
> <Babu.Moger@amd.com>; James Morse <james.morse@arm.com>; Ravi V
> Shankar <ravi.v.shankar@intel.com>; Sai Praneeth Prakhya
> <sai.praneeth.prakhya@intel.com>; Arshiya Hayatkhan Pathan
> <arshiya.hayatkhan.pathan@intel.com>
> Cc: linux-kernel <linux-kernel@vger.kernel.org>; Fenghua Yu
> <fenghua.yu@intel.com>
> Subject: [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest
> 
> With more and more resctrl features are being added by Intel, AMD
> and ARM, a test tool is becoming more and more useful to validate
> that both hardware and software functionalities work as expected.
> 
> We introduce resctrl selftest to cover resctrl features on both
> X86 and ARM architectures. It first implements MBM (Memory Bandwidth
> Monitoring) and MBA (Memory Bandwidth Allocation) tests. We can
> enhance
> the selftest tool to include more functionality tests in future.
> 
> There is an existing resctrl test suit 'intel_cmt_cat'. But the major
> purpose of the tool is to test Intel(R) RDT hardware via writing and
> reading MSR registers. It does access resctrl file system; but the
> functionalities are very limited. And it doesn't support automatic test
> and a lot of manual verifications are involved.
> 
> So the selftest tool we are introducing here provides a convenient
> tool which does automatic resctrl testing, is easily available in kernel
> tree, and will be extended to AMD QoS and ARM MPAM.
> 
> The selftest tool is in tools/testing/selftests/resctrl in order to have
> generic test code for all architectures.
> 
> Changelog:
> v2:
> - Change code based on comments from Babu Moger
> - Clean up other places.
> 
> Arshiya Hayatkhan Pathan (2):
>   selftests/resctrl: Add mbm test
>   selftests/resctrl: Add mba test

I suggest to use MBM and MBA(all caps) while talking about these features.  Same applies in each individual patches.

> 
> Fenghua Yu (2):
>   selftests/resctrl: Add README for resctrl tests
>   selftests/resctrl: Add the test in MAINTAINERS
> 
> Sai Praneeth Prakhya (4):
>   selftests/resctrl: Add basic resctrl file system operations and data
>   selftests/resctrl: Read memory bandwidth from perf IMC counter and
>     from resctrl file system
>   selftests/resctrl: Add callback to start a benchmark
>   selftests/resctrl: Add built in benchmark
> 
>  MAINTAINERS                                     |   1 +
>  tools/testing/selftests/resctrl/Makefile        |  16 +
>  tools/testing/selftests/resctrl/README          |  53 ++
>  tools/testing/selftests/resctrl/fill_buf.c      | 175 ++++++
>  tools/testing/selftests/resctrl/mba_test.c      | 175 ++++++
>  tools/testing/selftests/resctrl/mbm_test.c      | 143 +++++
>  tools/testing/selftests/resctrl/membw.c         | 678
> ++++++++++++++++++++++++
>  tools/testing/selftests/resctrl/resctrl.h       |  88 +++
>  tools/testing/selftests/resctrl/resctrl_tests.c | 138 +++++
>  tools/testing/selftests/resctrl/resctrlfs.c     | 465 ++++++++++++++++
>  10 files changed, 1932 insertions(+)
>  create mode 100644 tools/testing/selftests/resctrl/Makefile
>  create mode 100644 tools/testing/selftests/resctrl/README
>  create mode 100644 tools/testing/selftests/resctrl/fill_buf.c
>  create mode 100644 tools/testing/selftests/resctrl/mba_test.c
>  create mode 100644 tools/testing/selftests/resctrl/mbm_test.c
>  create mode 100644 tools/testing/selftests/resctrl/membw.c
>  create mode 100644 tools/testing/selftests/resctrl/resctrl.h
>  create mode 100644 tools/testing/selftests/resctrl/resctrl_tests.c
>  create mode 100644 tools/testing/selftests/resctrl/resctrlfs.c
> 
> --
> 2.5.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Fwd: [PATCH v2 2/8] selftests/resctrl: Add basic resctrl file system operations and data
       [not found]     ` <CALBSrqDRKbth2NAUFM+emBHVv7eZPX=O9Ki-=vUxi-QQc1wVnQ@mail.gmail.com>
@ 2018-10-30 18:35       ` Fenghua Yu
  2018-10-30 22:26         ` Moger, Babu
  0 siblings, 1 reply; 16+ messages in thread
From: Fenghua Yu @ 2018-10-30 18:35 UTC (permalink / raw)
  To: Babu Moger, Fenghua Yu
  Cc: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Tony Luck,
	Peter Zijlstra, Reinette Chatre, James Morse, Ravi V Shankar,
	Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan, linux-kernel

> From: Moger, Babu <Babu.Moger@amd.com>
> > From: Fenghua Yu <fenghua.yu@intel.com>
> > From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> >
> > The basic resctrl file system operations and data are added for future
> > usage by resctrl selftest tool.
> >
> > +     return 0;
> > +}
> > +
> > +char get_sock_num(int cpu_no)
> > +{
> > +     char sock_num, phys_pkg_path[1024];
> > +     FILE *fp;
> > +
> > +     sprintf(phys_pkg_path, "%s%d/topology/physical_package_id",
> > +             PHYS_ID_PATH, cpu_no);
> > +     fp = fopen(phys_pkg_path, "r");
> 
> There should corresponding fclose for this. In general, I would check all the
> fopens in this series. I found few of the files not closed while returning.
> More comments below.
> 
> > +     if (!fp || fscanf(fp, "%c", &sock_num) <= 0 || fclose(fp) == EOF) {

fclose is here.

> > +             perror("Could not get socket number");
> > +
> > +             return -1;
> > +     }
> > +
> > +      */
> > +     if (!freopen("/dev/null", "w", stdout))
> > +             PARENT_EXIT("Unable to direct BM op to /dev/null");
> 
> Do you need fclose for this before returning from this function?

This fclose is missing. I will add it.

> > +     /* Write child pid to con_mon grp */
> > +     fp = fopen(controlgroup, "w");
> 
> I don't see corresponding fclose.
> 
> > +     if (!fp || fprintf(fp, "%d\n", bm_pid) <= 0 || fclose(fp) == EOF) {

fclose is here:)

> > +             perror("Failed to write child to con_mon grp");
> > +
> > +             return errno;
> > +     }
> > +
> > +     /* Write child pid to mon grp, only for "mbm" */
> > +     if ((strcmp(resctrl_val, "mbm") == 0)) {
> > +             if (mongrp) {
> > +                     fp = fopen(monitorgroup, "w");
> > +                     if (!fp || fprintf(fp, "%d\n", bm_pid) <= 0 ||
> > +                         fclose(fp) == EOF) {
> 
> 
> I feel too many checks at one place.  If fprintf fails,  will it fclose the
> file? I suggest to separate these checks.

You are right. I will separate the checks in multiple lines.

> 
> > +
> > +             fp = fopen(controlgroup, "w");
> > +             if (!fp || fprintf(fp, "%s\n", schema) <= 0 ||
> > +                 fclose(fp) == EOF) {
> 
> Same comment as above.. If fprintf fails,  will it fclose the file? I suggest
> to separate these checks.

Sure.

I will change code based on your comments.

Thanks.

-Fenghua

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: Fwd: [PATCH v2 2/8] selftests/resctrl: Add basic resctrl file system operations and data
  2018-10-30 18:35       ` Fwd: " Fenghua Yu
@ 2018-10-30 22:26         ` Moger, Babu
  0 siblings, 0 replies; 16+ messages in thread
From: Moger, Babu @ 2018-10-30 22:26 UTC (permalink / raw)
  To: Fenghua Yu
  Cc: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Tony Luck,
	Peter Zijlstra, Reinette Chatre, James Morse, Ravi V Shankar,
	Sai Praneeth Prakhya, Arshiya Hayatkhan Pathan, linux-kernel

Fenghua,

> -----Original Message-----
> From: Fenghua Yu <fenghua.yu@intel.com>
> Sent: Tuesday, October 30, 2018 1:36 PM
> To: Moger, Babu <Babu.Moger@amd.com>; Fenghua Yu
> <fenghua.yu@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> <mingo@redhat.com>; H Peter Anvin <hpa@zytor.com>; Tony Luck
> <tony.luck@intel.com>; Peter Zijlstra <peterz@infradead.org>; Reinette
> Chatre <reinette.chatre@intel.com>; James Morse
> <james.morse@arm.com>; Ravi V Shankar <ravi.v.shankar@intel.com>; Sai
> Praneeth Prakhya <sai.praneeth.prakhya@intel.com>; Arshiya Hayatkhan
> Pathan <arshiya.hayatkhan.pathan@intel.com>; linux-kernel <linux-
> kernel@vger.kernel.org>
> Subject: Re: Fwd: [PATCH v2 2/8] selftests/resctrl: Add basic resctrl file
> system operations and data
> 
> > From: Moger, Babu <Babu.Moger@amd.com>
> > > From: Fenghua Yu <fenghua.yu@intel.com>
> > > From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> > >
> > > The basic resctrl file system operations and data are added for future
> > > usage by resctrl selftest tool.
> > >
> > > +     return 0;
> > > +}
> > > +
> > > +char get_sock_num(int cpu_no)
> > > +{
> > > +     char sock_num, phys_pkg_path[1024];
> > > +     FILE *fp;
> > > +
> > > +     sprintf(phys_pkg_path, "%s%d/topology/physical_package_id",
> > > +             PHYS_ID_PATH, cpu_no);
> > > +     fp = fopen(phys_pkg_path, "r");
> >
> > There should corresponding fclose for this. In general, I would check all the
> > fopens in this series. I found few of the files not closed while returning.
> > More comments below.
> >
> > > +     if (!fp || fscanf(fp, "%c", &sock_num) <= 0 || fclose(fp) == EOF) {
> 
> fclose is here.

Yes. It is there. Let's take a case(hypothetical) where fp is valid and fscanf returns -1.

             If( false || true || fclose(fp))

Will this execute fclose?  Or am I missing something here?


> 
> > > +             perror("Could not get socket number");
> > > +
> > > +             return -1;
> > > +     }
> > > +
> > > +      */
> > > +     if (!freopen("/dev/null", "w", stdout))
> > > +             PARENT_EXIT("Unable to direct BM op to /dev/null");
> >
> > Do you need fclose for this before returning from this function?
> 
> This fclose is missing. I will add it.
> 
> > > +     /* Write child pid to con_mon grp */
> > > +     fp = fopen(controlgroup, "w");
> >
> > I don't see corresponding fclose.
> >
> > > +     if (!fp || fprintf(fp, "%d\n", bm_pid) <= 0 || fclose(fp) == EOF) {
> 
> fclose is here:)
> 
> > > +             perror("Failed to write child to con_mon grp");
> > > +
> > > +             return errno;
> > > +     }
> > > +
> > > +     /* Write child pid to mon grp, only for "mbm" */
> > > +     if ((strcmp(resctrl_val, "mbm") == 0)) {
> > > +             if (mongrp) {
> > > +                     fp = fopen(monitorgroup, "w");
> > > +                     if (!fp || fprintf(fp, "%d\n", bm_pid) <= 0 ||
> > > +                         fclose(fp) == EOF) {
> >
> >
> > I feel too many checks at one place.  If fprintf fails,  will it fclose the
> > file? I suggest to separate these checks.
> 
> You are right. I will separate the checks in multiple lines.
> 
> >
> > > +
> > > +             fp = fopen(controlgroup, "w");
> > > +             if (!fp || fprintf(fp, "%s\n", schema) <= 0 ||
> > > +                 fclose(fp) == EOF) {
> >
> > Same comment as above.. If fprintf fails,  will it fclose the file? I suggest
> > to separate these checks.
> 
> Sure.
> 
> I will change code based on your comments.
> 
> Thanks.
> 
> -Fenghua

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-10-30 22:26 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-25 23:06 [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Fenghua Yu
2018-10-25 23:06 ` [PATCH v2 1/8] selftests/resctrl: Add README for resctrl tests Fenghua Yu
2018-10-25 23:07 ` [PATCH v2 2/8] selftests/resctrl: Add basic resctrl file system operations and data Fenghua Yu
2018-10-29 21:51   ` Moger, Babu
     [not found]     ` <CALBSrqDRKbth2NAUFM+emBHVv7eZPX=O9Ki-=vUxi-QQc1wVnQ@mail.gmail.com>
2018-10-30 18:35       ` Fwd: " Fenghua Yu
2018-10-30 22:26         ` Moger, Babu
2018-10-25 23:07 ` [PATCH v2 3/8] selftests/resctrl: Read memory bandwidth from perf IMC counter and from resctrl file system Fenghua Yu
2018-10-29 22:00   ` Moger, Babu
2018-10-29 22:14   ` Moger, Babu
2018-10-25 23:07 ` [PATCH v2 4/8] selftests/resctrl: Add callback to start a benchmark Fenghua Yu
2018-10-29 22:03   ` Moger, Babu
2018-10-25 23:07 ` [PATCH v2 5/8] selftests/resctrl: Add built in benchmark Fenghua Yu
2018-10-25 23:07 ` [PATCH v2 6/8] selftests/resctrl: Add mbm test Fenghua Yu
2018-10-25 23:07 ` [PATCH v2 7/8] selftests/resctrl: Add mba test Fenghua Yu
2018-10-25 23:07 ` [PATCH v2 8/8] selftests/resctrl: Add the test in MAINTAINERS Fenghua Yu
2018-10-29 22:56 ` [PATCH v2 0/8] selftests/resctrl: Add resctrl selftest Moger, Babu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).