RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"

RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"

[Metzger, Markus T]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2289 bytes --]

>-----Original Message-----
>From: tip-bot for Ingo Molnar [mailto:mingo@elte.hu]
>Sent: Thursday, June 11, 2009 1:37 AM
>To: linux-tip-commits@vger.kernel.org
>Cc: hpa@zytor.com; mingo@redhat.com; peterz@infradead.org; Metzger, Markus T; oleg@redhat.com;
>tglx@linutronix.de; mingo@elte.hu
>Subject: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>
>Commit-ID:  511b01bdf64ad8a38414096eab283c7784aebfc4
>Gitweb:     http://git.kernel.org/tip/511b01bdf64ad8a38414096eab283c7784aebfc4
>Author:     Ingo Molnar <mingo@elte.hu>
>AuthorDate: Thu, 11 Jun 2009 00:32:00 +0200
>Committer:  Ingo Molnar <mingo@elte.hu>
>CommitDate: Thu, 11 Jun 2009 00:32:00 +0200
>
>Revert "x86, bts: reenable ptrace branch trace support"
>
>This reverts commit 7e0bfad24d85de7cf2202a7b0ce51de11a077b21.
>
>A late objection to the ABI has arrived:
>
>   http://lkml.org/lkml/2009/6/10/253

I thought that this has been resolved. See for example http://lkml.org/lkml/2009/6/10/257.

Peters concerns were that Debug Store details are exposed to user space, which is
not the case. Debug Store itself is fully in-kernel and the expectation of a 
user-defined buffer can be implemented on top of the Debug Store changes that
Peter expects are needed to support PEBS.

A user-defined trace buffer size is required to support different usage models.
Some users only need a small amount of trace, whereas others need a big amount.
The interface will have to reflect that in some way.


regards,
markus.

---------------------------------------------------------------------Intel GmbHDornacher Strasse 185622 Feldkirchen/Muenchen GermanySitz der Gesellschaft: Feldkirchen bei MuenchenGeschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes SchwadererRegistergericht: Muenchen HRB 47456 Ust.-IdNr.VAT Registration No.: DE129385895Citibank Frankfurt (BLZ 502 109 00) 600119052
This e-mail and any attachments may contain confidential material forthe sole use of the intended recipient(s). Any review or distributionby others is strictly prohibited. If you are not the intendedrecipient, please contact the sender and delete all copies.ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"

[Peter Zijlstra]

On Thu, 2009-06-11 at 07:30 +0100, Metzger, Markus T wrote:
> >-----Original Message-----
> >From: tip-bot for Ingo Molnar [mailto:mingo@elte.hu]
> >Sent: Thursday, June 11, 2009 1:37 AM
> >To: linux-tip-commits@vger.kernel.org
> >Cc: hpa@zytor.com; mingo@redhat.com; peterz@infradead.org; Metzger, Markus T; oleg@redhat.com;
> >tglx@linutronix.de; mingo@elte.hu
> >Subject: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
> >
> >Commit-ID:  511b01bdf64ad8a38414096eab283c7784aebfc4
> >Gitweb:     http://git.kernel.org/tip/511b01bdf64ad8a38414096eab283c7784aebfc4
> >Author:     Ingo Molnar <mingo@elte.hu>
> >AuthorDate: Thu, 11 Jun 2009 00:32:00 +0200
> >Committer:  Ingo Molnar <mingo@elte.hu>
> >CommitDate: Thu, 11 Jun 2009 00:32:00 +0200
> >
> >Revert "x86, bts: reenable ptrace branch trace support"
> >
> >This reverts commit 7e0bfad24d85de7cf2202a7b0ce51de11a077b21.
> >
> >A late objection to the ABI has arrived:
> >
> >   http://lkml.org/lkml/2009/6/10/253
> 
> I thought that this has been resolved. See for example http://lkml.org/lkml/2009/6/10/257.
> 
> Peters concerns were that Debug Store details are exposed to user space, which is
> not the case. Debug Store itself is fully in-kernel and the expectation of a 
> user-defined buffer can be implemented on top of the Debug Store changes that
> Peter expects are needed to support PEBS.
> 
> A user-defined trace buffer size is required to support different usage models.
> Some users only need a small amount of trace, whereas others need a big amount.
> The interface will have to reflect that in some way.

Right, your last email did explain how we could keep per task in-kernel
buffers and fill them from the DS and still have them of user-specified
size.

That would indeed keep the proposed ABI workable, what I'm still not
liking is that this buffer is in-kernel, but I guess that might be
something for other people to have an opinion on.

RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"

[Metzger, Markus T]

>-----Original Message-----
>From: Peter Zijlstra [mailto:peterz@infradead.org]
>Sent: Thursday, June 11, 2009 8:36 AM
>To: Metzger, Markus T
>Cc: linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com; oleg@redhat.com;
>tglx@linutronix.de; mingo@elte.hu; linux-tip-commits@vger.kernel.org
>Subject: RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>
>On Thu, 2009-06-11 at 07:30 +0100, Metzger, Markus T wrote:
>> >-----Original Message-----
>> >From: tip-bot for Ingo Molnar [mailto:mingo@elte.hu]
>> >Sent: Thursday, June 11, 2009 1:37 AM
>> >To: linux-tip-commits@vger.kernel.org
>> >Cc: hpa@zytor.com; mingo@redhat.com; peterz@infradead.org; Metzger, Markus T; oleg@redhat.com;
>> >tglx@linutronix.de; mingo@elte.hu
>> >Subject: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>> >
>> >Commit-ID:  511b01bdf64ad8a38414096eab283c7784aebfc4
>> >Gitweb:     http://git.kernel.org/tip/511b01bdf64ad8a38414096eab283c7784aebfc4
>> >Author:     Ingo Molnar <mingo@elte.hu>
>> >AuthorDate: Thu, 11 Jun 2009 00:32:00 +0200
>> >Committer:  Ingo Molnar <mingo@elte.hu>
>> >CommitDate: Thu, 11 Jun 2009 00:32:00 +0200
>> >
>> >Revert "x86, bts: reenable ptrace branch trace support"
>> >
>> >This reverts commit 7e0bfad24d85de7cf2202a7b0ce51de11a077b21.
>> >
>> >A late objection to the ABI has arrived:
>> >
>> >   http://lkml.org/lkml/2009/6/10/253
>>
>> I thought that this has been resolved. See for example http://lkml.org/lkml/2009/6/10/257.
>>
>> Peters concerns were that Debug Store details are exposed to user space, which is
>> not the case. Debug Store itself is fully in-kernel and the expectation of a
>> user-defined buffer can be implemented on top of the Debug Store changes that
>> Peter expects are needed to support PEBS.
>>
>> A user-defined trace buffer size is required to support different usage models.
>> Some users only need a small amount of trace, whereas others need a big amount.
>> The interface will have to reflect that in some way.
>
>Right, your last email did explain how we could keep per task in-kernel
>buffers and fill them from the DS and still have them of user-specified
>size.
>
>That would indeed keep the proposed ABI workable, what I'm still not
>liking is that this buffer is in-kernel, but I guess that might be
>something for other people to have an opinion on.

The alternative would be to give a user-allocated buffer to the tracing h/w.

We would need to take precautions to prevent the user from messing around
with that buffer while the h/w is writing to it. Other code uses the kernel-
allocated buffer plus copy_to_user() approach, as well.

