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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170220141117.GJ4109@kernel.org>
Date:   Mon, 20 Feb 2017 11:11:17 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>
Cc:     Ingo Molnar <mingo@...nel.org>, masami.hiramatsu.pt@...achi.com,
        mingo@...hat.com, brendan.d.gregg@...il.com, peterz@...radead.org,
        alexander.shishkin@...ux.intel.com, wangnan0@...wei.com,
        jolsa@...nel.org, ak@...ux.intel.com, treeze.taeung@...il.com,
        mathieu.poirier@...aro.org, hekuang@...wei.com,
        sukadev@...ux.vnet.ibm.com, ananth@...ibm.com,
        naveen.n.rao@...ux.vnet.ibm.com, colin.ing@...onical.com,
        adrian.hunter@...el.com, linux-kernel@...r.kernel.org,
        hemant@...ux.vnet.ibm.com
Subject: Re: [RFC] perf/sdt: Directly record SDT event with 'perf record'

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@...ux.vnet.ibm.com> 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