[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160520180507.GC8897@kernel.org>
Date: Fri, 20 May 2016 15:05:07 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Wang Nan <wangnan0@...wei.com>
Cc: pi3orama@....com, linux-kernel@...r.kernel.org,
He Kuang <hekuang@...wei.com>,
Arnaldo Carvalho de Melo <acme@...hat.com>,
Jiri Olsa <jolsa@...nel.org>,
Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
Namhyung Kim <namhyung@...nel.org>,
Zefan Li <lizefan@...wei.com>
Subject: Re: [PATCH v2 03/13] perf tools: Add API to pause a evlist
Em Fri, May 20, 2016 at 04:38:25PM +0000, Wang Nan escreveu:
> perf_evlist__toggle_paused() is introduced to pause/resume events
> in an evlist. Utilize PERF_EVENT_IOC_PAUSE_OUTPUT ioctl. Following
> commits use perf_evlist__toggle_paused() to ensure overwrite
> ring buffer is paused before reading.
> +{
int perf_evlist__toggle_paused(struct perf_evlist *evlist, bool pause)
Humm, toggle means, 1 -> 0 or 0 -> 1, not set(n), i.e. you either call
this:
int perf_evlist__toggle_paused(struct perf_evlist *evlist)
and keep an evsel->paused that starts with false and then you, in this
function, use:
err = ioctl(fd, PERF_EVENT_IOC_PAUSE_OUTPUT, evsel->pause ? 1 : 0);
if (!err)
evsel->paused = !evsel->paused;
Or:
int perf_evlist__set_paused(struct perf_evlist *evlist, bool pause)
and do as you did below.
And you could do, to make it shorter, more intuitive:
static inline int perf_evlist__pause(struct perf_evlist *evlist)
{
return perf_evlist__set_paused(evlist, true);
}
static inline int perf_evlist__continue(struct perf_evlist *evlist)
{
return perf_evlist__set_paused(evlist, false);
}
I applied the first two patches.
- Arnaldo
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Signed-off-by: He Kuang <hekuang@...wei.com>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Zefan Li <lizefan@...wei.com>
> Cc: pi3orama@....com
> ---
> tools/perf/util/evlist.c | 22 ++++++++++++++++++++++
> tools/perf/util/evlist.h | 1 +
> 2 files changed, 23 insertions(+)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 1a370db..f4bb8be 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -679,6 +679,28 @@ static struct perf_evsel *perf_evlist__event2evsel(struct perf_evlist *evlist,
> return NULL;
> }
>
> +int perf_evlist__toggle_paused(struct perf_evlist *evlist, bool pause)
> +{
> + int i;
> +
> + for (i = 0; i < evlist->nr_mmaps; i++) {
> + int fd = evlist->mmap[i].fd;
> + int err;
> +
> + if (fd < 0)
> + continue;
> + err = ioctl(fd, PERF_EVENT_IOC_PAUSE_OUTPUT,
> + pause ? 1 : 0);
> + if (err) {
> + err = (errno == 0 ? -EINVAL : -errno);
> + pr_err("Unable to pause output on %d: %s\n",
> + fd, strerror(-err));
> + return err;
> + }
> + }
> + return 0;
> +}
> +
> /* When check_messup is true, 'end' must points to a good entry */
> static union perf_event *
> perf_mmap__read(struct perf_mmap *md, bool check_messup, u64 start,
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 0d165b1..13234dc 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -136,6 +136,7 @@ void perf_evlist__mmap_read_catchup(struct perf_evlist *evlist, int idx);
>
> void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx);
>
> +int perf_evlist__toggle_paused(struct perf_evlist *evlist, bool pause);
> int perf_evlist__open(struct perf_evlist *evlist);
> void perf_evlist__close(struct perf_evlist *evlist);
>
> --
> 1.8.3.4
Powered by blists - more mailing lists