Further, it would require the user to interpret the various tracing formats,
whereas the existing interface provides an architecture-independent format.


Does anybody have concerns on using an in-kernel buffer and providing a 
copy_to_user() interface?


regards,
markus.
---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"

[Peter Zijlstra]

On Thu, 2009-06-11 at 08:17 +0100, Metzger, Markus T wrote:
> >That would indeed keep the proposed ABI workable, what I'm still not
> >liking is that this buffer is in-kernel, but I guess that might be
> >something for other people to have an opinion on.
> 
> The alternative would be to give a user-allocated buffer to the tracing h/w.
> 
> We would need to take precautions to prevent the user from messing around
> with that buffer while the h/w is writing to it. Other code uses the kernel-
> allocated buffer plus copy_to_user() approach, as well.
> 
> Further, it would require the user to interpret the various tracing formats,
> whereas the existing interface provides an architecture-independent format.
> 
> 
> Does anybody have concerns on using an in-kernel buffer and providing a 
> copy_to_user() interface?

Ah, if you mmap() you can do without copy_to_user().

Either way, you have to make sure the buffer is mlocked() anyway, since
you're wanting to fill it from interrupt context.

The advantage (imo) from letting the user set it up is that you don't
need those separate allocation routines.

But yes, it would expose the data to the user, but one could keep it
opaque data, without requiring the user to be able to interpret it.

Of course a data format aligned with the interface capabilities would be
nicer ;-)

RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"

[Metzger, Markus T]

>-----Original Message-----
>From: Peter Zijlstra [mailto:peterz@infradead.org]
>Sent: Thursday, June 11, 2009 10:09 AM
>To: Metzger, Markus T
>Cc: linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com; oleg@redhat.com;
>tglx@linutronix.de; mingo@elte.hu; linux-tip-commits@vger.kernel.org
>Subject: RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>
>On Thu, 2009-06-11 at 08:17 +0100, Metzger, Markus T wrote:
>> >That would indeed keep the proposed ABI workable, what I'm still not
>> >liking is that this buffer is in-kernel, but I guess that might be
>> >something for other people to have an opinion on.
>>
>> The alternative would be to give a user-allocated buffer to the tracing h/w.
>>
>> We would need to take precautions to prevent the user from messing around
>> with that buffer while the h/w is writing to it. Other code uses the kernel-
>> allocated buffer plus copy_to_user() approach, as well.
>>
>> Further, it would require the user to interpret the various tracing formats,
>> whereas the existing interface provides an architecture-independent format.
>>
>>
>> Does anybody have concerns on using an in-kernel buffer and providing a
>> copy_to_user() interface?
>
>Ah, if you mmap() you can do without copy_to_user().

The user would still need to call the kernel to interpret the trace data,
which would then require copy_to_user() for the interpreted data.


>Either way, you have to make sure the buffer is mlocked() anyway, since
>you're wanting to fill it from interrupt context.
>
>The advantage (imo) from letting the user set it up is that you don't
>need those separate allocation routines.

I would still need to mlock() the buffer. Those separate allocation routines
do the accounting work of mlock() on an already non-pageable buffer.

If you don't like those separate allocation routines, we could extract the
accounting part from mlock() into separate functions and I could use those.
These changes would again be in-kernel without impacting the API.


>But yes, it would expose the data to the user, but one could keep it
>opaque data, without requiring the user to be able to interpret it.

See above, the user would then need to call the kernel to interpret the data.
We would achieve exactly the same thing in a more complicated way:
- there's a buffer which the user cannot use directly
- the buffer must be locked and must not go away
- the user needs to call the kernel to read trace records from the buffer

If we let the user setup the buffer, all we gain is some additional overhead.
The user needs to request pages from the kernel. Malloc needs to administrate
this memory. The user then gives the memory back to the kernel, which locks it
and gives it to the h/w.


regards,
markus.

---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, 2009-06-11 at 07:30 +0100, Metzger, Markus T wrote:
> > >-----Original Message-----
> > >From: tip-bot for Ingo Molnar [mailto:mingo@elte.hu]
> > >Sent: Thursday, June 11, 2009 1:37 AM
> > >To: linux-tip-commits@vger.kernel.org
> > >Cc: hpa@zytor.com; mingo@redhat.com; peterz@infradead.org; Metzger, Markus T; oleg@redhat.com;
> > >tglx@linutronix.de; mingo@elte.hu
> > >Subject: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
> > >
> > >Commit-ID:  511b01bdf64ad8a38414096eab283c7784aebfc4
> > >Gitweb:     http://git.kernel.org/tip/511b01bdf64ad8a38414096eab283c7784aebfc4
> > >Author:     Ingo Molnar <mingo@elte.hu>
> > >AuthorDate: Thu, 11 Jun 2009 00:32:00 +0200
> > >Committer:  Ingo Molnar <mingo@elte.hu>
> > >CommitDate: Thu, 11 Jun 2009 00:32:00 +0200
> > >
> > >Revert "x86, bts: reenable ptrace branch trace support"
> > >
> > >This reverts commit 7e0bfad24d85de7cf2202a7b0ce51de11a077b21.
> > >
> > >A late objection to the ABI has arrived:
> > >
> > >   http://lkml.org/lkml/2009/6/10/253
> > 
> > I thought that this has been resolved. See for example http://lkml.org/lkml/2009/6/10/257.
> > 
> > Peters concerns were that Debug Store details are exposed to user space, which is
> > not the case. Debug Store itself is fully in-kernel and the expectation of a 
> > user-defined buffer can be implemented on top of the Debug Store changes that
> > Peter expects are needed to support PEBS.
> > 
> > A user-defined trace buffer size is required to support 
> > different usage models. Some users only need a small amount of 
> > trace, whereas others need a big amount. The interface will have 
> > to reflect that in some way.
> 
> Right, your last email did explain how we could keep per task 
> in-kernel buffers and fill them from the DS and still have them of 
> user-specified size.
> 
> That would indeed keep the proposed ABI workable, what I'm still 
> not liking is that this buffer is in-kernel, but I guess that 
> might be something for other people to have an opinion on.

Hm. Wrt. the ABI, wouldnt it make more sense to expose this PMU 
feature via perfcounters: a sampling hw-branch-executions counter, 
with interval=1.

That would give the exact existing semantics, plus a lot lot more. 
Markus?

	Ingo

RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"

[Metzger, Markus T]

