Re: [igt-dev] [PATCH i-g-t v2 4/4] tests: DRM selftests: switch to KUnit

2022-11-01 Thread Daniel Latypov
On Tue, Nov 1, 2022, 6:53 AM Mauro Carvalho Chehab
 wrote:
> > Ah, the selection is in kunit.ko itself. kunit.filter_glob, set from
> > kunit.py with
> >
> > positional arguments:
> >   filter_glob   Filter which KUnit test suites/tests run at 
> > boot-time, e.g. list* or list*.*del_test
> > \
>
> Gah, permissions are 0:
>
> module_param_named(filter_glob, filter_glob_param, charp, 0);
>
> You can't even see what filters were set via sysfs:
>
> $ ls /sys/module/kunit/parameters/
> stats_enabled
>
> So, once set, there's no way to change it.
>
> I wonder why this isn't 0x644.
>
> It sounds that, if we want to control it at runtime, we'll need to
> build with kunit as module, and unload/reload it every time if/when
> we want to run IGT with a different filtering ruleset.

kunit.filter_glob only works on built-in tests, which is why you can
only set it once atm.
There is no currently no way for KUnit to run a subset of tests in modules.

The module init code is here:
https://elixir.bootlin.com/linux/v6.0/source/lib/kunit/test.c#L620
As you can see, that func it calls just initializes and runs all the
suites: https://elixir.bootlin.com/linux/v6.0/source/lib/kunit/test.c#L589

This can of course be changed, just needs some work on the KUnit-side.

Daniel


Re: [igt-dev] [PATCH i-g-t v2 4/4] tests: DRM selftests: switch to KUnit

2022-11-01 Thread Mauro Carvalho Chehab
On Tue, 1 Nov 2022 15:16:02 +0200
Petri Latvala  wrote:

