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

[Alexander Potapenko]

On Thu, Nov 9, 2023 at 3:32 PM Alexander Lobakin
<aleksander.lobakin@xxxxxxxxx> wrote:
>
> From: Alexander Potapenko <glider@xxxxxxxxxx>
> Date: Thu, 9 Nov 2023 15:28:56 +0100
>
> >>
> >> Could we maybe rather extend __check_eq_uint to take ulongs? Doesn't
> >> seem like they differ a lot.
> >
> > We could redefine expect_eq_uint as:
> >
> > #define expect_eq_uint(x, y)           expect_eq_ulong((unsigned
> > int)(x), (unsigned int)(y))
>
> Do we need explicit casts here tho?
>
> >
> > and throw __expect_eq_uint away.
> >
> >
> >>> +     }
> >>> +     time = ktime_get() - time;
> >>> +     pr_err("Time spent in %s:\t%llu\n", __func__, time);
> >>
> >> pr_err() is for printing errors and is shown in red by some log readers.
> >> Maybe use pr_info() or pr_notice()? Definitely not an error or even warning.
> >
> > Note that test_bitmap.c has 17 calls of pr_err() and 7 calls of
> > pr_warn(), which aren't really consistent (e.g. they are used in
> > certain __check helpers instead of pr_err()), and the existing
> > performance tests are calling pr_err().
>
> Correct, and that's what caught my attention: visual grepping for bitmap
> messages makes no sense because some of them are red even thought all
> tests pass correctly.
>
> > I can change that in a separate patch, if you think it's worth the
> > effort: the error messages should probably remain pr_err(), but the
> > informational ones could be made pr_info().
>
> Sounds good to be, would be nice to see!
>
> [...]
>
> >>> @@ -1237,6 +1411,9 @@ static void __init selftest(void)
> >>>       test_bitmap_cut();
> >>>       test_bitmap_print_buf();
> >>>       test_bitmap_const_eval();
> >>> +     test_bitmap_read_write();
> >>> +     test_bitmap_read_perf();
> >>> +     test_bitmap_write_perf();
> >>>
> >>>       test_find_nth_bit();
> >>>       test_for_each_set_bit();
> >>
> >> Thanks,
> >> Olek
> >
> >
> >
> > --
> > 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
>
> Thanks,
> Olek



-- 
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