Messages in this thread | | | Date | Fri, 5 Jun 2020 15:57:43 +0200 | From | Jiri Olsa <> | Subject | Re: [PATCH v5 13/13] perf record: introduce --ctl-fd[-ack] options |
| |
On Fri, Jun 05, 2020 at 04:15:52PM +0300, Alexey Budankov wrote: > > On 05.06.2020 13:51, Jiri Olsa wrote: > > On Tue, Jun 02, 2020 at 04:43:58PM +0300, Adrian Hunter wrote: > >> On 2/06/20 12:12 pm, Alexey Budankov wrote: > >>> > >>> On 02.06.2020 11:32, Alexey Budankov wrote: > >>>> > >>>> On 02.06.2020 2:37, Andi Kleen wrote: > >>>>>>> or a pathname, or including also the event default of "disabled". > >>>>>> > >>>>>> For my cases conversion of pathnames into open fds belongs to external > >>>>>> controlling process e.g. like in the examples provided in the patch set. > >>>>>> Not sure about "event default of 'disabled'" > >>>>> > >>>>> It would be nicer for manual use cases if perf supported the path names > >>>>> directly like in Adrian's example, not needing a complex wrapper script. > >>>> > >>>> fds interface is required for VTune integration since VTune wants control > >>>> over files creation aside of Perf tool process. The script demonstrates > >>>> just one possible use case. > >>>> > >>>> Control files could easily be implemented on top of fds making open operations > >>>> for paths and then initializing fds. Interface below is vague and with explicit > >>>> options like below it could be more explicit: > >>>> --ctl-file /tmp/my-perf.fifo --ctl-file-ack /tmp/my-perf-ack.fifo > >>> > >>> Or even clearer: > >>> > >>> --ctl-fifo /tmp/my-perf --ctl-fifo-ack /tmp/my-perf-ack > >> > >> If people are OK with having so many options, then that is fine by me. > > > > the single option Adrian suggested seems better to me: > > > > --control > > --control 11 > > --control 11,15 > > What if a user specifies fifos named like this above, not fds? > > > --control 11,15,disabled > > --control 11,,disabled > > --control /tmp/my-perf.fifo > > --control /tmp/my-perf.fifo,/tmp/my-perf-ack.fifo > > What if a user wants not fifos but other type of comm channels? > > > --control /tmp/my-perf.fifo,/tmp/my-perf-ack.fifo,disabled > > --control /tmp/my-perf.fifo,,disabled > > > > we already support this kind of options arguments, like for --call-graph > > > > jirka > > > > IMHO, > this interface, of course, looks more compact (in amount of options) however > the other side it is less user friendly. One simple option for one simple > purpose is more convenient as for users as for developers. Also complex > option syntax tends to have limitations and there are probably more > non-obvious ones. > > Please speak up. I might have missed something meaningful.
how about specify the type like:
--control fd:1,2,... --control fifo:/tmp/fifo1,/tmp/fifo2 --control xxx:....
this way we can extend the functionality in the future and stay backward compatible, while keeping single option
jirka
| |