| Message ID | 20200715031120.1002016-1-vitor@massaru.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | kunit: add support to use modules | expand |
On Wed, Jul 15, 2020 at 11:11 AM Vitor Massaru Iha <vitor@massaru.org> wrote: > > Currently, KUnit does not allow the use of tests as a module. > This prevents the implementation of tests that require userspace. If this is what I think it is, thanks! I'll hopefully get a chance to play with it over the next few days. Can we clarify what this means: the current description is a little misleading, as KUnit tests can already be built and run as modules, and "tests that require userspace" is a bit broad. As I understand it, this patchset does three things: - Let kunit_tool install modules to a root filesystem and boot UML with that filesystem. - Have tests inherit the mm of the process that started them, which (if the test is in a module), provides a user-space memory context so that copy_{from,to}_user() works. - Port the test_user_copy.c tests to KUnit, using this new feature. A few comments from my quick glance over it: - The rootfs support is useful: I'm curious how it'll interact with non-UML architectures in [1]. It'd be nice for this to be extensible and to not explicitly state UML where possible. - The inheriting of the mm stuff still means that copy_{from,to}_user() will only work if loaded as a module. This really needs to be documented. (Ideally, we'd find a way of having this work even for built-in tests, but I don't have any real ideas as to how that could be done). - It'd be nice to split the test_user_copy.c test port into a separate commit. In fact, it may make sense to also split the kunit_tool changes and the mm changes into separate series, as they're both quite useful independently. Cheers, -- David > This patchset makes this possible by introducing the use of > the root filesystem in KUnit. And it allows the use of tests > that can be compiled as a module > > Vitor Massaru Iha (3): > kunit: tool: Add support root filesystem in kunit-tool > lib: Allows to borrow mm in userspace on KUnit > lib: Convert test_user_copy to KUnit test > > include/kunit/test.h | 1 + > lib/Kconfig.debug | 17 ++ > lib/Makefile | 2 +- > lib/kunit/try-catch.c | 15 +- > lib/{test_user_copy.c => user_copy_kunit.c} | 196 +++++++++----------- > tools/testing/kunit/kunit.py | 37 +++- > tools/testing/kunit/kunit_kernel.py | 105 +++++++++-- > 7 files changed, 238 insertions(+), 135 deletions(-) > rename lib/{test_user_copy.c => user_copy_kunit.c} (55%) > > > base-commit: 725aca9585956676687c4cb803e88f770b0df2b2 > prerequisite-patch-id: 582b6d9d28ce4b71628890ec832df6522ca68de0 > -- > 2.26.2 >
On Wed, Jul 15, 2020 at 11:47:11AM +0800, David Gow wrote: > - The inheriting of the mm stuff still means that > copy_{from,to}_user() will only work if loaded as a module. This > really needs to be documented. (Ideally, we'd find a way of having > this work even for built-in tests, but I don't have any real ideas as > to how that could be done). I'd like to better understand this ... are there conditions where vm_mmap() doesn't work? I thought this would either use current() (e.g. how LKDTM uses it when getting triggered from debugfs), or use init_mm. I'd really like to see the mm patch more well described/justified.
On Wed, 2020-07-15 at 11:47 +0800, David Gow wrote: > On Wed, Jul 15, 2020 at 11:11 AM Vitor Massaru Iha <vitor@massaru.org > > wrote: > > Currently, KUnit does not allow the use of tests as a module. > > This prevents the implementation of tests that require userspace. > > If this is what I think it is, thanks! I'll hopefully get a chance to > play with it over the next few days. > > Can we clarify what this means: the current description is a little > misleading, as KUnit tests can already be built and run as modules, > and "tests that require userspace" is a bit broad. > > As I understand it, this patchset does three things: > - Let kunit_tool install modules to a root filesystem and boot UML > with that filesystem. > - Have tests inherit the mm of the process that started them, which > (if the test is in a module), provides a user-space memory context so > that copy_{from,to}_user() works. > - Port the test_user_copy.c tests to KUnit, using this new feature. > > A few comments from my quick glance over it: > - The rootfs support is useful: I'm curious how it'll interact with > non-UML architectures in [1]. It'd be nice for this to be extensible > and to not explicitly state UML where possible. Hm, I didn't think about other architectures. Which ones are you thinking ? > - The inheriting of the mm stuff still means that > copy_{from,to}_user() will only work if loaded as a module. This > really needs to be documented. (Ideally, we'd find a way of having > this work even for built-in tests, but I don't have any real ideas as > to how that could be done). Sure, I'll write the documentation. > - It'd be nice to split the test_user_copy.c test port into a > separate > commit. In fact, it may make sense to also split the kunit_tool > changes and the mm changes into separate series, as they're both > quite > useful independently. > I'll do it. Thanks for the review.
On Wed, 2020-07-15 at 19:41 -0700, Kees Cook wrote: > On Wed, Jul 15, 2020 at 11:47:11AM +0800, David Gow wrote: > > - The inheriting of the mm stuff still means that > > copy_{from,to}_user() will only work if loaded as a module. This > > really needs to be documented. (Ideally, we'd find a way of having > > this work even for built-in tests, but I don't have any real ideas > > as > > to how that could be done). > > I'd like to better understand this ... are there conditions where > vm_mmap() doesn't work? I thought this would either use current() > (e.g. > how LKDTM uses it when getting triggered from debugfs), or use > init_mm. > > I'd really like to see the mm patch more well described/justified. > Sure, I'll describe the patch better. Thanks for the review.
Currently, KUnit does not allow the use of tests as a module. This prevents the implementation of tests that require userspace. This patchset makes this possible by introducing the use of the root filesystem in KUnit. And it allows the use of tests that can be compiled as a module Vitor Massaru Iha (3): kunit: tool: Add support root filesystem in kunit-tool lib: Allows to borrow mm in userspace on KUnit lib: Convert test_user_copy to KUnit test include/kunit/test.h | 1 + lib/Kconfig.debug | 17 ++ lib/Makefile | 2 +- lib/kunit/try-catch.c | 15 +- lib/{test_user_copy.c => user_copy_kunit.c} | 196 +++++++++----------- tools/testing/kunit/kunit.py | 37 +++- tools/testing/kunit/kunit_kernel.py | 105 +++++++++-- 7 files changed, 238 insertions(+), 135 deletions(-) rename lib/{test_user_copy.c => user_copy_kunit.c} (55%) base-commit: 725aca9585956676687c4cb803e88f770b0df2b2 prerequisite-patch-id: 582b6d9d28ce4b71628890ec832df6522ca68de0