mbox series

[v3,00/10] iov_iter: kunit: Cleanup, abstraction and more tests

Message ID 20231115154946.3933808-1-dhowells@redhat.com (mailing list archive)
Headers show
Series iov_iter: kunit: Cleanup, abstraction and more tests | expand

Message

David Howells Nov. 15, 2023, 3:49 p.m. UTC
Hi Christian,

Can you take this through the filesystem tree?

These patches make some changes to the kunit tests previously added for
iov_iter testing, in particular adding testing of UBUF/IOVEC iterators and
some benchmarking:

 (1) Clean up a couple of checkpatch style complaints.

 (2) Consolidate some repeated bits of code into helper functions and use
     the same struct to represent straight offset/address ranges and
     partial page lists.

 (3) Add a function to set up a userspace VM, attach the VM to the kunit
     testing thread, create an anonymous file, stuff some pages into the
     file and map the file into the VM to act as a buffer that can be used
     with UBUF/IOVEC iterators.

     I map an anonymous file with pages attached rather than using MAP_ANON
     so that I can check the pages obtained from iov_iter_extract_pages()
     without worrying about them changing due to swap, migrate, etc..

     [?] Is this the best way to do things?  Mirroring execve, it requires
     a number of extra core symbols to be exported.  Should this be done in
     the core code?

 (4) Add tests for copying into and out of UBUF and IOVEC iterators.

 (5) Add tests for extracting pages from UBUF and IOVEC iterators.

 (6) Add tests to benchmark copying 256MiB to UBUF, IOVEC, KVEC, BVEC and
     XARRAY iterators.

 (7) Add a test to bencmark copying 256MiB from an xarray that gets decanted
     into 256-page BVEC iterators to model batching from the pagecache.

 (8) Add a test to benchmark copying 256MiB through dynamically allocated
     256-page bvecs to simulate bio construction.

Example benchmarks output:

	iov_kunit_benchmark_ubuf: avg 4474 uS, stddev 1340 uS
	iov_kunit_benchmark_iovec: avg 6619 uS, stddev 23 uS
	iov_kunit_benchmark_kvec: avg 2672 uS, stddev 14 uS
	iov_kunit_benchmark_bvec: avg 3189 uS, stddev 19 uS
	iov_kunit_benchmark_bvec_split: avg 3403 uS, stddev 8 uS
	iov_kunit_benchmark_xarray: avg 3709 uS, stddev 7 uS

I've pushed the patches here also:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=iov-kunit

David

Changes
=======
ver #3)
 - #include <linux/personality.h> to get READ_IMPLIES_EXEC.
 - Add a test to benchmark decanting an xarray into bio_vecs.

ver #2)
 - Use MAP_ANON to make the user buffer if we don't want a list of pages.
 - KUNIT_ASSERT_NOT_ERR_OR_NULL() doesn't like __user pointers as the
   condition, so cast.
 - Make the UBUF benchmark loop, doing an iterator per page so that the
   overhead from the iterator code is not negligible.
 - Make the KVEC benchmark use an iovec per page so that the iteration is
   not not negligible.
 - Switch the benchmarking to use copy_from_iter() so that only a single
   page is needed in the userspace buffer (as it can be shared R/O), not
   256MiB's worth.

Link: https://lore.kernel.org/r/20230914221526.3153402-1-dhowells@redhat.com/ # v1
Link: https://lore.kernel.org/r/20230920130400.203330-1-dhowells@redhat.com/ # v2
Link: https://lore.kernel.org/r/20230922113038.1135236-1-dhowells@redhat.com/ # v3

David Howells (10):
  iov_iter: Fix some checkpatch complaints in kunit tests
  iov_iter: Consolidate some of the repeated code into helpers
  iov_iter: Consolidate the test vector struct in the kunit tests
  iov_iter: Consolidate bvec pattern checking
  iov_iter: Create a function to prepare userspace VM for UBUF/IOVEC
    tests
  iov_iter: Add copy kunit tests for ITER_UBUF and ITER_IOVEC
  iov_iter: Add extract kunit tests for ITER_UBUF and ITER_IOVEC
  iov_iter: Add benchmarking kunit tests
  iov_iter: Add kunit to benchmark decanting of xarray to bvec
  iov_iter: Add benchmarking kunit tests for UBUF/IOVEC

 arch/s390/kernel/vdso.c |    1 +
 fs/anon_inodes.c        |    1 +
 kernel/fork.c           |    2 +
 lib/kunit_iov_iter.c    | 1317 +++++++++++++++++++++++++++++++++------
 mm/mmap.c               |    1 +
 mm/util.c               |    3 +
 6 files changed, 1139 insertions(+), 186 deletions(-)

Comments

Linus Torvalds Nov. 15, 2023, 4:04 p.m. UTC | #1
On Wed, 15 Nov 2023 at 10:50, David Howells <dhowells@redhat.com> wrote:
>
>  (3) Add a function to set up a userspace VM, attach the VM to the kunit
>      testing thread, create an anonymous file, stuff some pages into the
>      file and map the file into the VM to act as a buffer that can be used
>      with UBUF/IOVEC iterators.
>
>      I map an anonymous file with pages attached rather than using MAP_ANON
>      so that I can check the pages obtained from iov_iter_extract_pages()
>      without worrying about them changing due to swap, migrate, etc..
>
>      [?] Is this the best way to do things?  Mirroring execve, it requires
>      a number of extra core symbols to be exported.  Should this be done in
>      the core code?

Do you really need to do this as a kunit test in the kernel itself?

Why not just make it a user-space test as part of tools/testing/selftests?

That's what it smells like to me. You're doing user-level tests, but
you're doing them in the wrong place, so you need to jump through all
these hoops that you really shouldn't.

                Linus