On Thu, Feb 24, 2022 at 2:54 PM Stephen Boyd <sboyd@xxxxxxxxxx> wrote: > > Quoting Daniel Latypov (2022-02-23 14:50:59) > > On Wed, Feb 23, 2022 at 2:56 AM Maxime Ripard <maxime@xxxxxxxxxx> wrote: > > > > > > Let's test various parts of the rate-related clock API with the kunit > > > testing framework. > > > > > > Cc: kunit-dev@xxxxxxxxxxxxxxxx > > > Suggested-by: Stephen Boyd <sboyd@xxxxxxxxxx> > > > Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx> > > > > Tested-by: Daniel Latypov <dlatypov@xxxxxxxxxx> > > > > 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.
