Messages in this thread | | | Date | Wed, 23 Aug 2006 03:29:32 -0700 | From | Stephane Eranian <> | Subject | Re: [PATCH 18/18] 2.6.17.9 perfmon2 patch for review: new x86_64 files |
| |
Andi,
On Wed, Aug 23, 2006 at 12:19:44PM +0200, Andi Kleen wrote: > > + > > +config X86_64_PERFMON_EM64T > > + tristate "Support Intel EM64T hardware performance counters" > > + depends on PERFMON > > + default m > > + help > > + Enables support for the Intel EM64T hardware performance > > + counters. Does not work with AMD64 processors. > > + If unsure, say m. > > Does that include the Core 2 support that you had in the i386 patch? > This overs P4 in 64-bit mode only!
What you have in i386 is the architectural perfmon support as documented by Intel for Core Duo/Core Solo and possibly others. Core 2 is something different but hopfully backward compatible with this.
> In general I would prefer to call it P4, not EM64T which is just > a generic architecture name and at least on P4 performance counters > are not really architected yet. > Agreed.
> > > + * > > + * This file implements the PEBS sampling format for Intel > > + * EM64T Intel Pentium 4/Xeon processors. It does not work > > + * with Intel 32-bit P4/Xeon processors. > > Why not anyways? The registers are basically the same. What's so different > in 64bit? oprofile shares that code too. > Well, no. the PEBS record format is differnet between 32 and 64-bit mode. In 64-bit they add r8-r15 in each sample. The rest of the logic is exactly the same. Until now, I have kept the 32 and 64 bit format completely separate. But I will merge them in my next patch to cut down on the amount of code.
> The file seems a bit underdocumented. At least some brief description > what PEBS is and maybe at least one sentence for each function? > > > + */ > > +#ifndef __PERFMON_EM64T_PEBS_SMPL_H__ > > +#define __PERFMON_EM64T_PEBS_SMPL_H__ 1 > > + > > +#define PFM_EM64T_PEBS_SMPL_UUID { \ > > + 0x36, 0xbe, 0x97, 0x94, 0x1f, 0xbf, 0x41, 0xdf,\ > > + 0xb4, 0x63, 0x10, 0x62, 0xeb, 0x72, 0x9b, 0xad} > > What does it need the UUID for? > Every sampling format is identified by a UUID. This is how an application can identify the format it wants to use when it creates a context.
> > + > > +/* > > + * format specific parameters (passed at context creation) > > + * > > + * intr_thres: index from start of buffer of entry where the > > + * PMU interrupt must be triggered. It must be several samples > > + * short of the end of the buffer. > > + */ > > +struct pfm_em64t_pebs_smpl_arg { > > + size_t buf_size; /* size of the buffer in bytes */ > > + size_t intr_thres; /* index of interrupt threshold entry */ > > + u32 flags; /* buffer specific flags */ > > + u64 cnt_reset; /* counter reset value */ > > + u32 res1; /* for future use */ > > + u64 reserved[2]; /* for future use */ > > I hope you double checked the alignment comes up everywhere correctly. > u64 alignment is different on the 32bit and 64bit ABIs. That can screw > > Normally it's safer to use aligned_u64 on files that can be used on > 32bit too, because that avoids that problem.
As of now this file (perfmon_em64t_pebs_smpl.c) is only compiled in 64-bit mode. Once I merge with 32-bit I will force types to avoid alignment problems.
> Where is the actual code that implements the code that you hooked > into arch/x86_64/*? I must have missed that. > It is in the patch I call modified x86_64 files.
Thanks for you quick and valuable feedback.
I will make the change you suggested on this part.
-- -Stephane - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |