* David Ahern <[email protected]> wrote:
> + /* for MMAP based file writes */
> + void *mmap_addr;
> + u64 bytes_at_mmap_start; /* bytes in file when mmap
> use starts */
> + u64 mmap_offset; /* current location within mmap
> */
> + size_t mmap_size; /* size of mmap segments */
> + bool use_mmap;
> + if (!rec->opts.pipe_output && stat(output_name, &st) == 0) {
> + rec->use_mmap = true;
> + rec->bytes_at_mmap_start = st.st_size - rec->bytes_written;
> + }
1)
I think __cmd_record() has become way too large, nearly 300 lines of code.
It would be nice to split it into 2-3 helpers that operate on 'struct
perf_record' or so.
2)
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?
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().
That means that this:
rec->bytes_at_mmap_start = st.st_size - rec->bytes_written;
is really:
rec->bytes_at_mmap_start = st.st_size;
furthermore, since we don't allow appends anymore, st.st_size ought to be
zero as well.
Which means that ->bytes_at_mmap_start is always zero - and could be
eliminated altogether.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
