linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Levi Yun <yeoreum.yun@arm.com>
To: peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
	namhyung@kernel.org, mark.rutland@arm.com,
	alexander.shishkin@linux.intel.com, jolsa@kernel.org,
	irogers@google.com, adrian.hunter@intel.com,
	james.clark@linaro.org, asmadeus@codewreck.org
Cc: nd@arm.com, linux-perf-users@vger.kernel.org,
	linux-kernel@vger.kernel.org, Levi Yun <yeoreum.yun@arm.com>
Subject: [PATCH v2 1/2] perf stat: Close cork_fd when create_perf_stat_counter() failed
Date: Fri, 13 Sep 2024 03:02:08 +0100	[thread overview]
Message-ID: <20240913020209.520104-2-yeoreum.yun@arm.com> (raw)
In-Reply-To: <20240913020209.520104-1-yeoreum.yun@arm.com>

When create_perf_stat_counter() failed, it doesn't close workload.cork_fd
open in evlist__prepare_workload(). This could make too many open file
error while __run_perf_stat() repeats.

Introduce evlist__cancel_workload to close workload.cork_fd and
wait workload.child_pid until exit to clear child process
when create_perf_stat_counter() is failed with COUNTER_FATAL.

Signed-off-by: Levi Yun <yeoreum.yun@arm.com>
Reviewed-by: James Clark <james.clark@linaro.org>
---
 tools/perf/builtin-stat.c |  4 ++++
 tools/perf/util/evlist.c  | 14 +++++++++++++-
 tools/perf/util/evlist.h  |  1 +
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 661832756a24..954eb37ce7b8 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -763,6 +763,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)

 			switch (stat_handle_error(counter)) {
 			case COUNTER_FATAL:
+				if (forks)
+					evlist__cancel_workload(evsel_list);
 				return -1;
 			case COUNTER_RETRY:
 				goto try_again;
@@ -804,6 +806,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)

 				switch (stat_handle_error(counter)) {
 				case COUNTER_FATAL:
+					if (forks)
+						evlist__cancel_workload(evsel_list);
 					return -1;
 				case COUNTER_RETRY:
 					goto try_again_reset;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 3a719edafc7a..51a221679c92 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -46,6 +46,7 @@
 #include <sys/mman.h>
 #include <sys/prctl.h>
 #include <sys/timerfd.h>
+#include <sys/wait.h>

 #include <linux/bitops.h>
 #include <linux/hash.h>
@@ -1465,7 +1466,7 @@ int evlist__prepare_workload(struct evlist *evlist, struct target *target, const
 		 * For cancelling the workload without actually running it,
 		 * the parent will just close workload.cork_fd, without writing
 		 * anything, i.e. read will return zero and we just exit()
-		 * here.
+		 * here (See evlist__cancel_workload()).
 		 */
 		if (ret != 1) {
 			if (ret == -1)
@@ -1546,6 +1547,17 @@ int evlist__start_workload(struct evlist *evlist)
 	return 0;
 }

+void evlist__cancel_workload(struct evlist *evlist)
+{
+	int status;
+
+	if (evlist->workload.cork_fd > 0) {
+		close(evlist->workload.cork_fd);
+		evlist->workload.cork_fd = -1;
+		waitpid(evlist->workload.pid, &status, WNOHANG);
+	}
+}
+
 int evlist__parse_sample(struct evlist *evlist, union perf_event *event, struct perf_sample *sample)
 {
 	struct evsel *evsel = evlist__event2evsel(evlist, event);
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index cb91dc9117a2..12f929ffdf92 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -184,6 +184,7 @@ int evlist__prepare_workload(struct evlist *evlist, struct target *target,
 			     const char *argv[], bool pipe_output,
 			     void (*exec_error)(int signo, siginfo_t *info, void *ucontext));
 int evlist__start_workload(struct evlist *evlist);
+void evlist__cancel_workload(struct evlist *evlist);

 struct option;

--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}


  reply	other threads:[~2024-09-13  2:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-13  2:02 [PATCH v2 0/2] Minor fixes error handling of perf stat Levi Yun
2024-09-13  2:02 ` Levi Yun [this message]
2024-09-13  2:02 ` [PATCH v2 2/2] perf stat: Stop repeating when ref_perf_stat() returns -1 Levi Yun
2024-09-13  8:36   ` James Clark
2024-09-13  8:45     ` James Clark
2024-09-13  9:15     ` Yeo Reum Yun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240913020209.520104-2-yeoreum.yun@arm.com \
    --to=yeoreum.yun@arm.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=asmadeus@codewreck.org \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=nd@arm.com \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).