Re: [PATCH 0/7] Make core VMA operations internal and testable

[SeongJae Park <sj@kernel.org>]

On Wed, 3 Jul 2024 21:33:00 +0100 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:

> On Wed, Jul 03, 2024 at 01:26:53PM GMT, Andrew Morton wrote:
> > On Wed,  3 Jul 2024 12:57:31 +0100 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> >
> > > Kernel functionality is stubbed and shimmed as needed in tools/testing/vma/
> > > which contains a fully functional userland vma_internal.h file and which
> > > imports mm/vma.c and mm/vma.h to be directly tested from userland.
> >
> > Cool stuff.
> 
> Thanks :)
> 
> >
> > Now we need to make sure that anyone who messes with vma code has run
> > the tests.  And has added more testcases, if appropriate.
> >
> > Does it make sense to execute this test under selftests/ in some
> > fashion?  Quite a few people appear to be running the selftest code
> > regularly and it would be good to make them run this as well.
> 
> I think it will be useful to do that, yes, but as the tests are currently a
> skeleton to both provide the stubbing out and to provide essentially an
> example of how you might test (though enough that it'd now be easy to add a
> _ton_ of tests), it's not quite ready to be run just yet.

If we will eventually move the files under selftests/, why dont' we place the
files there from the beginning?  Is there a strict rule saying files that not
really involved with running tests or not ready cannot be added there?  If so,
could adding the files after the tests are ready to be run be an option?
Cc-ing Shuah since I think she might have a comment.

Also, I haven't had enough time to read the patches in detail but just the
cover letter a little bit.  My humble impression from that is that this might
better to eventually be kunit tests.  I know there was a discussion with Kees
on RFC v1 [1] which you kindly explained why you decide to implement this in
user space.  To my understanding, at least some of the problems are not real
problems.  For two things as examples,

1. I understand that you concern the test speed [2].  I think Kunit could be
slower than the dedicated user space tests, but to my experience, it's not that
bad when using the default UML-based execution.

2. My next humble undrestanding is that you want to test functions that you
don't want to export [2,3] to kernel modules.  To my understanding it's not
limited on Kunit.  I'm testing such DAMON functions using KUnit by including
test code in the c file but protecting it via a config.  For an example, please
refer to DAMON_KUNIT_TEST.

I understand above are only small parts of the reason for your decision, and
some of those would really unsupported by Kunit.  In the case, I think adding
this user space tests as is is good.  Nonetheless, I think it would be good to
hear some comments from Kunit developers.  IMHO, letting them know the
limitations will hopefully help setting their future TODO items.  Cc-ing
Brendan, David and Rae for that.

To recap, I have no strong opinions about this patch, but I think knowing how
Selftests and KUnit developers think could be helpful.


[1] https://lore.kernel.org/202406270957.C0E5E8057@keescook
[2] https://lore.kernel.org/5zuowniex4sxy6l7erbsg5fiirf4d4f5fbpz2upay2igiwa2xk@vuezoh2wbqf4
[3] https://lore.kernel.org/f005a7b0-ca31-4d39-b2d5-00f5546d610a@lucifer.local


Thanks,
SJ

[...]