lkml.org 
[lkml]   [2013]   [Oct]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] perf record: mmap output file - v2
On 10/15/13 1:09 AM, Namhyung Kim wrote:
>> The stat() seems superfluous, here in __cmd_record() we've just checked
>> the output_name and made sure it exists. Can that stat() call ever fail?
>
> AFAICS it's needed to check current file size. But I think it's better
> to use fstat().

Sure fstat could be used over stat -- if it ends up staying.


>>
>> 3)
>>
>> The rec->bytes_at_mmap_start field feels a bit weird. If I read the code
>> correctly, in every 'perf record' invocation, rec->bytes_written starts at
>> 0 - i.e. we don't have repeat invocations of cmd_record().
>
> rec->bytes_written is updated when it writes to the output file for
> synthesizing COMM/MMAP events (this mmap output is not used at that time).

Ingo: I went through a number of itereations before using the
bytes_at_mmap_start. One of those was to use the bytes_written counter.
All failed. Header + synthesized events are written to the file before
we start farming the ring buffers.

Perhaps a good code cleanup will help figure out why. I needed the
functionality ASAP for use with perf-trace -a so I stuck with the new
variable. Since this change is working out well, I will look at a code
clean up on the next round.

I am traveling to LinuxCon / KVM Forum / Tracing Forum on Friday.
Perhaps the clean up and followup patch can be done on the long plane
ride; more likely when I return which means 3.14 material.

> Actually I worried about the mmap offset not being aligned to page
> size. But it seems that's not a problem.

This code snippet makes sure the mmap offset is a multiple of 64M
(rec->mmap_size). offset is the argument to mmap; mmap_offset is the
where we are within the mmap for the next copy:

+ offset = rec->bytes_at_mmap_start + rec->bytes_written;
+ if (offset < (ssize_t) rec->mmap_size) {
+ rec->mmap_offset = offset;
+ offset = 0;
+ } else
+ rec->mmap_offset = 0;


David


\
 
 \ /
  Last update: 2013-10-15 15:41    [W:0.123 / U:0.300 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site