From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Wang Nan <wangnan0@huawei.com>
Cc: linux-kernel@vger.kernel.org, pi3orama@163.com,
Arnaldo Carvalho de Melo <acme@redhat.com>,
Jiri Olsa <jolsa@kernel.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Namhyung Kim <namhyung@kernel.org>, Zefan Li <lizefan@huawei.com>,
He Kuang <hekuang@huawei.com>
Subject: Re: [PATCH v8 3/8] perf tests: Add testcase for auxiliary evlist
Date: Tue, 21 Jun 2016 18:05:38 -0300 [thread overview]
Message-ID: <20160621210538.GE4213@kernel.org> (raw)
In-Reply-To: <1466419645-75551-4-git-send-email-wangnan0@huawei.com>
Em Mon, Jun 20, 2016 at 10:47:20AM +0000, Wang Nan escreveu:
> Improve test backward-ring-buffer, trace both enter and exit event of
> prctl() syscall, utilize auxiliary evlist to mmap enter and exit event
> into separated mmaps.
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: He Kuang <hekuang@huawei.com>
> ---
> tools/perf/tests/backward-ring-buffer.c | 87 ++++++++++++++++++++++++++-------
> 1 file changed, 68 insertions(+), 19 deletions(-)
>
> diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c
> index d9ba991..76e34c0 100644
> --- a/tools/perf/tests/backward-ring-buffer.c
> +++ b/tools/perf/tests/backward-ring-buffer.c
> @@ -31,16 +31,19 @@ static int count_samples(struct perf_evlist *evlist, int *sample_count,
> for (i = 0; i < evlist->nr_mmaps; i++) {
> union perf_event *event;
>
> - perf_evlist__mmap_read_catchup(evlist, i);
> - while ((event = perf_evlist__mmap_read_backward(evlist, i)) != NULL) {
> + if (evlist->backward)
> + perf_evlist__mmap_read_catchup(evlist, i);
So, shouldn't this be done at perf_evlist__mmap_read_catchup()? I.e. you
will use this only when you know that one of the evlists count_samples()
will deal with can be a backwards one...
I.e. do with perf_evlist__mmap_read_catchup the same thing you did in
perf_evlist__mmap_read, checking there this evlist->backward.
This can be done on top, so I'll continue tentatively merging this.
> + while ((event = perf_evlist__mmap_read(evlist, i)) != NULL) {
> const u32 type = event->header.type;
>
> switch (type) {
> case PERF_RECORD_SAMPLE:
> - (*sample_count)++;
> + if (sample_count)
> + (*sample_count)++;
> break;
> case PERF_RECORD_COMM:
> - (*comm_count)++;
> + if (comm_count)
> + (*comm_count)++;
You could've avoided all this by passing some dummy integer pointer for
the enter_sample_count case. Making the patch smaller helps reviewing
:-)
> break;
> default:
> pr_err("Unexpected record of type %d\n", type);
> @@ -51,34 +54,53 @@ static int count_samples(struct perf_evlist *evlist, int *sample_count,
> return TEST_OK;
> }
>
> -static int do_test(struct perf_evlist *evlist, int mmap_pages,
> - int *sample_count, int *comm_count)
> +static int do_test(struct perf_evlist *evlist,
> + struct perf_evlist *aux_evlist,
> + int mmap_pages,
> + int *enter_sample_count,
> + int *exit_sample_count,
> + int *comm_count)
> {
> int err;
> char sbuf[STRERR_BUFSIZE];
>
> - err = perf_evlist__mmap(evlist, mmap_pages, true);
> + err = perf_evlist__mmap(evlist, mmap_pages, false);
> if (err < 0) {
> pr_debug("perf_evlist__mmap: %s\n",
> strerror_r(errno, sbuf, sizeof(sbuf)));
> return TEST_FAIL;
> }
>
> + err = perf_evlist__mmap(aux_evlist, mmap_pages, true);
> + if (err < 0) {
> + pr_debug("perf_evlist__mmap for aux_evlist: %s\n",
> + strerror_r(errno, sbuf, sizeof(sbuf)));
> + return TEST_FAIL;
> + }
> +
> perf_evlist__enable(evlist);
> testcase();
> perf_evlist__disable(evlist);
>
> - err = count_samples(evlist, sample_count, comm_count);
> + err = count_samples(aux_evlist, exit_sample_count, comm_count);
> + if (err)
> + goto errout;
> + err = count_samples(evlist, enter_sample_count, NULL);
> + if (err)
> + goto errout;
> +errout:
> perf_evlist__munmap(evlist);
> + perf_evlist__munmap(aux_evlist);
> return err;
> }
>
>
> int test__backward_ring_buffer(int subtest __maybe_unused)
> {
> - int ret = TEST_SKIP, err, sample_count = 0, comm_count = 0;
> + int ret = TEST_SKIP, err;
> + int enter_sample_count = 0, exit_sample_count = 0, comm_count = 0;
> char pid[16], sbuf[STRERR_BUFSIZE];
> - struct perf_evlist *evlist;
> + struct perf_evlist *evlist, *aux_evlist = NULL;
> struct perf_evsel *evsel __maybe_unused;
> struct parse_events_error parse_error;
> struct record_opts opts = {
> @@ -115,11 +137,22 @@ int test__backward_ring_buffer(int subtest __maybe_unused)
> goto out_delete_evlist;
> }
>
> - perf_evlist__config(evlist, &opts, NULL);
> + /*
> + * Set backward bit, ring buffer should be writing from end. Record
> + * it in aux evlist
> + */
> + perf_evlist__last(evlist)->overwrite = true;
> + perf_evlist__last(evlist)->attr.write_backward = 1;
>
> - /* Set backward bit, ring buffer should be writing from end */
> - evlist__for_each(evlist, evsel)
> - evsel->attr.write_backward = 1;
> + err = parse_events(evlist, "syscalls:sys_exit_prctl", &parse_error);
> + if (err) {
> + pr_debug("Failed to parse tracepoint event, try use root\n");
> + ret = TEST_SKIP;
> + goto out_delete_evlist;
> + }
> + /* Don't set backward bit for exit event. Record it in main evlist */
> +
> + perf_evlist__config(evlist, &opts, NULL);
>
> err = perf_evlist__open(evlist);
> if (err < 0) {
> @@ -128,24 +161,40 @@ int test__backward_ring_buffer(int subtest __maybe_unused)
> goto out_delete_evlist;
> }
>
> + aux_evlist = perf_evlist__new_aux(evlist);
> + if (!aux_evlist) {
> + pr_debug("perf_evlist__new_aux failed\n");
> + goto out_delete_evlist;
> + }
> + aux_evlist->backward = true;
> +
> ret = TEST_FAIL;
> - err = do_test(evlist, opts.mmap_pages, &sample_count,
> + err = do_test(evlist, aux_evlist, opts.mmap_pages,
> + &enter_sample_count, &exit_sample_count,
> &comm_count);
> if (err != TEST_OK)
> goto out_delete_evlist;
>
> - if ((sample_count != NR_ITERS) || (comm_count != NR_ITERS)) {
> - pr_err("Unexpected counter: sample_count=%d, comm_count=%d\n",
> - sample_count, comm_count);
> + if (enter_sample_count != exit_sample_count) {
> + pr_err("Unexpected counter: enter_sample_count=%d, exit_sample_count=%d\n",
> + enter_sample_count, exit_sample_count);
> + goto out_delete_evlist;
> + }
> +
> + if ((exit_sample_count != NR_ITERS) || (comm_count != NR_ITERS)) {
> + pr_err("Unexpected counter: exit_sample_count=%d, comm_count=%d\n",
> + exit_sample_count, comm_count);
> goto out_delete_evlist;
> }
>
> - err = do_test(evlist, 1, &sample_count, &comm_count);
> + err = do_test(evlist, aux_evlist, 1, NULL, NULL, NULL);
> if (err != TEST_OK)
> goto out_delete_evlist;
>
> ret = TEST_OK;
> out_delete_evlist:
> + if (aux_evlist)
> + perf_evlist__delete(aux_evlist);
> perf_evlist__delete(evlist);
> return ret;
> }
> --
> 1.8.3.4
next prev parent reply other threads:[~2016-06-21 21:15 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-20 10:47 [PATCH v8 0/8] perf tools: Support overwritable ring buffer Wang Nan
2016-06-20 10:47 ` [PATCH v8 1/8] perf tools: Fix write_backwards fallback Wang Nan
2016-06-20 10:47 ` [PATCH v8 2/8] perf evlist: Introduce aux evlist Wang Nan
2016-06-20 20:36 ` Arnaldo Carvalho de Melo
2016-06-21 1:31 ` Wangnan (F)
2016-06-20 10:47 ` [PATCH v8 3/8] perf tests: Add testcase for auxiliary evlist Wang Nan
2016-06-21 21:05 ` Arnaldo Carvalho de Melo [this message]
2016-06-22 4:10 ` Wangnan (F)
2016-06-20 10:47 ` [PATCH v8 4/8] perf record: Introduce rec->overwrite_evlist for overwritable events Wang Nan
2016-06-21 21:30 ` Arnaldo Carvalho de Melo
2016-06-20 10:47 ` [PATCH v8 5/8] perf record: Toggle overwrite ring buffer for reading Wang Nan
2016-06-20 10:47 ` [PATCH v8 6/8] perf tools: Enable overwrite settings Wang Nan
2016-06-21 21:49 ` Arnaldo Carvalho de Melo
2016-06-20 10:47 ` [PATCH v8 7/8] perf tools: Don't warn about out of order event if write_backward is used Wang Nan
2016-06-20 10:47 ` [PATCH v8 8/8] perf tools: Add --tail-synthesize option Wang Nan
-- strict thread matches above, loose matches on Subject: below --
2016-06-15 2:23 [PATCH v7 0/8] perf tools: Support overwritable ring buffer Wang Nan
2016-06-15 2:23 ` [PATCH v7 1/8] perf evlist: Introduce aux evlist Wang Nan
2016-06-15 2:23 ` [PATCH v7 2/8] perf tests: Add testcase for auxiliary evlist Wang Nan
2016-06-15 2:23 ` [PATCH v7 3/8] perf record: Introduce rec->overwrite_evlist for overwritable events Wang Nan
2016-06-15 2:23 ` [PATCH v7 4/8] perf record: Toggle overwrite ring buffer for reading Wang Nan
2016-06-15 2:23 ` [PATCH v7 5/8] perf tools: Enable overwrite settings Wang Nan
2016-06-15 2:23 ` [PATCH v7 6/8] perf tools: Don't warn about out of order event if write_backward is used Wang Nan
2016-06-15 2:23 ` [PATCH v7 7/8] perf tools: Check write_backward during evlist config Wang Nan
2016-06-16 21:47 ` Arnaldo Carvalho de Melo
2016-06-20 4:09 ` Wangnan (F)
2016-06-22 7:43 ` [tip:perf/core] perf evsel: Fix write_backwards fallback tip-bot for Arnaldo Carvalho de Melo
2016-06-15 2:23 ` [PATCH v7 8/8] perf record: Unmap overwrite evlist when event terminate Wang Nan
2016-06-16 20:59 ` Arnaldo Carvalho de Melo
2016-06-20 8:04 ` Wangnan (F)
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=20160621210538.GE4213@kernel.org \
--to=acme@kernel.org \
--cc=acme@redhat.com \
--cc=hekuang@huawei.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lizefan@huawei.com \
--cc=mhiramat@kernel.org \
--cc=namhyung@kernel.org \
--cc=pi3orama@163.com \
--cc=wangnan0@huawei.com \
/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
Powered by JetHome