[PATCH v6 02/12] clk: Introduce Kunit Tests for the framework

[Daniel Latypov]

On Thu, Feb 24, 2022 at 2:54 PM Stephen Boyd <sboyd at kernel.org> wrote:
>
> Quoting Daniel Latypov (2022-02-23 14:50:59)
> > On Wed, Feb 23, 2022 at 2:56 AM Maxime Ripard <maxime at cerno.tech> wrote:
> > >
> > > Let's test various parts of the rate-related clock API with the kunit
> > > testing framework.
> > >
> > > Cc: kunit-dev at googlegroups.com
> > > Suggested-by: Stephen Boyd <sboyd at kernel.org>
> > > Signed-off-by: Maxime Ripard <maxime at cerno.tech>
> >
> > Tested-by: Daniel Latypov <dlatypov at google.com>
> >
> > Looks good to me on the KUnit side.
> > Two small nits below.
> >
> > FYI, I computed the incremental coverage for this series, i.e.:
> > 1) applied the full series
> > 2) computed the absolute coverage
> >
> > $  ./tools/testing/kunit/kunit.py run  --kunitconfig=drivers/clk
> > --make_options=CC=/usr/bin/gcc-6 --kconfig_add=CONFIG_DEBUG_KERNEL=y
> > --kconfig_add=CONFIG_DEBUG_INFO=y --kconfig_add=CONFIG_GCOV=y
> > $ lcov -t "clk_tests" -o coverage.info -c -d .kunit/ --gcov-tool=/usr/bin/gcov-6
>
> This is cool. Thanks! Is it possible to add some 'coverage' command to
> kunit so we don't have to recall this invocation?

This is documented at
https://www.kernel.org/doc/html/latest/dev-tools/kunit/running_tips.html#generating-code-coverage-reports-under-uml
It also includes pointers on how to use lcov to process the .gcda files.
I wrote it before --kconfig_add existed, so it just looks a bit different.

The main blockers to directly supporting this in kunit.py are
1.) this only works on UML
2.) it needs gcc-6 or lower (and the kernel's min version is 5.1, iirc)...
3.) in kernels older than 5.14, this requires some more hacks to get
working. So for the large portion of us stuck dealing with somewhat
older kernels, we'd have to do stuff manually anyway.

For #1, we'd need different kconfig options and kunit.py's QEMU would
need some sort of userspace (busybox should be sufficient).
For #2, I don't recall what the precise issues were anymore. But I
think there were some more issues in gcc 8 or 9... :(

>
> >
> > 3) intersected that with the total diff
>
> This would also be cool to do automatically with a revision range.

Hmm, can you elaborate?
I assume you mean other revision ranges beyond this patch set?

My script roughly looks like

$ incremental_cli --diff_file=a.diff --lcov_file=coverage.info
I can generate a.diff in any way I want.

For these numbers I did
$ git diff clk-next/clk-next > a.diff
But I could do
$ git diff HEAD~3 > a.diff
or anything else.

Just need to make sure the endpoint of the range is the one we
generated coverage at so the line #s match up.

So if you have some specific requests, I can get those generated as well.

I would share my incremental_cli script, but it depends on some
internal code (an LCOV parser).
I don't quite understand how to use lcov's --diff option, but maybe it
supports this?
I just saw "to merge coverage data from different source code levels"
in the man page and figured it wasn't relevant.

>
> >
> > Incremental coverage for 3/9 files in --diff_file
> > Total incremental: 99.29% coverage (281/283 lines)
> >   drivers/clk/clk.c: 84.62% coverage (11/13 lines)
> >   drivers/clk/clk_test.c: 100.00% coverage (269/269 lines)
> >   include/linux/clk.h: 100.00% coverage (1/1 lines)
> >
> > Missing lines are drivers/clk/clk.c:2397-8, i.e. this part of the diff:
> > +       if (ret) {
> > +               /* rollback the changes */
> > +               clk->min_rate = old_min; <- 2397
> > +               clk->max_rate = old_max; <- 2398
> >
> > These are from before and were just moved around.
>
> We could trigger a failure in the provider when the rate is set, and
> then we could call round_rate() again and make sure the boundaries from
> before are maintained.