Re: [RFC] perf/sdt: Directly record SDT event with 'perf record'

[Ravi Bangoria]

Thanks Arnaldo,

I'm working on this but it's taking bit longer time. Will post out a patch within
few days.

Ravi

On Monday 20 February 2017 07:41 PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 20, 2017 at 04:31:50PM +0530, Ravi Bangoria escreveu:
>> Thanks Ingo,
>>
>> On Monday 20 February 2017 02:12 PM, Ingo Molnar wrote:
>>> * Ravi Bangoria <ravi.bangoria@xxxxxxxxxxxxxxxxxx> wrote:
>>>
>>>> What should be the behavior of the tool? Should it record only one
>>>> 'sdt_libpthread:mutex_entry' which exists in uprobe_events? Or it
>>>> should record all the SDT events from libpthread? We can choose either
>>>> of two but both the cases are ambiguous.
>>> They are not ambiguous really if coded right: just pick one of the outcomes and 
>>> maybe print a warning to inform the user that something weird is going on because 
>>> not all markers are enabled?
>>>
>>> As a user I'd expect 'perf record' to enable all markers and print a warning that 
>>> the markers were in a partial state. This would result in consistent behaviour.
>> Yes, makes sense.
>>
>>> Does it make sense to only enable some of the markers that alias on the same name? 
>>> If not then maybe disallow that in perf probe - or change perf probe to do the 
>>> same thing as perf record.
>> 'perf probe' is doing that correctly. It fetches all events with given name from
>> probe-cache and creates entries for them in uprobe_events.
>>
>> The problem is the 2-step process of adding probes and then recording,
>> allowing users to select individual markers to record on.
> So, the more streamlined one works for most people, i.e. just use perf
> record, no need to perf probe anything. But, for people who "know what
> they are doing", perf probe can be used first to control exactly which
> SDT probes one wants in place, and then those will be used.
>
> We need to make sure that when processing the file there is information
> that says which probes were in place and enabled in the record session,
> tho. Is that possible?
>
>>> I.e. this is IMHO an artificial problem that users should not be exposed to and 
>>> which can be solved by tooling.
>>>
>>> In particular if it's possible to enable only a part of the markers then perf 
>>> record not continuing would be a failure mode: if for example a previous perf 
>>> record session segfaulted (or ran out of RAM or was killed in the wrong moment or 
>>> whatever) then it would not be possible to (easily) clean up the mess.
>> Agreed. We need to make this more robust.
> Right, disambiguating a 'probes left by a session that did auto-probing'
> from a 'hey, those probes are there intentionally, just use those' is
> important.
>
>>>> Not allowing 'perf probe' for SDT event will solve all such issues.
>>>> Also it will make user interface simple and consistent. Other current
>>>> tooling (systemtap, for instance) also do not allow probing individual
>>>> markers when there are multiple markers with the same name.
>>> In any case if others agree with your change in UI flow too then it's fine by me, 
>>> but please make it robust, i.e. if perf record sees partially enabled probes it 
>>> should still continue.
>> @Masami, can you please provide your thoughts as well.
> Yeah, if technically possible to allow both variants, we should leave
> it up to users to decide what is best?
>
> I.e. most people will do auto-probing, not using 'perf probe' at all,
> documentation should state the pitfalls in doing so.
>
> So, after writing the above, perhaps we should warn the user that
> pre-existing probes are being used, as this will be the odd case?
>
> The normal flow will be just using perf record with SDT probes, that
> will auto-probe them and remove on exit, or better drop a reference to
> them, as simultaneous use also needs to be covered?
>
> - Arnaldo
>