public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@infradead.org>
To: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>,
	linux-kernel@vger.kernel.org,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Mike Galbraith <efault@gmx.de>, Paul Mackerras <paulus@samba.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH 4/4] perf test: Validate PERF_RECORD_ events and perf_sample fields
Date: Fri, 2 Dec 2011 17:06:52 -0200	[thread overview]
Message-ID: <20111202190652.GB4544@infradead.org> (raw)
In-Reply-To: <4ED905DF.2000501@gmail.com>

Em Fri, Dec 02, 2011 at 10:07:43AM -0700, David Ahern escreveu:
> 
> 
> On 12/02/2011 09:19 AM, Arnaldo Carvalho de Melo wrote:
> > +found_exit:
> > +	if (!found_vdso_mmap) {
> > +		pr_debug("PERF_RECORD_MMAP for %s missing!\n", "[vdso]");
> > +		goto out_err;
> > +	}
> 
> 
> Since this is a test why not show all missed events rather than punt to
> out_err on the first missed one?

Right! And it was so, pushed this one there.

- Arnaldo

>From f71c49e5e985897850406a152ab99001cfb86f2a Mon Sep 17 00:00:00 2001
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Fri, 2 Dec 2011 13:53:04 -0200
Subject: [PATCH 1/1] perf test: Soft errors shouldn't stop the "Validate
 PERF_RECORD_" test

For errors that don't preclude checking for further errors, aka "soft"
errors, just  continue testing for other errors.

Better coverage in verbose mode.

Suggested-by: David Ahern <dsahern@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-jafcokbj26m845dsgm2hx6az@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-test.c |   45 +++++++++++++++++++++------------------------
 1 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index f1e3611..6173f78 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -903,7 +903,7 @@ static int test__PERF_RECORD(void)
 	     found_libc_mmap = false,
 	     found_vdso_mmap = false,
 	     found_ld_mmap = false;
-	int err = -1, i, wakeups = 0, sample_size;
+	int err = -1, errs = 0, i, wakeups = 0, sample_size;
 	u32 cpu;
 	int total_events = 0, nr_events[PERF_RECORD_MAX] = { 0, };
 
@@ -1012,8 +1012,6 @@ static int test__PERF_RECORD(void)
 	 */
 	perf_evlist__start_workload(evlist);
 
