Hi,
On Fri, Oct 05, 2018 at 12:39:10PM +0300, Alexey Budankov wrote:
> Hi,
>
> On 05.10.2018 11:48, Namhyung Kim wrote:
> > On Fri, Oct 05, 2018 at 11:31:11AM +0300, Alexey Budankov wrote:
> <SNIP>
> >>
> >> Well, this could be implemented like this avoiding lseek() in else branch:
> >>
> >> off = lseek(trace_fd, 0, SEEK_CUR);
> >> ret = record__aio_write(cblock, trace_fd, bf, size, off);
> >> if (!ret) {
> >> lseek(trace_fd, off + size, SEEK_SET);
> >> rec->bytes_written += size;
> >>
> >> if (switch_output_size(rec))
> >> trigger_hit(&switch_output_trigger);
> >> }
> >
> > Oh I meant the both like:
> >
> > off = rec->bytes_written;
> > ret = record__aio_write(cblock, trace_fd, bf, size, off);
> > if (!ret) {
> > rec->bytes_written += size;
> >
> > ...
> >
>
> It still have to adjust the file pos thru lseek() prior leaving
> record__aio_pushfn() so space in trace file would be pre-allocated for
> enqueued record and file pos be moved beyond the record data,
> possibly for the next record.
For that purpose, isn't it better calling ftruncate() with a
reasonable batch size to reduce number of syscalls?
>
> >
> <SNIP>
> >
> > Why not exposing opts.nr_cblocks regardless of the #ifdef? It'll have
> > 0 when it's not compiled in. Then it could be like below (assuming
> > you have all the dummy aio funcitons):
> >
> >
> >>
> >> if (map->base) {
> >> if (!rec->opts.nr_cblocks) {
> >> if (perf_mmap__push(map, rec, record__pushfn)
> >> != 0) {
> >> rc = -1;
> >> goto out;
> >> }
> >> } else {
> >> int idx;
> >> /*
> >> * Call record__aio_sync() to wait till
> >> map->data buffer
> >> * becomes available after previous aio write
> >> request.
> >> */
> >> idx = record__aio_sync(map, false);
> >> if (perf_mmap__aio_push(map, rec, idx,
> >> record__aio_pushfn) != 0) {
> >> rc = -1;
> >> goto out;
> >> }
> >> }
> >> }
> >>
>
> Well, if it has AIO symbols + opts.nr_cblocks exposed unconditionally of
> HAVE_AIO_SUPPORT, but keeps the symbols implementation under the define, then
> as far aio-cblocks option is not exposed thru command line, we end up in
> whole bunch of symbols referenced under the else branch that, after all,
> can cause Perf binary size increase, which is, probably, worth avoiding.
I think it's ok as long as they're empty.
Thanks,
Namhyung
