Bugzilla – Full Text Bug Listing
| Summary: | OProfile plugin issues with Power processors | ||||||
|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Daniel Henrique Barboza <danielhb> | ||||
| Component: | LinuxTools | Assignee: | OProfile Inbox <linux.oprofile-inbox> | ||||
| Status: | CLOSED FIXED | QA Contact: | |||||
| Severity: | critical | ||||||
| Priority: | P3 | CC: | danielhb, kksebasti, maynardj, overholt, sgehwolf | ||||
| Version: | unspecified | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | Other | ||||||
| OS: | Linux | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 343955 | ||||||
| Attachments: |
|
||||||
|
Description
Daniel Henrique Barboza
Answer from Maynard Johnson on the mailing list: I personally haven't had time to look at the oprofile plugin code, so I don't how it might be using the returned value 'ophelp <event_name>'. But I can tell you that the purpose of this particular incantation of ophelp is for opcontrol (oprofile's control script) to obtain the event encoding which is passed to the oprofile kernel driver via the oprofilefs pseudo filesystem. The kernel driver uses that information to program the performance monitor unit hardware. On Intel, this event encoding is a simple integer value; on Power systems, the encoding is done via several register values. The opcontrol script has code that can detect when the returned value is more than a simple integer, and it handles that extra data. So IMHO, I can't see any valid purpose in using that ophelp call from within the GUI. ophelp has an '--xml' option which should provide GUI tools with all the information they need about events in an architected manner -- guaranteed to work on any hardware arch. If there is something missing in the XML output that a GUI tool *really* needs, we'll gladly address it. Thanks. -Maynard Created attachment 189848 [details]
Patch to fix hex number parsing.
Daniel, have you tried this simple ugly hack? What happens if you use that? Thanks!
Hi Severin, I tried the hack and it didn't work. The usage of the 'ophelp <event_name>' output goes deeper and deeper than I expected. So far what I've got is that the output (interpreted as an integer) is used on EventCache.idMap to map an integer with a String value. These values are retrieved and used by the methods: - getEventNameWithID - getElementWithID Both these methods are used in CheckEventAdapter.isValidEvent() - getEventIDwithName This method is used in InfoAdapter.getValue and TestCheckEventsPreParse.setUp This is as far as I dig up, but each time I change the code another dependency breaks. This is why I thought that if we're goint that far to just amend it, maybe we should take the time to make it right from the start. I offered to help out on this issue, and here's what I've found.
I did some investigation into how/where the event val (returned from 'ophelp <event_name>') is used in the oprofile plugin. There are two maps that use the integer value: the idMap (which uses the event val as a key) and the nameMap (which uses the event name as the key and the event val as the object). Looking at the nameMap, I see it's accessed by way of getEventIDWithName() -- from org.eclipse.linuxtools.oprofile.core.opxml.EventIdCache.java, the same java file as where the map is created. The getEventIDWithName function is only used in a couple places: the InfoAdapter file in the org.eclipse.linuxtools.oprofile.core.opxml.info package and the TestCheckEventsPreParse file in org.eclipse.linuxtools.oprofile.core.tests.package. Chasing these two takes me to org.eclipse.linuxtools.oprofile.core.linux.OpxmlRunner.java:run(), which creates XML from stored profile data. And that should be dead code, since the oprofile plugin now uses the XML output from the native oprofile tools.
So, since it seems that the event val is not truly needed in the plugin right now. I tried a quick and dirty hack in EventIdCache.java file, line 101:
//int val = Integer.parseInt(line);
int val = i;
We tested this hack successfully on both Power and Intel systems. I suggest this hack can be accepted upstream now. Then, as time and resources allow, the entire plugin code can be scrubbed for dead, unused functions.
A fix has been pushed to master (Revision c8c730c). Please verify that it works on Power. Thanks! I've created a tracker bug 339945 for the task of going through the code and remove what is not necessary anymore. There seems to be some left over code due to the opxml removal. Thanks! Managed to checkout the revision and tested it. Runs as expected on Power. I'm not aware of the process here, but are you making this new revision available through the nightly build update site? Or are you going to update the plugin inside the 0.7 version? I'm just a bit confused about how we can easily install this patched version on Eclipse. Short answer: it'll be in the nightly update site soon and seems like a fine candidate for 0.7.1 Long answer: Our nightly builds are not running at present due to our switch to git causing some releng changes. We have things ready to go but are waiting for Chris Aniszczyk to be approved as a committer so that his Tycho work can be imported without having to wait for CQs. HTH, Andrew For the moment you can use: https://hudson.eclipse.org/hudson/job/cbi-linuxtools-Indigo/lastSuccessfulBuild/artifact/releng/org.eclipse.linuxtools.releng-site/target/repository/ It is built from master. I haven't yet finished back-porting the Tycho work to stable-0.7 so we don't have a nightly build for that yet, sorry. The latest build towards what will be our Indigo release (0.8) is here: http://download.eclipse.org/technology/linuxtools/updates-nightly/ This is fixed, closing. |