>-----Original Message-----
>From: Ingo Molnar [mailto:mingo@elte.hu]
>Sent: Thursday, June 11, 2009 12:22 PM
>To: Peter Zijlstra
>Cc: Metzger, Markus T; linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com; oleg@redhat.com;
>tglx@linutronix.de; linux-tip-commits@vger.kernel.org
>Subject: Re: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>
>
>* Peter Zijlstra <peterz@infradead.org> wrote:
>
>> On Thu, 2009-06-11 at 07:30 +0100, Metzger, Markus T wrote:
>> > >-----Original Message-----
>> > >From: tip-bot for Ingo Molnar [mailto:mingo@elte.hu]
>> > >Sent: Thursday, June 11, 2009 1:37 AM
>> > >To: linux-tip-commits@vger.kernel.org
>> > >Cc: hpa@zytor.com; mingo@redhat.com; peterz@infradead.org; Metzger, Markus T; oleg@redhat.com;
>> > >tglx@linutronix.de; mingo@elte.hu
>> > >Subject: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>> > >
>> > >Commit-ID:  511b01bdf64ad8a38414096eab283c7784aebfc4
>> > >Gitweb:     http://git.kernel.org/tip/511b01bdf64ad8a38414096eab283c7784aebfc4
>> > >Author:     Ingo Molnar <mingo@elte.hu>
>> > >AuthorDate: Thu, 11 Jun 2009 00:32:00 +0200
>> > >Committer:  Ingo Molnar <mingo@elte.hu>
>> > >CommitDate: Thu, 11 Jun 2009 00:32:00 +0200
>> > >
>> > >Revert "x86, bts: reenable ptrace branch trace support"
>> > >
>> > >This reverts commit 7e0bfad24d85de7cf2202a7b0ce51de11a077b21.
>> > >
>> > >A late objection to the ABI has arrived:
>> > >
>> > >   http://lkml.org/lkml/2009/6/10/253
>> >
>> > I thought that this has been resolved. See for example http://lkml.org/lkml/2009/6/10/257.
>> >
>> > Peters concerns were that Debug Store details are exposed to user space, which is
>> > not the case. Debug Store itself is fully in-kernel and the expectation of a
>> > user-defined buffer can be implemented on top of the Debug Store changes that
>> > Peter expects are needed to support PEBS.
>> >
>> > A user-defined trace buffer size is required to support
>> > different usage models. Some users only need a small amount of
>> > trace, whereas others need a big amount. The interface will have
>> > to reflect that in some way.
>>
>> Right, your last email did explain how we could keep per task
>> in-kernel buffers and fill them from the DS and still have them of
>> user-specified size.
>>
>> That would indeed keep the proposed ABI workable, what I'm still
>> not liking is that this buffer is in-kernel, but I guess that
>> might be something for other people to have an opinion on.
>
>Hm. Wrt. the ABI, wouldnt it make more sense to expose this PMU
>feature via perfcounters: a sampling hw-branch-executions counter,
>with interval=1.
>
>That would give the exact existing semantics, plus a lot lot more.
>Markus?

What more would we get?

I take it that you don't want to implement branch tracing via PEBS,
which would be possible but rather inefficient since the BTS format
is much more compact than the PEBS format.

So we would still implement it via BTS and we would still like to present
a branch trace specific format to the user.

Are you suggesting to use a common ABI for sampling and branch tracing?

The existing ABI is tailored towards the expected users: debuggers.
I do believe that a ptrace based interface makes a lot of sense for
this debugging-related feature, since debuggers already speak ptrace.

Branch tracing and sampling are used by different classes of user-mode
applications. I don't think that a common ABI would benefit user-mode.
Since we do need different implementations in the kernel, I don't see
how a common ABI would help here, either.


I rather see this as two independent, unrelated hardware features that
happen to use the same technique to allow arbitrary-sized buffers and
that therefore share some hardware real-estate.


regards,
markus.

---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"

[Ingo Molnar]

* Metzger, Markus T <markus.t.metzger@intel.com> wrote:

> >-----Original Message-----
> >From: Ingo Molnar [mailto:mingo@elte.hu]
> >Sent: Thursday, June 11, 2009 12:22 PM
> >To: Peter Zijlstra
> >Cc: Metzger, Markus T; linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com; oleg@redhat.com;
> >tglx@linutronix.de; linux-tip-commits@vger.kernel.org
> >Subject: Re: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
> >
> >
> >* Peter Zijlstra <peterz@infradead.org> wrote:
> >
> >> On Thu, 2009-06-11 at 07:30 +0100, Metzger, Markus T wrote:
> >> > >-----Original Message-----
> >> > >From: tip-bot for Ingo Molnar [mailto:mingo@elte.hu]
> >> > >Sent: Thursday, June 11, 2009 1:37 AM
> >> > >To: linux-tip-commits@vger.kernel.org
> >> > >Cc: hpa@zytor.com; mingo@redhat.com; peterz@infradead.org; Metzger, Markus T; oleg@redhat.com;
> >> > >tglx@linutronix.de; mingo@elte.hu
> >> > >Subject: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
> >> > >
> >> > >Commit-ID:  511b01bdf64ad8a38414096eab283c7784aebfc4
> >> > >Gitweb:     http://git.kernel.org/tip/511b01bdf64ad8a38414096eab283c7784aebfc4
> >> > >Author:     Ingo Molnar <mingo@elte.hu>
> >> > >AuthorDate: Thu, 11 Jun 2009 00:32:00 +0200
> >> > >Committer:  Ingo Molnar <mingo@elte.hu>
> >> > >CommitDate: Thu, 11 Jun 2009 00:32:00 +0200
> >> > >
> >> > >Revert "x86, bts: reenable ptrace branch trace support"
> >> > >
> >> > >This reverts commit 7e0bfad24d85de7cf2202a7b0ce51de11a077b21.
> >> > >
> >> > >A late objection to the ABI has arrived:
> >> > >
> >> > >   http://lkml.org/lkml/2009/6/10/253
> >> >
> >> > I thought that this has been resolved. See for example http://lkml.org/lkml/2009/6/10/257.
> >> >
> >> > Peters concerns were that Debug Store details are exposed to user space, which is
> >> > not the case. Debug Store itself is fully in-kernel and the expectation of a
> >> > user-defined buffer can be implemented on top of the Debug Store changes that
> >> > Peter expects are needed to support PEBS.
> >> >
> >> > A user-defined trace buffer size is required to support
> >> > different usage models. Some users only need a small amount of
> >> > trace, whereas others need a big amount. The interface will have
> >> > to reflect that in some way.
> >>
> >> Right, your last email did explain how we could keep per task
> >> in-kernel buffers and fill them from the DS and still have them of
> >> user-specified size.
> >>
> >> That would indeed keep the proposed ABI workable, what I'm still
> >> not liking is that this buffer is in-kernel, but I guess that
> >> might be something for other people to have an opinion on.
> >
> > Hm. Wrt. the ABI, wouldnt it make more sense to expose this PMU 
> > feature via perfcounters: a sampling hw-branch-executions 
> > counter, with interval=1.
> >
> > That would give the exact existing semantics, plus a lot lot 
> > more. Markus?
> 
> What more would we get?

There's numerous direct functionality advantages:

 - We will get all the sampling features of perfcounters such as 
   timed samples, CPU ID samples. Some will be approximate (timing), 
   some precise (CPU ID).

 - We will get the advanced workflow isolation features: we could
   sample on a per CPU basis (system-wide BTS), and we could sample
   child tasks automatically. The current code is limited to a
   single task.

 - We will sample other types of information into the same outgoing 
   event buffer: for example branch-miss events, intermixed with BTS
   records. This could help not just the narrow purpose of 
   debugging, but also the purpose of performance analysis.

 - There's a rich and fast/efficient VFS based APIs to wait for
   event overflows: poll(), read(), mmap().

 - Remote sampling via perfcounters is transparent, while ptrace 
   sampling can be seen by apps.

There's maintenance advantages as well from the x86 architecture and 
scheduler maintenance point of view:

 - We would have a single facility handling the Debug Store, and
   we'd have almost all pieces in place for PEBS support in
   perfcounters as well so there's good synergy.

There are performance advantages as well:

 - There's lazy-switching optimizations in perfcounters avoiding the
   DS buffer switching overhead.

> I take it that you don't want to implement branch tracing via 
> PEBS, which would be possible but rather inefficient since the BTS 
> format is much more compact than the PEBS format.

Sampling could be done via PEBS too, if someone wants to take 
advantage of the instruction latency field for example on Nehalem.

But yes, i agree that for the simple case of 
branch-executions+period=1 case we want that to use BTS, as those 
records are a lot more compact than the all-general-purpose-regs 
bloated records of PEBS.

> So we would still implement it via BTS and we would still like to 
> present a branch trace specific format to the user.
> 
> Are you suggesting to use a common ABI for sampling and branch 
> tracing?

Yes, that makes sense.

> The existing ABI is tailored towards the expected users: 
> debuggers. I do believe that a ptrace based interface makes a lot 
> of sense for this debugging-related feature, since debuggers 
> already speak ptrace.
> 
> Branch tracing and sampling are used by different classes of 
> user-mode applications. I don't think that a common ABI would 
> benefit user-mode. Since we do need different implementations in 
> the kernel, I don't see how a common ABI would help here, either.
> 
> I rather see this as two independent, unrelated hardware features 
> that happen to use the same technique to allow arbitrary-sized 
> buffers and that therefore share some hardware real-estate.

I'd rather not maintain two separate pieces of infrastructure and 
ABIs.

We had a _lot_ of problems with the BTS code, and there's still that 
unresolved crash from akpm. (i too reported crashes in the past) 

That is the problem with such rarely used ABIs: almost nobody tests 
them.

With perfcounters that dynamics changes quite profoundly: the DS and 
the overflow handling will be used for PEBS anyway, so there's good 
overlap and good sharing in facilities.

	Ingo

RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"

[Metzger, Markus T]

>-----Original Message-----
>From: Ingo Molnar [mailto:mingo@elte.hu]
>Sent: Thursday, June 11, 2009 11:41 PM
>To: Metzger, Markus T
>Cc: Peter Zijlstra; linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com; oleg@redhat.com;
>tglx@linutronix.de; linux-tip-commits@vger.kernel.org
>Subject: Re: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>
>
>* Metzger, Markus T <markus.t.metzger@intel.com> wrote:
>
>> >-----Original Message-----
>> >From: Ingo Molnar [mailto:mingo@elte.hu]
>> >Sent: Thursday, June 11, 2009 12:22 PM
>> >To: Peter Zijlstra
>> >Cc: Metzger, Markus T; linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
>oleg@redhat.com;
>> >tglx@linutronix.de; linux-tip-commits@vger.kernel.org
>> >Subject: Re: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>> >
>> >
>> >* Peter Zijlstra <peterz@infradead.org> wrote:
>> >
>> >> On Thu, 2009-06-11 at 07:30 +0100, Metzger, Markus T wrote:
>> >> > >-----Original Message-----
>> >> > >From: tip-bot for Ingo Molnar [mailto:mingo@elte.hu]
>> >> > >Sent: Thursday, June 11, 2009 1:37 AM
>> >> > >To: linux-tip-commits@vger.kernel.org
>> >> > >Cc: hpa@zytor.com; mingo@redhat.com; peterz@infradead.org; Metzger, Markus T; oleg@redhat.com;
>> >> > >tglx@linutronix.de; mingo@elte.hu
>> >> > >Subject: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>> >> > >
>> >> > >Commit-ID:  511b01bdf64ad8a38414096eab283c7784aebfc4
>> >> > >Gitweb:     http://git.kernel.org/tip/511b01bdf64ad8a38414096eab283c7784aebfc4
>> >> > >Author:     Ingo Molnar <mingo@elte.hu>
>> >> > >AuthorDate: Thu, 11 Jun 2009 00:32:00 +0200
>> >> > >Committer:  Ingo Molnar <mingo@elte.hu>
>> >> > >CommitDate: Thu, 11 Jun 2009 00:32:00 +0200
>> >> > >
>> >> > >Revert "x86, bts: reenable ptrace branch trace support"
>> >> > >
>> >> > >This reverts commit 7e0bfad24d85de7cf2202a7b0ce51de11a077b21.
>> >> > >
>> >> > >A late objection to the ABI has arrived:
>> >> > >
>> >> > >   http://lkml.org/lkml/2009/6/10/253
>> >> >
>> >> > I thought that this has been resolved. See for example http://lkml.org/lkml/2009/6/10/257.
>> >> >
>> >> > Peters concerns were that Debug Store details are exposed to user space, which is
>> >> > not the case. Debug Store itself is fully in-kernel and the expectation of a
>> >> > user-defined buffer can be implemented on top of the Debug Store changes that
>> >> > Peter expects are needed to support PEBS.
>> >> >
>> >> > A user-defined trace buffer size is required to support
>> >> > different usage models. Some users only need a small amount of
>> >> > trace, whereas others need a big amount. The interface will have
>> >> > to reflect that in some way.
>> >>
>> >> Right, your last email did explain how we could keep per task
>> >> in-kernel buffers and fill them from the DS and still have them of
>> >> user-specified size.
>> >>
>> >> That would indeed keep the proposed ABI workable, what I'm still
>> >> not liking is that this buffer is in-kernel, but I guess that
>> >> might be something for other people to have an opinion on.
>> >
>> > Hm. Wrt. the ABI, wouldnt it make more sense to expose this PMU
>> > feature via perfcounters: a sampling hw-branch-executions
>> > counter, with interval=1.
>> >
>> > That would give the exact existing semantics, plus a lot lot
>> > more. Markus?
>>
>> What more would we get?
>
>There's numerous direct functionality advantages:
>
> - We will get all the sampling features of perfcounters such as
>   timed samples, CPU ID samples. Some will be approximate (timing),
>   some precise (CPU ID).
>
> - We will get the advanced workflow isolation features: we could
>   sample on a per CPU basis (system-wide BTS), and we could sample
>   child tasks automatically. The current code is limited to a
>   single task.
>
> - We will sample other types of information into the same outgoing
>   event buffer: for example branch-miss events, intermixed with BTS
>   records. This could help not just the narrow purpose of
>   debugging, but also the purpose of performance analysis.
>
> - There's a rich and fast/efficient VFS based APIs to wait for
>   event overflows: poll(), read(), mmap().
>
> - Remote sampling via perfcounters is transparent, while ptrace
>   sampling can be seen by apps.
>
>There's maintenance advantages as well from the x86 architecture and
>scheduler maintenance point of view:
>
> - We would have a single facility handling the Debug Store, and
>   we'd have almost all pieces in place for PEBS support in
>   perfcounters as well so there's good synergy.

I'm not exactly sure what you are proposing.

There currently is a single facility handling Debug Store which is used
by perfmon for PEBS sampling and by BTS for branch tracing. The ptrace
interface is a thin layer that allows debuggers to easily access the
part that is relevant for them.


Are you proposing a single abstraction of sampling and branch tracing events
using records of different sizes in one big stream?

regards,
markus.

---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"

[Metzger, Markus T]

>-----Original Message-----
>From: Metzger, Markus T
>Sent: Friday, June 12, 2009 1:05 PM
>To: Ingo Molnar
>Cc: Peter Zijlstra; linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com; oleg@redhat.com;
>tglx@linutronix.de; linux-tip-commits@vger.kernel.org
>Subject: RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>
>>-----Original Message-----
>>From: Ingo Molnar [mailto:mingo@elte.hu]
>>Sent: Thursday, June 11, 2009 11:41 PM
>>To: Metzger, Markus T
>>Cc: Peter Zijlstra; linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com; oleg@redhat.com;
>>tglx@linutronix.de; linux-tip-commits@vger.kernel.org
>>Subject: Re: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>>
>>
>>* Metzger, Markus T <markus.t.metzger@intel.com> wrote:
>>
>>> >-----Original Message-----
>>> >From: Ingo Molnar [mailto:mingo@elte.hu]
>>> >Sent: Thursday, June 11, 2009 12:22 PM
>>> >To: Peter Zijlstra
>>> >Cc: Metzger, Markus T; linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
>>oleg@redhat.com;
>>> >tglx@linutronix.de; linux-tip-commits@vger.kernel.org
>>> >Subject: Re: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>>> >
>>> >
>>> >* Peter Zijlstra <peterz@infradead.org> wrote:
>>> >
>>> >> On Thu, 2009-06-11 at 07:30 +0100, Metzger, Markus T wrote:
>>> >> > >-----Original Message-----
>>> >> > >From: tip-bot for Ingo Molnar [mailto:mingo@elte.hu]
>>> >> > >Sent: Thursday, June 11, 2009 1:37 AM
>>> >> > >To: linux-tip-commits@vger.kernel.org
>>> >> > >Cc: hpa@zytor.com; mingo@redhat.com; peterz@infradead.org; Metzger, Markus T;
>oleg@redhat.com;
>>> >> > >tglx@linutronix.de; mingo@elte.hu
>>> >> > >Subject: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>>> >> > >
>>> >> > >Commit-ID:  511b01bdf64ad8a38414096eab283c7784aebfc4
>>> >> > >Gitweb:     http://git.kernel.org/tip/511b01bdf64ad8a38414096eab283c7784aebfc4
>>> >> > >Author:     Ingo Molnar <mingo@elte.hu>
>>> >> > >AuthorDate: Thu, 11 Jun 2009 00:32:00 +0200
>>> >> > >Committer:  Ingo Molnar <mingo@elte.hu>
>>> >> > >CommitDate: Thu, 11 Jun 2009 00:32:00 +0200
>>> >> > >
>>> >> > >Revert "x86, bts: reenable ptrace branch trace support"
>>> >> > >
>>> >> > >This reverts commit 7e0bfad24d85de7cf2202a7b0ce51de11a077b21.
>>> >> > >
>>> >> > >A late objection to the ABI has arrived:
>>> >> > >
>>> >> > >   http://lkml.org/lkml/2009/6/10/253
>>> >> >
>>> >> > I thought that this has been resolved. See for example http://lkml.org/lkml/2009/6/10/257.
>>> >> >
>>> >> > Peters concerns were that Debug Store details are exposed to user space, which is
>>> >> > not the case. Debug Store itself is fully in-kernel and the expectation of a
>>> >> > user-defined buffer can be implemented on top of the Debug Store changes that
>>> >> > Peter expects are needed to support PEBS.
>>> >> >
>>> >> > A user-defined trace buffer size is required to support
>>> >> > different usage models. Some users only need a small amount of
>>> >> > trace, whereas others need a big amount. The interface will have
>>> >> > to reflect that in some way.
>>> >>
>>> >> Right, your last email did explain how we could keep per task
>>> >> in-kernel buffers and fill them from the DS and still have them of
>>> >> user-specified size.
>>> >>
>>> >> That would indeed keep the proposed ABI workable, what I'm still
>>> >> not liking is that this buffer is in-kernel, but I guess that
>>> >> might be something for other people to have an opinion on.
>>> >
>>> > Hm. Wrt. the ABI, wouldnt it make more sense to expose this PMU
>>> > feature via perfcounters: a sampling hw-branch-executions
>>> > counter, with interval=1.
>>> >
>>> > That would give the exact existing semantics, plus a lot lot
>>> > more. Markus?
>>>
>>> What more would we get?
>>
>>There's numerous direct functionality advantages:
>>
>> - We will get all the sampling features of perfcounters such as
>>   timed samples, CPU ID samples. Some will be approximate (timing),
>>   some precise (CPU ID).
>>
>> - We will get the advanced workflow isolation features: we could
>>   sample on a per CPU basis (system-wide BTS), and we could sample
>>   child tasks automatically. The current code is limited to a
>>   single task.
>>
>> - We will sample other types of information into the same outgoing
>>   event buffer: for example branch-miss events, intermixed with BTS
>>   records. This could help not just the narrow purpose of
>>   debugging, but also the purpose of performance analysis.
>>
>> - There's a rich and fast/efficient VFS based APIs to wait for
>>   event overflows: poll(), read(), mmap().
>>
>> - Remote sampling via perfcounters is transparent, while ptrace
>>   sampling can be seen by apps.
>>
>>There's maintenance advantages as well from the x86 architecture and
>>scheduler maintenance point of view:
>>
>> - We would have a single facility handling the Debug Store, and
>>   we'd have almost all pieces in place for PEBS support in
>>   perfcounters as well so there's good synergy.
>
>I'm not exactly sure what you are proposing.
>
>There currently is a single facility handling Debug Store which is used
>by perfmon for PEBS sampling and by BTS for branch tracing. The ptrace
>interface is a thin layer that allows debuggers to easily access the
>part that is relevant for them.
>
>
>Are you proposing a single abstraction of sampling and branch tracing events
>using records of different sizes in one big stream?

Ingo,

I did not hear from you.
Could you be a bit more specific about what you have in mind regarding this?

regards,
markus.
---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"

[Metzger, Markus T]

Ingo,

I did not hear from you.

If you have further concerns regarding the ABI, would you please continue the discussion.
If you don't have further concerns, would you please reenable the feature.

thanks and regards,
markus.


>-----Original Message-----
>From: Metzger, Markus T
>Sent: Thursday, June 18, 2009 12:24 PM
>To: Ingo Molnar
>Cc: Peter Zijlstra; linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com; oleg@redhat.com;
>tglx@linutronix.de; linux-tip-commits@vger.kernel.org
>Subject: RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>
>>-----Original Message-----
>>From: Metzger, Markus T
>>Sent: Friday, June 12, 2009 1:05 PM
>>To: Ingo Molnar
>>Cc: Peter Zijlstra; linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com; oleg@redhat.com;
>>tglx@linutronix.de; linux-tip-commits@vger.kernel.org
>>Subject: RE: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>>
>>>-----Original Message-----
>>>From: Ingo Molnar [mailto:mingo@elte.hu]
>>>Sent: Thursday, June 11, 2009 11:41 PM
>>>To: Metzger, Markus T
>>>Cc: Peter Zijlstra; linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com; oleg@redhat.com;
>>>tglx@linutronix.de; linux-tip-commits@vger.kernel.org
>>>Subject: Re: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>>>
>>>
>>>* Metzger, Markus T <markus.t.metzger@intel.com> wrote:
>>>
>>>> >-----Original Message-----
>>>> >From: Ingo Molnar [mailto:mingo@elte.hu]
>>>> >Sent: Thursday, June 11, 2009 12:22 PM
>>>> >To: Peter Zijlstra
>>>> >Cc: Metzger, Markus T; linux-kernel@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
>>>oleg@redhat.com;
>>>> >tglx@linutronix.de; linux-tip-commits@vger.kernel.org
>>>> >Subject: Re: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>>>> >
>>>> >
>>>> >* Peter Zijlstra <peterz@infradead.org> wrote:
>>>> >
>>>> >> On Thu, 2009-06-11 at 07:30 +0100, Metzger, Markus T wrote:
>>>> >> > >-----Original Message-----
>>>> >> > >From: tip-bot for Ingo Molnar [mailto:mingo@elte.hu]
>>>> >> > >Sent: Thursday, June 11, 2009 1:37 AM
>>>> >> > >To: linux-tip-commits@vger.kernel.org
>>>> >> > >Cc: hpa@zytor.com; mingo@redhat.com; peterz@infradead.org; Metzger, Markus T;
>>oleg@redhat.com;
>>>> >> > >tglx@linutronix.de; mingo@elte.hu
>>>> >> > >Subject: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"
>>>> >> > >
>>>> >> > >Commit-ID:  511b01bdf64ad8a38414096eab283c7784aebfc4
>>>> >> > >Gitweb:     http://git.kernel.org/tip/511b01bdf64ad8a38414096eab283c7784aebfc4
>>>> >> > >Author:     Ingo Molnar <mingo@elte.hu>
>>>> >> > >AuthorDate: Thu, 11 Jun 2009 00:32:00 +0200
>>>> >> > >Committer:  Ingo Molnar <mingo@elte.hu>
>>>> >> > >CommitDate: Thu, 11 Jun 2009 00:32:00 +0200
>>>> >> > >
>>>> >> > >Revert "x86, bts: reenable ptrace branch trace support"
>>>> >> > >
>>>> >> > >This reverts commit 7e0bfad24d85de7cf2202a7b0ce51de11a077b21.
>>>> >> > >
>>>> >> > >A late objection to the ABI has arrived:
>>>> >> > >
>>>> >> > >   http://lkml.org/lkml/2009/6/10/253
>>>> >> >
>>>> >> > I thought that this has been resolved. See for example http://lkml.org/lkml/2009/6/10/257.
>>>> >> >
>>>> >> > Peters concerns were that Debug Store details are exposed to user space, which is
>>>> >> > not the case. Debug Store itself is fully in-kernel and the expectation of a
>>>> >> > user-defined buffer can be implemented on top of the Debug Store changes that
>>>> >> > Peter expects are needed to support PEBS.
>>>> >> >
>>>> >> > A user-defined trace buffer size is required to support
>>>> >> > different usage models. Some users only need a small amount of
>>>> >> > trace, whereas others need a big amount. The interface will have
>>>> >> > to reflect that in some way.
>>>> >>
>>>> >> Right, your last email did explain how we could keep per task
>>>> >> in-kernel buffers and fill them from the DS and still have them of
>>>> >> user-specified size.
>>>> >>
>>>> >> That would indeed keep the proposed ABI workable, what I'm still
>>>> >> not liking is that this buffer is in-kernel, but I guess that
>>>> >> might be something for other people to have an opinion on.
>>>> >
>>>> > Hm. Wrt. the ABI, wouldnt it make more sense to expose this PMU
>>>> > feature via perfcounters: a sampling hw-branch-executions
>>>> > counter, with interval=1.
>>>> >
>>>> > That would give the exact existing semantics, plus a lot lot
>>>> > more. Markus?
>>>>
>>>> What more would we get?
>>>
>>>There's numerous direct functionality advantages:
>>>
>>> - We will get all the sampling features of perfcounters such as
>>>   timed samples, CPU ID samples. Some will be approximate (timing),
>>>   some precise (CPU ID).
>>>
>>> - We will get the advanced workflow isolation features: we could
>>>   sample on a per CPU basis (system-wide BTS), and we could sample
>>>   child tasks automatically. The current code is limited to a
>>>   single task.
>>>
>>> - We will sample other types of information into the same outgoing
>>>   event buffer: for example branch-miss events, intermixed with BTS
>>>   records. This could help not just the narrow purpose of
>>>   debugging, but also the purpose of performance analysis.
>>>
>>> - There's a rich and fast/efficient VFS based APIs to wait for
>>>   event overflows: poll(), read(), mmap().
>>>
>>> - Remote sampling via perfcounters is transparent, while ptrace
>>>   sampling can be seen by apps.
>>>
>>>There's maintenance advantages as well from the x86 architecture and
>>>scheduler maintenance point of view:
>>>
>>> - We would have a single facility handling the Debug Store, and
>>>   we'd have almost all pieces in place for PEBS support in
>>>   perfcounters as well so there's good synergy.
>>
>>I'm not exactly sure what you are proposing.
>>
>>There currently is a single facility handling Debug Store which is used
>>by perfmon for PEBS sampling and by BTS for branch tracing. The ptrace
>>interface is a thin layer that allows debuggers to easily access the
>>part that is relevant for them.
>>
>>
>>Are you proposing a single abstraction of sampling and branch tracing events
>>using records of different sizes in one big stream?
>
>Ingo,
>
>I did not hear from you.
>Could you be a bit more specific about what you have in mind regarding this?
>
>regards,
>markus.
---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

bts & perf_counters

[Metzger, Markus T]

>-----Original Message-----
>From: Ingo Molnar [mailto:mingo@elte.hu]
>Sent: Monday, June 29, 2009 10:20 PM
>To: Metzger, Markus T
>Cc: Thomas Gleixner; H. Peter Anvin; Peter Zijlstra; Markus Metzger
>Subject: Re: [tip:tracing/core] Revert "x86, bts: reenable ptrace branch trace support"


>> Would you want to re-add klml when we're discussing technical
>> things?
>
>Sure - feel free to start new threads with lkml Cc:-ed.

[cc-ing lkml]


>> I looked at the user-mode code and a little bit at the kernel
>> code. I'm not yet finished but I already have a handful of
>> questions.
>>
>> In general, I think that it might indeed make sense to use the
>> perfcounter interface for branch tracing if we can clarify a few
>> open points.
>
>Note, i consider the BTS (and PEBS) use-case a primary future
>component of perfcounters, so 'clarify' can (and should) mean the
>changing of perfcounters to suit the BTS model better, when needed.

That sounds good. I hope it is not necessary in too many places.


>> Both perf-record and perf-top try to read the data while the
>> profiled process is running. They start at offset zero; if they
>> detect they're falling behind too much, they drop samples. This
>> requires that tracer and tracee run tightly interleaved or truly
>> in parallel - at least when the sampling intervals get smaller.
>
>The alternative is to use a larger buffer when needed. The default
>limit is 512K buffer per unprivileged user. Privileged or power
>users can have an mlock limit of a few megs easily.
>
>> What if they're behind from the very beginning, say, after the
>> first buffer overflow? Would they be able to detect that they're
>> reading nonsense data? Would it be possible to stop the traced
>> process to guarantee a full trace?
>
>The latest perfcounters code (already upstream as of .31-rc1)
>implements a 'precise' and 'reliable' eventing flow: the kernel
>never overwrites records but stops and increases a 'lost events'
>counter. User-space should never get 'nonsense' data and will always
>be able to get a measure of how good the data is.
>
>We could add a new attribute to 'struct perf_counter_attr' that puts
>the kernel into 'tail overwrite' mode.

That would improve things for profilers but it would not help branch
tracers, e.g. debuggers.

