linux-kernel - Re: [PATCH 03/10] perf tool: new iterfaces to read event from ring buffer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 10 Oct 2017 15:15:20 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     kan.liang@...el.com
Cc:     peterz@...radead.org, mingo@...hat.com,
        linux-kernel@...r.kernel.org, jolsa@...nel.org,
        wangnan0@...wei.com, hekuang@...wei.com, namhyung@...nel.org,
        alexander.shishkin@...ux.intel.com, adrian.hunter@...el.com,
        ak@...ux.intel.com
Subject: Re: [PATCH 03/10] perf tool: new iterfaces to read event from ring
 buffer

Em Tue, Oct 10, 2017 at 10:20:16AM -0700, kan.liang@...el.com escreveu:
> From: Kan Liang <kan.liang@...el.com>
> 
> The perf_evlist__mmap_read only support forward mode. It needs a common
> function to support both forward and backward mode.

> The perf_evlist__mmap_read_backward is buggy.

So, what is the bug? You state that it is buggy, but don't spell out the
bug, please do so.

If it fixes an existing bug, then it should go separate from this
patchkit, right?

- Arnaldo
 
> Introduce a new mmap read interface to support both forward and backward
> mode. It can read event one by one from the specific range of ring
> buffer.
> 
> The standard process to mmap__read event would be as below.
>  perf_mmap__read_init
>  while (event = perf_mmap__read_event) {
>    //process the event
>    perf_mmap__consume
>  }
>  perf_mmap__read_done
> 
> The following patches will use it to replace the old interfaces.
> 
> Signed-off-by: Kan Liang <kan.liang@...el.com>
> ---
>  tools/perf/util/evlist.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/evlist.h |  2 ++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 7d23cf5..b36211e 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -897,6 +897,47 @@ perf_mmap__read(struct perf_mmap *md, bool check_messup, u64 start,
>  	return event;
>  }
>  
> +/*
> + * Read the first event in the specified range of the ring buffer.
> + * Used by most of the perf tools and tests
> + */
> +union perf_event *
> +perf_mmap__read_event(struct perf_mmap_read *read)
> +{
> +	struct perf_mmap *md = read->md;
> +	union perf_event *event;
> +
> +	/*
> +	 * Check if event was unmapped due to a POLLHUP/POLLERR.
> +	 */
> +	if (!refcount_read(&md->refcnt))
> +		return NULL;
> +
> +	/* In backward, the ringbuffer is already paused. */
> +	if (!read->backward) {
> +		read->end = perf_mmap__read_head(md);
> +		if (!read->end)
> +			return NULL;
> +	}
> +
> +	event = perf_mmap__read(md, !read->backward && read->overwrite,
> +				read->start, read->end, &md->prev);
> +	read->start = md->prev;
> +	return event;
> +}
> +
> +/*
> + * Mandatory for backward
> + * The direction of backward mmap__read_event is from head to tail.
> + * The last mmap__read_event will set tail to md->prev.
> + * Need to correct the md->prev.
> + */
> +void
> +perf_mmap__read_done(struct perf_mmap_read *read)
> +{
> +	read->md->prev = read->head;
> +}
> +
>  union perf_event *perf_mmap__read_forward(struct perf_mmap *md, bool check_messup)
>  {
>  	u64 head;
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 1ce4857..53baf26 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -207,6 +207,8 @@ int perf_mmap__read_init(struct perf_mmap *md, struct perf_mmap_read *read,
>  			 bool overwrite, bool backward);
>  int perf_mmap__read_to_file(struct perf_mmap_read *read,
>  			    struct perf_data_file *file);
> +union perf_event *perf_mmap__read_event(struct perf_mmap_read *read);
> +void perf_mmap__read_done(struct perf_mmap_read *read);
>  
>  int perf_evlist__open(struct perf_evlist *evlist);
>  void perf_evlist__close(struct perf_evlist *evlist);
> -- 
> 2.5.5