Groups > linux.kernel > #1237913 > unrolled thread

[RFC/PATCH 00/38] perf tools: Speed-up perf report by using multi thread (v5)

Started by	Namhyung Kim <namhyung@kernel.org>
First post	2015-10-02 07:30 +0200
Last post	2015-10-12 16:40 +0200
Articles	20 on this page of 57 — 4 participants

Back to article view | Back to linux.kernel

  [RFC/PATCH 00/38] perf tools: Speed-up perf report by using multi thread (v5) Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 04/38] perf tools: pass perf_mmap desc directly Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
      Re: [RFC/PATCH 04/38] perf tools: pass perf_mmap desc directly Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-10-02 20:50 +0200
    [RFC/PATCH 27/38] perf hists: Pass hists struct to hist_entry_iter struct Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 06/38] perf tools: Extend perf_evlist__mmap_ex() to use track mmap Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 18/38] perf tools: Introduce thread__find_addr_location_by_time() and friends Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
      Re: [RFC/PATCH 18/38] perf tools: Introduce  thread__find_addr_location_by_time() and friends Jiri Olsa <jolsa@redhat.com> - 2015-10-12 15:40 +0200
    [RFC/PATCH 21/38] perf tools: Save timestamp of a map creation Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 15/38] perf tools: Introduce machine__find*_thread_by_time() Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
      Re: [RFC/PATCH 15/38] perf tools: Introduce  machine__find*_thread_by_time() Jiri Olsa <jolsa@redhat.com> - 2015-10-08 14:30 +0200
        Re: [RFC/PATCH 15/38] perf tools: Introduce  machine__find*_thread_by_time() Namhyung Kim <namhyung@kernel.org> - 2015-10-09 08:10 +0200
    [RFC/PATCH 01/38] perf tools: Use a software dummy event to track task/mmap events Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
      Re: [RFC/PATCH 01/38] perf tools: Use a software dummy event to  track task/mmap events Jiri Olsa <jolsa@redhat.com> - 2015-10-05 15:00 +0200
        Re: [RFC/PATCH 01/38] perf tools: Use a software dummy event to  track task/mmap events Namhyung Kim <namhyung@kernel.org> - 2015-10-06 10:40 +0200
    [RFC/PATCH 28/38] perf tools: Move BUILD_ID_SIZE definition to perf.h Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 13/38] perf tools: Use thread__comm_by_time() when adding hist entries Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 14/38] perf tools: Convert dead thread list into rbtree Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 22/38] perf tools: Introduce map_groups__{insert,find}_by_time() Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 26/38] perf session: Pass struct events stats to event processing functions Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 09/38] perf record: Add --index option for building index table Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
      Re: [RFC/PATCH 09/38] perf record: Add --index option for building  index table Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-10-02 21:00 +0200
      Re: [RFC/PATCH 09/38] perf record: Add --index option for building  index table Jiri Olsa <jolsa@redhat.com> - 2015-10-05 15:50 +0200
        Re: [RFC/PATCH 09/38] perf record: Add --index option for building  index table Namhyung Kim <namhyung@kernel.org> - 2015-10-07 10:30 +0200
          Re: [RFC/PATCH 09/38] perf record: Add --index option for building  index table Jiri Olsa <jolsa@redhat.com> - 2015-10-07 14:20 +0200
    [RFC/PATCH 23/38] perf tools: Use map_groups__find_addr_by_time() Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 28/38] perf tools: Move BUILD_ID_SIZE definition to perf.h Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 24/38] perf tools: Add testcase for managing maps with time Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 11/38] perf tools: Introduce thread__comm(_str)_by_time() helpers Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
    [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:30 +0200
      Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a  leader thread Jiri Olsa <jolsa@redhat.com> - 2015-10-08 15:00 +0200
        Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a  leader thread Namhyung Kim <namhyung@kernel.org> - 2015-10-09 09:10 +0200
          Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a  leader thread Jiri Olsa <jolsa@redhat.com> - 2015-10-12 14:50 +0200
      Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a  leader thread Jiri Olsa <jolsa@redhat.com> - 2015-10-08 15:00 +0200
        Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a  leader thread Namhyung Kim <namhyung@kernel.org> - 2015-10-09 08:30 +0200
    [RFC/PATCH 08/38] perf tools: Handle indexed data file properly Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:40 +0200
    [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:40 +0200
      Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-10-02 20:50 +0200
        Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask Namhyung Kim <namhyung@kernel.org> - 2015-10-06 10:40 +0200
      Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask Jiri Olsa <jolsa@redhat.com> - 2015-10-08 12:20 +0200
        Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask Namhyung Kim <namhyung@kernel.org> - 2015-10-09 08:10 +0200
          Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask Jiri Olsa <jolsa@redhat.com> - 2015-10-12 14:50 +0200
    [RFC/PATCH 07/38] perf tools: Add HEADER_DATA_INDEX feature Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:40 +0200
    [RFC/PATCH 10/38] perf report: Skip dummy tracking event Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:40 +0200
    [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:40 +0200
      Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct  perf_evlist Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-10-02 20:50 +0200
        Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct  perf_evlist Adrian Hunter <adrian.hunter@intel.com> - 2015-10-05 13:40 +0200
          Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist Namhyung Kim <namhyung@kernel.org> - 2015-10-06 11:10 +0200
            Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct  perf_evlist Adrian Hunter <adrian.hunter@intel.com> - 2015-10-06 11:30 +0200
              Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist Namhyung Kim <namhyung@kernel.org> - 2015-10-07 11:10 +0200
                Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct  perf_evlist Adrian Hunter <adrian.hunter@intel.com> - 2015-10-08 18:10 +0200
                  Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct  perf_evlist Namhyung Kim <namhyung@kernel.org> - 2015-10-09 10:00 +0200
        Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist Namhyung Kim <namhyung@kernel.org> - 2015-10-06 11:00 +0200
      Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct  perf_evlist Jiri Olsa <jolsa@redhat.com> - 2015-10-05 15:20 +0200
        Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct  perf_evlist Namhyung Kim <namhyung@kernel.org> - 2015-10-06 10:50 +0200
      Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct  perf_evlist Jiri Olsa <jolsa@redhat.com> - 2015-10-08 12:20 +0200
    [RFC/PATCH 05/38] perf tools: Create separate mmap for dummy tracking event Namhyung Kim <namhyung@kernel.org> - 2015-10-02 07:40 +0200
    Re: [RFC/PATCH 28/38] perf tools: Move BUILD_ID_SIZE definition to  perf.h Jiri Olsa <jolsa@redhat.com> - 2015-10-12 16:40 +0200

Page 2 of 3 — ← Prev page 1 [2] 3 Next page →

#1238533 — Re: [RFC/PATCH 09/38] perf record: Add --index option for building index table

From	Arnaldo Carvalho de Melo <acme@kernel.org>
Date	2015-10-02 21:00 +0200
Subject	Re: [RFC/PATCH 09/38] perf record: Add --index option for building index table
Message-ID	<qfdIK-3ul-13@gated-at.bofh.it>
In reply to	#1237928

Em Fri, Oct 02, 2015 at 02:18:50PM +0900, Namhyung Kim escreveu:
> The new --index option will create indexed data file which can be
> processed by multiple threads parallelly.  It saves meta event and
> sample data in separate files and merges them with an index table.
> 
> If there's an index table in the data file, the HEADER_DATA_INDEX
> feature bit is set and session->header.index[0] will point to the meta
> event area, and rest are sample data.  It'd look like below:

So this is all about perf.data files, i.e. we will traverse it all
looking for metadata events, then the samples itself will be processed
using multiple threads. I.e. two stages, touching the whole perf.data
file looking for the metadata events, then touching it all again to
process the files, right?

The model processing events for 'perf report' and for 'perf top' will
differ, right?

Can't we have one thread reading the events, another batching then to
merge up to those FINISHED_ROUND when it needs to sort things (in
parallel with reading more events for the next round) and then pass the
sorted batch to one thread per CPU to actually process the samples as in
hist processing, etc?

Need to try this, see if 'perf top' works, which I think it will, but
you haven't mentioned anything about it in the cover letter for this
patchkit.

But no speedups should be expected there, as no 'perf.data' file is
involved...

- Arnaldo
 
>         +---------------------+
>         |     file header     |
>         |---------------------|
>         |                     |
>         |    meta events[0] <-+--+
>         |                     |  |
>         |---------------------|  |
>         |                     |  |
>         |    sample data[1] <-+--+
>         |                     |  |
>         |---------------------|  |
>         |                     |  |
>         |    sample data[2] <-|--+
>         |                     |  |
>         |---------------------|  |
>         |         ...         | ...
>         |---------------------|  |
>         |     feature data    |  |
>         |   (contains index) -+--+
>         +---------------------+
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/Documentation/perf-record.txt |   4 +
>  tools/perf/builtin-record.c              | 178 ++++++++++++++++++++++++++++---
>  tools/perf/perf.h                        |   1 +
>  tools/perf/util/header.c                 |   2 +
>  tools/perf/util/session.c                |   1 +
>  5 files changed, 173 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index 2e9ce77b5e14..71a9520b10b0 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -308,6 +308,10 @@ This option sets the time out limit. The default value is 500 ms.
>  Record context switch events i.e. events of type PERF_RECORD_SWITCH or
>  PERF_RECORD_SWITCH_CPU_WIDE.
>  
> +--index::
> +Build an index table for sample data.  This will speed up perf report by
> +parallel processing.
> +
>  SEE ALSO
>  --------
>  linkperf:perf-stat[1], linkperf:perf-list[1]
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 623984c81478..096634c4c5ea 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -43,6 +43,7 @@ struct record {
>  	u64			bytes_written;
>  	struct perf_data_file	file;
>  	struct auxtrace_record	*itr;
> +	int			*fds;
>  	struct perf_evlist	*evlist;
>  	struct perf_session	*session;
>  	const char		*progname;
> @@ -52,9 +53,16 @@ struct record {
>  	long			samples;
>  };
>  
> -static int record__write(struct record *rec, void *bf, size_t size)
> +static int record__write(struct record *rec, void *bf, size_t size, int idx)
>  {
> -	if (perf_data_file__write(rec->session->file, bf, size) < 0) {
> +	int fd;
> +
> +	if (rec->fds && idx >= 0)
> +		fd = rec->fds[idx];
> +	else
> +		fd = perf_data_file__fd(rec->session->file);
> +
> +	if (writen(fd, bf, size) < 0) {
>  		pr_err("failed to write perf data, error: %m\n");
>  		return -1;
>  	}
> @@ -69,7 +77,7 @@ static int process_synthesized_event(struct perf_tool *tool,
>  				     struct machine *machine __maybe_unused)
>  {
>  	struct record *rec = container_of(tool, struct record, tool);
> -	return record__write(rec, event, event->header.size);
> +	return record__write(rec, event, event->header.size, -1);
>  }
>  
>  static int record__mmap_read(struct record *rec, int idx)
> @@ -94,7 +102,7 @@ static int record__mmap_read(struct record *rec, int idx)
>  		size = md->mask + 1 - (old & md->mask);
>  		old += size;
>  
> -		if (record__write(rec, buf, size) < 0) {
> +		if (record__write(rec, buf, size, idx) < 0) {
>  			rc = -1;
>  			goto out;
>  		}
> @@ -104,7 +112,7 @@ static int record__mmap_read(struct record *rec, int idx)
>  	size = head - old;
>  	old += size;
>  
> -	if (record__write(rec, buf, size) < 0) {
> +	if (record__write(rec, buf, size, idx) < 0) {
>  		rc = -1;
>  		goto out;
>  	}
> @@ -151,6 +159,7 @@ static int record__process_auxtrace(struct perf_tool *tool,
>  	struct perf_data_file *file = &rec->file;
>  	size_t padding;
>  	u8 pad[8] = {0};
> +	int idx = event->auxtrace.idx;
>  
>  	if (!perf_data_file__is_pipe(file)) {
>  		off_t file_offset;
> @@ -171,11 +180,11 @@ static int record__process_auxtrace(struct perf_tool *tool,
>  	if (padding)
>  		padding = 8 - padding;
>  
> -	record__write(rec, event, event->header.size);
> -	record__write(rec, data1, len1);
> +	record__write(rec, event, event->header.size, idx);
> +	record__write(rec, data1, len1, idx);
>  	if (len2)
> -		record__write(rec, data2, len2);
> -	record__write(rec, &pad, padding);
> +		record__write(rec, data2, len2, idx);
> +	record__write(rec, &pad, padding, idx);
>  
>  	return 0;
>  }
> @@ -268,6 +277,110 @@ int auxtrace_record__snapshot_start(struct auxtrace_record *itr __maybe_unused)
>  
>  #endif
>  
> +#define INDEX_FILE_FMT  "%s.dir/perf.data.%d"
> +
> +static int record__create_index_files(struct record *rec, int nr_index)
> +{
> +	int i = 0;
> +	int ret = -1;
> +	char path[PATH_MAX];
> +	struct perf_data_file *file = &rec->file;
> +
> +	rec->fds = malloc(nr_index * sizeof(int));
> +	if (rec->fds == NULL)
> +		return -ENOMEM;
> +
> +	scnprintf(path, sizeof(path), "%s.dir", file->path);
> +	if (rm_rf(path) < 0 || mkdir(path, S_IRWXU) < 0)
> +		goto out_err;
> +
> +	for (i = 0; i < nr_index; i++) {
> +		scnprintf(path, sizeof(path), INDEX_FILE_FMT, file->path, i);
> +		ret = open(path, O_RDWR|O_CREAT|O_TRUNC, S_IRUSR|S_IWUSR);
> +		if (ret < 0)
> +			goto out_err;
> +
> +		rec->fds[i] = ret;
> +	}
> +	return 0;
> +
> +out_err:
> +	while (--i >= 1)
> +		close(rec->fds[i]);
> +	zfree(&rec->fds);
> +
> +	scnprintf(path, sizeof(path), "%s.dir", file->path);
> +	rm_rf(path);
> +
> +	return ret;
> +}
> +
> +static int record__merge_index_files(struct record *rec, int nr_index)
> +{
> +	int i;
> +	int ret = -ENOMEM;
> +	u64 offset;
> +	char path[PATH_MAX];
> +	struct perf_file_section *idx;
> +	struct perf_data_file *file = &rec->file;
> +	struct perf_session *session = rec->session;
> +	int output_fd = perf_data_file__fd(file);
> +
> +	/* +1 for header file itself */
> +	nr_index++;
> +
> +	idx = calloc(nr_index, sizeof(*idx));
> +	if (idx == NULL)
> +		goto out_close;
> +
> +	offset = lseek(output_fd, 0, SEEK_END);
> +
> +	idx[0].offset = session->header.data_offset;
> +	idx[0].size   = offset - idx[0].offset;
> +
> +	for (i = 1; i < nr_index; i++) {
> +		struct stat stbuf;
> +		int fd = rec->fds[i - 1];
> +
> +		ret = fstat(fd, &stbuf);
> +		if (ret < 0)
> +			goto out_close;
> +
> +		idx[i].offset = offset;
> +		idx[i].size   = stbuf.st_size;
> +
> +		offset += stbuf.st_size;
> +
> +		if (idx[i].size == 0)
> +			continue;
> +
> +		ret = copyfile_offset(fd, 0, output_fd, idx[i].offset,
> +				      idx[i].size);
> +		if (ret < 0)
> +			goto out_close;
> +	}
> +
> +	session->header.index = idx;
> +	session->header.nr_index = nr_index;
> +
> +	perf_has_index = true;
> +
> +	ret = 0;
> +
> +out_close:
> +	if (ret < 0)
> +		pr_err("failed to merge index files: %d\n", ret);
> +
> +	for (i = 0; i < nr_index - 1; i++)
> +		close(rec->fds[i]);
> +
> +	scnprintf(path, sizeof(path), "%s.dir", file->path);
> +	rm_rf(path);
> +
> +	zfree(&rec->fds);
> +	return ret;
> +}
> +
>  static int record__open(struct record *rec)
>  {
>  	char msg[512];
> @@ -306,7 +419,8 @@ try_again:
>  
>  	if (perf_evlist__mmap_ex(evlist, opts->mmap_pages, false,
>  				 opts->auxtrace_mmap_pages,
> -				 opts->auxtrace_snapshot_mode, false) < 0) {
> +				 opts->auxtrace_snapshot_mode,
> +				 opts->index) < 0) {
>  		if (errno == EPERM) {
>  			pr_err("Permission error mapping pages.\n"
>  			       "Consider increasing "
> @@ -323,6 +437,14 @@ try_again:
>  		goto out;
>  	}
>  
> +	if (opts->index) {
> +		rc = record__create_index_files(rec, evlist->nr_mmaps);
> +		if (rc < 0) {
> +			pr_err("failed to create index file: %d\n", rc);
> +			goto out;
> +		}
> +	}
> +
>  	session->evlist = evlist;
>  	perf_session__set_id_hdr_size(session);
>  out:
> @@ -347,7 +469,9 @@ static int process_buildids(struct record *rec)
>  	struct perf_data_file *file  = &rec->file;
>  	struct perf_session *session = rec->session;
>  
> -	if (file->size == 0)
> +	/* update file size after merging sample files with index */
> +	u64 size = lseek(perf_data_file__fd(file), 0, SEEK_END);
> +	if (size == 0)
>  		return 0;
>  
>  	/*
> @@ -414,6 +538,13 @@ static int record__mmap_read_all(struct record *rec)
>  			}
>  		}
>  
> +		if (rec->evlist->track_mmap && rec->evlist->track_mmap[i].base) {
> +			if (record__mmap_read(rec, track_mmap_idx(i)) != 0) {
> +				rc = -1;
> +				goto out;
> +			}
> +		}
> +
>  		if (mm->base && !rec->opts.auxtrace_snapshot_mode &&
>  		    record__auxtrace_mmap_read(rec, mm) != 0) {
>  			rc = -1;
> @@ -426,7 +557,8 @@ static int record__mmap_read_all(struct record *rec)
>  	 * at least one event.
>  	 */
>  	if (bytes_written != rec->bytes_written)
> -		rc = record__write(rec, &finished_round_event, sizeof(finished_round_event));
> +		rc = record__write(rec, &finished_round_event,
> +				   sizeof(finished_round_event), -1);
>  
>  out:
>  	return rc;
> @@ -452,7 +584,8 @@ static void record__init_features(struct record *rec)
>  	if (!rec->opts.full_auxtrace)
>  		perf_header__clear_feat(&session->header, HEADER_AUXTRACE);
>  
> -	perf_header__clear_feat(&session->header, HEADER_DATA_INDEX);
> +	if (!rec->opts.index)
> +		perf_header__clear_feat(&session->header, HEADER_DATA_INDEX);
>  }
>  
>  static volatile int workload_exec_errno;
> @@ -520,6 +653,11 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>  		}
>  	}
>  
> +	if (file->is_pipe && opts->index) {
> +		pr_warning("Indexing is disabled for pipe output\n");
> +		opts->index = false;
> +	}
> +
>  	if (record__open(rec) != 0) {
>  		err = -1;
>  		goto out_child;
> @@ -753,6 +891,9 @@ out_child:
>  		rec->session->header.data_size += rec->bytes_written;
>  		file->size = lseek(perf_data_file__fd(file), 0, SEEK_CUR);
>  
> +		if (rec->opts.index)
> +			record__merge_index_files(rec, rec->evlist->nr_mmaps);
> +
>  		if (!rec->no_buildid) {
>  			process_buildids(rec);
>  			/*
> @@ -1119,6 +1260,8 @@ struct option __record_options[] = {
>  			"per thread proc mmap processing timeout in ms"),
>  	OPT_BOOLEAN(0, "switch-events", &record.opts.record_switch_events,
>  		    "Record context switch events"),
> +	OPT_BOOLEAN(0, "index", &record.opts.index,
> +		    "make index for sample data to speed-up processing"),
>  	OPT_END()
>  };
>  
> @@ -1186,6 +1329,15 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
>  		goto out_symbol_exit;
>  	}
>  
> +	if (rec->opts.index) {
> +		if (!rec->opts.sample_time) {
> +			pr_err("Sample timestamp is required for indexing\n");
> +			goto out_symbol_exit;
> +		}
> +
> +		perf_evlist__add_dummy_tracking(rec->evlist);
> +	}
> +
>  	if (rec->opts.target.tid && !rec->opts.no_inherit_set)
>  		rec->opts.no_inherit = true;
>  
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index f4b4d7d8752c..df7c208abb74 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -60,6 +60,7 @@ struct record_opts {
>  	bool	     full_auxtrace;
>  	bool	     auxtrace_snapshot_mode;
>  	bool	     record_switch_events;
> +	bool	     index;
>  	unsigned int freq;
>  	unsigned int mmap_pages;
>  	unsigned int auxtrace_mmap_pages;
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index c357f7f47d32..13ba1402ec1b 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2706,6 +2706,8 @@ int perf_session__read_header(struct perf_session *session)
>  						   session->tevent.pevent))
>  		goto out_delete_evlist;
>  
> +	perf_has_index = perf_header__has_feat(&session->header, HEADER_DATA_INDEX);
> +
>  	return 0;
>  out_errno:
>  	return -errno;
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 91fa9647f565..7546c4d147b9 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -182,6 +182,7 @@ void perf_session__delete(struct perf_session *session)
>  	machines__exit(&session->machines);
>  	if (session->file)
>  		perf_data_file__close(session->file);
> +	free(session->header.index);
>  	free(session);
>  }
>  
> -- 
> 2.6.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1239570 — Re: [RFC/PATCH 09/38] perf record: Add --index option for building index table

From	Jiri Olsa <jolsa@redhat.com>
Date	2015-10-05 15:50 +0200
Subject	Re: [RFC/PATCH 09/38] perf record: Add --index option for building index table
Message-ID	<qgejn-uJ-3@gated-at.bofh.it>
In reply to	#1237928

On Fri, Oct 02, 2015 at 02:18:50PM +0900, Namhyung Kim wrote:

SNIP

> +static int record__merge_index_files(struct record *rec, int nr_index)
> +{
> +	int i;
> +	int ret = -ENOMEM;
> +	u64 offset;
> +	char path[PATH_MAX];
> +	struct perf_file_section *idx;
> +	struct perf_data_file *file = &rec->file;
> +	struct perf_session *session = rec->session;
> +	int output_fd = perf_data_file__fd(file);
> +
> +	/* +1 for header file itself */
> +	nr_index++;
> +
> +	idx = calloc(nr_index, sizeof(*idx));
> +	if (idx == NULL)
> +		goto out_close;
> +
> +	offset = lseek(output_fd, 0, SEEK_END);
> +
> +	idx[0].offset = session->header.data_offset;
> +	idx[0].size   = offset - idx[0].offset;
> +
> +	for (i = 1; i < nr_index; i++) {
> +		struct stat stbuf;
> +		int fd = rec->fds[i - 1];
> +
> +		ret = fstat(fd, &stbuf);
> +		if (ret < 0)
> +			goto out_close;
> +
> +		idx[i].offset = offset;
> +		idx[i].size   = stbuf.st_size;
> +
> +		offset += stbuf.st_size;
> +
> +		if (idx[i].size == 0)
> +			continue;
> +
> +		ret = copyfile_offset(fd, 0, output_fd, idx[i].offset,
> +				      idx[i].size);
> +		if (ret < 0)
> +			goto out_close;
> +	}
> +
> +	session->header.index = idx;
> +	session->header.nr_index = nr_index;
> +
> +	perf_has_index = true;

I might have asked earlier, but why is this global? seems like
perf_session member to me..

thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241233 — Re: [RFC/PATCH 09/38] perf record: Add --index option for building index table

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-10-07 10:30 +0200
Subject	Re: [RFC/PATCH 09/38] perf record: Add --index option for building index table
Message-ID	<qgSgO-7JK-21@gated-at.bofh.it>
In reply to	#1239570

Hi Jiri,

On Mon, Oct 5, 2015 at 10:46 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Fri, Oct 02, 2015 at 02:18:50PM +0900, Namhyung Kim wrote:
>
> SNIP
>
>> +static int record__merge_index_files(struct record *rec, int nr_index)
>> +{
>> +     int i;
>> +     int ret = -ENOMEM;
>> +     u64 offset;
>> +     char path[PATH_MAX];
>> +     struct perf_file_section *idx;
>> +     struct perf_data_file *file = &rec->file;
>> +     struct perf_session *session = rec->session;
>> +     int output_fd = perf_data_file__fd(file);
>> +
>> +     /* +1 for header file itself */
>> +     nr_index++;
>> +
>> +     idx = calloc(nr_index, sizeof(*idx));
>> +     if (idx == NULL)
>> +             goto out_close;
>> +
>> +     offset = lseek(output_fd, 0, SEEK_END);
>> +
>> +     idx[0].offset = session->header.data_offset;
>> +     idx[0].size   = offset - idx[0].offset;
>> +
>> +     for (i = 1; i < nr_index; i++) {
>> +             struct stat stbuf;
>> +             int fd = rec->fds[i - 1];
>> +
>> +             ret = fstat(fd, &stbuf);
>> +             if (ret < 0)
>> +                     goto out_close;
>> +
>> +             idx[i].offset = offset;
>> +             idx[i].size   = stbuf.st_size;
>> +
>> +             offset += stbuf.st_size;
>> +
>> +             if (idx[i].size == 0)
>> +                     continue;
>> +
>> +             ret = copyfile_offset(fd, 0, output_fd, idx[i].offset,
>> +                                   idx[i].size);
>> +             if (ret < 0)
>> +                     goto out_close;
>> +     }
>> +
>> +     session->header.index = idx;
>> +     session->header.nr_index = nr_index;
>> +
>> +     perf_has_index = true;
>
> I might have asked earlier, but why is this global? seems like
> perf_session member to me..

Yes you did. :-)

https://lkml.org/lkml/2015/5/19/110


Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1241418 — Re: [RFC/PATCH 09/38] perf record: Add --index option for building index table

From	Jiri Olsa <jolsa@redhat.com>
Date	2015-10-07 14:20 +0200
Subject	Re: [RFC/PATCH 09/38] perf record: Add --index option for building index table
Message-ID	<qgVRn-4wo-1@gated-at.bofh.it>
In reply to	#1241233

On Wed, Oct 07, 2015 at 05:21:46PM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Mon, Oct 5, 2015 at 10:46 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> > On Fri, Oct 02, 2015 at 02:18:50PM +0900, Namhyung Kim wrote:
> >
> > SNIP
> >
> >> +static int record__merge_index_files(struct record *rec, int nr_index)
> >> +{
> >> +     int i;
> >> +     int ret = -ENOMEM;
> >> +     u64 offset;
> >> +     char path[PATH_MAX];
> >> +     struct perf_file_section *idx;
> >> +     struct perf_data_file *file = &rec->file;
> >> +     struct perf_session *session = rec->session;
> >> +     int output_fd = perf_data_file__fd(file);
> >> +
> >> +     /* +1 for header file itself */
> >> +     nr_index++;
> >> +
> >> +     idx = calloc(nr_index, sizeof(*idx));
> >> +     if (idx == NULL)
> >> +             goto out_close;
> >> +
> >> +     offset = lseek(output_fd, 0, SEEK_END);
> >> +
> >> +     idx[0].offset = session->header.data_offset;
> >> +     idx[0].size   = offset - idx[0].offset;
> >> +
> >> +     for (i = 1; i < nr_index; i++) {
> >> +             struct stat stbuf;
> >> +             int fd = rec->fds[i - 1];
> >> +
> >> +             ret = fstat(fd, &stbuf);
> >> +             if (ret < 0)
> >> +                     goto out_close;
> >> +
> >> +             idx[i].offset = offset;
> >> +             idx[i].size   = stbuf.st_size;
> >> +
> >> +             offset += stbuf.st_size;
> >> +
> >> +             if (idx[i].size == 0)
> >> +                     continue;
> >> +
> >> +             ret = copyfile_offset(fd, 0, output_fd, idx[i].offset,
> >> +                                   idx[i].size);
> >> +             if (ret < 0)
> >> +                     goto out_close;
> >> +     }
> >> +
> >> +     session->header.index = idx;
> >> +     session->header.nr_index = nr_index;
> >> +
> >> +     perf_has_index = true;
> >
> > I might have asked earlier, but why is this global? seems like
> > perf_session member to me..
> 
> Yes you did. :-)
> 
> https://lkml.org/lkml/2015/5/19/110

ah right ;-) ok

thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1237930 — [RFC/PATCH 23/38] perf tools: Use map_groups__find_addr_by_time()

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-10-02 07:30 +0200
Subject	[RFC/PATCH 23/38] perf tools: Use map_groups__find_addr_by_time()
Message-ID	<qf14T-2jW-51@gated-at.bofh.it>
In reply to	#1237913

Use timestamp to find a corresponding map so that it can find a match
symbol eventually.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/event.c  | 81 ++++++++++++++++++++++++++++++++++++++++++------
 tools/perf/util/thread.c |  8 +++--
 2 files changed, 77 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index c960cbcd30d4..d7997105ee7a 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -895,12 +895,11 @@ int perf_event__process(struct perf_tool *tool __maybe_unused,
 	return machine__process_event(machine, event, sample);
 }
 
-static void map_groups__find_addr_map(struct map_groups *mg, u8 cpumode,
-				      enum map_type type, u64 addr,
-				      struct addr_location *al)
+static bool map_groups__set_addr_location(struct map_groups *mg,
+					  struct addr_location *al,
+					  u8 cpumode, u64 addr)
 {
 	struct machine *machine = mg->machine;
-	bool load_map = false;
 
 	al->machine = machine;
 	al->addr = addr;
@@ -909,21 +908,17 @@ static void map_groups__find_addr_map(struct map_groups *mg, u8 cpumode,
 
 	if (machine == NULL) {
 		al->map = NULL;
-		return;
+		return true;
 	}
 
 	BUG_ON(mg == NULL);
 
 	if (cpumode == PERF_RECORD_MISC_KERNEL && perf_host) {
 		al->level = 'k';
-		mg = &machine->kmaps;
-		load_map = true;
 	} else if (cpumode == PERF_RECORD_MISC_USER && perf_host) {
 		al->level = '.';
 	} else if (cpumode == PERF_RECORD_MISC_GUEST_KERNEL && perf_guest) {
 		al->level = 'g';
-		mg = &machine->kmaps;
-		load_map = true;
 	} else if (cpumode == PERF_RECORD_MISC_GUEST_USER && perf_guest) {
 		al->level = 'u';
 	} else {
@@ -939,8 +934,27 @@ static void map_groups__find_addr_map(struct map_groups *mg, u8 cpumode,
 			!perf_host)
 			al->filtered |= (1 << HIST_FILTER__HOST);
 
+		return true;
+	}
+	return false;
+}
+
+static void map_groups__find_addr_map(struct map_groups *mg, u8 cpumode,
+				      enum map_type type, u64 addr,
+				      struct addr_location *al)
+{
+	struct machine *machine = mg->machine;
+	bool load_map = false;
+
+	if (map_groups__set_addr_location(mg, al, cpumode, addr))
 		return;
+
+	if ((cpumode == PERF_RECORD_MISC_KERNEL && perf_host) ||
+	    (cpumode == PERF_RECORD_MISC_GUEST_KERNEL && perf_guest)) {
+		mg = &machine->kmaps;
+		load_map = true;
 	}
+
 try_again:
 	al->map = map_groups__find(mg, type, al->addr);
 	if (al->map == NULL) {
@@ -971,6 +985,53 @@ try_again:
 	}
 }
 
+static void map_groups__find_addr_map_by_time(struct map_groups *mg, u8 cpumode,
+					      enum map_type type, u64 addr,
+					      struct addr_location *al,
+					      u64 timestamp)
+{
+	struct machine *machine = mg->machine;
+	bool load_map = false;
+
+	if (map_groups__set_addr_location(mg, al, cpumode, addr))
+		return;
+
+	if ((cpumode == PERF_RECORD_MISC_KERNEL && perf_host) ||
+	    (cpumode == PERF_RECORD_MISC_GUEST_KERNEL && perf_guest)) {
+		mg = &machine->kmaps;
+		load_map = true;
+	}
+
+try_again:
+	al->map = map_groups__find_by_time(mg, type, al->addr, timestamp);
+	if (al->map == NULL) {
+		/*
+		 * If this is outside of all known maps, and is a negative
+		 * address, try to look it up in the kernel dso, as it might be
+		 * a vsyscall or vdso (which executes in user-mode).
+		 *
+		 * XXX This is nasty, we should have a symbol list in the
+		 * "[vdso]" dso, but for now lets use the old trick of looking
+		 * in the whole kernel symbol list.
+		 */
+		if (cpumode == PERF_RECORD_MISC_USER && machine &&
+		    mg != &machine->kmaps &&
+		    machine__kernel_ip(machine, al->addr)) {
+			mg = &machine->kmaps;
+			load_map = true;
+			goto try_again;
+		}
+	} else {
+		/*
+		 * Kernel maps might be changed when loading symbols so loading
+		 * must be done prior to using kernel maps.
+		 */
+		if (load_map)
+			map__load(al->map, machine->symbol_filter);
+		al->addr = al->map->map_ip(al->map, al->addr);
+	}
+}
+
 void thread__find_addr_map(struct thread *thread, u8 cpumode,
 			   enum map_type type, u64 addr,
 			   struct addr_location *al)
@@ -991,7 +1052,7 @@ void thread__find_addr_map_by_time(struct thread *thread, u8 cpumode,
 		mg = thread->mg;
 
 	al->thread = thread;
-	map_groups__find_addr_map(mg, cpumode, type, addr, al);
+	map_groups__find_addr_map_by_time(mg, cpumode, type, addr, al, timestamp);
 }
 
 void thread__find_addr_location(struct thread *thread,
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index efd510d5d966..21de681415f4 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -337,8 +337,12 @@ size_t thread__fprintf(struct thread *thread, FILE *fp)
 
 void thread__insert_map(struct thread *thread, struct map *map)
 {
-	map_groups__fixup_overlappings(thread->mg, map, stderr);
-	map_groups__insert(thread->mg, map);
+	if (perf_has_index) {
+		map_groups__insert_by_time(thread->mg, map);
+	} else {
+		map_groups__fixup_overlappings(thread->mg, map, stderr);
+		map_groups__insert(thread->mg, map);
+	}
 }
 
 static int thread__clone_map_groups(struct thread *thread,
-- 
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1237931 — [RFC/PATCH 28/38] perf tools: Move BUILD_ID_SIZE definition to perf.h

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-10-02 07:30 +0200
Subject	[RFC/PATCH 28/38] perf tools: Move BUILD_ID_SIZE definition to perf.h
Message-ID	<qf14T-2jW-53@gated-at.bofh.it>
In reply to	#1237913

The util/event.h includes util/build-id.h only for BUILD_ID_SIZE.
This is a problem when I include util/event.h from util/tool.h which
is also included by util/build-id.h since it now makes a circular
dependency resulting in incomplete type error.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/perf.h          | 3 +++
 tools/perf/util/build-id.h | 3 ---
 tools/perf/util/dso.h      | 1 +
 tools/perf/util/event.h    | 1 -
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index df7c208abb74..d21b5c63f244 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -31,6 +31,9 @@ static inline unsigned long long rdclock(void)
 
 #define MAX_NR_CPUS			1024
 
+#define BUILD_ID_SIZE			20
+#define SBUILD_ID_SIZE	(BUILD_ID_SIZE * 2 + 1)
+
 extern const char *input_name;
 extern bool perf_host, perf_guest;
 extern const char perf_version_string[];
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index 27a14a8a945b..8f9a5720bc5e 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -1,9 +1,6 @@
 #ifndef PERF_BUILD_ID_H_
 #define PERF_BUILD_ID_H_ 1
 
-#define BUILD_ID_SIZE	20
-#define SBUILD_ID_SIZE	(BUILD_ID_SIZE * 2 + 1)
-
 #include "tool.h"
 #include "strlist.h"
 #include <linux/types.h>
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index fc8db9c764ac..416b9a57fcb9 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -9,6 +9,7 @@
 #include <linux/types.h>
 #include <linux/bitops.h>
 #include "map.h"
+#include "perf.h"
 #include "build-id.h"
 
 enum dso_binary_type {
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index a0dbcbd4f6d8..3812d645362c 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -6,7 +6,6 @@
 
 #include "../perf.h"
 #include "map.h"
-#include "build-id.h"
 #include "perf_regs.h"
 
 struct mmap_event {
-- 
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1237932 — [RFC/PATCH 24/38] perf tools: Add testcase for managing maps with time

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-10-02 07:30 +0200
Subject	[RFC/PATCH 24/38] perf tools: Add testcase for managing maps with time
Message-ID	<qf14T-2jW-55@gated-at.bofh.it>
In reply to	#1237913

This tests new map_groups__{insert,find}_by_time() API working
correctly by using 3 * 100 maps.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/Build             |  1 +
 tools/perf/tests/builtin-test.c    |  4 ++
 tools/perf/tests/tests.h           |  1 +
 tools/perf/tests/thread-map-time.c | 90 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+)
 create mode 100644 tools/perf/tests/thread-map-time.c

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index d287b99ff3bb..cc4e3af3e0fd 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -28,6 +28,7 @@ perf-y += thread-comm.o
 perf-y += thread-mg-share.o
 perf-y += thread-lookup-time.o
 perf-y += thread-mg-time.o
+perf-y += thread-map-time.o
 perf-y += switch-tracking.o
 perf-y += keep-tracking.o
 perf-y += code-reading.o
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 62de08a89e0e..d5f9fcef5571 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -203,6 +203,10 @@ static struct test {
 		.func = test__thread_mg_time,
 	},
 	{
+		.desc = "Test thread map lookup with time",
+		.func = test__thread_map_lookup_time,
+	},
+	{
 		.func = NULL,
 	},
 };
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 03dcaccb570f..e498b23f1580 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -68,6 +68,7 @@ int test_session_topology(void);
 int test__thread_comm(void);
 int test__thread_lookup_time(void);
 int test__thread_mg_time(void);
+int test__thread_map_lookup_time(void);
 
 #if defined(__x86_64__) || defined(__i386__) || defined(__arm__) || defined(__aarch64__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
diff --git a/tools/perf/tests/thread-map-time.c b/tools/perf/tests/thread-map-time.c
new file mode 100644
index 000000000000..6f28975faeb5
--- /dev/null
+++ b/tools/perf/tests/thread-map-time.c
@@ -0,0 +1,90 @@
+#include "debug.h"
+#include "tests.h"
+#include "machine.h"
+#include "thread.h"
+#include "map.h"
+
+#define PERF_MAP_START  0x40000
+#define LIBC_MAP_START  0x80000
+#define VDSO_MAP_START  0x7F000
+
+#define NR_MAPS  100
+
+static int lookup_maps(struct map_groups *mg)
+{
+	struct map *map;
+	int i, ret = -1;
+	size_t n;
+	struct {
+		const char *path;
+		u64 start;
+	} maps[] = {
+		{ "/usr/bin/perf",	PERF_MAP_START },
+		{ "/usr/lib/libc.so",	LIBC_MAP_START },
+		{ "[vdso]",		VDSO_MAP_START },
+	};
+
+	/* this is needed to insert/find map by time */
+	perf_has_index = true;
+
+	for (n = 0; n < ARRAY_SIZE(maps); n++) {
+		for (i = 0; i < NR_MAPS; i++) {
+			map = map__new2(maps[n].start, dso__new(maps[n].path),
+					MAP__FUNCTION, i * 10000);
+			if (map == NULL) {
+				pr_debug("memory allocation failed\n");
+				goto out;
+			}
+
+			map->end = map->start + 0x1000;
+			map_groups__insert_by_time(mg, map);
+		}
+	}
+
+	if (verbose > 1)
+		map_groups__fprintf(mg, stderr);
+
+	for (n = 0; n < ARRAY_SIZE(maps); n++) {
+		for (i = 0; i < NR_MAPS; i++) {
+			u64 timestamp = i * 10000;
+
+			map = map_groups__find_by_time(mg, MAP__FUNCTION,
+						       maps[n].start,
+						       timestamp);
+
+			TEST_ASSERT_VAL("cannot find map", map);
+			TEST_ASSERT_VAL("addr not matched",
+					map->start == maps[n].start);
+			TEST_ASSERT_VAL("pathname not matched",
+					!strcmp(map->dso->name, maps[n].path));
+			TEST_ASSERT_VAL("timestamp not matched",
+					map->timestamp == timestamp);
+		}
+	}
+
+	ret = 0;
+out:
+	return ret;
+}
+
+/*
+ * This test creates large number of overlapping maps for increasing
+ * time and find a map based on timestamp.
+ */
+int test__thread_map_lookup_time(void)
+{
+	struct machines machines;
+	struct machine *machine;
+	struct thread *t;
+	int ret;
+
+	machines__init(&machines);
+	machine = &machines.host;
+
+	t = machine__findnew_thread(machine, 0, 0);
+
+	ret = lookup_maps(t->mg);
+
+	machine__delete_threads(machine);
+	return ret;
+}
-- 
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1237933 — [RFC/PATCH 11/38] perf tools: Introduce thread__comm(_str)_by_time() helpers

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-10-02 07:30 +0200
Subject	[RFC/PATCH 11/38] perf tools: Introduce thread__comm(_str)_by_time() helpers
Message-ID	<qf14T-2jW-57@gated-at.bofh.it>
In reply to	#1237913

When data file indexing is enabled, it processes all task, comm and mmap
events first and then goes to the sample events.  So all it sees is the
last comm of a thread although it has information at the time of sample.

Sort thread's comm by time so that it can find appropriate comm at the
sample time.  The thread__comm_by_time() will mostly work even if
PERF_SAMPLE_TIME bit is off since in that case, sample->time will be
-1 so it'll take the last comm anyway.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/thread.c | 33 ++++++++++++++++++++++++++++++++-
 tools/perf/util/thread.h |  2 ++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 0a9ae8014729..8244397753fd 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -121,6 +121,21 @@ struct comm *thread__exec_comm(const struct thread *thread)
 	return last;
 }
 
+struct comm *thread__comm_by_time(const struct thread *thread, u64 timestamp)
+{
+	struct comm *comm;
+
+	list_for_each_entry(comm, &thread->comm_list, list) {
+		if (timestamp >= comm->start)
+			return comm;
+	}
+
+	if (list_empty(&thread->comm_list))
+		return NULL;
+
+	return list_last_entry(&thread->comm_list, struct comm, list);
+}
+
 int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
 		       bool exec)
 {
@@ -136,7 +151,13 @@ int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
 		new = comm__new(str, timestamp, exec);
 		if (!new)
 			return -ENOMEM;
-		list_add(&new->list, &thread->comm_list);
+
+		/* sort by time */
+		list_for_each_entry(curr, &thread->comm_list, list) {
+			if (timestamp >= curr->start)
+				break;
+		}
+		list_add_tail(&new->list, &curr->list);
 
 		if (exec)
 			unwind__flush_access(thread);
@@ -157,6 +178,16 @@ const char *thread__comm_str(const struct thread *thread)
 	return comm__str(comm);
 }
 
+const char *thread__comm_str_by_time(const struct thread *thread, u64 timestamp)
+{
+	const struct comm *comm = thread__comm_by_time(thread, timestamp);
+
+	if (!comm)
+		return NULL;
+
+	return comm__str(comm);
+}
+
 /* CHECKME: it should probably better return the max comm len from its comm list */
 int thread__comm_len(struct thread *thread)
 {
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index a0ac0317affb..33418e6dc64a 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -68,7 +68,9 @@ static inline int thread__set_comm(struct thread *thread, const char *comm,
 int thread__comm_len(struct thread *thread);
 struct comm *thread__comm(const struct thread *thread);
 struct comm *thread__exec_comm(const struct thread *thread);
+struct comm *thread__comm_by_time(const struct thread *thread, u64 timestamp);
 const char *thread__comm_str(const struct thread *thread);
+const char *thread__comm_str_by_time(const struct thread *thread, u64 timestamp);
 void thread__insert_map(struct thread *thread, struct map *map);
 int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp);
 size_t thread__fprintf(struct thread *thread, FILE *fp);
-- 
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1237934 — [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-10-02 07:30 +0200
Subject	[RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread
Message-ID	<qf14T-2jW-59@gated-at.bofh.it>
In reply to	#1237913

To support multi-threaded perf report, we need to maintain time-sorted
map groups.  Add ->mg_list member to struct thread and sort the list
by time.  Now leader threads have one more refcnt for map groups in
the list so also update the thread-mg-share test case.

Currently only add a new map groups when an exec (comm) event is
received.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/thread-mg-share.c |   7 ++-
 tools/perf/util/event.c            |   2 +
 tools/perf/util/machine.c          |  15 ++++-
 tools/perf/util/map.c              |   3 +
 tools/perf/util/map.h              |   2 +
 tools/perf/util/thread.c           | 111 ++++++++++++++++++++++++++++++++++++-
 tools/perf/util/thread.h           |   3 +
 7 files changed, 138 insertions(+), 5 deletions(-)

diff --git a/tools/perf/tests/thread-mg-share.c b/tools/perf/tests/thread-mg-share.c
index 01fabb19d746..b258d5298b9b 100644
--- a/tools/perf/tests/thread-mg-share.c
+++ b/tools/perf/tests/thread-mg-share.c
@@ -23,6 +23,9 @@ int test__thread_mg_share(void)
 	 * with several threads and checks they properly share and
 	 * maintain map groups info (struct map_groups).
 	 *
+	 * Note that a leader thread has one more refcnt for its
+	 * (current) map groups.
+	 *
 	 * thread group (pid: 0, tids: 0, 1, 2, 3)
 	 * other  group (pid: 4, tids: 4, 5)
 	*/
@@ -43,7 +46,7 @@ int test__thread_mg_share(void)
 			leader && t1 && t2 && t3 && other);
 
 	mg = leader->mg;
-	TEST_ASSERT_EQUAL("wrong refcnt", atomic_read(&mg->refcnt), 4);
+	TEST_ASSERT_EQUAL("wrong refcnt", atomic_read(&mg->refcnt), 5);
 
 	/* test the map groups pointer is shared */
 	TEST_ASSERT_VAL("map groups don't match", mg == t1->mg);
@@ -71,7 +74,7 @@ int test__thread_mg_share(void)
 	machine__remove_thread(machine, other_leader);
 
 	other_mg = other->mg;
-	TEST_ASSERT_EQUAL("wrong refcnt", atomic_read(&other_mg->refcnt), 2);
+	TEST_ASSERT_EQUAL("wrong refcnt", atomic_read(&other_mg->refcnt), 3);
 
 	TEST_ASSERT_VAL("map groups don't match", other_mg == other_leader->mg);
 
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 3dff1b5cd4cc..887f18266ab5 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -914,6 +914,8 @@ void thread__find_addr_map(struct thread *thread, u8 cpumode,
 		return;
 	}
 
+	BUG_ON(mg == NULL);
+
 	if (cpumode == PERF_RECORD_MISC_KERNEL && perf_host) {
 		al->level = 'k';
 		mg = &machine->kmaps;
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 7cfaa2c3f131..3373e8455945 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -349,8 +349,19 @@ static void machine__update_thread_pid(struct machine *machine,
 	if (!leader)
 		goto out_err;
 
-	if (!leader->mg)
-		leader->mg = map_groups__new(machine);
+	if (!leader->mg) {
+		struct map_groups *mg = map_groups__new(machine);
+
+		if (mg == NULL) {
+			pr_err("Not enough memory for map groups\n");
+			return;
+		}
+
+		if (thread__set_map_groups(leader, mg, 0) < 0) {
+			map_groups__put(mg);
+			goto out_err;
+		}
+	}
 
 	if (!leader->mg)
 		goto out_err;
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 4e38c396a897..addd4b323027 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -471,6 +471,8 @@ void map_groups__init(struct map_groups *mg, struct machine *machine)
 	}
 	mg->machine = machine;
 	atomic_set(&mg->refcnt, 1);
+	mg->timestamp = 0;
+	INIT_LIST_HEAD(&mg->list);
 }
 
 static void __maps__purge(struct maps *maps)
@@ -527,6 +529,7 @@ struct map_groups *map_groups__new(struct machine *machine)
 void map_groups__delete(struct map_groups *mg)
 {
 	map_groups__exit(mg);
+	list_del(&mg->list);
 	free(mg);
 }
 
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 7309d64ce39e..1e3313a22d3a 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -68,6 +68,8 @@ struct map_groups {
 	struct maps	 maps[MAP__NR_TYPES];
 	struct machine	 *machine;
 	atomic_t	 refcnt;
+	u64		 timestamp;
+	struct list_head list;
 };
 
 struct map_groups *map_groups__new(struct machine *machine);
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index ad7c2a00bff8..33de8b010282 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -11,13 +11,79 @@
 #include "unwind.h"
 #include "machine.h"
 
+struct map_groups *thread__get_map_groups(struct thread *thread, u64 timestamp)
+{
+	struct map_groups *mg;
+	struct thread *leader = thread;
+
+	BUG_ON(thread->mg == NULL);
+
+	if (thread->tid != thread->pid_) {
+		leader = machine__find_thread_by_time(thread->mg->machine,
+						      thread->pid_, thread->pid_,
+						      timestamp);
+		if (leader == NULL)
+			goto out;
+	}
+
+	list_for_each_entry(mg, &leader->mg_list, list)
+		if (timestamp >= mg->timestamp)
+			return mg;
+
+out:
+	return thread->mg;
+}
+
+int thread__set_map_groups(struct thread *thread, struct map_groups *mg,
+			   u64 timestamp)
+{
+	struct list_head *pos;
+	struct map_groups *old;
+
+	if (mg == NULL)
+		return -ENOMEM;
+
+	/*
+	 * Only a leader thread can have map groups list - others
+	 * reference it through map_groups__get.  This means the
+	 * leader thread will have one more refcnt than others.
+	 */
+	if (thread->tid != thread->pid_)
+		return -EINVAL;
+
+	if (thread->mg) {
+		BUG_ON(atomic_read(&thread->mg->refcnt) <= 1);
+		map_groups__put(thread->mg);
+	}
+
+	/* sort by time */
+	list_for_each(pos, &thread->mg_list) {
+		old = list_entry(pos, struct map_groups, list);
+		if (timestamp > old->timestamp)
+			break;
+	}
+
+	list_add_tail(&mg->list, pos);
+	mg->timestamp = timestamp;
+
+	/* set current ->mg to most recent one */
+	thread->mg = list_first_entry(&thread->mg_list, struct map_groups, list);
+	/* increase one more refcnt for current */
+	map_groups__get(thread->mg);
+
+	return 0;
+}
+
 int thread__init_map_groups(struct thread *thread, struct machine *machine)
 {
 	struct thread *leader;
 	pid_t pid = thread->pid_;
 
 	if (pid == thread->tid || pid == -1) {
-		thread->mg = map_groups__new(machine);
+		struct map_groups *mg = map_groups__new(machine);
+
+		if (thread__set_map_groups(thread, mg, 0) < 0)
+			map_groups__put(mg);
 	} else {
 		leader = __machine__findnew_thread(machine, pid, pid);
 		if (leader)
@@ -39,6 +105,7 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 		thread->ppid = -1;
 		thread->cpu = -1;
 		INIT_LIST_HEAD(&thread->comm_list);
+		INIT_LIST_HEAD(&thread->mg_list);
 
 		if (unwind__prepare_access(thread) < 0)
 			goto err_thread;
@@ -69,6 +136,7 @@ err_thread:
 void thread__delete(struct thread *thread)
 {
 	struct comm *comm, *tmp;
+	struct map_groups *mg, *tmp_mg;
 
 	BUG_ON(!RB_EMPTY_NODE(&thread->rb_node));
 	BUG_ON(!list_empty(&thread->tid_list));
@@ -79,6 +147,10 @@ void thread__delete(struct thread *thread)
 		map_groups__put(thread->mg);
 		thread->mg = NULL;
 	}
+	/* only leader threads have mg list */
+	list_for_each_entry_safe(mg, tmp_mg, &thread->mg_list, list)
+		map_groups__put(mg);
+
 	list_for_each_entry_safe(comm, tmp, &thread->comm_list, list) {
 		list_del(&comm->list);
 		comm__free(comm);
@@ -152,6 +224,9 @@ struct comm *thread__comm_by_time(const struct thread *thread, u64 timestamp)
 	return list_last_entry(&thread->comm_list, struct comm, list);
 }
 
+static int thread__clone_map_groups(struct thread *thread,
+				    struct thread *parent);
+
 int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
 		       bool exec)
 {
@@ -182,6 +257,40 @@ int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
 			unwind__flush_access(thread);
 	}
 
+	if (exec) {
+		struct machine *machine;
+
+		BUG_ON(thread->mg == NULL || thread->mg->machine == NULL);
+
+		machine = thread->mg->machine;
+
+		if (thread->tid != thread->pid_) {
+			struct map_groups *old = thread->mg;
+			struct thread *leader;
+
+			leader = machine__findnew_thread(machine, thread->pid_,
+							 thread->pid_);
+
+			/* now it'll be a new leader */
+			thread->pid_ = thread->tid;
+
+			thread->mg = map_groups__new(old->machine);
+			if (thread->mg == NULL)
+				return -ENOMEM;
+
+			/* save current mg in the new leader */
+			thread__clone_map_groups(thread, leader);
+
+			/* current mg of leader thread needs one more refcnt */
+			map_groups__get(thread->mg);
+
+			thread__set_map_groups(thread, thread->mg, old->timestamp);
+		}
+
+		/* create a new mg for newly executed binary */
+		thread__set_map_groups(thread, map_groups__new(machine), timestamp);
+	}
+
 	thread->comm_set = true;
 
 	return 0;
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 97026a9660ec..c8463d08a6dd 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -16,6 +16,7 @@ struct thread {
 	struct rb_node	 	rb_node;
 	struct list_head 	tid_list;
 	struct map_groups	*mg;
+	struct list_head	mg_list;
 	pid_t			pid_; /* Not all tools update this */
 	pid_t			tid;
 	pid_t			ppid;
@@ -71,6 +72,8 @@ struct comm *thread__exec_comm(const struct thread *thread);
 struct comm *thread__comm_by_time(const struct thread *thread, u64 timestamp);
 const char *thread__comm_str(const struct thread *thread);
 const char *thread__comm_str_by_time(const struct thread *thread, u64 timestamp);
+struct map_groups *thread__get_map_groups(struct thread *thread, u64 timestamp);
+int thread__set_map_groups(struct thread *thread, struct map_groups *mg, u64 timestamp);
 void thread__insert_map(struct thread *thread, struct map *map);
 int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp);
 size_t thread__fprintf(struct thread *thread, FILE *fp);
-- 
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242316 — Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread

From	Jiri Olsa <jolsa@redhat.com>
Date	2015-10-08 15:00 +0200
Subject	Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread
Message-ID	<qhiXE-3Z7-1@gated-at.bofh.it>
In reply to	#1237934

On Fri, Oct 02, 2015 at 02:18:58PM +0900, Namhyung Kim wrote:

SNIP

>  int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
>  		       bool exec)
>  {
> @@ -182,6 +257,40 @@ int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
>  			unwind__flush_access(thread);
>  	}
>  
> +	if (exec) {
> +		struct machine *machine;
> +
> +		BUG_ON(thread->mg == NULL || thread->mg->machine == NULL);
> +
> +		machine = thread->mg->machine;
> +
> +		if (thread->tid != thread->pid_) {
> +			struct map_groups *old = thread->mg;
> +			struct thread *leader;
> +
> +			leader = machine__findnew_thread(machine, thread->pid_,
> +							 thread->pid_);
> +
> +			/* now it'll be a new leader */
> +			thread->pid_ = thread->tid;
> +
> +			thread->mg = map_groups__new(old->machine);
> +			if (thread->mg == NULL)
> +				return -ENOMEM;
> +
> +			/* save current mg in the new leader */
> +			thread__clone_map_groups(thread, leader);
> +
> +			/* current mg of leader thread needs one more refcnt */
> +			map_groups__get(thread->mg);
> +
> +			thread__set_map_groups(thread, thread->mg, old->timestamp);
> +		}
> +
> +		/* create a new mg for newly executed binary */
> +		thread__set_map_groups(thread, map_groups__new(machine), timestamp);

should this     ^^^^ be in the else case of above condition?

also thread__fork calls thread__clone_map_groups once again,
I have some difficulty to sort this out ATM.. is that correct?

some comment on how we treat map groups in general (for fork/clone/exit)
would be awesome ;-)

thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243075 — Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-10-09 09:10 +0200
Subject	Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread
Message-ID	<qhzYu-3mM-15@gated-at.bofh.it>
In reply to	#1242316

On Thu, Oct 08, 2015 at 02:58:00PM +0200, Jiri Olsa wrote:
> On Fri, Oct 02, 2015 at 02:18:58PM +0900, Namhyung Kim wrote:
> 
> SNIP
> 
> >  int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
> >  		       bool exec)
> >  {
> > @@ -182,6 +257,40 @@ int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
> >  			unwind__flush_access(thread);
> >  	}
> >  
> > +	if (exec) {
> > +		struct machine *machine;
> > +
> > +		BUG_ON(thread->mg == NULL || thread->mg->machine == NULL);
> > +
> > +		machine = thread->mg->machine;
> > +
> > +		if (thread->tid != thread->pid_) {
> > +			struct map_groups *old = thread->mg;
> > +			struct thread *leader;
> > +
> > +			leader = machine__findnew_thread(machine, thread->pid_,
> > +							 thread->pid_);
> > +
> > +			/* now it'll be a new leader */
> > +			thread->pid_ = thread->tid;
> > +
> > +			thread->mg = map_groups__new(old->machine);
> > +			if (thread->mg == NULL)
> > +				return -ENOMEM;
> > +
> > +			/* save current mg in the new leader */
> > +			thread__clone_map_groups(thread, leader);
> > +
> > +			/* current mg of leader thread needs one more refcnt */
> > +			map_groups__get(thread->mg);
> > +
> > +			thread__set_map_groups(thread, thread->mg, old->timestamp);
> > +		}
> > +
> > +		/* create a new mg for newly executed binary */
> > +		thread__set_map_groups(thread, map_groups__new(machine), timestamp);
> 
> should this     ^^^^ be in the else case of above condition?

Nop.  Above condition is to make the thread a new leader thread and
for that purpose, it clones old thread->mg and add into the mg_list.
Because a non-leader thread don't have mg_list.

After that, we can add a new mg to the now-available mg_list.

> 
> also thread__fork calls thread__clone_map_groups once again,
> I have some difficulty to sort this out ATM.. is that correct?

In fork case, above code will not be called since it's only for exec path.

> 
> some comment on how we treat map groups in general (for fork/clone/exit)
> would be awesome ;-)

I admit that this code is subtle and confusing..  How about this?

Managing map groups is subtle in that we basically want to share a map
groups between threads in a process.  When a new process is created
(forked), the child clones (current) map groups from the parent.  But
if a new thread is called it only gets a reference of the leader's mg.

Complication comes from the exec as we also want to keep the history
of a thread's execution, so the map groups are now managed by mg_list.
This mg_list is maintained by leader threads only, and non-leader
threads have a reference a mg at the time in the mg_list.  It uses a
timestamp at the event to find out the correct mg in the mg_list.

One corner case is when exec is called from a non-leader thread.  We
want to add a new mg to the mg_list in the thread.  But it doesn't
have a mg_list since it was not a leader.  So it sets up a mg_list and
insert a cloned mg from the old leader.  Now it can handle exec as
usual - create a new mg and insert it to the mg_list.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1244661 — Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread

From	Jiri Olsa <jolsa@redhat.com>
Date	2015-10-12 14:50 +0200
Subject	Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread
Message-ID	<qiKIb-7EQ-39@gated-at.bofh.it>
In reply to	#1243075

On Fri, Oct 09, 2015 at 03:58:49PM +0900, Namhyung Kim wrote:

SNIP

> 
> 
> > 
> > some comment on how we treat map groups in general (for fork/clone/exit)
> > would be awesome ;-)
> 
> I admit that this code is subtle and confusing..  How about this?
> 
> 
> Managing map groups is subtle in that we basically want to share a map
> groups between threads in a process.  When a new process is created
> (forked), the child clones (current) map groups from the parent.  But
> if a new thread is called it only gets a reference of the leader's mg.
> 
> Complication comes from the exec as we also want to keep the history
> of a thread's execution, so the map groups are now managed by mg_list.
> This mg_list is maintained by leader threads only, and non-leader
> threads have a reference a mg at the time in the mg_list.  It uses a
> timestamp at the event to find out the correct mg in the mg_list.
> 
> One corner case is when exec is called from a non-leader thread.  We
> want to add a new mg to the mg_list in the thread.  But it doesn't
> have a mg_list since it was not a leader.  So it sets up a mg_list and
> insert a cloned mg from the old leader.  Now it can handle exec as
> usual - create a new mg and insert it to the mg_list.

seems ok, thanks

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242321 — Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread

From	Jiri Olsa <jolsa@redhat.com>
Date	2015-10-08 15:00 +0200
Subject	Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread
Message-ID	<qhiXF-3Z7-23@gated-at.bofh.it>
In reply to	#1237934

On Fri, Oct 02, 2015 at 02:18:58PM +0900, Namhyung Kim wrote:

SNIP

> +static int thread__clone_map_groups(struct thread *thread,
> +				    struct thread *parent);
> +
>  int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
>  		       bool exec)
>  {
> @@ -182,6 +257,40 @@ int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
>  			unwind__flush_access(thread);
>  	}
>  
> +	if (exec) {
> +		struct machine *machine;
> +
> +		BUG_ON(thread->mg == NULL || thread->mg->machine == NULL);
> +
> +		machine = thread->mg->machine;
> +
> +		if (thread->tid != thread->pid_) {
> +			struct map_groups *old = thread->mg;
> +			struct thread *leader;
> +
> +			leader = machine__findnew_thread(machine, thread->pid_,
> +							 thread->pid_);
> +
> +			/* now it'll be a new leader */
> +			thread->pid_ = thread->tid;
> +
> +			thread->mg = map_groups__new(old->machine);
> +			if (thread->mg == NULL)
> +				return -ENOMEM;

hum, isn't this leaking thread->mg ?
should we call map_groups__put(old) at the end of block?


jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243049 — Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-10-09 08:30 +0200
Subject	Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread
Message-ID	<qhzlM-2nJ-1@gated-at.bofh.it>
In reply to	#1242321

On Thu, Oct 08, 2015 at 02:51:43PM +0200, Jiri Olsa wrote:
> On Fri, Oct 02, 2015 at 02:18:58PM +0900, Namhyung Kim wrote:
> 
> SNIP
> 
> > +static int thread__clone_map_groups(struct thread *thread,
> > +				    struct thread *parent);
> > +
> >  int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
> >  		       bool exec)
> >  {
> > @@ -182,6 +257,40 @@ int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
> >  			unwind__flush_access(thread);
> >  	}
> >  
> > +	if (exec) {
> > +		struct machine *machine;
> > +
> > +		BUG_ON(thread->mg == NULL || thread->mg->machine == NULL);
> > +
> > +		machine = thread->mg->machine;
> > +
> > +		if (thread->tid != thread->pid_) {
> > +			struct map_groups *old = thread->mg;
> > +			struct thread *leader;
> > +
> > +			leader = machine__findnew_thread(machine, thread->pid_,
> > +							 thread->pid_);
> > +
> > +			/* now it'll be a new leader */
> > +			thread->pid_ = thread->tid;
> > +
> > +			thread->mg = map_groups__new(old->machine);
> > +			if (thread->mg == NULL)
> > +				return -ENOMEM;
> 
> hum, isn't this leaking thread->mg ?
> should we call map_groups__put(old) at the end of block?

You're right!  Will fix.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1237935 — [RFC/PATCH 08/38] perf tools: Handle indexed data file properly

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-10-02 07:40 +0200
Subject	[RFC/PATCH 08/38] perf tools: Handle indexed data file properly
Message-ID	<qf1ex-2v1-1@gated-at.bofh.it>
In reply to	#1237913

When perf detects data file has index table, process header part first
and then rest data files in a row.  Note that the indexed sample data is
recorded for each cpu/thread separately, it's already ordered with
respect to themselves so no need to use the ordered event queue
interface.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/perf.c         |  1 +
 tools/perf/perf.h         |  2 ++
 tools/perf/util/session.c | 55 +++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 1fded922bcc8..9664d84a9f8c 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -28,6 +28,7 @@ const char perf_more_info_string[] =
 int use_browser = -1;
 static int use_pager = -1;
 const char *input_name;
+bool perf_has_index;
 
 struct cmd_struct {
 	const char *cmd;
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 90129accffbe..f4b4d7d8752c 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -39,6 +39,8 @@ void pthread__unblock_sigwinch(void);
 
 #include "util/target.h"
 
+extern bool perf_has_index;
+
 struct record_opts {
 	struct target target;
 	bool	     group;
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 428149bc64d2..91fa9647f565 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1586,7 +1586,9 @@ static int __perf_session__process_events(struct perf_session *session,
 	mmap_size = MMAP_SIZE;
 	if (mmap_size > file_size) {
 		mmap_size = file_size;
-		session->one_mmap = true;
+
+		if (!perf_has_index)
+			session->one_mmap = true;
 	}
 
 	memset(mmaps, 0, sizeof(mmaps));
@@ -1664,28 +1666,63 @@ out:
 	err = perf_session__flush_thread_stacks(session);
 out_err:
 	ui_progress__finish();
-	perf_session__warn_about_errors(session);
 	ordered_events__free(&session->ordered_events);
 	auxtrace__free_events(session);
 	session->one_mmap = false;
 	return err;
 }
 
+static int __perf_session__process_indexed_events(struct perf_session *session)
+{
+	struct perf_data_file *file = session->file;
+	struct perf_tool *tool = session->tool;
+	u64 size = perf_data_file__size(file);
+	int err = 0, i;
+
+	for (i = 0; i < (int)session->header.nr_index; i++) {
+		struct perf_file_section *idx = &session->header.index[i];
+
+		if (!idx->size)
+			continue;
+
+		/*
+		 * For indexed data file, samples are processed for
+		 * each cpu/thread so it's already ordered.  However
+		 * meta-events at index 0 should be processed in order.
+		 */
+		if (i > 0)
+			tool->ordered_events = false;
+
+		err = __perf_session__process_events(session, idx->offset,
+						     idx->size, size);
+		if (err < 0)
+			break;
+	}
+
+	perf_session__warn_about_errors(session);
+	return err;
+}
+
 int perf_session__process_events(struct perf_session *session)
 {
-	u64 size = perf_data_file__size(session->file);
+	struct perf_data_file *file = session->file;
+	u64 size = perf_data_file__size(file);
 	int err;
 
 	if (perf_session__register_idle_thread(session) == NULL)
 		return -ENOMEM;
 
-	if (!perf_data_file__is_pipe(session->file))
-		err = __perf_session__process_events(session,
-						     session->header.data_offset,
-						     session->header.data_size, size);
-	else
-		err = __perf_session__process_pipe_events(session);
+	if (perf_data_file__is_pipe(file))
+		return __perf_session__process_pipe_events(session);
+	if (perf_has_index)
+		return __perf_session__process_indexed_events(session);
+
+	err = __perf_session__process_events(session,
+					     session->header.data_offset,
+					     session->header.data_size,
+					     size);
 
+	perf_session__warn_about_errors(session);
 	return err;
 }
 
-- 
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1237936 — [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-10-02 07:40 +0200
Subject	[RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask
Message-ID	<qf1ex-2v1-3@gated-at.bofh.it>
In reply to	#1237913

It is more convenient saving mmap length rather than (bit) mask.  With
this patch, we can eliminate dependency to perf_evlist other than
getting mmap_desc for dealing with mmaps.  The mask and length can be
converted using perf_evlist__mmap_mask/len().

Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/evlist.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index c5180a29db1b..e46adcd5b408 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -29,6 +29,8 @@
 
 static void perf_evlist__mmap_put(struct perf_evlist *evlist, int idx);
 static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx);
+static size_t perf_evlist__mmap_mask(size_t len);
+static size_t perf_evlist__mmap_len(size_t mask);
 
 #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
 #define SID(e, x, y) xyarray__entry(e->sample_id, x, y)
@@ -871,7 +873,9 @@ void __weak auxtrace_mmap_params__set_idx(
 static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
 {
 	if (evlist->mmap[idx].base != NULL) {
-		munmap(evlist->mmap[idx].base, evlist->mmap_len);
+		size_t mmap_len = perf_evlist__mmap_len(evlist->mmap[idx].mask);
+
+		munmap(evlist->mmap[idx].base, mmap_len);
 		evlist->mmap[idx].base = NULL;
 		atomic_set(&evlist->mmap[idx].refcnt, 0);
 	}
@@ -901,8 +905,8 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
 }
 
 struct mmap_params {
-	int prot;
-	int mask;
+	int	prot;
+	size_t	len;
 	struct auxtrace_mmap_params auxtrace_mp;
 };
 
@@ -924,8 +928,8 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
 	 */
 	atomic_set(&evlist->mmap[idx].refcnt, 2);
 	evlist->mmap[idx].prev = 0;
-	evlist->mmap[idx].mask = mp->mask;
-	evlist->mmap[idx].base = mmap(NULL, evlist->mmap_len, mp->prot,
+	evlist->mmap[idx].mask = perf_evlist__mmap_mask(mp->len);
+	evlist->mmap[idx].base = mmap(NULL, mp->len, mp->prot,
 				      MAP_SHARED, fd, 0);
 	if (evlist->mmap[idx].base == MAP_FAILED) {
 		pr_debug2("failed to mmap perf event ring buffer, error %d\n",
@@ -1071,6 +1075,21 @@ static size_t perf_evlist__mmap_size(unsigned long pages)
 	return (pages + 1) * page_size;
 }
 
+static size_t perf_evlist__mmap_mask(size_t len)
+{
+	BUG_ON(len <= page_size);
+	BUG_ON((len % page_size) != 0);
+
+	return len - page_size - 1;
+}
+
+static size_t perf_evlist__mmap_len(size_t mask)
+{
+	BUG_ON(((mask + 1) % page_size) != 0);
+
+	return mask + 1 + page_size;
+}
+
 static long parse_pages_arg(const char *str, unsigned long min,
 			    unsigned long max)
 {
@@ -1176,7 +1195,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
 	evlist->overwrite = overwrite;
 	evlist->mmap_len = perf_evlist__mmap_size(pages);
 	pr_debug("mmap size %zuB\n", evlist->mmap_len);
-	mp.mask = evlist->mmap_len - page_size - 1;
+	mp.len = evlist->mmap_len;
 
 	auxtrace_mmap_params__init(&mp.auxtrace_mp, evlist->mmap_len,
 				   auxtrace_pages, auxtrace_overwrite);
-- 
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1238519 — Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask

From	Arnaldo Carvalho de Melo <acme@kernel.org>
Date	2015-10-02 20:50 +0200
Subject	Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask
Message-ID	<qfdz3-3iS-11@gated-at.bofh.it>
In reply to	#1237936

Em Fri, Oct 02, 2015 at 02:18:43PM +0900, Namhyung Kim escreveu:
> It is more convenient saving mmap length rather than (bit) mask.  With
> this patch, we can eliminate dependency to perf_evlist other than
> getting mmap_desc for dealing with mmaps.  The mask and length can be
> converted using perf_evlist__mmap_mask/len().
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/evlist.c | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index c5180a29db1b..e46adcd5b408 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -29,6 +29,8 @@
>  
>  static void perf_evlist__mmap_put(struct perf_evlist *evlist, int idx);
>  static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx);
> +static size_t perf_evlist__mmap_mask(size_t len);
> +static size_t perf_evlist__mmap_len(size_t mask);

Are these "perf_evlist" methods? I don't think so, those are related to
"perf_mmap".

>  #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
>  #define SID(e, x, y) xyarray__entry(e->sample_id, x, y)
> @@ -871,7 +873,9 @@ void __weak auxtrace_mmap_params__set_idx(
>  static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
>  {
>  	if (evlist->mmap[idx].base != NULL) {
> -		munmap(evlist->mmap[idx].base, evlist->mmap_len);
> +		size_t mmap_len = perf_evlist__mmap_len(evlist->mmap[idx].mask);

I.e. here you could have it as:

		size_t mmap_len = perf_mmap__len(evlist->mmap[idx]);

> +
> +		munmap(evlist->mmap[idx].base, mmap_len);
>  		evlist->mmap[idx].base = NULL;
>  		atomic_set(&evlist->mmap[idx].refcnt, 0);
>  	}
> @@ -901,8 +905,8 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
>  }
>  
>  struct mmap_params {
> -	int prot;
> -	int mask;
> +	int	prot;
> +	size_t	len;
>  	struct auxtrace_mmap_params auxtrace_mp;
>  };
>  
> @@ -924,8 +928,8 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
>  	 */
>  	atomic_set(&evlist->mmap[idx].refcnt, 2);
>  	evlist->mmap[idx].prev = 0;
> -	evlist->mmap[idx].mask = mp->mask;
> -	evlist->mmap[idx].base = mmap(NULL, evlist->mmap_len, mp->prot,
> +	evlist->mmap[idx].mask = perf_evlist__mmap_mask(mp->len);

Here, since you're not using a perf_mmap instance, but the calculation
is relative to a perf_mmap property, we would use:

	evlist->mmap[idx].mask = __perf_mmap__mask(mp->len);
	

> +	evlist->mmap[idx].base = mmap(NULL, mp->len, mp->prot,
>  				      MAP_SHARED, fd, 0);
>  	if (evlist->mmap[idx].base == MAP_FAILED) {
>  		pr_debug2("failed to mmap perf event ring buffer, error %d\n",
> @@ -1071,6 +1075,21 @@ static size_t perf_evlist__mmap_size(unsigned long pages)
>  	return (pages + 1) * page_size;
>  }
>  
> +static size_t perf_evlist__mmap_mask(size_t len)
> +{
> +	BUG_ON(len <= page_size);
> +	BUG_ON((len % page_size) != 0);
> +
> +	return len - page_size - 1;
> +}
> +
> +static size_t perf_evlist__mmap_len(size_t mask)
> +{
> +	BUG_ON(((mask + 1) % page_size) != 0);
> +
> +	return mask + 1 + page_size;
> +}
> +
>  static long parse_pages_arg(const char *str, unsigned long min,
>  			    unsigned long max)
>  {
> @@ -1176,7 +1195,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
>  	evlist->overwrite = overwrite;
>  	evlist->mmap_len = perf_evlist__mmap_size(pages);
>  	pr_debug("mmap size %zuB\n", evlist->mmap_len);
> -	mp.mask = evlist->mmap_len - page_size - 1;
> +	mp.len = evlist->mmap_len;
>  
>  	auxtrace_mmap_params__init(&mp.auxtrace_mp, evlist->mmap_len,
>  				   auxtrace_pages, auxtrace_overwrite);
> -- 
> 2.6.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240252 — Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-10-06 10:40 +0200
Subject	Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask
Message-ID	<qgvWX-S5-31@gated-at.bofh.it>
In reply to	#1238519

Hi Arnaldo,

On Fri, Oct 02, 2015 at 03:44:33PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Oct 02, 2015 at 02:18:43PM +0900, Namhyung Kim escreveu:
> > It is more convenient saving mmap length rather than (bit) mask.  With
> > this patch, we can eliminate dependency to perf_evlist other than
> > getting mmap_desc for dealing with mmaps.  The mask and length can be
> > converted using perf_evlist__mmap_mask/len().
> > 
> > Cc: Jiri Olsa <jolsa@redhat.com>
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/evlist.c | 31 +++++++++++++++++++++++++------
> >  1 file changed, 25 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > index c5180a29db1b..e46adcd5b408 100644
> > --- a/tools/perf/util/evlist.c
> > +++ b/tools/perf/util/evlist.c
> > @@ -29,6 +29,8 @@
> >  
> >  static void perf_evlist__mmap_put(struct perf_evlist *evlist, int idx);
> >  static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx);
> > +static size_t perf_evlist__mmap_mask(size_t len);
> > +static size_t perf_evlist__mmap_len(size_t mask);
> 
> Are these "perf_evlist" methods? I don't think so, those are related to
> "perf_mmap".

Agreed.

> 
> >  #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
> >  #define SID(e, x, y) xyarray__entry(e->sample_id, x, y)
> > @@ -871,7 +873,9 @@ void __weak auxtrace_mmap_params__set_idx(
> >  static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
> >  {
> >  	if (evlist->mmap[idx].base != NULL) {
> > -		munmap(evlist->mmap[idx].base, evlist->mmap_len);
> > +		size_t mmap_len = perf_evlist__mmap_len(evlist->mmap[idx].mask);
> 
> I.e. here you could have it as:
> 
> 		size_t mmap_len = perf_mmap__len(evlist->mmap[idx]);

OK.

> 
> > +
> > +		munmap(evlist->mmap[idx].base, mmap_len);
> >  		evlist->mmap[idx].base = NULL;
> >  		atomic_set(&evlist->mmap[idx].refcnt, 0);
> >  	}
> > @@ -901,8 +905,8 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
> >  }
> >  
> >  struct mmap_params {
> > -	int prot;
> > -	int mask;
> > +	int	prot;
> > +	size_t	len;
> >  	struct auxtrace_mmap_params auxtrace_mp;
> >  };
> >  
> > @@ -924,8 +928,8 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
> >  	 */
> >  	atomic_set(&evlist->mmap[idx].refcnt, 2);
> >  	evlist->mmap[idx].prev = 0;
> > -	evlist->mmap[idx].mask = mp->mask;
> > -	evlist->mmap[idx].base = mmap(NULL, evlist->mmap_len, mp->prot,
> > +	evlist->mmap[idx].mask = perf_evlist__mmap_mask(mp->len);
> 
> Here, since you're not using a perf_mmap instance, but the calculation
> is relative to a perf_mmap property, we would use:
> 
> 	evlist->mmap[idx].mask = __perf_mmap__mask(mp->len);

Will change.

Thanks,
Namhyung


> 	
> 
> > +	evlist->mmap[idx].base = mmap(NULL, mp->len, mp->prot,
> >  				      MAP_SHARED, fd, 0);
> >  	if (evlist->mmap[idx].base == MAP_FAILED) {
> >  		pr_debug2("failed to mmap perf event ring buffer, error %d\n",
> > @@ -1071,6 +1075,21 @@ static size_t perf_evlist__mmap_size(unsigned long pages)
> >  	return (pages + 1) * page_size;
> >  }
> >  
> > +static size_t perf_evlist__mmap_mask(size_t len)
> > +{
> > +	BUG_ON(len <= page_size);
> > +	BUG_ON((len % page_size) != 0);
> > +
> > +	return len - page_size - 1;
> > +}
> > +
> > +static size_t perf_evlist__mmap_len(size_t mask)
> > +{
> > +	BUG_ON(((mask + 1) % page_size) != 0);
> > +
> > +	return mask + 1 + page_size;
> > +}
> > +
> >  static long parse_pages_arg(const char *str, unsigned long min,
> >  			    unsigned long max)
> >  {
> > @@ -1176,7 +1195,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
> >  	evlist->overwrite = overwrite;
> >  	evlist->mmap_len = perf_evlist__mmap_size(pages);
> >  	pr_debug("mmap size %zuB\n", evlist->mmap_len);
> > -	mp.mask = evlist->mmap_len - page_size - 1;
> > +	mp.len = evlist->mmap_len;
> >  
> >  	auxtrace_mmap_params__init(&mp.auxtrace_mp, evlist->mmap_len,
> >  				   auxtrace_pages, auxtrace_overwrite);
> > -- 
> > 2.6.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242158 — Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask

From	Jiri Olsa <jolsa@redhat.com>
Date	2015-10-08 12:20 +0200
Subject	Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask
Message-ID	<qhgsP-u9-23@gated-at.bofh.it>
In reply to	#1237936

On Fri, Oct 02, 2015 at 02:18:43PM +0900, Namhyung Kim wrote:
> It is more convenient saving mmap length rather than (bit) mask.  With
> this patch, we can eliminate dependency to perf_evlist other than
> getting mmap_desc for dealing with mmaps.  The mask and length can be
> converted using perf_evlist__mmap_mask/len().
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

after this patch I'm hitting:

[jolsa@krava perf]$ ./perf record  kill
kill: not enough arguments
perf: util/evlist.c:1003: perf_evlist__mmap_len: Assertion `!((mask & page_size) != 0)' failed.
Aborted (core dumped)
[jolsa@krava perf]$ 


jirka

> ---
>  tools/perf/util/evlist.c | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index c5180a29db1b..e46adcd5b408 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -29,6 +29,8 @@
>  
>  static void perf_evlist__mmap_put(struct perf_evlist *evlist, int idx);
>  static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx);
> +static size_t perf_evlist__mmap_mask(size_t len);
> +static size_t perf_evlist__mmap_len(size_t mask);
>  
>  #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
>  #define SID(e, x, y) xyarray__entry(e->sample_id, x, y)
> @@ -871,7 +873,9 @@ void __weak auxtrace_mmap_params__set_idx(
>  static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
>  {
>  	if (evlist->mmap[idx].base != NULL) {
> -		munmap(evlist->mmap[idx].base, evlist->mmap_len);
> +		size_t mmap_len = perf_evlist__mmap_len(evlist->mmap[idx].mask);
> +
> +		munmap(evlist->mmap[idx].base, mmap_len);
>  		evlist->mmap[idx].base = NULL;
>  		atomic_set(&evlist->mmap[idx].refcnt, 0);
>  	}
> @@ -901,8 +905,8 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
>  }
>  
>  struct mmap_params {
> -	int prot;
> -	int mask;
> +	int	prot;
> +	size_t	len;
>  	struct auxtrace_mmap_params auxtrace_mp;
>  };
>  
> @@ -924,8 +928,8 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
>  	 */
>  	atomic_set(&evlist->mmap[idx].refcnt, 2);
>  	evlist->mmap[idx].prev = 0;
> -	evlist->mmap[idx].mask = mp->mask;
> -	evlist->mmap[idx].base = mmap(NULL, evlist->mmap_len, mp->prot,
> +	evlist->mmap[idx].mask = perf_evlist__mmap_mask(mp->len);
> +	evlist->mmap[idx].base = mmap(NULL, mp->len, mp->prot,
>  				      MAP_SHARED, fd, 0);
>  	if (evlist->mmap[idx].base == MAP_FAILED) {
>  		pr_debug2("failed to mmap perf event ring buffer, error %d\n",
> @@ -1071,6 +1075,21 @@ static size_t perf_evlist__mmap_size(unsigned long pages)
>  	return (pages + 1) * page_size;
>  }
>  
> +static size_t perf_evlist__mmap_mask(size_t len)
> +{
> +	BUG_ON(len <= page_size);
> +	BUG_ON((len % page_size) != 0);
> +
> +	return len - page_size - 1;
> +}
> +
> +static size_t perf_evlist__mmap_len(size_t mask)
> +{
> +	BUG_ON(((mask + 1) % page_size) != 0);
> +
> +	return mask + 1 + page_size;
> +}
> +
>  static long parse_pages_arg(const char *str, unsigned long min,
>  			    unsigned long max)
>  {
> @@ -1176,7 +1195,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
>  	evlist->overwrite = overwrite;
>  	evlist->mmap_len = perf_evlist__mmap_size(pages);
>  	pr_debug("mmap size %zuB\n", evlist->mmap_len);
> -	mp.mask = evlist->mmap_len - page_size - 1;
> +	mp.len = evlist->mmap_len;
>  
>  	auxtrace_mmap_params__init(&mp.auxtrace_mp, evlist->mmap_len,
>  				   auxtrace_pages, auxtrace_overwrite);
> -- 
> 2.6.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243048 — Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-10-09 08:10 +0200
Subject	Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask
Message-ID	<qhz2q-213-5@gated-at.bofh.it>
In reply to	#1242158

Hi Jiri,

On Thu, Oct 08, 2015 at 12:17:11PM +0200, Jiri Olsa wrote:
> On Fri, Oct 02, 2015 at 02:18:43PM +0900, Namhyung Kim wrote:
> > It is more convenient saving mmap length rather than (bit) mask.  With
> > this patch, we can eliminate dependency to perf_evlist other than
> > getting mmap_desc for dealing with mmaps.  The mask and length can be
> > converted using perf_evlist__mmap_mask/len().
> > 
> > Cc: Jiri Olsa <jolsa@redhat.com>
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> after this patch I'm hitting:
> 
> [jolsa@krava perf]$ ./perf record  kill
> kill: not enough arguments
> perf: util/evlist.c:1003: perf_evlist__mmap_len: Assertion `!((mask & page_size) != 0)' failed.
> Aborted (core dumped)
> [jolsa@krava perf]$ 

This is strange..  I think I fixed it already.  And the expression in
the assertion is different than the code in the patch:

  static size_t perf_evlist__mmap_len(size_t mask)
  {
         BUG_ON(((mask + 1) % page_size) != 0);
  
         return mask + 1 + page_size;
  }

Could you please double check?

Thanks,
Namhyung


> 
> 
> jirka
> 
> > ---
> >  tools/perf/util/evlist.c | 31 +++++++++++++++++++++++++------
> >  1 file changed, 25 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > index c5180a29db1b..e46adcd5b408 100644
> > --- a/tools/perf/util/evlist.c
> > +++ b/tools/perf/util/evlist.c
> > @@ -29,6 +29,8 @@
> >  
> >  static void perf_evlist__mmap_put(struct perf_evlist *evlist, int idx);
> >  static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx);
> > +static size_t perf_evlist__mmap_mask(size_t len);
> > +static size_t perf_evlist__mmap_len(size_t mask);
> >  
> >  #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
> >  #define SID(e, x, y) xyarray__entry(e->sample_id, x, y)
> > @@ -871,7 +873,9 @@ void __weak auxtrace_mmap_params__set_idx(
> >  static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
> >  {
> >  	if (evlist->mmap[idx].base != NULL) {
> > -		munmap(evlist->mmap[idx].base, evlist->mmap_len);
> > +		size_t mmap_len = perf_evlist__mmap_len(evlist->mmap[idx].mask);
> > +
> > +		munmap(evlist->mmap[idx].base, mmap_len);
> >  		evlist->mmap[idx].base = NULL;
> >  		atomic_set(&evlist->mmap[idx].refcnt, 0);
> >  	}
> > @@ -901,8 +905,8 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
> >  }
> >  
> >  struct mmap_params {
> > -	int prot;
> > -	int mask;
> > +	int	prot;
> > +	size_t	len;
> >  	struct auxtrace_mmap_params auxtrace_mp;
> >  };
> >  
> > @@ -924,8 +928,8 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
> >  	 */
> >  	atomic_set(&evlist->mmap[idx].refcnt, 2);
> >  	evlist->mmap[idx].prev = 0;
> > -	evlist->mmap[idx].mask = mp->mask;
> > -	evlist->mmap[idx].base = mmap(NULL, evlist->mmap_len, mp->prot,
> > +	evlist->mmap[idx].mask = perf_evlist__mmap_mask(mp->len);
> > +	evlist->mmap[idx].base = mmap(NULL, mp->len, mp->prot,
> >  				      MAP_SHARED, fd, 0);
> >  	if (evlist->mmap[idx].base == MAP_FAILED) {
> >  		pr_debug2("failed to mmap perf event ring buffer, error %d\n",
> > @@ -1071,6 +1075,21 @@ static size_t perf_evlist__mmap_size(unsigned long pages)
> >  	return (pages + 1) * page_size;
> >  }
> >  
> > +static size_t perf_evlist__mmap_mask(size_t len)
> > +{
> > +	BUG_ON(len <= page_size);
> > +	BUG_ON((len % page_size) != 0);
> > +
> > +	return len - page_size - 1;
> > +}
> > +
> > +static size_t perf_evlist__mmap_len(size_t mask)
> > +{
> > +	BUG_ON(((mask + 1) % page_size) != 0);
> > +
> > +	return mask + 1 + page_size;
> > +}
> > +
> >  static long parse_pages_arg(const char *str, unsigned long min,
> >  			    unsigned long max)
> >  {
> > @@ -1176,7 +1195,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
> >  	evlist->overwrite = overwrite;
> >  	evlist->mmap_len = perf_evlist__mmap_size(pages);
> >  	pr_debug("mmap size %zuB\n", evlist->mmap_len);
> > -	mp.mask = evlist->mmap_len - page_size - 1;
> > +	mp.len = evlist->mmap_len;
> >  
> >  	auxtrace_mmap_params__init(&mp.auxtrace_mp, evlist->mmap_len,
> >  				   auxtrace_pages, auxtrace_overwrite);
> > -- 
> > 2.6.0
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 2 of 3 — ← Prev page 1 [2] 3 Next page →

csiph-web

[RFC/PATCH 00/38] perf tools: Speed-up perf report by using multi thread (v5)

Contents

#1238533 — Re: [RFC/PATCH 09/38] perf record: Add --index option for building index table

#1239570 — Re: [RFC/PATCH 09/38] perf record: Add --index option for building index table

#1241233 — Re: [RFC/PATCH 09/38] perf record: Add --index option for building index table

#1241418 — Re: [RFC/PATCH 09/38] perf record: Add --index option for building index table

#1237930 — [RFC/PATCH 23/38] perf tools: Use map_groups__find_addr_by_time()

#1237931 — [RFC/PATCH 28/38] perf tools: Move BUILD_ID_SIZE definition to perf.h

#1237932 — [RFC/PATCH 24/38] perf tools: Add testcase for managing maps with time

#1237933 — [RFC/PATCH 11/38] perf tools: Introduce thread__comm(_str)_by_time() helpers

#1237934 — [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread

#1242316 — Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread

#1243075 — Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread

#1244661 — Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread

#1242321 — Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread

#1243049 — Re: [RFC/PATCH 17/38] perf tools: Maintain map groups list in a leader thread

#1237935 — [RFC/PATCH 08/38] perf tools: Handle indexed data file properly

#1237936 — [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask

#1238519 — Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask

#1240252 — Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask

#1242158 — Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask

#1243048 — Re: [RFC/PATCH 02/38] perf tools: Save mmap_param.len instead of mask