> On Tue, Nov 01, 2022 at 01:54:25PM +0100, Mauro Carvalho Chehab wrote:
> > On Tue, 20 Sep 2022 11:18:25 +0300
> > Petri Latvala  wrote:
> > 
> > > On Sun, Aug 28, 2022 at 09:09:20PM -0300, Isabella Basso wrote:
> > > > As the DRM selftests are now using KUnit [1], update IGT tests as well.
> > > > 
> > > > [1] - 
> > > > https://lore.kernel.org/all/20220708203052.236290-1-maira.ca...@usp.br/
> > > > 
> > > > Signed-off-by: Isabella Basso 
> > > > ---
> > > >  tests/drm_buddy.c|  7 ---
> > > >  tests/drm_mm.c   |  7 ---
> > > >  tests/kms_selftest.c | 12 +---
> > > >  3 files changed, 17 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/tests/drm_buddy.c b/tests/drm_buddy.c
> > > > index 06876e0c..74c06dce 100644
> > > > --- a/tests/drm_buddy.c
> > > > +++ b/tests/drm_buddy.c
> > > > @@ -6,9 +6,10 @@
> > > >  #include "igt.h"
> > > >  #include "igt_kmod.h"
> > > >  
> > > > -IGT_TEST_DESCRIPTION("Basic sanity check of DRM's buddy allocator 
> > > > (struct drm_buddy)");
> > > > +IGT_TEST_DESCRIPTION("Basic sanity check of DRM's buddy allocator 
> > > > (struct \
> > > > + drm_buddy) using KUnit");
> > > >  
> > > > -igt_main
> > > > +igt_simple_main
> > > >  {
> > > > -   igt_kselftests("test-drm_buddy", NULL, NULL, NULL);
> > > > +   igt_kunit("drm_buddy_test", NULL);
> > > >  }  
> > > 
> > > With kselftests we had the ability to only run a specified set of
> > > tests, and with or without selection, we had sub-results for each
> > > through dynamic subtests. Does kunit have that possibility? I mean I
> > > know kunit itself does but what about igt_kunit()?
> > 
> > I don't think so. I created a KUnit with the mock selftests:
> > 
> > $ modinfo test-i915-mock
> > filename:   
> > /lib/modules/6.1.0-rc2-drm-b1cab854a639+/kernel/drivers/gpu/drm/i915/test-i915-mock.ko
> > import_ns:  I915_SELFTEST
> > license:GPL v2
> > depends:i915
> > retpoline:  Y
> > intree: Y
> > name:   test_i915_mock
> > vermagic:   6.1.0-rc2-drm-b1cab854a639+ SMP preempt mod_unload 
> > 
> > There's no module parameters. Just loading it makes all tests
> > there to run.
> 
> Ah, the selection is in kunit.ko itself. kunit.filter_glob, set from
> kunit.py with
> 
> positional arguments:
>   filter_glob   Filter which KUnit test suites/tests run at 
> boot-time, e.g. list* or list*.*del_test
> \

Gah, permissions are 0:

module_param_named(filter_glob, filter_glob_param, charp, 0);

You can't even see what filters were set via sysfs:

$ ls /sys/module/kunit/parameters/
stats_enabled

So, once set, there's no way to change it.

I wonder why this isn't 0x644.

It sounds that, if we want to control it at runtime, we'll need to 
build with kunit as module, and unload/reload it every time if/when
we want to run IGT with a different filtering ruleset.

Regards,
Mauro


Re: [igt-dev] [PATCH i-g-t v2 4/4] tests: DRM selftests: switch to KUnit

2022-11-01 Thread Petri Latvala
On Tue, Nov 01, 2022 at 01:54:25PM +0100, Mauro Carvalho Chehab wrote:
> On Tue, 20 Sep 2022 11:18:25 +0300
> Petri Latvala  wrote:
> 
> > On Sun, Aug 28, 2022 at 09:09:20PM -0300, Isabella Basso wrote:
> > > As the DRM selftests are now using KUnit [1], update IGT tests as well.
> > > 
> > > [1] - 
> > > https://lore.kernel.org/all/20220708203052.236290-1-maira.ca...@usp.br/
> > > 
> > > Signed-off-by: Isabella Basso 
> > > ---
> > >  tests/drm_buddy.c|  7 ---
> > >  tests/drm_mm.c   |  7 ---
> > >  tests/kms_selftest.c | 12 +---
> > >  3 files changed, 17 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/tests/drm_buddy.c b/tests/drm_buddy.c
> > > index 06876e0c..74c06dce 100644
> > > --- a/tests/drm_buddy.c
> > > +++ b/tests/drm_buddy.c
> > > @@ -6,9 +6,10 @@
> > >  #include "igt.h"
> > >  #include "igt_kmod.h"
> > >  
> > > -IGT_TEST_DESCRIPTION("Basic sanity check of DRM's buddy allocator 
> > > (struct drm_buddy)");
> > > +IGT_TEST_DESCRIPTION("Basic sanity check of DRM's buddy allocator 
> > > (struct \
> > > +   drm_buddy) using KUnit");
> > >  
> > > -igt_main
> > > +igt_simple_main
> > >  {
> > > - igt_kselftests("test-drm_buddy", NULL, NULL, NULL);
> > > + igt_kunit("drm_buddy_test", NULL);
> > >  }  
> > 
> > With kselftests we had the ability to only run a specified set of
> > tests, and with or without selection, we had sub-results for each
> > through dynamic subtests. Does kunit have that possibility? I mean I
> > know kunit itself does but what about igt_kunit()?
> 
> I don't think so. I created a KUnit with the mock selftests:
> 
>   $ modinfo test-i915-mock
>   filename:   
> /lib/modules/6.1.0-rc2-drm-b1cab854a639+/kernel/drivers/gpu/drm/i915/test-i915-mock.ko
>   import_ns:  I915_SELFTEST
>   license:GPL v2
>   depends:i915
>   retpoline:  Y
>   intree: Y
>   name:   test_i915_mock
>   vermagic:   6.1.0-rc2-drm-b1cab854a639+ SMP preempt mod_unload 
> 
> There's no module parameters. Just loading it makes all tests
> there to run.

Ah, the selection is in kunit.ko itself. kunit.filter_glob, set from
kunit.py with

positional arguments:
  filter_glob   Filter which KUnit test suites/tests run at boot-time, 
e.g. list* or list*.*del_test



-- 
Petri Latvala


Re: [igt-dev] [PATCH i-g-t v2 4/4] tests: DRM selftests: switch to KUnit

2022-11-01 Thread Mauro Carvalho Chehab
On Tue, 20 Sep 2022 11:18:25 +0300
Petri Latvala  wrote:

> On Sun, Aug 28, 2022 at 09:09:20PM -0300, Isabella Basso wrote:
> > As the DRM selftests are now using KUnit [1], update IGT tests as well.
> > 
> > [1] - 
> > https://lore.kernel.org/all/20220708203052.236290-1-maira.ca...@usp.br/
> > 
> > Signed-off-by: Isabella Basso 
> > ---
> >  tests/drm_buddy.c|  7 ---
> >  tests/drm_mm.c   |  7 ---
> >  tests/kms_selftest.c | 12 +---
> >  3 files changed, 17 insertions(+), 9 deletions(-)
> > 
> > diff --git a/tests/drm_buddy.c b/tests/drm_buddy.c
> > index 06876e0c..74c06dce 100644
> > --- a/tests/drm_buddy.c
> > +++ b/tests/drm_buddy.c
> > @@ -6,9 +6,10 @@
> >  #include "igt.h"
> >  #include "igt_kmod.h"
> >  
> > -IGT_TEST_DESCRIPTION("Basic sanity check of DRM's buddy allocator (struct 
> > drm_buddy)");
> > +IGT_TEST_DESCRIPTION("Basic sanity check of DRM's buddy allocator (struct \
> > + drm_buddy) using KUnit");
> >  
> > -igt_main
> > +igt_simple_main
> >  {
> > -   igt_kselftests("test-drm_buddy", NULL, NULL, NULL);
> > +   igt_kunit("drm_buddy_test", NULL);
> >  }  
> 
> With kselftests we had the ability to only run a specified set of
> tests, and with or without selection, we had sub-results for each
> through dynamic subtests. Does kunit have that possibility? I mean I
> know kunit itself does but what about igt_kunit()?

I don't think so. I created a KUnit with the mock selftests:

$ modinfo test-i915-mock
filename:   
/lib/modules/6.1.0-rc2-drm-b1cab854a639+/kernel/drivers/gpu/drm/i915/test-i915-mock.ko
import_ns:  I915_SELFTEST
license:GPL v2
depends:i915
retpoline:  Y
intree: Y
name:   test_i915_mock
vermagic:   6.1.0-rc2-drm-b1cab854a639+ SMP preempt mod_unload 

There's no module parameters. Just loading it makes all tests
there to run.

OK, with the current implementation, we could have one module per subtest, 
but that sounds overkill. 

Another possibility would be to implement it like:

test-i915-mock-fences
test-i915-mock-hugepages
...

which would allow a finer control. Another possibility would be to
change kunit_suite and kunit_test_suites() to optionally create
a modprobe parameter to allow specifying what tests from the test
suites would run.

On the other hand, not sure if are there any gains implementing it,
at least for the hardware-independent tests. I mean, if one of such
tests fail after a patch, the change is broken and should be reverted,
as this is a regression.

> Orthogonal to that question, drm_mm and test-drm_modeset are _now_
> using kunit but in LTS kernels they're not, and we'd like to be able
> to keep testing those. That means having both launchers here and just
> letting the "incorrect" one produce a 'skip'.

Agreed. Patch 4/4 should be checking if the test module is there.
If not, fall back to selftest.

Regards,
Mauro



Re: [igt-dev] [PATCH i-g-t v2 4/4] tests: DRM selftests: switch to KUnit

2022-09-20 Thread Petri Latvala
On Sun, Aug 28, 2022 at 09:09:20PM -0300, Isabella Basso wrote:
> As the DRM selftests are now using KUnit [1], update IGT tests as well.
> 
> [1] - https://lore.kernel.org/all/20220708203052.236290-1-maira.ca...@usp.br/
> 
> Signed-off-by: Isabella Basso 
> ---
>  tests/drm_buddy.c|  7 ---
>  tests/drm_mm.c   |  7 ---
>  tests/kms_selftest.c | 12 +---
>  3 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/drm_buddy.c b/tests/drm_buddy.c
> index 06876e0c..74c06dce 100644
> --- a/tests/drm_buddy.c
> +++ b/tests/drm_buddy.c
> @@ -6,9 +6,10 @@
>  #include "igt.h"
>  #include "igt_kmod.h"
>  
> -IGT_TEST_DESCRIPTION("Basic sanity check of DRM's buddy allocator (struct 
> drm_buddy)");
> +IGT_TEST_DESCRIPTION("Basic sanity check of DRM's buddy allocator (struct \
> +   drm_buddy) using KUnit");
>  
> -igt_main
> +igt_simple_main
>  {
> - igt_kselftests("test-drm_buddy", NULL, NULL, NULL);
> + igt_kunit("drm_buddy_test", NULL);
>  }

With kselftests we had the ability to only run a specified set of
tests, and with or without selection, we had sub-results for each
through dynamic subtests. Does kunit have that possibility? I mean I
know kunit itself does but what about igt_kunit()?

Orthogonal to that question, drm_mm and test-drm_modeset are _now_
using kunit but in LTS kernels they're not, and we'd like to be able
to keep testing those. That means having both launchers here and just
letting the "incorrect" one produce a 'skip'.


-- 
Petri Latvala


> diff --git a/tests/drm_mm.c b/tests/drm_mm.c
> index 2052b115..75fc6682 100644
> --- a/tests/drm_mm.c
> +++ b/tests/drm_mm.c
> @@ -24,9 +24,10 @@
>  #include "igt.h"
>  #include "igt_kmod.h"
>  
> -IGT_TEST_DESCRIPTION("Basic sanity check of DRM's range manager (struct 
> drm_mm)");
> +IGT_TEST_DESCRIPTION("Basic sanity check of DRM's range manager (struct 
> drm_mm)\
> +  using KUnit");
>  
> -igt_main
> +igt_simple_main
>  {
> - igt_kselftests("test-drm_mm", NULL, NULL, NULL);
> + igt_kunit("drm_mm_test", NULL);
>  }
> diff --git a/tests/kms_selftest.c b/tests/kms_selftest.c
> index abc4bfe9..bbf24c2b 100644
> --- a/tests/kms_selftest.c
> +++ b/tests/kms_selftest.c
> @@ -24,9 +24,15 @@
>  #include "igt.h"
>  #include "igt_kmod.h"
>  
> -IGT_TEST_DESCRIPTION("Basic sanity check of KMS selftests.");
> +IGT_TEST_DESCRIPTION("Basic sanity check of KMS selftests using KUnit");
>  
> -igt_main
> +igt_simple_main
>  {
> - igt_kselftests("test-drm_modeset", NULL, NULL, NULL);
> + igt_kunit("drm_cmdline_parser_test", NULL);
> + igt_kunit("drm_damage_helper_test", NULL);
> + igt_kunit("drm_dp_mst_helper_test", NULL);
> + igt_kunit("drm_format_helper_test", NULL);
> + igt_kunit("drm_format_test", NULL);
> + igt_kunit("drm_framebuffer_test", NULL);
> + igt_kunit("drm_plane_helper_test", NULL);
>  }
> -- 
> 2.37.2
>