Messages in this thread | | | Date | Fri, 23 Dec 2011 17:17:01 +0100 | From | Robert Richter <> | Subject | Re: [PATCH 4/4] perf script: Add script to collect and display IBS samples |
| |
On 23.12.11 15:40:40, Ingo Molnar wrote: > > * Peter Zijlstra <peterz@infradead.org> wrote: > > > On Fri, 2011-12-23 at 14:53 +0100, Ingo Molnar wrote: > > > * Robert Richter <robert.richter@amd.com> wrote: > > > > > > > > Also, could you quote example output of "perf script report > > > > > ibs"? > > > > > > > > IBS_FETCH sample on cpu6 IBS0: 0x00170003186a186a IBS1: 0x0000000000444780 IBS2:0x000000041af26780 > > > > IBS_FETCH sample on cpu6 IBS0: 0x00170003186a186a IBS1: 0x00007f5efb44e3b2 IBS2:0x000000042fcff3b2 > > > > IBS_FETCH sample on cpu6 IBS0: 0x01370003186a186a IBS1: 0xffffffff81065273 IBS2:0x0000000001065273 > > > > IBS_FETCH sample on cpu6 IBS0: 0x01370003186a186a IBS1: 0xffffffff811a6320 IBS2:0x00000000011a6320 > > > > IBS_FETCH sample on cpu6 IBS0: 0x01370003186a186a IBS1: 0xffffffff81065255 IBS2:0x0000000001065255 > > > > IBS_FETCH sample on cpu7 IBS0: 0x00170004186a186a IBS1: 0x00007fbf0c687ca0 IBS2:0x000000041d345ca0 > > > > IBS_FETCH sample on cpu7 IBS0: 0x00170003186a186a IBS1: 0x000000000043bb80 IBS2:0x000000041c351b80 > > > > IBS_FETCH sample on cpu7 IBS0: 0x01370003186a186a IBS1: 0xffffffff813d5790 IBS2:0x00000000013d5790 > > > > IBS_FETCH sample on cpu7 IBS0: 0x00030001186a186a IBS1: 0xffffffff8102bd00 IBS2:0x00000000013d5d00 > > > > > > That does not look very usable to users. So why should we merge > > > this new ABI in its incomplete form with no complete usecase? > > > Being usable is clearly not outside the scope of a new feature > > > ...
So is it not useful to pull ibs data with native perf tools to userspace? Is it not useful to have a dump of the raw data too? Is it not useful to have perf script support that allows user to easy implement their own script to parse the data they are interested in? IBS is for experienced users, even I don't know *everything* about what this data can be used for, but I know people who know it. And they want to get this data out of the kernel, using the perf_event syscall. Maybe they also improve the perf tool with their knowledge.
Oprofile also has support for IBS in a very raw way, and there are users for it. Remember, there are also plans to rewrite the oprofile daemon to use perf_event. If there isn't any confidence that perf_event will get the same features so it can be used for this, people might better stick with that what they have.
> > > Integrating it into perf report should not be *that* hard, > > > you've done most of the hard work already. But without that > > > step we just don't know how complete and usable the whole > > > thing is.
We don't need a feature complete (which features?) implementation to see use cases for this. AFAIK there is no doubt anymore about the user/kernel interface implementation. So your argument is questionable. If you look at other kernel features (also other perf features) and how they got in, most of them are not perfect in the beginning.
We also should give others the opportunity to actually use IBS, to give feedback, to improve the tools, to work with it. Most of the current patches wont change anymore, the code is ready. As long as the code is not in the repostitory it is much harder for people to work with it. They usually don't deal with some patches flying around.
We waste our time with reposting, reviewing (people seem to no longer do this, because they are tiered), rebasing it, testing it. Do you fear we stop working to improve the code as soon as you applied the patches? Have you ever had this feeling in the past? We invest a high amount of time in all this, please let us know if this is not worth the effort. Almost 2 years ago I posted my first IBS patches for perf...
And, is the kernel ever ready? There is no completion at all. There is an onging development in small steps. I don't see why you demand this 100%-fits-for-all-usecases solution, that you also don't know much about. There isn't one, and there wont be any. Relax your position that everything must be there in the beginning.
To me these are reasons enough to merge it.
Thanks,
-Robert
-- Advanced Micro Devices, Inc. Operating System Research Center
| |