Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201211085039.GA7619@ubuntu>
In-Reply-To: <20201211085039.GA7619@ubuntu>
From:   Yury Norov <yury.norov@gmail.com>
Date:   Fri, 11 Dec 2020 09:20:23 -0800
Message-ID: <CAAH8bW8piLSCYLKjVXYV45cJeHApFX3Z3G=Zx-nap3yCA1=DDg@mail.gmail.com>
Subject: Re: [PATCH] lib/find_bit_bench: fix the unmatched iterations cnt
To:     Levi Yun <ppbuk5246@gmail.com>
Cc:     dushistov@mail.ru, Arnd Bergmann <arnd@arndb.de>,
        Andrew Morton <akpm@linux-foundation.org>,
        "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
        William Breathitt Gray <vilhelm.gray@gmail.com>,
        richard.weiyang@linux.alibaba.com,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        joseph.qi@linux.alibaba.com, skalluru@marvell.com,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Dec 11, 2020 at 12:50 AM Levi Yun <ppbuk5246@gmail.com> wrote:
>
> We should have same iteration count when we walk the same bitmap
> regardless of using find_next_bit or find_last_b

I think it's not that important, because the difference is not measurable.
But if this part raises questions, I have nothing against aligning numbers.

> When we run the find_bit_benchmark.ko, we sometime get
> unmatched iterations count below:
>
>              Start testing find_bit() with random-filled bitmap
> [+...] find_next_bit:                  875085 ns, 163755 iterations <
> [+...] find_next_zero_bit:             865319 ns, 163926 iterations
> [+...] find_last_bit:                  611807 ns, 163756 iterations <
> [+...] find_first_bit:                1601016 ns,  16335 iterations
> [+...] find_next_and_bit:              400645 ns,  74040 iterations
> [+...]
>               Start testing find_bit() with sparse bitmap
> [+...] find_next_bit:                    9942 ns,    654 iterations
> [+...] find_next_zero_bit:            1678445 ns, 327027 iterations
> [+...] find_last_bit:                    7131 ns,    654 iterations
> [+...] find_first_bit:                 551383 ns,    654 iterations
> [+...] find_next_and_bit:                3027 ns,      1 iterations
>
> Normally, this is happen when the last bit of bitmap was set.

Can you please confirm that for bitmap 0001,
test_find_{first,next,next_and}_bit reports cnt == 0, and
test_find_last_bit() reports 1?

> This patch fix the unmatched iterations count between
> test_find_next_bit and test_find_last_bit.
>
> Signed-off-by: Levi Yun <ppbuk5246@gmail.com>
> ---
>  lib/find_bit_benchmark.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/lib/find_bit_benchmark.c b/lib/find_bit_benchmark.c
> index 5637c5711db9..766e0487852b 100644
> --- a/lib/find_bit_benchmark.c
> +++ b/lib/find_bit_benchmark.c
> @@ -35,14 +35,14 @@ static DECLARE_BITMAP(bitmap2, BITMAP_LEN) __initdata;
>   */
>  static int __init test_find_first_bit(void *bitmap, unsigned long len)
>  {
> -       unsigned long i, cnt;
> +       unsigned long i = 0, cnt = 0;
>         ktime_t time;
>
>         time = ktime_get();
> -       for (cnt = i = 0; i < len; cnt++) {
> +       do {
>                 i = find_first_bit(bitmap, len);
>                 __clear_bit(i, bitmap);
> -       }
> +       } while (i++ < len && ++cnt);

What for this check against ++cnt? I doubt that the counter can overflow.

>         time = ktime_get() - time;
>         pr_err("find_first_bit:     %18llu ns, %6ld\n", time, cnt);
>
> @@ -51,12 +51,13 @@ static int __init test_find_first_bit(void *bitmap, unsigned long len)
>
>  static int __init test_find_next_bit(const void *bitmap, unsigned long len)
>  {
> -       unsigned long i, cnt;
> +       unsigned long i = 0, cnt = 0;
>         ktime_t time;
>
>         time = ktime_get();
> -       for (cnt = i = 0; i < BITMAP_LEN; cnt++)
> -               i = find_next_bit(bitmap, BITMAP_LEN, i) + 1;
> +       do {
> +               i = find_next_bit(bitmap, BITMAP_LEN, i);
> +       } while (i++ < BITMAP_LEN && ++cnt);
>         time = ktime_get() - time;
>         pr_err("find_next_bit:      %18llu ns, %6ld iterations\n", time, cnt);
>
> @@ -65,12 +66,13 @@ static int __init test_find_next_bit(const void *bitmap, unsigned long len)
>
>  static int __init test_find_next_zero_bit(const void *bitmap, unsigned long len)
>  {
> -       unsigned long i, cnt;
> +       unsigned long i = 0, cnt = 0;
>         ktime_t time;
>
>         time = ktime_get();
> -       for (cnt = i = 0; i < BITMAP_LEN; cnt++)
> -               i = find_next_zero_bit(bitmap, len, i) + 1;
> +       do {
> +               i = find_next_zero_bit(bitmap, len, i);
> +       } while (i++ < BITMAP_LEN && ++cnt);
>         time = ktime_get() - time;
>         pr_err("find_next_zero_bit: %18llu ns, %6ld iterations\n", time, cnt);
>
> @@ -84,12 +86,11 @@ static int __init test_find_last_bit(const void *bitmap, unsigned long len)
>
>         time = ktime_get();
>         do {
> -               cnt++;
>                 l = find_last_bit(bitmap, len);
>                 if (l >= len)
>                         break;
>                 len = l;
> -       } while (len);
> +       } while (len >= 0 && ++cnt);

Why this?

>         time = ktime_get() - time;
>         pr_err("find_last_bit:      %18llu ns, %6ld iterations\n", time, cnt);
>
> @@ -99,12 +100,13 @@ static int __init test_find_last_bit(const void *bitmap, unsigned long len)
>  static int __init test_find_next_and_bit(const void *bitmap,
>                 const void *bitmap2, unsigned long len)
>  {
> -       unsigned long i, cnt;
> +       unsigned long i = 0, cnt = 0;
>         ktime_t time;
>
>         time = ktime_get();
> -       for (cnt = i = 0; i < BITMAP_LEN; cnt++)
> -               i = find_next_and_bit(bitmap, bitmap2, BITMAP_LEN, i + 1);
> +       do {
> +               i = find_next_and_bit(bitmap, bitmap2, BITMAP_LEN, i);
> +       } while (i++ < BITMAP_LEN && ++cnt);

Do you experience the same problem with find_next_and_bit() as well?

>         time = ktime_get() - time;
>         pr_err("find_next_and_bit:  %18llu ns, %6ld iterations\n", time, cnt);
>
> --
> 2.27.0