Re: [PATCH v8 2/2] lib/test_bitmap: add tests for bitmap_{read,write}()

From: Alexander Potapenko
Date: Mon Oct 23 2023 - 09:51:27 EST


On Mon, Oct 23, 2023 at 1:32 PM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> On Mon, Oct 23, 2023 at 12:23:27PM +0200, Alexander Potapenko wrote:
> > Add basic tests ensuring that values can be added at arbitrary positions
> > of the bitmap, including those spanning into the adjacent unsigned
> > longs.
>
> ...
>
> > + val = bitmap_read(bitmap, i, nbits);
> > + (void)val;
>
> Is it marked with __must_check? Otherwise why do we need this?

That was a weak attempt to prevent the compiler from completely
optimizing away the bitmap_read() calls, but I haven't really looked
at the result.
The reality is that even with this check the calls are deleted, and
the size of the function is only 68 bytes.
Replacing the val assignment with a WRITE_ONCE() actually ensures the
bitmap_read() calls are not deleted:

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index ba567f53feff1..ae57ae48ef3ad 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -1360,8 +1360,7 @@ static void __init test_bitmap_read_perf(void)
for (i = 0; i < TEST_BIT_LEN; i++) {
if (i + nbits > TEST_BIT_LEN)
break;
- val = bitmap_read(bitmap, i, nbits);
- (void)val;
+ WRITE_ONCE(val, bitmap_read(bitmap, i, nbits));
}
}
}


> --
> With Best Regards,
> Andy Shevchenko
>
>


--
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg