[PATCH v5 3/3] perf test: Don't count mlock budget wait time towards test timeout

[Ian Rogers]

When multiple parallel tests need room in kernel.perf_event_mlock_kb,
they take turns acquiring the shared mlock_budget->lock mutex in
run_test_case_mlocked() after being forked in start_test().

Because child->start_time is recorded in start_test() when the child
process is forked, any time a test spends waiting in the
mlock_budget->lock queue would otherwise be counted against both its
per-test timeout (causing tests later in the queue to time out before
they even start) and its JUnit XML testcase elapsed time.

Extend struct mlock_budget with a per-slot shared start_time and started
flag populated in run_test_case_mlocked() once the child acquires
mlock_budget->lock. In check_child_timeout(), skip tests that are still
waiting for mlock_budget->lock and measure the timeout from the lock
acquisition timestamp once started. Also use the post-lock start_time
when computing the test's elapsed duration.

Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
---
 tools/perf/tests/builtin-test.c | 81 +++++++++++++++++++++++++--------
 1 file changed, 63 insertions(+), 18 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 58dc75ba82cf..e1bfde78d34d 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -79,8 +79,17 @@ static unsigned int test_timeout = 120;
  * processes and robust so that a test killed while holding it doesn't leave
  * the budget locked, see mutex_init_pshared_robust().
  */
+struct mlock_child_time {
+	struct timespec start_time;
+	bool started;
+};
+
 struct mlock_budget {
 	struct mutex lock;
+	size_t size;
+	size_t nr_slots;
+	size_t next_slot;
+	struct mlock_child_time slots[];
 };
 
 static struct mlock_budget *mlock_budget;
@@ -384,6 +393,7 @@ struct child_test {
 	struct timespec start_time;
 	struct timespec end_time;
 	struct timespec timeout_time;
+	struct mlock_child_time *mlock_time;
 };
 
 static jmp_buf run_test_jmp_buf;
@@ -414,6 +424,8 @@ static int run_test_case_mlocked(struct child_test *child)
 	int err;
 
 	mutex_lock(&mlock_budget->lock);
+	clock_gettime(CLOCK_MONOTONIC, &child->mlock_time->start_time);
+	__atomic_store_n(&child->mlock_time->started, true, __ATOMIC_RELEASE);
 	err = run_test_case(child);
 	mutex_unlock(&mlock_budget->lock);
 
@@ -459,7 +471,6 @@ static int run_test_child(struct child_process *process)
 		SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGINT, SIGPIPE, SIGQUIT, SIGSEGV, SIGTERM,
 	};
 	struct child_test *child = container_of(process, struct child_test, process);
-	bool needs_mlock = mlock_budget && child->test->needs_mlock;
 	int err, sig;
 
 	/*
@@ -486,7 +497,7 @@ static int run_test_child(struct child_process *process)
 	pr_debug("---- start ----\n");
 	pr_debug("test child forked, pid %d\n", getpid());
 
-	if (needs_mlock)
+	if (child->mlock_time)
 		err = run_test_case_mlocked(child);
 	else
 		err = run_test_case(child);
@@ -1027,6 +1038,28 @@ static void kill_child_process(pid_t pid, int sig)
 		kill(pid, sig);
 }
 
+static bool child_test__started(const struct child_test *child, struct timespec *start_time)
+{
+	if (child->mlock_time) {
+		if (!__atomic_load_n(&child->mlock_time->started, __ATOMIC_ACQUIRE))
+			return false;
+		*start_time = child->mlock_time->start_time;
+		return true;
+	}
+	*start_time = child->start_time;
+	return true;
+}
+
+static double child_test__elapsed_secs(const struct child_test *child,
+				       const struct timespec *end_time)
+{
+	struct timespec start_time;
+
+	if (!child_test__started(child, &start_time))
+		start_time = child->start_time;
+	return timespec_diff_secs(end_time, &start_time);
+}
+
 static void check_child_timeout(struct child_test *child, const struct timespec *now)
 {
 	pid_t pid;
@@ -1043,7 +1076,12 @@ static void check_child_timeout(struct child_test *child, const struct timespec
 		return;
 
 	if (!child->timed_out) {
-		if (timespec_diff_secs(now, &child->start_time) >= test_timeout) {
+		struct timespec start_time;
+
+		if (!child_test__started(child, &start_time))
+			return;
+
+		if (timespec_diff_secs(now, &start_time) >= test_timeout) {
 			child->timed_out = true;
 			child->timeout_time = *now;
 			pr_debug3("Timeout (%us) for test %d.%d (pid %d), sending SIGTERM\n",
@@ -1258,8 +1296,7 @@ static void finish_test(struct child_test **child_tests, int running_test, int c
 		print_test_failure_snippet(stderr, err_output.buf);
 
 	clock_gettime(CLOCK_MONOTONIC, &end_time);
-	elapsed = (end_time.tv_sec - child_test->start_time.tv_sec) +
-		  (end_time.tv_nsec - child_test->start_time.tv_nsec) / 1000000000.0;
+	elapsed = child_test__elapsed_secs(child_test, &end_time);
 
 	print_test_result(t, curr_suite, curr_test_case, ret, width, /*running=*/0,
 			  err_output.buf, elapsed);
@@ -1532,9 +1569,7 @@ static int finish_tests_parallel(struct child_test **child_tests, size_t num_tes
 				 (child->result == TEST_FAIL || child->result == TEST_TIMEOUT))
 				print_test_failure_snippet(stderr, child->err_output.buf);
 
-			elapsed = (child->end_time.tv_sec - child->start_time.tv_sec) +
-				  (child->end_time.tv_nsec -
-				   child->start_time.tv_nsec) / 1000000000.0;
+			elapsed = child_test__elapsed_secs(child, &child->end_time);
 
 			print_test_result(child->test, child->suite_num, child->test_case_num,
 					  child->result, width, 0, child->err_output.buf, elapsed);
@@ -1606,6 +1641,9 @@ static int start_test(struct test_suite *test, int curr_suite, int curr_test_cas
 		(*child)->process.stdout_to_stderr = 1;
 	}
 	(*child)->process.no_exec_cmd = run_test_child;
+	if (mlock_budget && test->needs_mlock && !sequential && pass == 1 &&
+	    mlock_budget->next_slot < mlock_budget->nr_slots)
+		(*child)->mlock_time = &mlock_budget->slots[mlock_budget->next_slot++];
 	clock_gettime(CLOCK_MONOTONIC, &(*child)->start_time);
 	err = start_command(&(*child)->process);
 	if (err)
@@ -2199,10 +2237,10 @@ static struct test_suite **build_suites(void)
 #undef for_each_suite
 }
 
-static bool perf_test__shares_mlock_budget(struct test_suite **suites, int argc,
-					   const char *argv[], struct intlist *skiplist)
+static size_t perf_test__count_mlock_budget(struct test_suite **suites, int argc,
+					    const char *argv[], struct intlist *skiplist)
 {
-	int sharing = 0;
+	size_t sharing = 0;
 
 	/* The selection rules mirrored here are __cmd_test()'s: when they change, change both. */
 	for (int curr_suite = 0; suites[curr_suite]; curr_suite++) {
@@ -2224,10 +2262,8 @@ static bool perf_test__shares_mlock_budget(struct test_suite **suites, int argc,
 					       curr_suite, argc, argv))
 				sharing += runs_per_test;
 		}
-		if (sharing > 1)
-			return true;
 	}
-	return false;
+	return sharing;
 }
 
 /*
@@ -2250,6 +2286,8 @@ static bool perf_test__mlock_budget_exhaustible(void)
 static void perf_test__setup_mlock_budget(struct test_suite **suites, int argc,
 					  const char *argv[], struct intlist *skiplist)
 {
+	size_t sharing, size;
+
 	/* Nothing can be denied the budget, nothing to take turns for. */
 	if (!perf_test__mlock_budget_exhaustible()) {
 		pr_debug("Not taking turns using the perf mlock budget: nothing can exhaust it\n");
@@ -2257,19 +2295,26 @@ static void perf_test__setup_mlock_budget(struct test_suite **suites, int argc,
 	}
 
 	/* Nothing runs at the same time, nothing to share the budget with. */
-	if (sequential || !perf_test__shares_mlock_budget(suites, argc, argv, skiplist))
+	if (sequential)
+		return;
+
+	sharing = perf_test__count_mlock_budget(suites, argc, argv, skiplist);
+	if (sharing <= 1)
 		return;
 
-	mlock_budget = mmap(NULL, sizeof(*mlock_budget), PROT_READ | PROT_WRITE,
+	size = sizeof(*mlock_budget) + sharing * sizeof(mlock_budget->slots[0]);
+	mlock_budget = mmap(NULL, size, PROT_READ | PROT_WRITE,
 			    MAP_SHARED | MAP_ANONYMOUS, /*fd=*/-1, /*offset=*/0);
 	if (mlock_budget == MAP_FAILED) {
 		mlock_budget = NULL;
 		pr_err("Failed to map the shared area used to take turns using the perf mlock budget\n");
 		return;
 	}
+	mlock_budget->size = size;
+	mlock_budget->nr_slots = sharing;
 
 	if (!mutex_init_pshared_robust(&mlock_budget->lock)) {
-		munmap(mlock_budget, sizeof(*mlock_budget));
+		munmap(mlock_budget, size);
 		mlock_budget = NULL;
 		pr_debug("Not taking turns using the perf mlock budget\n");
 	}
@@ -2281,7 +2326,7 @@ static void perf_test__teardown_mlock_budget(void)
 		return;
 
 	mutex_destroy(&mlock_budget->lock);
-	munmap(mlock_budget, sizeof(*mlock_budget));
+	munmap(mlock_budget, mlock_budget->size);
 	mlock_budget = NULL;
 }
 
-- 
2.56.0.307.g5fe7bd680c-goog