perf tools: Exit recording if events have non matching sample type
Em Thu, Sep 29, 2011 at 04:55:23PM -0600, David Ahern escreveu:
Quoted 16 lines · Arnaldo Carvalho de Melo
Quoted 15 lines
Em Thu, Sep 29, 2011 at 06:01:08PM +0200, Jiri Olsa escreveu:
Quoted 8 lines
Following command hangs on my setup:
./perf record -o perf.data -e ext4:ext4_mb_new_group_pa \ -e LLC-loads -- date '+%F'because hardware and tracepoint events have different sample type.
With the patch applied the record command displays "Non matching
sample_type" message and exits.That is way too cryptic :-\
What is that makes the sample type not match in this case? Can we make it match instead?
Quoted 6 lines
Quoted 5 lines
This is something to be properly fixed by _allowing_ non matching sample types, the evsel/evlist abstractions are getting we close but not there yet, multiple files in a perf.data/ directory are needed.
From what I can see
sample_typehas to be the same for all samples:
We could add a PERF_SAMPLE_ID2 that if present would be guaranteed to be the last, or we can, as PeterZ prefers/suggests, to use one mmap per sample id, then we know that if we're getting it on that mmap, it has that sample_type.
For record that means we need to dump each per-sample_id mmap onto a separate file, i.e. use a directory, etc.
But what I suggested as a short term solution was to check what is the difference in the above case and then make them use a single sample type, i.e. bow to the current restriction.
- Arnaldo
Quoted 10 lines · Arnaldo Carvalho de Melo
Quoted 2 lines
Quoted 1 line
From what I can see
sample_typehas to be the same for all samples:We could add a
PERF_SAMPLE_ID2that if present would be guaranteed to be the last, or we can, as PeterZ prefers/suggests, to use one mmap per sample id, then we know that if we're getting it on that mmap, it has thatsample_type.For record that means we need to dump each per-sample_id mmap onto a separate file, i.e. use a directory, etc.
That seems like a major re-write of perf. Why not keep the existing restriction and have users who want different sample_types run multiple perf commands?
What's to be gained by refactoring the code to support multiple sample_types in a single command invocation?
David
Em Thu, Sep 29, 2011 at 05:33:42PM -0600, David Ahern escreveu:
Quoted 12 lines · Arnaldo Carvalho de Melo
Quoted 10 lines
Quoted 2 lines
Quoted 1 line
From what I can see
sample_typehas to be the same for all samples:We could add a
PERF_SAMPLE_ID2that if present would be guaranteed to be the last, or we can, as PeterZ prefers/suggests, to use one mmap per sample id, then we know that if we're getting it on that mmap, it has thatsample_type.For record that means we need to dump each per-sample_id mmap onto a separate file, i.e. use a directory, etc.
Quoted 6 lines
That seems like a major re-write of perf. Why not keep the existing restriction and have users who want different
sample_typesrun multiple perf commands?What's to be gained by refactoring the code to support multiple
sample_typesin a single command invocation?
This comes from time to time:
The whole point of sample_type is to ask for just what is needed for some specific event so that we reduce the per sample footprint.
So we allow for multiple types of events to be on the same stream, but not for them to have just what each need, its a limitation, one that we can solve. I see no reason not to solve it :-)
The evsel/evlist classes were designed to abstract away such details, i.e. perf_evlist__mmap should notice different sample_types and sort this out transparently.
And then perf_evlist__mmap_read(evlist, idx), evlist->nr_mmaps should be enough to process the events :-)
- Arnaldo
Quoted 31 lines · Arnaldo Carvalho de Melo
Em Thu, Sep 29, 2011 at 05:33:42PM -0600, David Ahern escreveu:
Quoted 12 lines · Arnaldo Carvalho de Melo
Quoted 10 lines
Quoted 2 lines
Quoted 1 line
From what I can see
sample_typehas to be the same for all samples:We could add a
PERF_SAMPLE_ID2that if present would be guaranteed to be the last, or we can, as PeterZ prefers/suggests, to use one mmap per sample id, then we know that if we're getting it on that mmap, it has thatsample_type.For record that means we need to dump each per-sample_id mmap onto a separate file, i.e. use a directory, etc.
Quoted 6 lines
That seems like a major re-write of perf. Why not keep the existing restriction and have users who want different
sample_typesrun multiple perf commands?What's to be gained by refactoring the code to support multiple
sample_typesin a single command invocation?This comes from time to time:
The whole point of
sample_typeis to ask for just what is needed for some specific event so that we reduce the per sample footprint.So we allow for multiple types of events to be on the same stream, but not for them to have just what each need, its a limitation, one that we can solve. I see no reason not to solve it :-)
Quoted 8 lines
The evsel/evlist classes were designed to abstract away such details, i.e.
perf_evlist__mmapshould notice differentsample_typesand sort this out transparently.And then
perf_evlist__mmap_read(evlist, idx),evlist->nr_mmapsshould be enough to process the events :-)- Arnaldo
An ABI change fixes the root cause of the current problem -- put an id in perf_event_header. The id maps to the evsel which has the sample_type. Right now you have to parse the sample to get the id to get the evsel.
With that change I have a patch that (mostly?) covers the rest of perf userspace to handle sample_type per evsel.
David
Em Thu, Sep 29, 2011 at 06:58:20PM -0600, David Ahern escreveu:
Quoted 19 lines · Arnaldo Carvalho de Melo
Quoted 18 lines
This comes from time to time:
The whole point of
sample_typeis to ask for just what is needed for some specific event so that we reduce the per sample footprint.So we allow for multiple types of events to be on the same stream, but not for them to have just what each need, its a limitation, one that we can solve. I see no reason not to solve it :-)
The evsel/evlist classes were designed to abstract away such details, i.e.
perf_evlist__mmapshould notice differentsample_typesand sort this out transparently.And then
perf_evlist__mmap_read(evlist, idx),evlist->nr_mmapsshould be enough to process the events :-)
Quoted 6 lines
Quoted 1 line
- Arnaldo
An ABI change fixes the root cause of the current problem -- put an id in
perf_event_header. The id maps to the evsel which has thesample_type. Right now you have to parse the sample to get the id to get the evsel.
This id is not always needed, so it shouldn't go to perf_event_header. Peter, can you comment here? ;-)
- Arnaldo
Quoted 22 lines · Arnaldo Carvalho de Melo
Em Thu, Sep 29, 2011 at 06:58:20PM -0600, David Ahern escreveu:
Quoted 21 lines · Arnaldo Carvalho de Melo
Quoted 19 lines
This comes from time to time:
The whole point of
sample_typeis to ask for just what is needed for some specific event so that we reduce the per sample footprint.So we allow for multiple types of events to be on the same stream, but not for them to have just what each need, its a limitation, one that we can solve. I see no reason not to solve it :-)
The evsel/evlist classes were designed to abstract away such details, i.e.
perf_evlist__mmapshould notice differentsample_typesand sort this out transparently.And then
perf_evlist__mmap_read(evlist, idx),evlist->nr_mmapsshould be enough to process the events :-)- Arnaldo
Quoted 7 lines
Quoted 4 lines
An ABI change fixes the root cause of the current problem -- put an id in
perf_event_header. The id maps to the evsel which has thesample_type. Right now you have to parse the sample to get the id to get the evsel.This id is not always needed, so it shouldn't go to
perf_event_header. Peter, can you comment here? ;-)
I'm talking about a new 'id' that would always be needed *for samples* -- it identifies the evsel it is associated with. The evsel has the sample type. The sample_type is then used to parse it. Seems like a simpler solution then having mmap's per sample type and then writing separate files.
David
Quoted 4 lines · David Ahern
Quoted 2 lines
For record that means we need to dump each per-sample_id mmap onto a separate file, i.e. use a directory, etc.
That seems like a major re-write of perf.
Well we want to go there anyway. A file per cpu stream can be a lot less overhead than one file for all cpu streams.
Quoted 9 lines · Peter Zijlstra
Quoted 4 lines · David Ahern
Quoted 2 lines
For record that means we need to dump each per-sample_id mmap onto a separate file, i.e. use a directory, etc.
That seems like a major re-write of perf.
Well we want to go there anyway. A file per cpu stream can be a lot less overhead than one file for all cpu streams.
really it becomes file per cpu stream and per sample type. So a dual socket, quad core with HT means 16 files per sample type. ie., the number of files explodes quick.
I understand the allure for simplicity during the data collection. Has any thought been given on management of the files in such a scenario? User specifies a directory path instead of a file path - or both to handle backward compatibility?
David
Quoted 11 lines · David Ahern
Quoted 9 lines · Peter Zijlstra
Quoted 4 lines · David Ahern
Quoted 2 lines
For record that means we need to dump each per-sample_id mmap onto a separate file, i.e. use a directory, etc.
That seems like a major re-write of perf.
Well we want to go there anyway. A file per cpu stream can be a lot less overhead than one file for all cpu streams.
Quoted 8 lines
really it becomes file per cpu stream and per sample type. So a dual socket, quad core with HT means 16 files per sample type. ie., the number of files explodes quick.
I understand the allure for simplicity during the data collection. Has any thought been given on management of the files in such a scenario? User specifies a directory path instead of a file path - or both to handle backward compatibility?
I think the idea was a directory and a script to convert old data files to the new format. But it was a while ago acme and I talked about this.
Em Tue, Oct 04, 2011 at 12:00:12PM +0200, Peter Zijlstra escreveu:
Quoted 15 lines · David Ahern
Quoted 13 lines · Peter Zijlstra
Quoted 8 lines · David Ahern
Quoted 4 lines
Quoted 2 lines
For record that means we need to dump each per-sample_id mmap onto a separate file, i.e. use a directory, etc.
That seems like a major re-write of perf.
Well we want to go there anyway. A file per cpu stream can be a lot less overhead than one file for all cpu streams.
really it becomes file per cpu stream and per sample type. So a dual socket, quad core with HT means 16 files per sample type. ie., the number of files explodes quick.
Quoted 7 lines
Quoted 4 lines
I understand the allure for simplicity during the data collection. Has any thought been given on management of the files in such a scenario? User specifies a directory path instead of a file path - or both to handle backward compatibility?
I think the idea was a directory and a script to convert old data files to the new format. But it was a while ago acme and I talked about this.
I don't remember the script idea, but that would work too.
WRT major rewrites, well, as we go on developing any piece of software, those kinds of things happen :-)
If it is to improve things, make them faster, cope with new hardware realities, keep our jobs, so be it! ;-)
- Arnaldo