On 12.10.2020 19:09, Andi Kleen wrote:
> On Mon, Oct 12, 2020 at 11:58:58AM +0300, Alexey Budankov wrote:
>>
>> Read trace files located at data directory into tool process memory.
>> Basic analysis support of data directories is provided for report
>> mode. Raw dump (-D) and aggregated reports are available for data
>> directories, still with no memory consumption optimizations. However
>> data directories collected with --compression-level option enabled
>> can be analyzed with little less memory because trace files are
>> unmaped from tool process memory after loading collected data.
>> The implementation is based on the prototype [1], [2].
>
> Should credit the author(s) of the prototypes.
Sure. Will explicitly add:
Suggested-by: Jiri Olsa <[email protected]>
Suggested-by: Namhyung Kim <[email protected]>
here and [PATCH v2 15/15], additionally to [1], [2] below.
Thanks for pointing this out!
>>
>> [1] git clone https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git -b perf/record_threads
>> [2] https://lore.kernel.org/lkml/[email protected]/
>>
>> Signed-off-by: Alexey Budankov <[email protected]>
>> ---
>> tools/perf/util/session.c | 48 +++++++++++++++++++++++++++++++++++++++
>> tools/perf/util/session.h | 1 +
>> 2 files changed, 49 insertions(+)
>>
>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>> index 6afc670fdf0c..0752eec19813 100644
>> --- a/tools/perf/util/session.c
>> +++ b/tools/perf/util/session.c
>> @@ -2212,6 +2212,17 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>> goto more;
>>
>> out:
>> + if (rd->unmap_file) {
>> + int i;
>> +
>> + for (i = 0; i < NUM_MMAPS; i++) {
>> + if (mmaps[i]) {
>> + munmap(mmaps[i], mmap_size);
>> + mmaps[i] = NULL;
>
> Okay so where is the mmap? Would make more sense to put that
> into the same patch as who adds the mmap. Or is the mmap
> code already in the perf source? In that case it should
> probably be some common helper with the existing users.
That mmap is already in the code. Agree, this part of the patch
can be applied prior the whole patch set.
Alexei
On 12.10.2020 19:49, Alexey Budankov wrote:
>
> On 12.10.2020 19:09, Andi Kleen wrote:
>> On Mon, Oct 12, 2020 at 11:58:58AM +0300, Alexey Budankov wrote:
>>>
>>> Read trace files located at data directory into tool process memory.
>>> Basic analysis support of data directories is provided for report
>>> mode. Raw dump (-D) and aggregated reports are available for data
>>> directories, still with no memory consumption optimizations. However
>>> data directories collected with --compression-level option enabled
>>> can be analyzed with little less memory because trace files are
>>> unmaped from tool process memory after loading collected data.
>>> The implementation is based on the prototype [1], [2].
>>
>> Should credit the author(s) of the prototypes.
>
> Sure. Will explicitly add:
>
> Suggested-by: Jiri Olsa <[email protected]>
> Suggested-by: Namhyung Kim <[email protected]>
>
> here and [PATCH v2 15/15], additionally to [1], [2] below.
>
> Thanks for pointing this out!
>
>>>
>>> [1] git clone https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git -b perf/record_threads
>>> [2] https://lore.kernel.org/lkml/[email protected]/
>>>
>>> Signed-off-by: Alexey Budankov <[email protected]>
>>> ---
>>> tools/perf/util/session.c | 48 +++++++++++++++++++++++++++++++++++++++
>>> tools/perf/util/session.h | 1 +
>>> 2 files changed, 49 insertions(+)
>>>
>>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>>> index 6afc670fdf0c..0752eec19813 100644
>>> --- a/tools/perf/util/session.c
>>> +++ b/tools/perf/util/session.c
>>> @@ -2212,6 +2212,17 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>>> goto more;
>>>
>>> out:
>>> + if (rd->unmap_file) {
>>> + int i;
>>> +
>>> + for (i = 0; i < NUM_MMAPS; i++) {
>>> + if (mmaps[i]) {
>>> + munmap(mmaps[i], mmap_size);
>>> + mmaps[i] = NULL;
>>
>> Okay so where is the mmap? Would make more sense to put that
>> into the same patch as who adds the mmap. Or is the mmap
>> code already in the perf source? In that case it should
>> probably be some common helper with the existing users.
>
> That mmap is already in the code. Agree, this part of the patch
> can be applied prior the whole patch set.
I take it back. Single trace file can't be unmapped yet since it also
contains not only compressed records but also other records backing
the data for aggregated analysis.
Alexei
Hi,
On Tue, Oct 13, 2020 at 8:30 PM Alexey Budankov
<[email protected]> wrote:
> On 12.10.2020 19:49, Alexey Budankov wrote:
> > On 12.10.2020 19:09, Andi Kleen wrote:
> >> On Mon, Oct 12, 2020 at 11:58:58AM +0300, Alexey Budankov wrote:
> >>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> >>> index 6afc670fdf0c..0752eec19813 100644
> >>> --- a/tools/perf/util/session.c
> >>> +++ b/tools/perf/util/session.c
> >>> @@ -2212,6 +2212,17 @@ reader__process_events(struct reader *rd, struct perf_session *session,
> >>> goto more;
> >>>
> >>> out:
> >>> + if (rd->unmap_file) {
> >>> + int i;
> >>> +
> >>> + for (i = 0; i < NUM_MMAPS; i++) {
> >>> + if (mmaps[i]) {
> >>> + munmap(mmaps[i], mmap_size);
> >>> + mmaps[i] = NULL;
> >>
> >> Okay so where is the mmap? Would make more sense to put that
> >> into the same patch as who adds the mmap. Or is the mmap
> >> code already in the perf source? In that case it should
> >> probably be some common helper with the existing users.
> >
> > That mmap is already in the code. Agree, this part of the patch
> > can be applied prior the whole patch set.
>
> I take it back. Single trace file can't be unmapped yet since it also
> contains not only compressed records but also other records backing
> the data for aggregated analysis.
Are you talking about the auxtrace?
I thought everything will be compressed when it's enabled.
But if it's only for the auxtrace, maybe we can unmap them
with checking it as it's not the common case?
Thanks
Namhyung
On 21.10.2020 9:54, Namhyung Kim wrote:
> Hi,
>
> On Tue, Oct 13, 2020 at 8:30 PM Alexey Budankov
> <[email protected]> wrote:
>> On 12.10.2020 19:49, Alexey Budankov wrote:
>>> On 12.10.2020 19:09, Andi Kleen wrote:
>>>> On Mon, Oct 12, 2020 at 11:58:58AM +0300, Alexey Budankov wrote:
>>>>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>>>>> index 6afc670fdf0c..0752eec19813 100644
>>>>> --- a/tools/perf/util/session.c
>>>>> +++ b/tools/perf/util/session.c
>>>>> @@ -2212,6 +2212,17 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>>>>> goto more;
>>>>>
>>>>> out:
>>>>> + if (rd->unmap_file) {
>>>>> + int i;
>>>>> +
>>>>> + for (i = 0; i < NUM_MMAPS; i++) {
>>>>> + if (mmaps[i]) {
>>>>> + munmap(mmaps[i], mmap_size);
>>>>> + mmaps[i] = NULL;
>>>>
>>>> Okay so where is the mmap? Would make more sense to put that
>>>> into the same patch as who adds the mmap. Or is the mmap
>>>> code already in the perf source? In that case it should
>>>> probably be some common helper with the existing users.
>>>
>>> That mmap is already in the code. Agree, this part of the patch
>>> can be applied prior the whole patch set.
>>
>> I take it back. Single trace file can't be unmapped yet since it also
>> contains not only compressed records but also other records backing
>> the data for aggregated analysis.
>
> Are you talking about the auxtrace?
>
> I thought everything will be compressed when it's enabled.
> But if it's only for the auxtrace, maybe we can unmap them
> with checking it as it's not the common case?
I am about primary trace file. Together with PERF_RECORD_COMPRESSED
records it contains the other records with uncompressed data (MMAP,
FORk, COMM etc.) so it is still unsafe to unmap the trace file after
loading.
Alexei
On Wed, Oct 21, 2020 at 7:25 PM Alexey Budankov
<[email protected]> wrote:
>
>
> On 21.10.2020 9:54, Namhyung Kim wrote:
> > Hi,
> >
> > On Tue, Oct 13, 2020 at 8:30 PM Alexey Budankov
> > <[email protected]> wrote:
> >> On 12.10.2020 19:49, Alexey Budankov wrote:
> >>> On 12.10.2020 19:09, Andi Kleen wrote:
> >>>> On Mon, Oct 12, 2020 at 11:58:58AM +0300, Alexey Budankov wrote:
> >>>>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> >>>>> index 6afc670fdf0c..0752eec19813 100644
> >>>>> --- a/tools/perf/util/session.c
> >>>>> +++ b/tools/perf/util/session.c
> >>>>> @@ -2212,6 +2212,17 @@ reader__process_events(struct reader *rd, struct perf_session *session,
> >>>>> goto more;
> >>>>>
> >>>>> out:
> >>>>> + if (rd->unmap_file) {
> >>>>> + int i;
> >>>>> +
> >>>>> + for (i = 0; i < NUM_MMAPS; i++) {
> >>>>> + if (mmaps[i]) {
> >>>>> + munmap(mmaps[i], mmap_size);
> >>>>> + mmaps[i] = NULL;
> >>>>
> >>>> Okay so where is the mmap? Would make more sense to put that
> >>>> into the same patch as who adds the mmap. Or is the mmap
> >>>> code already in the perf source? In that case it should
> >>>> probably be some common helper with the existing users.
> >>>
> >>> That mmap is already in the code. Agree, this part of the patch
> >>> can be applied prior the whole patch set.
> >>
> >> I take it back. Single trace file can't be unmapped yet since it also
> >> contains not only compressed records but also other records backing
> >> the data for aggregated analysis.
> >
> > Are you talking about the auxtrace?
> >
> > I thought everything will be compressed when it's enabled.
> > But if it's only for the auxtrace, maybe we can unmap them
> > with checking it as it's not the common case?
>
> I am about primary trace file. Together with PERF_RECORD_COMPRESSED
> records it contains the other records with uncompressed data (MMAP,
> FORk, COMM etc.) so it is still unsafe to unmap the trace file after
> loading.
Oh, I think all the events coming from the kernel share the mmap
buffer so they will be compressed with SAMPLEs. Note that
synthesized events have 0 timestamp which means that they
are not queued in the ordered events and processed immediately.
IIUC, here we unmap the original mmap buffer and the actual data
in MMAP/FORK/... is kept in the decomp data buffer, no?
Thanks
Namhyung
On 21.10.2020 13:57, Namhyung Kim wrote:
> On Wed, Oct 21, 2020 at 7:25 PM Alexey Budankov
> <[email protected]> wrote:
>>
>>
>> On 21.10.2020 9:54, Namhyung Kim wrote:
>>> Hi,
>>>
>>> On Tue, Oct 13, 2020 at 8:30 PM Alexey Budankov
>>> <[email protected]> wrote:
>>>> On 12.10.2020 19:49, Alexey Budankov wrote:
>>>>> On 12.10.2020 19:09, Andi Kleen wrote:
>>>>>> On Mon, Oct 12, 2020 at 11:58:58AM +0300, Alexey Budankov wrote:
>>>>>>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>>>>>>> index 6afc670fdf0c..0752eec19813 100644
>>>>>>> --- a/tools/perf/util/session.c
>>>>>>> +++ b/tools/perf/util/session.c
>>>>>>> @@ -2212,6 +2212,17 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>>>>>>> goto more;
>>>>>>>
>>>>>>> out:
>>>>>>> + if (rd->unmap_file) {
>>>>>>> + int i;
>>>>>>> +
>>>>>>> + for (i = 0; i < NUM_MMAPS; i++) {
>>>>>>> + if (mmaps[i]) {
>>>>>>> + munmap(mmaps[i], mmap_size);
>>>>>>> + mmaps[i] = NULL;
>>>>>>
>>>>>> Okay so where is the mmap? Would make more sense to put that
>>>>>> into the same patch as who adds the mmap. Or is the mmap
>>>>>> code already in the perf source? In that case it should
>>>>>> probably be some common helper with the existing users.
>>>>>
>>>>> That mmap is already in the code. Agree, this part of the patch
>>>>> can be applied prior the whole patch set.
>>>>
>>>> I take it back. Single trace file can't be unmapped yet since it also
>>>> contains not only compressed records but also other records backing
>>>> the data for aggregated analysis.
>>>
>>> Are you talking about the auxtrace?
>>>
>>> I thought everything will be compressed when it's enabled.
>>> But if it's only for the auxtrace, maybe we can unmap them
>>> with checking it as it's not the common case?
>>
>> I am about primary trace file. Together with PERF_RECORD_COMPRESSED
>> records it contains the other records with uncompressed data (MMAP,
>> FORk, COMM etc.) so it is still unsafe to unmap the trace file after
>> loading.
>
> Oh, I think all the events coming from the kernel share the mmap
> buffer so they will be compressed with SAMPLEs. Note that
> synthesized events have 0 timestamp which means that they
> are not queued in the ordered events and processed immediately.
Yes, they are and it will work as expected with this patch changes.
>
> IIUC, here we unmap the original mmap buffer and the actual data
> in MMAP/FORK/... is kept in the decomp data buffer, no?
Correct. It unmaps trace file from memory after COMPRESSED records
are decompressed into separately allocated decomp objects.
Alexei
>
> Thanks
> Namhyung
>