Subject: Re: [PATCH v2 1/4] selftests: add tests_sysfs module

From: Greg KH <[email-masked]>
Date: Wed, 21 Jul 2021 13:32:41 +0200

On Fri, Jul 02, 2021 at 05:46:29PM -0700, Luis Chamberlain wrote:
> This adds a new selftest module which can be used to test sysfs, which
> would otherwise require using an existing driver. This lets us muck
> with a template driver to test breaking things without affecting
> system behaviour or requiring the dependencies of a real device
> driver.
> 
> A series of 28 tests are added. Support for using two device types are
> supported:
> 
>   * misc
>   * block
> 
> Contrary to sysctls, sysfs requires a full write to happen at once, and
> so we reduce the digit tests to single writes. Two main sysfs knobs are
> provided for testing reading/storing, one which doesn't inclur any
> delays and another which can incur programmed delays. What locks are
> held, if any, are configurable, at module load time, or through dynamic
> configuration at run time.
> 
> Since sysfs is a technically filesystem, but a pseudo one, which
> requires a kernel user, our test_sysfs module and respective test script
> embraces fstests format for tests in the kernel ring bufffer. Likewise,
> a scraper for kernel crashes is provided which matches what fstests does
> as well.
> 
> Two tests are kept disabled as they currently cause a deadlock, and so
> this provides a mechanism to easily show proof and demo how the deadlock
> can happen:
> 
> Demos the deadlock with a device specific lock
> ./tools/testing/selftests/sysfs/sysfs.sh -t 0027
> 
> Demos the deadlock with rtnl_lock()
> ./tools/testing/selftests/sysfs/sysfs.sh -t 0028
> 
> Two separate solutions to the deadlock issue have been proposed,
> and so now its a matter of either documenting this limitation or
> eventually adopting a generic fix.
> 
> This selftests will shortly be expanded upon with more tests which
> require further kernel changes in order to provide better test
> coverage.

Why is this not using kunit?  We should not be adding new in-kernel
tests that are not using that api anymore.

> 
> Signed-off-by: Luis Chamberlain <[email-masked]>
> ---
>  MAINTAINERS                            |    7 +
>  lib/Kconfig.debug                      |   10 +
>  lib/Makefile                           |    1 +
>  lib/test_sysfs.c                       |  943 +++++++++++++++++++
>  tools/testing/selftests/sysfs/Makefile |   12 +
>  tools/testing/selftests/sysfs/config   |    2 +
>  tools/testing/selftests/sysfs/sysfs.sh | 1202 ++++++++++++++++++++++++
>  7 files changed, 2177 insertions(+)
>  create mode 100644 lib/test_sysfs.c
>  create mode 100644 tools/testing/selftests/sysfs/Makefile
>  create mode 100644 tools/testing/selftests/sysfs/config
>  create mode 100755 tools/testing/selftests/sysfs/sysfs.sh
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 66d047dc6880..fd369ed50040 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17958,6 +17958,13 @@ L:	[email-masked]
>  S:	Maintained
>  F:	drivers/mmc/host/sdhci-pci-dwc-mshc.c
>  
> +SYSFS TEST DRIVER
> +M:	Luis Chamberlain <[email-masked]>
> +L:	[email-masked]
> +S:	Maintained
> +F:	lib/test_sysfs.c
> +F:	tools/testing/selftests/sysfs/
> +
>  SYSTEM CONFIGURATION (SYSCON)
>  M:	Lee Jones <[email-masked]>
>  M:	Arnd Bergmann <[email-masked]>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index fb370c7c4756..568838ac1189 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2360,6 +2360,16 @@ config TEST_SYSCTL
>  
>  	  If unsure, say N.
>  
> +config TEST_SYSFS
> +	tristate "sysfs test driver"
> +	depends on SYSFS
> +	help
> +	  This builds the "test_sysfs" module. This driver enables to test the
> +	  sysfs file system safely without affecting production knobs which
> +	  might alter system functionality.
> +
> +	  If unsure, say N.
> +
>  config BITFIELD_KUNIT
>  	tristate "KUnit test bitfield functions at runtime"
>  	depends on KUNIT
> diff --git a/lib/Makefile b/lib/Makefile
> index 5efd1b435a37..effd1ef806f0 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o
>  obj-$(CONFIG_TEST_BITOPS) += test_bitops.o
>  CFLAGS_test_bitops.o += -Werror
>  obj-$(CONFIG_TEST_SYSCTL) += test_sysctl.o
> +obj-$(CONFIG_TEST_SYSFS) += test_sysfs.o
>  obj-$(CONFIG_TEST_HASH) += test_hash.o test_siphash.o
>  obj-$(CONFIG_TEST_IDA) += test_ida.o
>  obj-$(CONFIG_KASAN_KUNIT_TEST) += test_kasan.o
> diff --git a/lib/test_sysfs.c b/lib/test_sysfs.c
> new file mode 100644
> index 000000000000..bf43016d40b5
> --- /dev/null
> +++ b/lib/test_sysfs.c
> @@ -0,0 +1,943 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later

This does not match your "boiler-plate" text below, sorry.

thanks,

greg k-h