Re: [PATCH v11 01/10] perf record: Prepare mmap multiple evlists

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Mon, Jun 27, 2016 at 10:24:03AM +0000, Wang Nan escreveu:
> Following commits introduce multiple evlists to record. This patch
> extracts perf_evlist__mmap_ex() processing to a new function, creates
> record__mmap() and record__mmap_evlist() to wrap perf_evlist__mmap_ex()
> and its error processing. They will be improvemented to create mmap
> for all evlists.

Thanks for following my suggestion, as you can see this patch has value
irrespective of the other patches in this series and can be applied
straight away.

- Arnaldo
 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: He Kuang <hekuang@huawei.com>
> Cc: Nilay Vaish <nilayvaish@gmail.com>
> ---
>  tools/perf/builtin-record.c | 56 +++++++++++++++++++++++++++++----------------
>  1 file changed, 36 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 81411b1..7eb8d7d 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -342,6 +342,40 @@ int auxtrace_record__snapshot_start(struct auxtrace_record *itr __maybe_unused)
>  
>  #endif
>  
> +static int record__mmap_evlist(struct record *rec,
> +			       struct perf_evlist *evlist)
> +{
> +	struct record_opts *opts = &rec->opts;
> +	char msg[512];
> +
> +	if (perf_evlist__mmap_ex(evlist, opts->mmap_pages, false,
> +				 opts->auxtrace_mmap_pages,
> +				 opts->auxtrace_snapshot_mode) < 0) {
> +		if (errno == EPERM) {
> +			pr_err("Permission error mapping pages.\n"
> +			       "Consider increasing "
> +			       "/proc/sys/kernel/perf_event_mlock_kb,\n"
> +			       "or try again with a smaller value of -m/--mmap_pages.\n"
> +			       "(current value: %u,%u)\n",
> +			       opts->mmap_pages, opts->auxtrace_mmap_pages);
> +			return -errno;
> +		} else {
> +			pr_err("failed to mmap with %d (%s)\n", errno,
> +				strerror_r(errno, msg, sizeof(msg)));
> +			if (errno)
> +				return -errno;
> +			else
> +				return -EINVAL;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int record__mmap(struct record *rec)
> +{
> +	return record__mmap_evlist(rec, rec->evlist);
> +}
> +
>  static int record__open(struct record *rec)
>  {
>  	char msg[512];
> @@ -378,27 +412,9 @@ try_again:
>  		goto out;
>  	}
>  
> -	if (perf_evlist__mmap_ex(evlist, opts->mmap_pages, false,
> -				 opts->auxtrace_mmap_pages,
> -				 opts->auxtrace_snapshot_mode) < 0) {
> -		if (errno == EPERM) {
> -			pr_err("Permission error mapping pages.\n"
> -			       "Consider increasing "
> -			       "/proc/sys/kernel/perf_event_mlock_kb,\n"
> -			       "or try again with a smaller value of -m/--mmap_pages.\n"
> -			       "(current value: %u,%u)\n",
> -			       opts->mmap_pages, opts->auxtrace_mmap_pages);
> -			rc = -errno;
> -		} else {
> -			pr_err("failed to mmap with %d (%s)\n", errno,
> -				strerror_r(errno, msg, sizeof(msg)));
> -			if (errno)
> -				rc = -errno;
> -			else
> -				rc = -EINVAL;
> -		}
> +	rc = record__mmap(rec);
> +	if (rc)
>  		goto out;
> -	}
>  
>  	session->evlist = evlist;
>  	perf_session__set_id_hdr_size(session);
> -- 
> 1.8.3.4