Corey, Will wait for your patch. Let put your patch after what we already have, so after stats-debugfs.
On Wed, Jun 25, 2008 at 11:42 PM, Corey J Ashford <[EMAIL PROTECTED]> wrote: > Hi Stephane, > > Some responses below. > > "stephane eranian" <[EMAIL PROTECTED]> wrote on 06/25/2008 07:21:06 AM: >> Corey, >> >> >> I looked at the two patches and I have the following comments: >> >> In the base patch: >> >> - arch/powerpc/perfmon/Kconfig >> you rely on your patch being stacked AFTER the >> perfmon-stats-debugfs. I wonder if this is what we want > > I don't have a preference, really. > > Would you like the patch after or before the system-wide counting support > patch? > >> >> - arch/powerpc/perfmon/Makefile >> you make reference to perfmon_power6.c. This should be in the power6 >> patch > > Will fix. > >> >> - include/asm-powerpc/perfmon_kern.h >> >> * pfm_cacheflush() must disappear it will come with sampling > > Ok. > >> >> * in the pfm_arch_context structure you define fields (pmc6,pmc5) >> which, I believe, are there to support >> either Power6 or Cell. If so they should be added by these patch >> es respectively. > > Oh... hmm. Originally I had an empty arch context structure in the base > patch. > I must have messed that up somehow. I will fix and thanks for catching > that. > > As an aside, Cell's PMU is completely different and doesn't have a PMC5 or > PMC6 like > the other POWER chips. > >> >> * arch/powerpc/perfmon/perfmon.c >> >> pfm_arch_get_pmu_module_name() must disappear. This is needed >> only once we have custom sampling buffer >> format support. For now, everything is a compile time option only. >> > > Ok, I'll remove that. > >> - Signed-off-by are missing > > Will add that. > > I've found a couple of problems during testing that I will fix before the > next go around. I hope to have a new patch ready on Friday. > > Thanks! > > - Corey > >> >> Please fix those and resubmit. >> >> thanks. >> >> On Tue, Jun 24, 2008 at 2:28 AM, Corey Ashford <[EMAIL PROTECTED]> >> wrote: >> >> > Hi Stephane, >> > >> > I'm attaching the patches for the POWER port to the minimal perfmon2 >> > kernel. >> > I have built a working kernel based on this source, but I haven't >> > tested >> > perfmon2 yet. How are you testing? Can we use the existing libpfm >> > examples >> > without modification? I will try that next. >> > >> > In addition to a port of POWER to the mimimal perfmon2, there's also a >> > bug >> > fix which I plan to submit to the full perfmon2 git also. J K Rai >> > noticed >> > that there was a problem with the use of the call smp_processor_id(). >> > Our >> > code had a race condition in that the cpu number is obtained but there's >> > nothing to stop the CPU from being switched out from under the thread, >> > which >> > then goes on to read some CPU-specific registers and update data in the >> > pfm >> > context. >> > >> > To fix this, J K Rai correctly suggested using get_cpu() and put_cpu() >> > pairs >> > to disable preemption while we are using the cpu number. So I have made >> > those changes. >> > >> > I'd much appreciate a review of this code before submitting to the LKML >> > mailing list. >> > >> > Regards, >> > >> > - Corey >> > >> > >> > -- >> > Corey Ashford >> > Software Engineer >> > IBM Linux Technology Center, Linux Toolchain >> > Beaverton, OR >> > 503-578-3507 >> > [EMAIL PROTECTED] >> > > ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php _______________________________________________ perfmon2-devel mailing list perfmon2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
