linux-kernel - Re: [PATCH v8 2/2] lib/test_bitmap: add tests for bitmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAG_fn=VOJM5Ok+rvTMB08TKW=1xkU+LmMDWEPugxBaE0XgbL_w@mail.gmail.com>
Date:   Mon, 23 Oct 2023 15:50:42 +0200
From:   Alexander Potapenko <glider@...gle.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     catalin.marinas@....com, will@...nel.org, pcc@...gle.com,
        andreyknvl@...il.com, aleksander.lobakin@...el.com,
        linux@...musvillemoes.dk, yury.norov@...il.com,
        alexandru.elisei@....com, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, eugenis@...gle.com,
        syednwaris@...il.com, william.gray@...aro.org
Subject: Re: [PATCH v8 2/2] lib/test_bitmap: add tests for bitmap_{read,write}()

On Mon, Oct 23, 2023 at 1:32 PM Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> 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