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)
Thomas,

On Wed, Nov 26, 2008 at 10:18 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> 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 ?
>

Bitmask can be sparsed. Num represents the number of bits we have to find.
The idea is that we don't need to scan the entire bitmask, we stop as soon as
we have found all the bits we care about (i.e., all the bits that are set).

Example:
num = 3
bitmask=0000000010001001
^ we will iterate until we are
done with that bit.


\
 
 \ /
  Last update: 2008-11-26 22:39    [W:0.848 / U:0.144 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site