All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yury Norov <yury.norov@gmail.com>
To: Yury Norov <yury.norov@gmail.com>
Cc: Yi Sun <yi.sun@unisoc.com>,
	mnazarewicz@gmail.com, akpm@linux-foundation.org,
	mina86@mina86.com, akinobu.mita@gmail.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 2/2] lib/bitmap: add tests for bitmap_find_next_zero_area_off()
Date: Mon, 8 Jun 2026 15:24:43 -0400	[thread overview]
Message-ID: <aicW-4W4mSzb8lBS@yury> (raw)
In-Reply-To: <aicUjhv1iAs2F6Yf@yury>

(Sorry, hit 'send' prematurely with a fat finger)

On Mon, Jun 08, 2026 at 03:14:22PM -0400, Yury Norov wrote:
> On Mon, Jun 01, 2026 at 05:42:34PM +0800, Yi Sun wrote:
> > Add functional and performance tests
> > for bitmap_find_next_zero_area_off().
> > 
> > Signed-off-by: Yi Sun <yi.sun@unisoc.com>
> > ---
> >  lib/find_bit_benchmark.c | 17 +++++++++++++++++
> >  lib/test_bitmap.c        | 28 ++++++++++++++++++++++++++++
> >  2 files changed, 45 insertions(+)
> > 
> > diff --git a/lib/find_bit_benchmark.c b/lib/find_bit_benchmark.c
> > index 00d9dc61cd46..37fe76ad322e 100644
> > --- a/lib/find_bit_benchmark.c
> > +++ b/lib/find_bit_benchmark.c
> > @@ -149,6 +149,21 @@ static int __init test_find_next_and_bit(const void *bitmap,
> >  	return 0;
> >  }
> >  
> > +static int __init
> > +test_bitmap_find_next_zero_area_off(unsigned long *bitmap, unsigned long len)
> > +{
> > +	unsigned long i, cnt;
> > +	ktime_t time;
> > +
> > +	time = ktime_get();
> > +	for (cnt = i = 0; i < BITMAP_LEN; cnt++)
> > +		i = bitmap_find_next_zero_area_off(bitmap, BITMAP_LEN, i, 8, 0, 0) + 1;
> > +	time = ktime_get() - time;
> > +	pr_err("bitmap_find_next_zero_area_off:  %18llu ns, %6ld iterations\n", time, cnt);
> > +
> > +	return 0;
> > +}

I asked you at least 3 times to print the exact test output in the
commit message. If you didn't ignore it, you'd see something like:

[    0.230280] Start testing find_bit() with random-filled bitmap
[    0.231438] bitmap_find_next_zero_area_off:              352393 ns,   1235 iterations
[    0.232771] find_next_bit:                  586225 ns, 163838 iterations
[    0.234022] find_next_zero_bit:             631854 ns, 163843 iterations
[    0.235154] find_last_bit:                  489426 ns, 163839 iterations
[    0.238080] find_nth_bit:                  2283610 ns,  16471 iterations
[    0.239545] find_first_bit:                 844551 ns,  16472 iterations
[    0.248981] find_first_and_bit:            8840955 ns,  41232 iterations
[    0.250030] find_next_and_bit:              334729 ns,  81913 iterations
[    0.250728]
[    0.250728] Start testing find_bit() with sparse bitmap
[    0.253094] bitmap_find_next_zero_area_off:             1658964 ns, 322485 iterations
[    0.253851] find_next_bit:                    8053 ns,    655 iterations
[    0.255664] find_next_zero_bit:            1247380 ns, 327026 iterations
[    0.256292] find_last_bit:                    7444 ns,    655 iterations
[    0.257756] find_nth_bit:                   865494 ns,    654 iterations
[    0.258720] find_first_bit:                 284844 ns,    655 iterations
[    0.259347] find_first_and_bit:               1824 ns,      1 iterations
[    0.259992] find_next_and_bit:                1763 ns,      1 iterations

And it clearly breaks the test format. Please, when it comes to v5,
double check that you have all reviewers requests addressed.

> > +
> >  static int __init find_bit_test(void)
> >  {
> >  	unsigned long nbits = BITMAP_LEN / SPARSE;
> > @@ -158,6 +173,7 @@ static int __init find_bit_test(void)
> >  	get_random_bytes(bitmap, sizeof(bitmap));
> >  	get_random_bytes(bitmap2, sizeof(bitmap2));
> >  
> > +	test_bitmap_find_next_zero_area_off(bitmap, BITMAP_LEN);
> >  	test_find_next_bit(bitmap, BITMAP_LEN);
> >  	test_find_next_zero_bit(bitmap, BITMAP_LEN);
> >  	test_find_last_bit(bitmap, BITMAP_LEN);
> > @@ -181,6 +197,7 @@ static int __init find_bit_test(void)
> >  		__set_bit(get_random_u32_below(BITMAP_LEN), bitmap2);
> >  	}
> >  
> > +	test_bitmap_find_next_zero_area_off(bitmap, BITMAP_LEN);
> >  	test_find_next_bit(bitmap, BITMAP_LEN);
> >  	test_find_next_zero_bit(bitmap, BITMAP_LEN);
> >  	test_find_last_bit(bitmap, BITMAP_LEN);
> > diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
> > index 69813c10e6c0..fad2b19760c2 100644
> > --- a/lib/test_bitmap.c
> > +++ b/lib/test_bitmap.c
> > @@ -234,6 +234,34 @@ static void __init test_find_nth_bit(void)
> >  	}
> >  }
> >  
> > +static void __init
> > +test_bitmap_find_next_zero_area_off(void)
> 
> warning: ‘test_bitmap_find_next_zero_area_off’ defined but not used [-Wunused-function]
>   238 | test_bitmap_find_next_zero_area_off(void)
> 
> > +{
> > +	int bmap_len = 64 * 3;
> > +	DECLARE_BITMAP(bmap, bmap_len);
> 
> I believe, everyone can realize that 64*3 == 192, and we don't need a
> variable for it.
> 
> > +	bitmap_set(bmap, 0, bmap_len);
> > +
> > +	bitmap_clear(bmap, 0, 8);
> > +	__clear_bit(50, bmap);
> > +	bitmap_clear(bmap, 60, 10);
> > +	__clear_bit(80, bmap);
> > +	bitmap_clear(bmap, 100, 10);
> > +	__clear_bit(120, bmap);
> > +	bitmap_clear(bmap, 160, 32);
> 
> Can you also test a 'dirty' region:
> 
>         bitmap_clear(60, 18);
>         __set_bit(69);
>         // find nothing
>         bitmap_find_next_zero_area_off(bmap, bmap_len, 0, 10, 0, 0));
> 
> > +
> > +	expect_eq_uint(0,
> > +		bitmap_find_next_zero_area_off(bmap, bmap_len, 0, 8, 0, 0));
> > +	expect_eq_uint(60,
> > +		bitmap_find_next_zero_area_off(bmap, bmap_len, 1, 8, 0, 0));
> > +	expect_eq_uint(60,
> > +		bitmap_find_next_zero_area_off(bmap, bmap_len, 0, 10, 0, 0));
> > +	expect_eq_uint(160,
> > +		bitmap_find_next_zero_area_off(bmap, bmap_len, 0, 32, 0, 0));
> > +	expect_eq_uint(1,
> > +		!!(bitmap_find_next_zero_area_off(bmap, bmap_len, 0, 33, 0, 0) > bmap_len));
> 
> Two last parameters are zero in all cases. Doesn't sound like an
> exhaustive testing. Real users provide non-zero alignments, so please
> you do.
> 
> > +}
> > +
> >  static void __init test_fill_set(void)
> >  {
> >  	DECLARE_BITMAP(bmap, 1024);
> > -- 
> > 2.34.1

  reply	other threads:[~2026-06-08 19:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-01  9:42 [PATCH v4 0/2] Improve the performance of bitmap_find_next_zero_area_off() Yi Sun
2026-06-01  9:42 ` [PATCH v4 1/2] lib: bitmap: reduce the number of goto again in bitmap_find_next_zero_area_off() Yi Sun
2026-06-08 22:15   ` Yury Norov
2026-06-01  9:42 ` [PATCH v4 2/2] lib/bitmap: add tests for bitmap_find_next_zero_area_off() Yi Sun
2026-06-08 19:14   ` Yury Norov
2026-06-08 19:24     ` Yury Norov [this message]
2026-06-08  7:44 ` 答复: [PATCH v4 0/2] Improve the performance of bitmap_find_next_zero_area_off() 孙毅 (Yi Sun)
2026-06-08 21:54 ` Yury Norov
2026-06-09  1:06   ` Yury Norov
2026-06-09  2:09     ` John Stultz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aicW-4W4mSzb8lBS@yury \
    --to=yury.norov@gmail.com \
    --cc=akinobu.mita@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mina86@mina86.com \
    --cc=mnazarewicz@gmail.com \
    --cc=yi.sun@unisoc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.