lkml.org 
[lkml]   [2008]   [Nov]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch 05/24] perfmon: X86 generic code (x86)
On Wed, Nov 26, 2008 at 02:35:18PM +0100, Thomas Gleixner wrote:
> > + * does not work with other types of PMU registers.Thus, no
> > + * address is ever exposed by counters
> > + *
> > + * - there is never a dependency between one pmd register and
> > + * another
> > + */
> > + for (i = 0; num; i++) {
> > + if (likely(pfm_arch_bv_test_bit(i, set->used_pmds))) {
> > + pfm_write_pmd(ctx, i, set->pmds[i]);
> > + num--;
> > + }
> > + }
>
> This loop construct looks scary. It relies on set->nused_pmds >=
> bits set in set->used_pmds. I had to look more than once to
> understand that. It's used all over the code in variations.

FWIW this loop style tripped me up during review too.

> > + */
> > + pfm_arch_resend_irq(ctx);
>
> Do we really need this whole NMI business ?

Without it you cannot profile interrupts off regions well.

>
> 9 simple wrappers around generic bitops. The only reason you need
> those is because you use 64bit variables and that does not work on
> 32bit BE machines.
>
> I do not understand in the first place why you cant use simple
> unsigned longs for the bitfields, but if this is necessary for
> whatever non obvious reason, then its not an excuse to make this arch
> dependent code at all. You need a LE/BE64 and a BE32 version. So you
> need a generic and a special be32 version. That's not arch specific.

Or a unsigned long x[VALUE_DEPENDS_ON_WORD_SIZE]

-Andi


\
 
 \ /
  Last update: 2008-11-26 14:53    [from the cache]
©2003-2011 Jasper Spaans