lkml.org 
[lkml]   [2009]   [Feb]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] bitmap: Cleanup find_last_bit
    On Feb. 17, 2009, 1:10 +0200, Rusty Russell <rusty@rustcorp.com.au> wrote:
    > On Monday 16 February 2009 23:28:40 Benny Halevy wrote:
    >> Fix cut & paste error in header comment.
    >> Simplify implementation a bit.
    >
    > Hi Benny,
    >
    > Nice catch! But I disagree with one change:
    >
    >> /* Partial final word? */
    >> - if (size & (BITS_PER_LONG-1)) {
    >> - tmp = (addr[words] & (~0UL >> (BITS_PER_LONG
    >> - - (size & (BITS_PER_LONG-1)))));
    >> + tmp = size & (BITS_PER_LONG-1);
    >> + if (tmp) {
    >> + tmp = addr[words] & (~0UL >> (BITS_PER_LONG - tmp));
    >> if (tmp)
    >> goto found;
    >> }
    >
    > The overloading of tmp to the remainder size here is not really a cleanup: it
    > makes it into a dual-use var. I know it's called tmp, but that's a sign of
    > poor code too :)
    >
    > I'd prefer a new final_bits var: gcc will almost certainly just slap it in
    > a register anyway, but it's clearer.
    >
    > tmp could then be called something like... word?

    Yeah, I like that too.
    I'm about to go on a flight
    so the following was just tested to compile...

    diff --git a/include/linux/bitops.h b/include/linux/bitops.h
    index 6182913..886ebf0 100644
    --- a/include/linux/bitops.h
    +++ b/include/linux/bitops.h
    @@ -142,7 +142,7 @@ extern unsigned long find_first_zero_bit(const unsigned long *addr,
    * @addr: The address to start the search at
    * @size: The maximum size to search
    *
    - * Returns the bit number of the first set bit, or size.
    + * Returns the bit number of the last set bit, or size.
    */
    extern unsigned long find_last_bit(const unsigned long *addr,
    unsigned long size);
    diff --git a/lib/find_last_bit.c b/lib/find_last_bit.c
    index 5d202e3..6a35783 100644
    --- a/lib/find_last_bit.c
    +++ b/lib/find_last_bit.c
    @@ -17,29 +17,31 @@

    unsigned long find_last_bit(const unsigned long *addr, unsigned long size)
    {
    + unsigned long final_bits = size;
    unsigned long words;
    - unsigned long tmp;
    + unsigned long word;

    /* Start at final word. */
    - words = size / BITS_PER_LONG;
    + words = final_bits / BITS_PER_LONG;

    /* Partial final word? */
    - if (size & (BITS_PER_LONG-1)) {
    - tmp = (addr[words] & (~0UL >> (BITS_PER_LONG
    - - (size & (BITS_PER_LONG-1)))));
    - if (tmp)
    + size &= (BITS_PER_LONG-1);
    + if (size) {
    + word = addr[words] & (~0UL >> (BITS_PER_LONG - size));
    + if (word)
    goto found;
    }

    while (words) {
    - tmp = addr[--words];
    - if (tmp) {
    -found:
    - return words * BITS_PER_LONG + __fls(tmp);
    - }
    + word = addr[--words];
    + if (word)
    + goto found;
    }

    /* Not found */
    - return size;
    + return final_bits;
    +
    +found:
    + return words * BITS_PER_LONG + __fls(word);
    }
    EXPORT_SYMBOL(find_last_bit);
    --
    1.6.1.3

    >
    > Thanks!
    > Rusty.


    \
     
     \ /
      Last update: 2009-02-18 12:23    [W:0.028 / U:90.032 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site