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, 26 Nov 2008, Andi Kleen wrote:

> 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.

It's even worse than I thought when looking at it a second time:

All the loops iterate over an array which means in the worst case we
do full array_size iterations. In each iteration we check whether the
corresponding bit in the bitmask is set or not.

What a nonsense. We have a bitmask already. Why not iterate over the
bitmask and be done ?

Thanks,

tglx


\
 
 \ /
  Last update: 2008-11-26 22:21    [W:0.094 / U:79.012 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site