linux-kernel - Re: [patch 05/24] perfmon: X86 generic code (x86)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7c86c4470811261337ic1ea828me5e3ba0de469f4b6@mail.gmail.com>
Date:	Wed, 26 Nov 2008 22:37:07 +0100
From:	"stephane eranian" <eranian@...glemail.com>
To:	"Thomas Gleixner" <tglx@...utronix.de>
Cc:	"Andi Kleen" <andi@...stfloor.org>, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, mingo@...e.hu, x86@...nel.org,
	sfr@...b.auug.org.au
Subject: Re: [patch 05/24] perfmon: X86 generic code (x86)

Thomas,

On Wed, Nov 26, 2008 at 10:18 PM, Thomas Gleixner <tglx@...utronix.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/