-	err = -1;
-
 	while (1) {
 		int before = total_events;
 
@@ -1028,9 +1026,10 @@ static int test__PERF_RECORD(void)
 				if (type < PERF_RECORD_MAX)
 					nr_events[type]++;
 
-				if (perf_event__parse_sample(event, sample_type,
-							     sample_size, true,
-							     &sample, false) < 0) {
+				err = perf_event__parse_sample(event, sample_type,
+							       sample_size, true,
+							       &sample, false);
+				if (err < 0) {
 					if (verbose)
 						perf_event__fprintf(event, stderr);
 					pr_debug("Couldn't parse sample\n");
@@ -1045,7 +1044,7 @@ static int test__PERF_RECORD(void)
 				if (prev_time > sample.time) {
 					pr_debug("%s going backwards in time, prev=%" PRIu64 ", curr=%" PRIu64 "\n",
 						 name, prev_time, sample.time);
-					goto out_err;
+					++errs;
 				}
 
 				prev_time = sample.time;
@@ -1053,19 +1052,19 @@ static int test__PERF_RECORD(void)
 				if (sample.cpu != cpu) {
 					pr_debug("%s with unexpected cpu, expected %d, got %d\n",
 						 name, cpu, sample.cpu);
-					goto out_err;
+					++errs;
 				}
 
 				if ((pid_t)sample.pid != evlist->workload.pid) {
 					pr_debug("%s with unexpected pid, expected %d, got %d\n",
 						 name, evlist->workload.pid, sample.pid);
-					goto out_err;
+					++errs;
 				}
 
 				if ((pid_t)sample.tid != evlist->workload.pid) {
 					pr_debug("%s with unexpected tid, expected %d, got %d\n",
 						 name, evlist->workload.pid, sample.tid);
-					goto out_err;
+					++errs;
 				}
 
 				if ((type == PERF_RECORD_COMM ||
@@ -1074,21 +1073,21 @@ static int test__PERF_RECORD(void)
 				     type == PERF_RECORD_EXIT) &&
 				     (pid_t)event->comm.pid != evlist->workload.pid) {
 					pr_debug("%s with unexpected pid/tid\n", name);
-					goto out_err;
+					++errs;
 				}
 
 				if ((type == PERF_RECORD_COMM ||
 				     type == PERF_RECORD_MMAP) &&
 				     event->comm.pid != event->comm.tid) {
 					pr_debug("%s with different pid/tid!\n", name);
-					goto out_err;
+					++errs;
 				}
 
 				switch (type) {
 				case PERF_RECORD_COMM:
 					if (strcmp(event->comm.comm, cmd)) {
 						pr_debug("%s with unexpected comm!\n", name);
-						goto out_err;
+						++errs;
 					}
 					break;
 				case PERF_RECORD_EXIT:
@@ -1112,7 +1111,7 @@ static int test__PERF_RECORD(void)
 				default:
 					pr_debug("Unexpected perf_event->header.type %d!\n",
 						 type);
-					goto out_err;
+					++errs;
 				}
 			}
 		}
@@ -1128,42 +1127,40 @@ static int test__PERF_RECORD(void)
 		sleep(1);
 		if (++wakeups > 5) {
 			pr_debug("No PERF_RECORD_EXIT event!\n");
-			goto out_err;
+			break;
 		}
 	}
 
 found_exit:
 	if (nr_events[PERF_RECORD_COMM] > 1) {
 		pr_debug("Excessive number of PERF_RECORD_COMM events!\n");
-		goto out_err;
+		++errs;
 	}
 
 	if (nr_events[PERF_RECORD_COMM] == 0) {
 		pr_debug("Missing PERF_RECORD_COMM for %s!\n", cmd);
-		goto out_err;
+		++errs;
 	}
 
 	if (!found_cmd_mmap) {
 		pr_debug("PERF_RECORD_MMAP for %s missing!\n", cmd);
-		goto out_err;
+		++errs;
 	}
 
 	if (!found_libc_mmap) {
 		pr_debug("PERF_RECORD_MMAP for %s missing!\n", "libc");
-		goto out_err;
+		++errs;
 	}
 
 	if (!found_ld_mmap) {
 		pr_debug("PERF_RECORD_MMAP for %s missing!\n", "ld");
-		goto out_err;
+		++errs;
 	}
 
 	if (!found_vdso_mmap) {
 		pr_debug("PERF_RECORD_MMAP for %s missing!\n", "[vdso]");
-		goto out_err;
+		++errs;
 	}
-
-	err = 0;
 out_err:
 	perf_evlist__munmap(evlist);
 out_free_cpu_mask:
@@ -1171,7 +1168,7 @@ out_free_cpu_mask:
 out_delete_evlist:
 	perf_evlist__delete(evlist);
 out:
-	return err;
+	return (err < 0 || errs > 0) ? -1 : 0;
 }
 
 static struct test {
-- 
1.7.8.rc0.35.gee6df


  reply	other threads:[~2011-12-02 19:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-02 16:19 [GIT PULL 0/4] perf test improvements Arnaldo Carvalho de Melo
2011-12-02 16:19 ` [PATCH 1/4] perf evlist: Always do automatic allocation of pollfd and mmap structures Arnaldo Carvalho de Melo
2011-12-02 16:19 ` [PATCH 2/4] perf test: Allow running just a subset of the available tests Arnaldo Carvalho de Melo
2011-12-02 16:19 ` [PATCH 3/4] perf event: Introduce perf_event__fprintf Arnaldo Carvalho de Melo
2011-12-02 16:19 ` [PATCH 4/4] perf test: Validate PERF_RECORD_ events and perf_sample fields Arnaldo Carvalho de Melo
2011-12-02 17:07   ` David Ahern
2011-12-02 19:06     ` Arnaldo Carvalho de Melo [this message]
2011-12-02 17:07   ` David Ahern
2011-12-02 16:54 ` [GIT PULL 0/4] perf test improvements David Ahern
2011-12-02 18:22   ` Arnaldo Carvalho de Melo
2011-12-05 15:46     ` Ingo Molnar
2011-12-05  9:22 ` Ingo Molnar

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=20111202190652.GB4544@infradead.org \
    --to=acme@infradead.org \
    --cc=dsahern@gmail.com \
    --cc=efault@gmx.de \
    --cc=eranian@google.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    --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