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    [W:0.023 / U:149.560 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site