From: Alexey Budankov <alexey.budankov@linux.intel.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Ingo Molnar <mingo@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Namhyung Kim <namhyung@kernel.org>,
Andi Kleen <ak@linux.intel.com>,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v7 1/2]: perf util: map data buffer for preserving collected data
Date: Thu, 6 Sep 2018 14:50:56 +0300 [thread overview]
Message-ID: <50fea767-6424-44b5-0e68-f12a2d2c96cc@linux.intel.com> (raw)
In-Reply-To: <20180906110407.GE8248@krava>
On 06.09.2018 14:04, Jiri Olsa wrote:
> On Wed, Sep 05, 2018 at 10:19:56AM +0300, Alexey Budankov wrote:
>>
>> The map->data buffers are used to preserve map->base profiling data
>> for writing to disk. AIO map->cblocks are used to queue corresponding
>> map->data buffers for asynchronous writing. map->cblocks objects are
>> located in the last page of every map->data buffer.
>>
>> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
>> ---
>> Changes in v7:
>> - implemented handling record.aio setting from perfconfig file
>> Changes in v6:
>> - adjusted setting of priorities for cblocks;
>> Changes in v5:
>> - reshaped layout of data structures;
>> - implemented --aio option;
>> Changes in v4:
>> - converted mmap()/munmap() to malloc()/free() for mmap->data buffer management
>> Changes in v2:
>> - converted zalloc() to calloc() for allocation of mmap_aio array,
>> - cleared typo and adjusted fallback branch code;
>> ---
>> tools/perf/builtin-record.c | 15 ++++++++++++-
>> tools/perf/perf.h | 1 +
>> tools/perf/util/evlist.c | 7 +++---
>> tools/perf/util/evlist.h | 3 ++-
>> tools/perf/util/mmap.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
>> tools/perf/util/mmap.h | 6 ++++-
>> 6 files changed, 79 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index 22ebeb92ac51..f17a6f9cb1ba 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -326,7 +326,8 @@ static int record__mmap_evlist(struct record *rec,
>>
>> if (perf_evlist__mmap_ex(evlist, opts->mmap_pages,
>> opts->auxtrace_mmap_pages,
>> - opts->auxtrace_snapshot_mode) < 0) {
>> + opts->auxtrace_snapshot_mode,
>> + opts->nr_cblocks) < 0) {
>> if (errno == EPERM) {
>> pr_err("Permission error mapping pages.\n"
>> "Consider increasing "
>> @@ -1287,6 +1288,8 @@ static int perf_record_config(const char *var, const char *value, void *cb)
>> var = "call-graph.record-mode";
>> return perf_default_config(var, value, cb);
>> }
>> + if (!strcmp(var, "record.aio"))
>> + rec->opts.nr_cblocks = strtol(value, NULL, 0);
>>
>> return 0;
>> }
>> @@ -1519,6 +1522,7 @@ static struct record record = {
>> .default_per_cpu = true,
>> },
>> .proc_map_timeout = 500,
>> + .nr_cblocks = 2
>> },
>> .tool = {
>> .sample = process_sample_event,
>> @@ -1678,6 +1682,8 @@ static struct option __record_options[] = {
>> "signal"),
>> OPT_BOOLEAN(0, "dry-run", &dry_run,
>> "Parse options then exit"),
>> + OPT_INTEGER(0, "aio", &record.opts.nr_cblocks,
>> + "asynchronous trace write operations (min: 1, max: 32, default: 2)"),
>
> ok, so this got silently added in recent versions and I couldn't
> find any justification for it.. why do we use more aio blocks for
> single map now? also why the default is 2?
Having more blocks may improve thruput from kernel to userspace for
cases when we get more data at map->base but the started AIO is not
finished yet. That can easily happen between calls of
record__mmap_read_evlist().
>
> the option should be more specific like 'aio-blocks'
ok.
>
> the change is difficult enough.. we should start simple and add
> these additions with proper justification in separate patches
Setting default to 1 gives the simplest solution. I could provide
justification where spinning at record__aio_sync() becomes the hotspot.
>
> thanks,
> jirka
>
next prev parent reply other threads:[~2018-09-06 11:51 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-05 7:16 [PATCH v7 0/2]: perf: reduce data loss when profiling highly parallel CPU bound workloads Alexey Budankov
2018-09-05 7:19 ` [PATCH v7 1/2]: perf util: map data buffer for preserving collected data Alexey Budankov
2018-09-06 11:04 ` Jiri Olsa
2018-09-06 11:50 ` Alexey Budankov [this message]
2018-09-06 11:04 ` Jiri Olsa
2018-09-06 11:54 ` Alexey Budankov
2018-09-05 7:39 ` [PATCH v7 2/2]: perf record: enable asynchronous trace writing Alexey Budankov
2018-09-06 11:04 ` Jiri Olsa
2018-09-06 11:57 ` Alexey Budankov
2018-09-06 11:04 ` Jiri Olsa
2018-09-06 11:58 ` Alexey Budankov
2018-09-06 11:04 ` Jiri Olsa
2018-09-06 11:59 ` Alexey Budankov
2018-09-06 11:04 ` Jiri Olsa
2018-09-06 12:09 ` Alexey Budankov
2018-09-05 11:28 ` [PATCH v7 0/2]: perf: reduce data loss when profiling highly parallel CPU bound workloads Jiri Olsa
2018-09-05 17:37 ` Alexey Budankov
2018-09-05 18:51 ` Arnaldo Carvalho de Melo
2018-09-06 6:03 ` Alexey Budankov
2018-09-06 8:14 ` Jiri Olsa
2018-09-06 8:20 ` Alexey Budankov
2018-09-06 6:59 ` Alexey Budankov
2018-09-06 6:57 ` Alexey Budankov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50fea767-6424-44b5-0e68-f12a2d2c96cc@linux.intel.com \
--to=alexey.budankov@linux.intel.com \
--cc=acme@kernel.org \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).