A debugger needs the tail of the trace. It needs it to be precise,
i.e. without holes. And the debugger needs to be able to reliably
identify the beginning of that tail in the event stream.


>> A debugger is interested in the tail of the execution trace. It
>> won't poll the trace data (which would be far too much overhead).
>> How would a user synchronize on the profile stream when the
>> profiled process is stopped?
>
>Yeah, with a new perf_attr flag that activates overwrite this
>usecase would be solved, right? The debugger has to make sure the
>task is stopped before reading out the buffer, but that's pretty
>much all.

I'm not sure about that. The way I read struct perf_counter_mmap_page,
data_head points to the end of the stream (I would guess one byte
beyond the last record).

I think we can ignore data_tail in the debug scenario since debuggers
won't poll. We can further assume a buffer overflow no matter how big
the ring buffer - branch trace grows terribly fast and we don't want
normal uses to lock megabytes of memory, do we?

How would a debugger find the beginning of the event stream to start
reading?


>The profile stream will be serialized by ptrace if ptrace is used to
>stop the task.

Are you saying that we need ptrace to make sure the profile stream is
completely filled before notifying the tracer?


>> The mmap buffer that is used by perf is rather big, i.e. 128
>> pages. Don't you need to lock that buffer? The mlock rlimit is 64
>> pages.
>
>perfcounters has a per user allowance beyond the mlock limit. This
>is one of the reasons why i think it's better for BTS to use this
>facility: you get 512K of 'free' locked space, and that is a pretty
>practical amount of buffering/execution-history already.


Hmmm, couldn't that allowance be implemented anywhere in the kernel?
In fact, should this allowance not be per-user instead of per-feature?

I do agree that 64k (not pages, sorry, my mistake) is not enough for
branch-tracing (or profiling) multi-threaded apps.


>> The way I understand tools/perf/design.txt, you collect the
>> profile for child tasks into the same ring buffer; you just add
>> the task's pid as part of the sampling data. Is this correct?
>>
>> That won't work very well for branch tracing. Since we're sampling
>> every single branch, the buffer grows very fast. The branch trace
>> of a single thread will likely evict the trace for all other
>> threads. A debugger, on the other hand, would need an equally long
>> tail for each thread.
>
>A debugger knows all the child tasks and can create counters in each
>of them. (or in any desired sub-set of tasks - according to debugger
>settings/policy)
>
>The 'all child task events go into the parent buffer' portion is
>only true if you use inherited counter (perf_attr.inherit == 1). For
>normal PID counters you can mmap a separate buffer for each task.


We would lose one of the big improvements that comes with using the
perf_counter framework, though.


>> We will face the same problem if we want to combine branch tracing
>> with profiling, or - and you should have this problem already
>> today - if we want to sample different data using significantly
>> different sampling intervals. If we use a single buffer to hold
>> the combined data, the data with the smallest sampling interval
>> will evict all other data from that buffer.
>
>Could you clarify this use-case a bit more please? The basic BTS
>model is to use BTS as a 'flight recorder', to be read out when the
>debugger wants it. That model has no notion of interval.
>
>How does 'interval' get mixed with BTS?


We could view BTS as event-based sampling with interval=1. The sample
we collect is the <from, to> address pair of an executed branch and
the sampling interval is 1, i.e. we store a sample for every branch.
Wouldn't this be how BTS integrates into perf_counters?


One of the big advantages that comes with using the perf_counter
framework is that you could mix branch tracing with other forms
of profiling and sampling.


>> Would it be possible for a user to profile the same task twice? He
>> could then use different buffers for different sampling intervals.
>
>It's possibe to open multiple counters to the same task, yes.


That's good. And users could mmap every counter they open in order
to get multiple perf event streams?


>> I looked at the kernel code under the aspect how we could
>> integrate branch tracing.
>>
>> For one, we would need a separate locked buffer to collect BTS
>> (and later PEBS) and we would need to copy the data when either
>> the buffer overflows or the traced task is scheduled out. Do you
>> have an idea how big this copy overhead would be? At least for
>> BTS, users would likely never look at most of the trace.
>
>The best model for BTS is to 'drain' all BTS entries in the DS area
>when the counter overflows or when the task schedules out.
>
>To 'drain' means shuffling the entries from the per CPU DS area into
>the per counter mmap area. This will be very fast on any reasonably
>modern CPU - the DS area is already in the cache and the mmap
>bufferng area will be write-allocated.


OK. The existing implementation reconfigured DS area to have the h/w
already collect the trace into the correct buffer. The only copying
that is ever needed is to copy it into user-space while translating
the arch-specific format into an arch-independent format.

This is obviously only possible for a single user. Copying the data
is definitely more flexible if we expect multiple users of that data
with different-sized buffers.


>> We would need to extend the format to add a BTS record and we
>> would need to change generic code to not expect a counter value.
>> While we could omit the from address and only store the to address
>> using existing functionality, user code would not have a way to
>> re-sync if they do not understand an instruction, so I'd rather
>> keep the <from, to> pair.
>
>How about reusing these two existing sampling formats:
>
> PERF_SAMPLE_IP
> PERF_SAMPLE_ADDR
>
>For example for page-faul events, 'IP' is the faulting RIP, 'ADDR'
>is the faulting linear address.
>
>For BTS, 'IP' would the 'from' RIP, 'ADDR' could be the 'to' RIP.
>
>I.e. if you set the counter to:
>
>	attr->sample_type |= PERF_SAMPLE_IP;
>	attr->sample_type |= PERF_SAMPLE_DATA
>
>You'd get both the from-RIP and the to-RIP, without any BTS specific
>format extensions. Existing tooling uses the IP samples so we could
>get a branch profile via 'perf report' out of box.


That sounds good.


>> Sampling does not need to write the counters when the profiled
>> task schedules out, but I guess that those hooks can be added
>> easily. We might need to schedule_work() the stuff, though, so a
>> ptracer might see changes to the tracee's buffer even if the
>> tracee is currently stopped. I'm not sure how much of a problem
>> this could be, since debuggers read the trace from back to front.
>
>If a task schedules out then it will have its DS area drained
>already to the mmap buffer - i.e. it's all properly synchronized.


When is that draining done? Somewhere in schedule()? Wouldn't that
be quite expensive for a few pages of BTS buffer?


>> That's it, for now. I think it makes sense to continue the
>> discussion some more before I actually start with any code
>> changes.
>
>I'm quite sure about the model - i'm more sure about it than about
>any of the ptrace based BTS variants before ... so i'd suggest you
>try to send me a 'minimally working' prototype ASAP (within a few
>days), so that we can productize this ASAP. If you send me something
>i will try to tidy it up for upstream reactivation of the BTS code.

Hmmm, I'll see what I can do. Please don't expect a minimally working
prototype to be bug-free from the beginning.

I see identifying the beginning of the stream as well as random
accesses into the stream as bigger open points.

Maybe we could add a mode where records are zero-extended to a fixed size.
This would leave the choice to the user: compact format or random access.


thanks and regards,
markus.

---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: bts & perf_counters

[Ingo Molnar]

* Metzger, Markus T <markus.t.metzger@intel.com> wrote:

> > How does 'interval' get mixed with BTS?
> 
> We could view BTS as event-based sampling with interval=1. The 
> sample we collect is the <from, to> address pair of an executed 
> branch and the sampling interval is 1, i.e. we store a sample for 
> every branch. Wouldn't this be how BTS integrates into 
> perf_counters?

