This patch disables the mmap2 record format support
for now. The support needs a bit more work to cover
VM_CLONE cases.
The patch leaves attr->mmap2 defined, but returns
an error if it is set. That ensures no PERF_RECORD_MMAP2
record can be generated. Yet it minimizes the reverts in
the perf tool.
Signed-off-by: Stephane Eranian <[email protected]>
---
include/uapi/linux/perf_event.h | 18 +-----------------
kernel/events/core.c | 4 ++++
2 files changed, 5 insertions(+), 17 deletions(-)
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index da48837..8074b29 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -682,23 +682,7 @@ enum perf_event_type {
PERF_RECORD_SAMPLE = 9,
/*
- * The MMAP2 records are an augmented version of MMAP, they add
- * maj, min, ino numbers to be used to uniquely identify each mapping
- *
- * struct {
- * struct perf_event_header header;
- *
- * u32 pid, tid;
- * u64 addr;
- * u64 len;
- * u64 pgoff;
- * u32 maj;
- * u32 min;
- * u64 ino;
- * u64 ino_generation;
- * char filename[];
- * struct sample_id sample_id;
- * };
+ * reserved, do not use
*/
PERF_RECORD_MMAP2 = 10,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index c716385..d7152a6 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6782,6 +6782,10 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
if (attr->read_format & ~(PERF_FORMAT_MAX-1))
return -EINVAL;
+ /* disable for now */
+ if (attr->mmap2)
+ return -EINVAL;
+
if (attr->sample_type & PERF_SAMPLE_BRANCH_STACK) {
u64 mask = attr->branch_sample_type;
--
1.7.10.4
On 10/9/13 10:17 AM, Stephane Eranian wrote:
>
> This patch disables the mmap2 record format support
> for now. The support needs a bit more work to cover
> VM_CLONE cases.
>
> The patch leaves attr->mmap2 defined, but returns
> an error if it is set. That ensures no PERF_RECORD_MMAP2
> record can be generated. Yet it minimizes the reverts in
> the perf tool.
The userspace code generates MMAP2 events for synthesized mmap events.
That should be reverted to MMAP until the format of MMAP2 events is
finalized.
David
On Wed, Oct 9, 2013 at 6:21 PM, David Ahern <[email protected]> wrote:
> On 10/9/13 10:17 AM, Stephane Eranian wrote:
>>
>>
>> This patch disables the mmap2 record format support
>> for now. The support needs a bit more work to cover
>> VM_CLONE cases.
>>
>> The patch leaves attr->mmap2 defined, but returns
>> an error if it is set. That ensures no PERF_RECORD_MMAP2
>> record can be generated. Yet it minimizes the reverts in
>> the perf tool.
>
>
> The userspace code generates MMAP2 events for synthesized mmap events. That
> should be reverted to MMAP until the format of MMAP2 events is finalized.
>
Ah, yes, that is correct. I forgot about that ;-(
Let me send a revised patch then that covers both.
> David
On Wed, Oct 9, 2013 at 6:24 PM, Stephane Eranian <[email protected]> wrote:
> On Wed, Oct 9, 2013 at 6:21 PM, David Ahern <[email protected]> wrote:
>> On 10/9/13 10:17 AM, Stephane Eranian wrote:
>>>
>>>
>>> This patch disables the mmap2 record format support
>>> for now. The support needs a bit more work to cover
>>> VM_CLONE cases.
>>>
>>> The patch leaves attr->mmap2 defined, but returns
>>> an error if it is set. That ensures no PERF_RECORD_MMAP2
>>> record can be generated. Yet it minimizes the reverts in
>>> the perf tool.
>>
>>
>> The userspace code generates MMAP2 events for synthesized mmap events. That
>> should be reverted to MMAP until the format of MMAP2 events is finalized.
>>
> Ah, yes, that is correct. I forgot about that ;-(
> Let me send a revised patch then that covers both.
>
Looks like the easiest thing to do for perf is to revert:
Revert "perf tools: Add default handler for mmap2 events"
git revert 6adb0b0ae26fcc35cfec068d71f13863faac5b44
Revert "perf tools: Add attr->mmap2 support"
git revert 5c5e854bc760a2e2c878df3cfcf2afa4febcd511
Do you agree?
On 10/9/13 10:39 AM, Stephane Eranian wrote:
> Looks like the easiest thing to do for perf is to revert:
>
> Revert "perf tools: Add default handler for mmap2 events"
> git revert 6adb0b0ae26fcc35cfec068d71f13863faac5b44
>
> Revert "perf tools: Add attr->mmap2 support"
> git revert 5c5e854bc760a2e2c878df3cfcf2afa4febcd511
>
> Do you agree?
If you want to strip all of mmap2 you also need
5654a0257a86c093c2b38ea5d3ceaa17affd735b.
David
On Wed, Oct 9, 2013 at 7:19 PM, David Ahern <[email protected]> wrote:
> On 10/9/13 10:39 AM, Stephane Eranian wrote:
>>
>> Looks like the easiest thing to do for perf is to revert:
>>
>> Revert "perf tools: Add default handler for mmap2 events"
>> git revert 6adb0b0ae26fcc35cfec068d71f13863faac5b44
>>
>> Revert "perf tools: Add attr->mmap2 support"
>> git revert 5c5e854bc760a2e2c878df3cfcf2afa4febcd511
>>
>> Do you agree?
>
>
> If you want to strip all of mmap2 you also need
> 5654a0257a86c093c2b38ea5d3ceaa17affd735b.
>
You mean:
commit 384c671e33a7ddf905f2c60b433b9883f0e5a605
Author: David Ahern <[email protected]>
Date: Sun Sep 22 19:44:58 2013 -0600
perf trace: Add mmap2 handler
Right?
On 10/9/13 11:40 AM, Stephane Eranian wrote:
> On Wed, Oct 9, 2013 at 7:19 PM, David Ahern <[email protected]> wrote:
>> On 10/9/13 10:39 AM, Stephane Eranian wrote:
>>>
>>> Looks like the easiest thing to do for perf is to revert:
>>>
>>> Revert "perf tools: Add default handler for mmap2 events"
>>> git revert 6adb0b0ae26fcc35cfec068d71f13863faac5b44
>>>
>>> Revert "perf tools: Add attr->mmap2 support"
>>> git revert 5c5e854bc760a2e2c878df3cfcf2afa4febcd511
>>>
>>> Do you agree?
>>
>>
>> If you want to strip all of mmap2 you also need
>> 5654a0257a86c093c2b38ea5d3ceaa17affd735b.
>>
> You mean:
> commit 384c671e33a7ddf905f2c60b433b9883f0e5a605
> Author: David Ahern <[email protected]>
> Date: Sun Sep 22 19:44:58 2013 -0600
>
> perf trace: Add mmap2 handler
>
> Right?
Yes, a couple of those have 2 commit ids. I pulled the id's from acme's
perf/core branch
David
Em Wed, Oct 09, 2013 at 06:17:43PM +0200, Stephane Eranian escreveu:
> This patch disables the mmap2 record format support
> for now. The support needs a bit more work to cover
> VM_CLONE cases.
>
> The patch leaves attr->mmap2 defined, but returns
> an error if it is set. That ensures no PERF_RECORD_MMAP2
> record can be generated. Yet it minimizes the reverts in
> the perf tool.
Would be interesting to have a simple new tool that would detect all the
mmaps that are shared in a given perf.data file, using the
PERF_RECORD_MMAP2 information, and that tool should come together with
the reworked kernel ABI, showing how it is used.
A 'perf test' entry, based on the test programs you guys are using to
test this functionality would be even better, showing that all cases
you've been striving to cover are in fact covered by the new ABI.
- Arnaldo
>
> Signed-off-by: Stephane Eranian <[email protected]>
> ---
> include/uapi/linux/perf_event.h | 18 +-----------------
> kernel/events/core.c | 4 ++++
> 2 files changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index da48837..8074b29 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -682,23 +682,7 @@ enum perf_event_type {
> PERF_RECORD_SAMPLE = 9,
>
> /*
> - * The MMAP2 records are an augmented version of MMAP, they add
> - * maj, min, ino numbers to be used to uniquely identify each mapping
> - *
> - * struct {
> - * struct perf_event_header header;
> - *
> - * u32 pid, tid;
> - * u64 addr;
> - * u64 len;
> - * u64 pgoff;
> - * u32 maj;
> - * u32 min;
> - * u64 ino;
> - * u64 ino_generation;
> - * char filename[];
> - * struct sample_id sample_id;
> - * };
> + * reserved, do not use
> */
> PERF_RECORD_MMAP2 = 10,
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index c716385..d7152a6 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6782,6 +6782,10 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
> if (attr->read_format & ~(PERF_FORMAT_MAX-1))
> return -EINVAL;
>
> + /* disable for now */
> + if (attr->mmap2)
> + return -EINVAL;
> +
> if (attr->sample_type & PERF_SAMPLE_BRANCH_STACK) {
> u64 mask = attr->branch_sample_type;
>
> --
> 1.7.10.4
* Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Wed, Oct 09, 2013 at 06:17:43PM +0200, Stephane Eranian escreveu:
> > This patch disables the mmap2 record format support for now. The
> > support needs a bit more work to cover VM_CLONE cases.
> >
> > The patch leaves attr->mmap2 defined, but returns an error if it is
> > set. That ensures no PERF_RECORD_MMAP2 record can be generated. Yet it
> > minimizes the reverts in the perf tool.
>
> Would be interesting to have a simple new tool that would detect all the
> mmaps that are shared in a given perf.data file, using the
> PERF_RECORD_MMAP2 information, and that tool should come together with
> the reworked kernel ABI, showing how it is used.
Indeed. Let's make that a requirement for the next round of mmap2 patches.
Issues like this is really why I hate not getting a fully functional
usecase together with a new ABI - it's _so_ easy to come up with something
subtly (or not so subtly) incomplete.
> A 'perf test' entry, based on the test programs you guys are using to
> test this functionality would be even better, showing that all cases
> you've been striving to cover are in fact covered by the new ABI.
That too, in addition to an actual tool that people can use.
Thanks,
Ingo
Em Wed, Oct 09, 2013 at 12:03:58PM -0600, David Ahern escreveu:
> On 10/9/13 11:40 AM, Stephane Eranian wrote:
> >On Wed, Oct 9, 2013 at 7:19 PM, David Ahern <[email protected]> wrote:
> >>On 10/9/13 10:39 AM, Stephane Eranian wrote:
> >You mean:
> >commit 384c671e33a7ddf905f2c60b433b9883f0e5a605
> >Author: David Ahern <[email protected]>
> >Date: Sun Sep 22 19:44:58 2013 -0600
> >
> > perf trace: Add mmap2 handler
> >
> >Right?
>
> Yes, a couple of those have 2 commit ids. I pulled the id's from
> acme's perf/core branch
I'm merging my perf/core branch with Ingo's build speedup series, so
I've removed that patch already, will be ok as soon as the disabling is
done in perf/urgent and those branches get merged.
- Arnaldo
Hi Stephane:
On 10/9/13 11:40 AM, Stephane Eranian wrote:
> On Wed, Oct 9, 2013 at 7:19 PM, David Ahern <[email protected]> wrote:
>> On 10/9/13 10:39 AM, Stephane Eranian wrote:
>>>
>>> Looks like the easiest thing to do for perf is to revert:
>>>
>>> Revert "perf tools: Add default handler for mmap2 events"
>>> git revert 6adb0b0ae26fcc35cfec068d71f13863faac5b44
>>>
>>> Revert "perf tools: Add attr->mmap2 support"
>>> git revert 5c5e854bc760a2e2c878df3cfcf2afa4febcd511
>>>
>>> Do you agree?
>>
>>
>> If you want to strip all of mmap2 you also need
>> 5654a0257a86c093c2b38ea5d3ceaa17affd735b.
>>
> You mean:
> commit 384c671e33a7ddf905f2c60b433b9883f0e5a605
> Author: David Ahern <[email protected]>
> Date: Sun Sep 22 19:44:58 2013 -0600
>
> perf trace: Add mmap2 handler
>
Given that 3.12 rc5 will be out on Sunday and it takes time to go from
Arnaldo to Ingo to Linus the revert patches should go out today - in
Arnaldo's next perf/urgent request.
David
Arnaldo,
Could you revert all the mmap2 patches in your perf/urgent tree for now?
Thanks.
On Fri, Oct 11, 2013 at 8:40 PM, David Ahern <[email protected]> wrote:
> Hi Stephane:
>
>
> On 10/9/13 11:40 AM, Stephane Eranian wrote:
>>
>> On Wed, Oct 9, 2013 at 7:19 PM, David Ahern <[email protected]> wrote:
>>>
>>> On 10/9/13 10:39 AM, Stephane Eranian wrote:
>>>>
>>>>
>>>> Looks like the easiest thing to do for perf is to revert:
>>>>
>>>> Revert "perf tools: Add default handler for mmap2 events"
>>>> git revert 6adb0b0ae26fcc35cfec068d71f13863faac5b44
>>>>
>>>> Revert "perf tools: Add attr->mmap2 support"
>>>> git revert 5c5e854bc760a2e2c878df3cfcf2afa4febcd511
>>>>
>>>> Do you agree?
>>>
>>>
>>>
>>> If you want to strip all of mmap2 you also need
>>> 5654a0257a86c093c2b38ea5d3ceaa17affd735b.
>>>
>> You mean:
>> commit 384c671e33a7ddf905f2c60b433b9883f0e5a605
>> Author: David Ahern <[email protected]>
>> Date: Sun Sep 22 19:44:58 2013 -0600
>>
>> perf trace: Add mmap2 handler
>>
>
> Given that 3.12 rc5 will be out on Sunday and it takes time to go from
> Arnaldo to Ingo to Linus the revert patches should go out today - in
> Arnaldo's next perf/urgent request.
>
> David
>
* Stephane Eranian <[email protected]> wrote:
> Arnaldo,
>
> Could you revert all the mmap2 patches in your perf/urgent tree for now?
> Thanks.
I think it would be helpful if you re-sent the final patch, with a full
changelog, etc., in a separate thread, not inside an existing discussion
and such.
Thanks,
Ingo
On Tue, Oct 15, 2013 at 3:12 PM, Ingo Molnar <[email protected]> wrote:
>
> * Stephane Eranian <[email protected]> wrote:
>
>> Arnaldo,
>>
>> Could you revert all the mmap2 patches in your perf/urgent tree for now?
>> Thanks.
>
> I think it would be helpful if you re-sent the final patch, with a full
> changelog, etc., in a separate thread, not inside an existing discussion
> and such.
>
Are you talking about the perf tool or kernel patch?
I did not send any perf-related mmap2 revert patches.
> Thanks,
>
> Ingo
* Stephane Eranian <[email protected]> wrote:
> On Tue, Oct 15, 2013 at 3:12 PM, Ingo Molnar <[email protected]> wrote:
> >
> > * Stephane Eranian <[email protected]> wrote:
> >
> >> Arnaldo,
> >>
> >> Could you revert all the mmap2 patches in your perf/urgent tree for now?
> >> Thanks.
> >
> > I think it would be helpful if you re-sent the final patch, with a full
> > changelog, etc., in a separate thread, not inside an existing discussion
> > and such.
>
> Are you talking about the perf tool or kernel patch?
Both.
> I did not send any perf-related mmap2 revert patches.
Would be nice to have a single patch that reverts both.
Thanks,
Ingo
On Tue, Oct 15, 2013 at 3:34 PM, Ingo Molnar <[email protected]> wrote:
>
> * Stephane Eranian <[email protected]> wrote:
>
>> On Tue, Oct 15, 2013 at 3:12 PM, Ingo Molnar <[email protected]> wrote:
>> >
>> > * Stephane Eranian <[email protected]> wrote:
>> >
>> >> Arnaldo,
>> >>
>> >> Could you revert all the mmap2 patches in your perf/urgent tree for now?
>> >> Thanks.
>> >
>> > I think it would be helpful if you re-sent the final patch, with a full
>> > changelog, etc., in a separate thread, not inside an existing discussion
>> > and such.
>>
>> Are you talking about the perf tool or kernel patch?
>
> Both.
>
>> I did not send any perf-related mmap2 revert patches.
>
> Would be nice to have a single patch that reverts both.
>
I will send a single patch to remove everything (kernel + tool) then.
* Stephane Eranian <[email protected]> wrote:
> On Tue, Oct 15, 2013 at 3:34 PM, Ingo Molnar <[email protected]> wrote:
> >
> > * Stephane Eranian <[email protected]> wrote:
> >
> >> On Tue, Oct 15, 2013 at 3:12 PM, Ingo Molnar <[email protected]> wrote:
> >> >
> >> > * Stephane Eranian <[email protected]> wrote:
> >> >
> >> >> Arnaldo,
> >> >>
> >> >> Could you revert all the mmap2 patches in your perf/urgent tree for now?
> >> >> Thanks.
> >> >
> >> > I think it would be helpful if you re-sent the final patch, with a full
> >> > changelog, etc., in a separate thread, not inside an existing discussion
> >> > and such.
> >>
> >> Are you talking about the perf tool or kernel patch?
> >
> > Both.
> >
> >> I did not send any perf-related mmap2 revert patches.
> >
> > Would be nice to have a single patch that reverts both.
> >
> I will send a single patch to remove everything (kernel + tool) then.
Thanks!
Ingo