Em Thu, Feb 23, 2017 at 01:43:38PM +0530, Ravi Bangoria escreveu: > Thanks Arnaldo, > > I'm working on this but it's taking bit longer time. Will post out a patch > within > few days.
Take your time and thanks for giving consideration to my observations, Regards, - Arnaldo > 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.bango...@linux.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 > >