Yeah, this is how i view it too.

> One of the big advantages that comes with using the perf_counter 
> framework is that you could mix branch tracing with other forms of 
> profiling and sampling.

Correct.

> >> Would it be possible for a user to profile the same task twice? 
> >> He could then use different buffers for different sampling 
> >> intervals.
> >
> > It's possibe to open multiple counters to the same task, yes.
> 
> That's good. And users could mmap every counter they open in order 
> to get multiple perf event streams?

Yes.

> OK. The existing implementation reconfigured DS area to have the 
> h/w already collect the trace into the correct buffer. The only 
> copying that is ever needed is to copy it into user-space while 
> translating the arch-specific format into an arch-independent 
> format.
> 
> This is obviously only possible for a single user. Copying the 
> data is definitely more flexible if we expect multiple users of 
> that data with different-sized buffers.

Yeah. [ That decoupling is nice as it also allows multiplexing - 
there's nothing that prevents from two independent monitor tasks 
from sampling the same task. (beyond the inevitable runtime overhead 
that is inherent in BTS anyway.) ]

> > If a task schedules out then it will have its DS area drained 
> > already to the mmap buffer - i.e. it's all properly 
> > synchronized.
> 
> When is that draining done? Somewhere in schedule()? Wouldn't that 
> be quite expensive for a few pages of BTS buffer?

Well, it is an open question how frequently we want to move 
information from the DS area into the mmap pages.

The most direct approach would be to 'flush' the DS from two places: 
the threshold IRQ handler plus from the context switch code if the 
BTS counter gets deactivated. In the latter case BTS activities have 
to stop anyway, so the DS can be flushed to the mmap pages.

Or is your mental model for getting the BTS records from the DS to 
the mmap pages significantly different?

I think we should shoot for the simplest approach initially - we can 
do other, more sophisticated streaming modes later as well - they 
will not differ in functionality, only in performance.

> Hmmm, I'll see what I can do. Please don't expect a minimally 
> working prototype to be bug-free from the beginning.

Sure, i dont.

> I see identifying the beginning of the stream as well as random 
> accesses into the stream as bigger open points.
> 
> Maybe we could add a mode where records are zero-extended to a 
> fixed size. This would leave the choice to the user: compact 
> format or random access.

I agree that streaming is a problem because the debugger does not 
want to poll() really - such an output mode and a 'ignore data_tail 
and overwrite old entries' ring-buffer modus operandi should be 
added.

The latter would be useful for tracepoints too for example, so such 
a 'flight recorder' or 'history buffer' mode is not limited to BTS.

So feel free to add something that meets your constant-size records 
needs - and we'll make sure it fits well into the rest of 
perfcounters.

So based on your suggestion we'd have two streaming models:

 - 'no information loss' output model where user-space poll()s and 
   tries hard not to lose events (this is what profilers and 
   reliable tracers do)

 - 'history ring-buffer' model - this is useful for debuggers and is 
   useful for certain modes of tracing as well. (crash-tracing for 
   example)

	Ingo

Re: bts & perf_counters

[Peter Zijlstra]

On Tue, 2009-06-30 at 08:32 +0100, Metzger, Markus T wrote:
> 
> >> A debugger is interested in the tail of the execution trace. It
> >> won't poll the trace data (which would be far too much overhead).
> >> How would a user synchronize on the profile stream when the
> >> profiled process is stopped?
> >
> >Yeah, with a new perf_attr flag that activates overwrite this
> >usecase would be solved, right? The debugger has to make sure the
> >task is stopped before reading out the buffer, but that's pretty
> >much all.
> 
> I'm not sure about that. The way I read struct perf_counter_mmap_page,
> data_head points to the end of the stream (I would guess one byte
> beyond the last record).
> 
> I think we can ignore data_tail in the debug scenario since debuggers
> won't poll. We can further assume a buffer overflow no matter how big
> the ring buffer - branch trace grows terribly fast and we don't want
> normal uses to lock megabytes of memory, do we?
> 
> How would a debugger find the beginning of the event stream to start
> reading?

something like the below? (utterly untested)

---
 include/linux/perf_counter.h |    3 ++-
 kernel/perf_counter.c        |   35 +++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/include/linux/perf_counter.h b/include/linux/perf_counter.h
index 5e970c7..95b5257 100644
--- a/include/linux/perf_counter.h
+++ b/include/linux/perf_counter.h
@@ -180,8 +180,9 @@ struct perf_counter_attr {
 				freq           :  1, /* use freq, not period  */
 				inherit_stat   :  1, /* per task counts       */
 				enable_on_exec :  1, /* next exec enables     */
+				overwrite      :  1, /* overwrite mmap data   */
 
-				__reserved_1   : 51;
+				__reserved_1   : 50;
 
 	__u32			wakeup_events;	/* wakeup every n events */
 	__u32			__reserved_2;
diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index d55a50d..0c64d53 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -2097,6 +2097,13 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 	nr_pages = (vma_size / PAGE_SIZE) - 1;
 
 	/*
+	 * attr->overwrite and PROT_WRITE both use ->data_tail in an exclusive
+	 * manner, disallow this combination.
+	 */
+	if ((vma->vm_flags & VM_WRITE) && counter->attr.overwrite)
+		return -EINVAL;
+
+	/*
 	 * If we have data pages ensure they're a power-of-two number, so we
 	 * can do bitmasks instead of modulo.
 	 */
@@ -2329,6 +2336,7 @@ struct perf_output_handle {
 	struct perf_counter	*counter;
 	struct perf_mmap_data	*data;
 	unsigned long		head;
+	unsigned long		tail;
 	unsigned long		offset;
 	int			nmi;
 	int			sample;
@@ -2363,6 +2371,31 @@ static bool perf_output_space(struct perf_mmap_data *data,
 	return true;
 }
 
+static void perf_output_tail(struct perf_mmap_data *data, unsigned int head)
+{
+	__u64 *tailp = &data->user_page->data_tail;
+	struct perf_event_header *header;
+	unsigned long pages_mask, nr;
+	unsigned long tail, new;
+	unsigned long size;
+	void *ptr;
+
+	if (data->writable)
+		return;
+
+	size 	   = data->nr_pages << PAGE_SHIFT;
+	pages_mask = data->nr_pages - 1;
+	tail	   = ACCESS_ONCE(*tailp);
+
+	while (tail + size - head < 0) {
+		nr     = (tail >> PAGE_SHIFT) & pages_mask;
+		ptr    = data->pages[nr] + (tail & (PAGE_SIZE - 1));
+		header = (struct perf_event_header *)ptr;
+		new    = tail + header->size;
+		tail   = atomic64_cmpxchg(tailp, tail, new);
+	}
+}
+
 static void perf_output_wakeup(struct perf_output_handle *handle)
 {
 	atomic_set(&handle->data->poll, POLL_IN);
@@ -2535,6 +2568,8 @@ static int perf_output_begin(struct perf_output_handle *handle,
 		head += size;
 		if (unlikely(!perf_output_space(data, offset, head)))
 			goto fail;
+		if (unlikely(counter->attr.overwrite))
+			perf_output_tail(data, head);
 	} while (atomic_long_cmpxchg(&data->head, offset, head) != offset);
 
 	handle->offset	= offset;