mirror of https://lore.kernel.org/lkml/
 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: 13+ 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-17  8:55     ` 答复: " 孙毅 (Yi Sun)
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
2026-06-17  9:00     ` 答复: " 孙毅 (Yi Sun)
2026-06-11 10:56   ` Michał Nazarewicz

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox