: perf: reduce data loss when profiling highly parallel CPU bound workloads
v13 cover : perf: reduce data loss when profiling highly parallel CPU bound workloads
Currently in record mode the tool implements trace writing serially. The algorithm loops over mapped per-cpu data buffers and stores ready data chunks into a trace file using write() system call.
At some circumstances the kernel may lack free space in a buffer because the other buffer's half is not yet written to disk due to some other buffer's data writing by the tool at the moment.
Thus serial trace writing implementation may cause the kernel to loose profiling data and that is what observed when profiling highly parallel CPU bound workloads on machines with big number of cores.
Experiment with profiling matrix multiplication code executing 128 threads on Intel Xeon Phi (KNM) with 272 cores, like below, demonstrates data loss metrics value of 98%:
/usr/bin/time perf record -o /tmp/perf-ser.data -a -N -B -T -R -g \
--call-graph dwarf,1024 --user-regs=IP,SP,BP --switch-events \
-e cycles,instructions,ref-cycles,software/period=1,name=cs,config=0x3/Duk -- \
matrix.gccData loss metrics is the ratio lost_time/elapsed_time where lost_time is the sum of time intervals containing PERF_RECORD_LOST records and elapsed_time is the elapsed application run time under profiling.
Applying asynchronous trace streaming thru Posix AIO API [1] lowers data loss metrics value providing 2x improvement (from 98% to ~1%)
Asynchronous trace streaming is currently limited to glibc linkage. musl libc [5] also provides Posix AIO API implementation, however the patchkit is not tested with it. There may be other libc libraries linked by Perf tool that currently lack Posix AIO API support [2], [3], [4] so NO_AIO define may be used to limit Perf tool binary to serial streaming only.
---
Alexey Budankov (3):
perf util: map data buffer for preserving collected data perf record: enable asynchronous trace writing perf record: extend trace writing to multi AIO
tools/perf/Documentation/perf-record.txt | 5 + tools/perf/Makefile.config | 5 + tools/perf/Makefile.perf | 7 +- tools/perf/builtin-record.c | 249 ++++++++++++++++++++++++++++++- tools/perf/perf.h | 1 + tools/perf/util/evlist.c | 6 +- tools/perf/util/evlist.h | 2 +- tools/perf/util/mmap.c | 146 +++++++++++++++++- tools/perf/util/mmap.h | 26 +++- 9 files changed, 436 insertions(+), 11 deletions(-)
---
Changes in v13:
- named new functions with _aio_ word
- grouped aio functions under single #ifdef HAVE_AIO_SUPPORT
- moved perf_mmap__aio_push() stub into header
- removed trailed white space
Changes in v12:
- applied stub functions design for the whole patch kit
- grouped AIO related data into a struct under struct perf_mmap
- implemented record__aio_get/set_pos(), record__aio_enabled()
- implemented simple --aio option
- extended --aio option to --aio-cblocks=<n>
Changes in v11:
- replacing the both lseek() syscalls in every loop iteration by the only
two syscalls just before and after the loop at record__mmap_read_evlist() and advancing *in-flight* off file pos value at perf_mmap__aio_push()
Changes in v10:
- moved specific code to perf_mmap__aio_mmap(), perf_mmap__aio_munmap();
- adjusted error reporting by using %m
- avoided lseek() setting file pos back in case of record__aio_write() failure
- compacted code selecting between serial and AIO streaming
- optimized call places of record__mmap_read_sync()
- added description of aio-cblocks option into perf-record.txt
Changes in v9:
- enable AIO streaming only when --aio-cblocks option is specified explicitly
- enable AIO based implementation when linking with glibc only
- define NO_AIO to limit Perf binary to serial implementation
Changes in v8:
- run the whole thing thru checkpatch.pl and corrected found issues except lines longer than 80 symbols
- corrected comments alignment and formatting
- moved multi AIO implementation into 3rd patch in the series
- implemented explicit cblocks array allocation
- split AIO completion check into separate record__aio_complete()
- set nr_cblocks default to 1 and max allowed value to 4
Changes in v7:
- implemented handling record.aio setting from perfconfig file
Changes in v6:
- adjusted setting of priorities for cblocks;
- handled errno == EAGAIN case from aio_write() return;
Changes in v5:
- resolved livelock on perf record -e intel_pt// -- dd if=/dev/zero of=/dev/null count=100000
- data loss metrics decreased from 25% to 2x in trialed configuration;
- reshaped layout of data structures;
- implemented --aio option;
- avoided nanosleep() prior calling aio_suspend();
- switched to per-cpu aio multi buffer record__aio_sync();
- record_mmap_read_sync() now does global sync just before
switching trace file or collection stop;
Changes in v4:
- converted mmap()/munmap() to malloc()/free() for mmap->data buffer management
- converted void *bf to struct perf_mmap *md in signatures
- written comment in perf_mmap__push() just before perf_mmap__get();
- written comment in record__mmap_read_sync() on possible restarting
of aio_write() operation and releasing perf_mmap object after all;
- added perf_mmap__put() for the cases of failed aio_write();
Changes in v3:
- written comments about nanosleep(0.5ms) call prior aio_suspend()
to cope with intrusiveness of its implementation in glibc;
- written comments about rationale behind coping profiling data
into mmap->data buffer;
Changes in v2:
- converted zalloc() to calloc() for allocation of mmap_aio array,
- cleared typo and adjusted fallback branch code;
---
[1] http://man7.org/linux/man-pages/man7/aio.7.html
[2] https://android.googlesource.com/platform/bionic/+/master/docs/status.md
[3] https://www.uclibc.org/
[4] https://uclibc-ng.org/
[5] https://www.musl-libc.org/
PATCH v13 1/3 : perf util: map data buffer for preserving collected data
The map->data buffer is used to preserve map->base profiling data for writing to disk. AIO map->cblock is used to queue corresponding map->data buffer for asynchronous writing.
---
Changes in v13:
- grouped aio functions under single #ifdef HAVE_AIO_SUPPORT
Changes in v12:
- applied stub functions design for the whole patch kit
- grouped AIO related data into a struct under struct perf_mmap
Changes in v10:
- moved specific code to perf_mmap__aio_mmap(), perf_mmap__aio_munmap()
- adjusted error reporting by using %m
Changes in v9:
- implemented NO_AIO and HAVE_AIO_SUPPORT defines to cover cases of libc implementations without Posix AIO API support
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/Makefile.config | 5 +++++ tools/perf/Makefile.perf | 7 ++++++- tools/perf/util/evlist.c | 2 +- tools/perf/util/mmap.c | 49 +++++++++++++++++++++++++++++++++++++++++++++- tools/perf/util/mmap.h | 11 ++++++++++- 5 files changed, 70 insertions(+), 4 deletions(-)
tools/perf/Makefile.config +5 -0
@@ -355,6 +355,11 @@ endif # NO_LIBELF ifeq ($(feature-glibc), 1) CFLAGS += -DHAVE_GLIBC_SUPPORT + ifndef NO_AIO + ifndef BIONIC + CFLAGS += -DHAVE_AIO_SUPPORT + endif + endif endif ifdef NO_DWARF
tools/perf/Makefile.perf +6 -1
@@ -97,8 +97,13 @@ include ../scripts/utilities.mak # Define LIBCLANGLLVM if you DO want builtin clang and llvm support. # When selected, pass LLVM_CONFIG=/path/to/llvm-config to `make' if # llvm-config is not in $PATH. - +# # Define NO_CORESIGHT if you do not want support for CoreSight trace decoding. +# +# Define NO_AIO if you do not want support of Posix AIO based trace +# streaming for record mode. Currently Posix AIO trace streaming is +# supported only when linking with glibc. +# # As per kernel Makefile, avoid funny character set dependencies unexport LC_ALL
tools/perf/util/evlist.c +1 -1
@@ -1028,7 +1028,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,* Its value is decided by evsel's write_backward. * So &mp should not be passed through const pointer. */ -struct mmap_params mp; +struct mmap_params mp = { .nr_cblocks =0 };if (!evlist->mmap) evlist->mmap =perf_evlist__alloc_mmap (evlist, false);
tools/perf/util/mmap.c +48 -1
63 lines not on this page · raw message
tools/perf/util/mmap.h +10 -1
@@ -6,6 +6,9 @@#include <linux/types.h> #include <asm/barrier.h> #include <stdbool.h> +#ifdef HAVE_AIO_SUPPORT +#include <aio.h> +#endif #include "auxtrace.h" #include "event.h"
@@ -26,6 +29,12 @@ struct perf_mmap {bool overwrite;struct auxtrace_mmap auxtrace_mmap;char event_copy[PERF_SAMPLE_MAX_SIZE ]__aligned(8) ; +#ifdef HAVE_AIO_SUPPORT +struct { +void *data; +struct aiocb cblock; + } aio; +#endif };/*
@@ -57,7 +66,7 @@ enum bkw_mmap_state { };struct mmap_params { -int prot, mask; +int prot, mask, nr_cblocks;struct auxtrace_mmap_params auxtrace_mp; };
PATCH v13 2/3 : perf record: enable asynchronous trace writing
In reply to Alexey BudankovTrace file offset is read once before mmaps iterating loop and written back after all performance data enqueued for aio writing. Trace file offset is incremented linearly after every successful aio write operation.
record__aio_sync() blocks till completion of started AIO operation and then proceeds.
record__aio_mmap_read_sync() implements a barrier for all incomplete aio write requests.
---
Changes in v13:
- named new functions with _aio_ word
- grouped aio functions under single #ifdef HAVE_AIO_SUPPORT
- moved perf_mmap__aio_push() stub into header
- removed trailed white space
Changes in v12:
- implemented record__aio_get/set_pos(), record__aio_enabled()
- implemented simple --aio option
Changes in v11:
- replacing the both lseek() syscalls in every loop iteration by the only
two syscalls just before and after the loop at record__mmap_read_evlist() and advancing *in-flight* off file pos value at perf_mmap__aio_push()
Changes in v10:
- avoided lseek() setting file pos back in case of record__aio_write() failure
- compacted code selecting between serial and AIO streaming
- optimized call places of record__mmap_read_sync()
Changes in v9:
- enable AIO streaming only when --aio-cblocks option is specified explicitly Changes in v8:
- split AIO completion check into separate record__aio_complete()
Changes in v6:
- handled errno == EAGAIN case from aio_write();
Changes in v5:
- data loss metrics decreased from 25% to 2x in trialed configuration;
- avoided nanosleep() prior calling aio_suspend();
- switched to per cpu multi record__aio_sync() aio
- record_mmap_read_sync() now does global barrier just before
switching trace file or collection stop;
- resolved livelock on perf record -e intel_pt// -- dd if=/dev/zero of=/dev/null count=100000 Changes in v4:
- converted void *bf to struct perf_mmap *md in signatures
- written comment in perf_mmap__push() just before perf_mmap__get();
- written comment in record__mmap_read_sync() on possible restarting
of aio_write() operation and releasing perf_mmap object after all;
- added perf_mmap__put() for the cases of failed aio_write();
Changes in v3:
- written comments about nanosleep(0.5ms) call prior aio_suspend()
to cope with intrusiveness of its implementation in glibc;
- written comments about rationale behind coping profiling data
into mmap->data buffer;
--- tools/perf/Documentation/perf-record.txt | 5 + tools/perf/builtin-record.c | 216 ++++++++++++++++++++++++++++++- tools/perf/perf.h | 1 + tools/perf/util/evlist.c | 6 +- tools/perf/util/evlist.h | 2 +- tools/perf/util/mmap.c | 77 ++++++++++- tools/perf/util/mmap.h | 14 ++ 7 files changed, 312 insertions(+), 9 deletions(-)
tools/perf/Documentation/perf-record.txt +5 -0
@@ -435,6 +435,11 @@ Specify vmlinux path which has debuginfo. --buildid-all:: Record build-id of all DSOs regardless whether it's actually hit or not. +--aio:: +Enable asynchronous (Posix AIO) trace writing mode. +Asynchronous mode is supported only when linking Perf tool with libc library +providing implementation for Posix AIO API. + --all-kernel:: Configure all used events to run in kernel space.
tools/perf/builtin-record.c +212 -4
274 lines not on this page · raw message
tools/perf/perf.h +1 -0
@@ -82,6 +82,7 @@ struct record_opts {bool use_clockid;clockid_t clockid;unsigned int proc_map_timeout; +int nr_cblocks; };struct option ;
tools/perf/util/evlist.c +3 -3
@@ -1018,7 +1018,7 @@ int perf_evlist__parse_mmap_pages(const struct option *opt, const char *str,*/ int perf_evlist__mmap_ex (struct perf_evlist *evlist,unsigned int pages,unsigned int auxtrace_pages, -bool auxtrace_overwrite) +bool auxtrace_overwrite,int nr_cblocks) {struct perf_evsel *evsel;const struct cpu_map *cpus = evlist->cpus;
@@ -1028,7 +1028,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,* Its value is decided by evsel's write_backward. * So &mp should not be passed through const pointer. */ -struct mmap_params mp = { .nr_cblocks =0 }; +struct mmap_params mp = { .nr_cblocks = nr_cblocks };if (!evlist->mmap) evlist->mmap =perf_evlist__alloc_mmap (evlist, false);
@@ -1060,7 +1060,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,int perf_evlist__mmap (struct perf_evlist *evlist,unsigned int pages) { -return perf_evlist__mmap_ex (evlist, pages,0 , false); +return perf_evlist__mmap_ex (evlist, pages,0 , false,0 ); }int perf_evlist__create_maps (struct perf_evlist *evlist,struct target *target)
tools/perf/util/evlist.h +1 -1
@@ -162,7 +162,7 @@ unsigned long perf_event_mlock_kb_in_pages(void);int perf_evlist__mmap_ex (struct perf_evlist *evlist,unsigned int pages,unsigned int auxtrace_pages, -bool auxtrace_overwrite); +bool auxtrace_overwrite,int nr_cblocks);int perf_evlist__mmap (struct perf_evlist *evlist,unsigned int pages);void perf_evlist__munmap (struct perf_evlist *evlist);
tools/perf/util/mmap.c +76 -1
89 lines not on this page · raw message
tools/perf/util/mmap.h +14 -0
@@ -12,6 +12,7 @@#include "auxtrace.h" #include "event.h" +struct aiocb ;/** * struct perf_mmap - perf's ring buffer mmap details *
@@ -33,6 +34,7 @@ struct perf_mmap {struct {void *data;struct aiocb cblock; +int nr_cblocks; } aio;#endif };
@@ -103,6 +105,18 @@ union perf_event *perf_mmap__read_event(struct perf_mmap *map);int perf_mmap__push (struct perf_mmap *md,void *to,int push (struct perf_mmap *map,void *to,void *buf,size_t size)); +#ifdef HAVE_AIO_SUPPORT +int perf_mmap__aio_push (struct perf_mmap *md,void *to, +int push (void *to,struct aiocb *cblock,void *buf,size_t size,off_t off), +off_t *off); +#else +static inline int perf_mmap__aio_push (struct perf_mmap *md__maybe_unused ,void *to__maybe_unused , +int push (void *to,struct aiocb *cblock,void *buf,size_t size,off_t off)__maybe_unused , +off_t *off__maybe_unused ) +{ +return 0 ; +} +#endif size_t perf_mmap__mmap_len (struct perf_mmap *map);
PATCH v13 3/3 : perf record: extend trace writing to multi AIO
In reply to Alexey BudankovMulti AIO trace writing allows caching more kernel data into userspace memory postponing trace writing for the sake of overall profiling data thruput increase. It could be seen as kernel data buffer extension into userspace memory.
With aio option value different from 0, default value is 1, tool has capability to cache more and more data into user space along with delegating spill to AIO.
That allows avoiding suspend at record__aio_sync() between calls of record__mmap_read_evlist() and increase profiling data thruput for the cost of userspace memory.
---
Changes in v13:
- preserved --aio option name avoiding complication Changes in v12:
- extended --aio option to --aio-cblocks=<n>
Changes in v10:
- added description of aio-cblocks option into perf-record.txt
--- tools/perf/Documentation/perf-record.txt | 4 +- tools/perf/builtin-record.c | 63 +++++++++++++++++++++++-------- tools/perf/util/mmap.c | 64 +++++++++++++++++++++----------- tools/perf/util/mmap.h | 9 +++-- 4 files changed, 98 insertions(+), 42 deletions(-)
tools/perf/Documentation/perf-record.txt +2 -2
@@ -435,8 +435,8 @@ Specify vmlinux path which has debuginfo. --buildid-all:: Record build-id of all DSOs regardless whether it's actually hit or not. ---aio:: -Enable asynchronous (Posix AIO) trace writing mode. +--aio[=n]:: +Use <n> control blocks in asynchronous (Posix AIO) trace writing mode (default: 1, max: 4). Asynchronous mode is supported only when linking Perf tool with libc library providing implementation for Posix AIO API.
tools/perf/builtin-record.c +48 -15
122 lines not on this page · raw message
tools/perf/util/mmap.c +43 -21
112 lines not on this page · raw message
tools/perf/util/mmap.h +5 -4
@@ -32,8 +32,9 @@ struct perf_mmap {char event_copy[PERF_SAMPLE_MAX_SIZE ]__aligned(8) ;#ifdef HAVE_AIO_SUPPORT struct { -void *data; -struct aiocb cblock; +void **data; +struct aiocb *cblocks; +struct aiocb **aiocb;int nr_cblocks; } aio;#endif
@@ -106,11 +107,11 @@ union perf_event *perf_mmap__read_event(struct perf_mmap *map);int perf_mmap__push (struct perf_mmap *md,void *to,int push (struct perf_mmap *map,void *to,void *buf,size_t size));#ifdef HAVE_AIO_SUPPORT -int perf_mmap__aio_push (struct perf_mmap *md,void *to, +int perf_mmap__aio_push (struct perf_mmap *md,void *to,int idx,int push (void *to,struct aiocb *cblock,void *buf,size_t size,off_t off),off_t *off);#else -static inline int perf_mmap__aio_push (struct perf_mmap *md__maybe_unused ,void *to__maybe_unused , +static inline int perf_mmap__aio_push (struct perf_mmap *md__maybe_unused ,void *to__maybe_unused ,int idx__maybe_unused ,int push (void *to,struct aiocb *cblock,void *buf,size_t size,off_t off)__maybe_unused ,off_t *off__maybe_unused ) {
Re: [PATCH v13 3/3]: perf record: extend trace writing to multi AIO
Hi,
Quoted 23 lines · Alexey Budankov
Multi AIO trace writing allows caching more kernel data into userspace memory postponing trace writing for the sake of overall profiling data thruput increase. It could be seen as kernel data buffer extension into userspace memory.
With aio option value different from 0, default value is 1, tool has capability to cache more and more data into user space along with delegating spill to AIO.
That allows avoiding suspend at
record__aio_sync()between calls ofrecord__mmap_read_evlist()and increase profiling data thruput for the cost of userspace memory.Signed-off-by Alexey Budankov <[email protected]>---
Changes in v13:
- preserved --aio option name avoiding complication Changes in v12:
- extended --aio option to --aio-cblocks=<n>
Changes in v10:
- added description of aio-cblocks option into perf-record.txt
---
[SNIP]
tools/perf/builtin-record.c static struct option __record_options[] = { · line 1917
Quoted 8 lines
tools/perf/builtin-record.c +2
-2@@ -1882,8 +1913,8 @@ static struct option __record_options[] = { OPT_BOOLEAN(0, "dry-run", &dry_run, "Parse options then exit"),#ifdef HAVE_AIO_SUPPORT - OPT_CALLBACK_NOOPT(0, "aio", &record.opts, - NULL, "Enable asynchronous trace writing mode", + OPT_CALLBACK(0, "aio", &record.opts, + "n", "Use <n> control blocks in asynchronous trace writing mode (default: 1, max: 4)",
One question. It seems you used a very large N in your test result. Why did you limit it to 4 here? Is it something different? Maybe you'd better making it a macro constant for future changes..
Otherwise looks good to me. For the 3 patches
Thanks,
Namhyung
Quoted 11 lines
tools/perf/builtin-record.c +2
-0record__aio_parse),#endif OPT_END ()@@ -2078,6 +2109,8 @@ int cmd_record(int argc, const char **argv) goto out; } +if (rec->opts.nr_cblocks >4 ) + rec->opts.nr_cblocks =4 ;if (verbose >0 )pr_info ("nr_cblocks: %d\n" , rec->opts.nr_cblocks);
Re: [PATCH v13 3/3]: perf record: extend trace writing to multi AIO
Hi,
Quoted 4 lines · Namhyung Kim
Hi,
Quoted 1 line · Alexey Budankov
<SNIP>
Quoted 4 lines
Quoted 4 lines
tools/perf/builtin-record.c +0
-0@@ -1882,8 +1913,8 @@ static struct option __record_options[] = {OPT_BOOLEAN (0 ,"dry-run" , &dry_run,"Parse options then exit" ), #ifdef HAVE_AIO_SUPPORT
Quoted 8 lines
tools/perf/builtin-record.c static struct option __record_options[] = { · line 1917
Quoted 4 lines
tools/perf/builtin-record.c +2
-2- OPT_CALLBACK_NOOPT(0, "aio", &record.opts, - NULL, "Enable asynchronous trace writing mode", + OPT_CALLBACK(0, "aio", &record.opts, + "n", "Use <n> control blocks in asynchronous trace writing mode (default: 1, max: 4)",One question. It seems you used a very large N in your test result. Why did you limit it to 4 here? Is it something different? Maybe you'd better making it a macro constant for future changes..
Yes, it's less than values used during metrics measurements. There is still no good rationale behind top border value because currently it is a kind of implementation detail. Defining as a macro makes sense for easier source code management.
Thanks,
Alexey
Quoted 7 lines
Otherwise looks good to me. For the 3 patches
Acked-by Namhyung Kim <[email protected]>Thanks,
Namhyung
Re: [PATCH v13 3/3]: perf record: extend trace writing to multi AIO
In reply to Alexey BudankovOn Thu, Oct 11, 2018 at 09:59:19PM +0300, Alexey Budankov wrote:
SNIP
Quoted 15 lines
tools/perf/builtin-record.c +7
-0@@ -1490,6 +1514,13 @@ 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); } +#ifdef HAVE_AIO_SUPPORT +if (!strcmp (var,"record.aio" )) { + rec->opts.nr_cblocks =strtol (value,NULL ,0 ); +if (!rec->opts.nr_cblocks) + rec->opts.nr_cblocks =1 ; + } +#endif return 0 ; }
tools/perf/builtin-record.c static struct option __record_options[] = { · line 1918
Quoted 8 lines
tools/perf/builtin-record.c +2
-2@@ -1882,8 +1913,8 @@ static struct option __record_options[] = { OPT_BOOLEAN(0, "dry-run", &dry_run, "Parse options then exit"),#ifdef HAVE_AIO_SUPPORT -OPT_CALLBACK_NOOPT (0 ,"aio" , &record.opts, -NULL ,"Enable asynchronous trace writing mode" , +OPT_CALLBACK (0 ,"aio" , &record.opts, +"n" ,"Use <n> control blocks in asynchronous trace writing mode (default: 1, max: 4)" , record__aio_parse),
it stoped work with no argument:
[jolsa@krava perf]$ ./perf record --aio ls Error:
You may not have permission to collect system-wide stats.
jirka
Re: [PATCH v13 3/3]: perf record: extend trace writing to multi AIO
Hi,
Quoted 30 lines · Jiri Olsa
On Thu, Oct 11, 2018 at 09:59:19PM +0300, Alexey Budankov wrote:
SNIP
Quoted 15 lines
(quoted diff) +7
-0@@ -1490,6 +1514,13 @@ 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); } +#ifdef HAVE_AIO_SUPPORT + if (!strcmp(var, "record.aio")) { + rec->opts.nr_cblocks = strtol(value, NULL, 0); + if (!rec->opts.nr_cblocks) + rec->opts.nr_cblocks = 1; + } +#endif return 0; }Review static struct option __record_options[] = { · line 1918
Quoted 8 lines
(quoted diff) +2
-2@@ -1882,8 +1913,8 @@ static struct option __record_options[] = { OPT_BOOLEAN(0, "dry-run", &dry_run, "Parse options then exit"), #ifdef HAVE_AIO_SUPPORT - OPT_CALLBACK_NOOPT(0, "aio", &record.opts, - NULL, "Enable asynchronous trace writing mode", + OPT_CALLBACK(0, "aio", &record.opts, + "n", "Use <n> control blocks in asynchronous trace writing mode (default: 1, max: 4)", record__aio_parse),it stoped work with no argument:
Quoted 3 lines
[jolsa@krava perf]$ ./perf record --aio ls Error:
You may not have permission to collect system-wide stats.
This is the fix below. However, perf record --aio -- ls works without it. I will resend the whole patch kit.
Thanks,
Alexey
--- tools/perf/builtin-record.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
tools/perf/builtin-record.c +5 -3
@@ -284,13 +284,15 @@ static int record__aio_parse(const struct option *opt,if (unset) { opts->nr_cblocks =0 ; }else { - opts->nr_cblocks =strtol (str,NULL ,0 ); +if (str) + opts->nr_cblocks =strtol (str,NULL ,0 );if (!opts->nr_cblocks) opts->nr_cblocks =1 ; } return 0; } +static int nr_cblocks_default =1 ;#else /* HAVE_AIO_SUPPORT */ static int record__aio_sync (struct perf_mmap *md__maybe_unused ,bool sync_all__maybe_unused ) {
@@ -1913,8 +1915,8 @@ static struct option __record_options[] = { OPT_BOOLEAN(0, "dry-run", &dry_run, "Parse options then exit"),#ifdef HAVE_AIO_SUPPORT -OPT_CALLBACK (0 ,"aio" , &record.opts, -"n" ,"Use <n> control blocks in asynchronous trace writing mode (default: 1, max: 4)" , +OPT_CALLBACK_OPTARG (0 ,"aio" , &record.opts, + &nr_cblocks_default,"n" ,"Use <n> control blocks in asynchronous trace writing mode (default: 1, max: 4)" , record__aio_parse), #endifOPT_END ()
Re: [PATCH v13 3/3]: perf record: extend trace writing to multi AIO
Quoted 33 lines · Alexey Budankov
Hi,
Quoted 30 lines · Jiri Olsa
On Thu, Oct 11, 2018 at 09:59:19PM +0300, Alexey Budankov wrote:
SNIP
Quoted 15 lines
tools/perf/builtin-record.c +7
-0@@ -1490,6 +1514,13 @@ 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); } +#ifdef HAVE_AIO_SUPPORT +if (!strcmp (var,"record.aio" )) { + rec->opts.nr_cblocks =strtol (value,NULL ,0 ); +if (!rec->opts.nr_cblocks) + rec->opts.nr_cblocks =1 ; + } +#endif return 0 ; }tools/perf/builtin-record.c static struct option __record_options[] = { · line 1918
Quoted 8 lines
tools/perf/builtin-record.c +2
-2@@ -1882,8 +1913,8 @@ static struct option __record_options[] = { OPT_BOOLEAN(0, "dry-run", &dry_run, "Parse options then exit"),#ifdef HAVE_AIO_SUPPORT -OPT_CALLBACK_NOOPT (0 ,"aio" , &record.opts, -NULL ,"Enable asynchronous trace writing mode" , +OPT_CALLBACK (0 ,"aio" , &record.opts, +"n" ,"Use <n> control blocks in asynchronous trace writing mode (default: 1, max: 4)" , record__aio_parse),it stoped work with no argument:
Quoted 6 lines
Quoted 3 lines
[jolsa@krava perf]$ ./perf record --aio ls Error:
You may not have permission to collect system-wide stats.This is the fix below. However, perf record --aio -- ls works without it. I will resend the whole patch kit.
looks good, thanks
jirka