2023-12-14 08:50:00

by David Gow

[permalink] [raw]
Subject: [PATCH v3 0/5] kunit: Add helpers for creating test-managed devices

KUnit tests often need to provide a struct device, and thus far have
mostly been using root_device_register() or platform devices to create
a 'fake device' for use with, e.g., code which uses device-managed
resources. This has several disadvantages, including not being designed
for test use, scattering files in sysfs, and requiring manual teardown
on test exit, which may not always be possible in case of failure.

Instead, introduce a set of helper functions which allow devices
(internally a struct kunit_device) to be created and managed by KUnit --
i.e., they will be automatically unregistered on test exit. These
helpers can either use a user-provided struct device_driver, or have one
automatically created and managed by KUnit. In both cases, the device
lives on a new kunit_bus.

This is a follow-up to a previous proposal here:
https://lore.kernel.org/linux-kselftest/[email protected]/

(The kunit_defer() function in the first patch there has since been
merged as the 'deferred actions' feature.)

My intention is to take this whole series in via the kselftest/kunit
branch, but I'm equally okay with splitting up the later patches which
use this to go via the various subsystem trees in case there are merge
conflicts.

Cheers,
-- David

Signed-off-by: David Gow <[email protected]>
---
Changes in v3:
- Port the DRM tests to these new helpers (Thanks, Maxime!)
- Include the lib/kunit/device-impl.h file, which was missing from the
previous revision.
- Fix a use-after-free bug in kunit_device_driver_test, which resulted
in memory corruption on some clang-built UML builds.
- The 'test_state' is now allocated with kunit_kzalloc(), not on the
stack, as the stack will be gone when cleanup occurs.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Simplify device/driver/bus matching, removing the no-longer-required
kunit_bus_match function. (Thanks, Greg)
- The return values are both more consistent (kunit_device_register now
returns an explicit error pointer, rather than failing the test), and
better documented.
- Add some basic documentation to the implementations as well as the
headers. The documentation in the headers is still more complete, and
is now properly compiled into the HTML documentation (under
dev-tools/kunit/api/resources.html). (Thanks, Matti)
- Moved the internal-only kunit_bus_init() function to a private header,
lib/kunit/device-impl.h to avoid polluting the public headers, and
match other internal-only headers. (Thanks, Greg)
- Alphabetise KUnit includes in other test modules. (Thanks, Amadeusz.)
- Several code cleanups, particularly around error handling and
allocation. (Thanks Greg, Maxime)
- Several const-correctness and casting improvements. (Thanks, Greg)
- Added a new test to verify KUnit cleanup triggers device cleanup.
(Thanks, Maxime).
- Improved the user-specified device test to verify that probe/remove
hooks are called correctly. (Thanks, Maxime).
- The overflow test no-longer needlessly calls
kunit_device_unregister().
- Several other minor cleanups and documentation improvements, which
hopefully make this a bit clearer and more robust.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
David Gow (4):
kunit: Add APIs for managing devices
fortify: test: Use kunit_device
overflow: Replace fake root_device with kunit_device
ASoC: topology: Replace fake root_device with kunit_device in tests

Maxime Ripard (1):
drm/tests: Switch to kunit devices

Documentation/dev-tools/kunit/api/resource.rst | 9 ++
Documentation/dev-tools/kunit/usage.rst | 50 +++++++
drivers/gpu/drm/tests/drm_kunit_helpers.c | 66 +--------
include/kunit/device.h | 80 +++++++++++
lib/fortify_kunit.c | 5 +-
lib/kunit/Makefile | 3 +-
lib/kunit/device-impl.h | 17 +++
lib/kunit/device.c | 181 +++++++++++++++++++++++++
lib/kunit/kunit-test.c | 134 +++++++++++++++++-
lib/kunit/test.c | 3 +
lib/overflow_kunit.c | 5 +-
sound/soc/soc-topology-test.c | 10 +-
12 files changed, 485 insertions(+), 78 deletions(-)
---
base-commit: b285ba6f8cc1b2bfece0b4350fdb92c8780bc698
change-id: 20230718-kunit_bus-ab19c4ef48dc

Best regards,
--
David Gow <[email protected]>


2023-12-14 08:50:09

by David Gow

[permalink] [raw]
Subject: [PATCH v3 3/5] overflow: Replace fake root_device with kunit_device

Using struct root_device to create fake devices for tests is something
of a hack. The new struct kunit_device is meant for this purpose, so use
it instead.

Reviewed-by: Matti Vaittinen <[email protected]>
Signed-off-by: David Gow <[email protected]>
---
lib/overflow_kunit.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c
index 34db0b3aa502..c527f6b75789 100644
--- a/lib/overflow_kunit.c
+++ b/lib/overflow_kunit.c
@@ -6,6 +6,7 @@
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

+#include <kunit/device.h>
#include <kunit/test.h>
#include <linux/device.h>
#include <linux/kernel.h>
@@ -618,7 +619,7 @@ static void overflow_allocation_test(struct kunit *test)
} while (0)

/* Create dummy device for devm_kmalloc()-family tests. */
- dev = root_device_register(device_name);
+ dev = kunit_device_register(test, device_name);
KUNIT_ASSERT_FALSE_MSG(test, IS_ERR(dev),
"Cannot register test device\n");

@@ -634,8 +635,6 @@ static void overflow_allocation_test(struct kunit *test)
check_allocation_overflow(devm_kmalloc);
check_allocation_overflow(devm_kzalloc);

- device_unregister(dev);
-
kunit_info(test, "%d allocation overflow tests finished\n", count);
#undef check_allocation_overflow
}

--
2.43.0.472.g3155946c3a-goog

2023-12-14 08:51:00

by David Gow

[permalink] [raw]
Subject: [PATCH v3 4/5] ASoC: topology: Replace fake root_device with kunit_device in tests

Using struct root_device to create fake devices for tests is something
of a hack. The new struct kunit_device is meant for this purpose, so use
it instead.

Acked-by: Mark Brown <[email protected]>
Signed-off-by: David Gow <[email protected]>
---
sound/soc/soc-topology-test.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/sound/soc/soc-topology-test.c b/sound/soc/soc-topology-test.c
index 2cd3540cec04..70cbccc42a42 100644
--- a/sound/soc/soc-topology-test.c
+++ b/sound/soc/soc-topology-test.c
@@ -9,6 +9,7 @@
#include <sound/core.h>
#include <sound/soc.h>
#include <sound/soc-topology.h>
+#include <kunit/device.h>
#include <kunit/test.h>

/* ===== HELPER FUNCTIONS =================================================== */
@@ -21,26 +22,19 @@
*/
static struct device *test_dev;

-static struct device_driver test_drv = {
- .name = "sound-soc-topology-test-driver",
-};
-
static int snd_soc_tplg_test_init(struct kunit *test)
{
- test_dev = root_device_register("sound-soc-topology-test");
+ test_dev = kunit_device_register(test, "sound-soc-topology-test");
test_dev = get_device(test_dev);
if (!test_dev)
return -ENODEV;

- test_dev->driver = &test_drv;
-
return 0;
}

static void snd_soc_tplg_test_exit(struct kunit *test)
{
put_device(test_dev);
- root_device_unregister(test_dev);
}

/*

--
2.43.0.472.g3155946c3a-goog

2023-12-14 08:51:13

by David Gow

[permalink] [raw]
Subject: [PATCH v3 5/5] drm/tests: Switch to kunit devices

From: Maxime Ripard <[email protected]>

Kunit recently gained helpers to create test managed devices. This means
that we no longer have to roll our own helpers in KMS and we can reuse
them.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/tests/drm_kunit_helpers.c | 66 ++-----------------------------
1 file changed, 3 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c
index c251e6b34de0..ca4f8e4c5d5d 100644
--- a/drivers/gpu/drm/tests/drm_kunit_helpers.c
+++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c
@@ -5,6 +5,7 @@
#include <drm/drm_kunit_helpers.h>
#include <drm/drm_managed.h>

+#include <kunit/device.h>
#include <kunit/resource.h>

#include <linux/device.h>
@@ -15,28 +16,6 @@
static const struct drm_mode_config_funcs drm_mode_config_funcs = {
};

-static int fake_probe(struct platform_device *pdev)
-{
- return 0;
-}
-
-static struct platform_driver fake_platform_driver = {
- .probe = fake_probe,
- .driver = {
- .name = KUNIT_DEVICE_NAME,
- },
-};
-
-KUNIT_DEFINE_ACTION_WRAPPER(kunit_action_platform_driver_unregister,
- platform_driver_unregister,
- struct platform_driver *);
-KUNIT_DEFINE_ACTION_WRAPPER(kunit_action_platform_device_put,
- platform_device_put,
- struct platform_device *);
-KUNIT_DEFINE_ACTION_WRAPPER(kunit_action_platform_device_del,
- platform_device_del,
- struct platform_device *);
-
/**
* drm_kunit_helper_alloc_device - Allocate a mock device for a KUnit test
* @test: The test context object
@@ -54,34 +33,7 @@ KUNIT_DEFINE_ACTION_WRAPPER(kunit_action_platform_device_del,
*/
struct device *drm_kunit_helper_alloc_device(struct kunit *test)
{
- struct platform_device *pdev;
- int ret;
-
- ret = platform_driver_register(&fake_platform_driver);
- KUNIT_ASSERT_EQ(test, ret, 0);
-
- ret = kunit_add_action_or_reset(test,
- kunit_action_platform_driver_unregister,
- &fake_platform_driver);
- KUNIT_ASSERT_EQ(test, ret, 0);
-
- pdev = platform_device_alloc(KUNIT_DEVICE_NAME, PLATFORM_DEVID_NONE);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
-
- ret = kunit_add_action_or_reset(test,
- kunit_action_platform_device_put,
- pdev);
- KUNIT_ASSERT_EQ(test, ret, 0);
-
- ret = platform_device_add(pdev);
- KUNIT_ASSERT_EQ(test, ret, 0);
-
- ret = kunit_add_action_or_reset(test,
- kunit_action_platform_device_del,
- pdev);
- KUNIT_ASSERT_EQ(test, ret, 0);
-
- return &pdev->dev;
+ return kunit_device_register(test, KUNIT_DEVICE_NAME);
}
EXPORT_SYMBOL_GPL(drm_kunit_helper_alloc_device);

@@ -94,19 +46,7 @@ EXPORT_SYMBOL_GPL(drm_kunit_helper_alloc_device);
*/
void drm_kunit_helper_free_device(struct kunit *test, struct device *dev)
{
- struct platform_device *pdev = to_platform_device(dev);
-
- kunit_release_action(test,
- kunit_action_platform_device_del,
- pdev);
-
- kunit_release_action(test,
- kunit_action_platform_device_put,
- pdev);
-
- kunit_release_action(test,
- kunit_action_platform_driver_unregister,
- &fake_platform_driver);
+ kunit_device_unregister(test, dev);
}
EXPORT_SYMBOL_GPL(drm_kunit_helper_free_device);


--
2.43.0.472.g3155946c3a-goog

2023-12-14 12:26:51

by David Gow

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] drm/tests: Switch to kunit devices

On Thu, 14 Dec 2023 at 16:49, <[email protected]> wrote:
>
> From: Maxime Ripard <[email protected]>
>
> Kunit recently gained helpers to create test managed devices. This means
> that we no longer have to roll our own helpers in KMS and we can reuse
> them.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---

I've tested this over a few different architectures and configs, and
it works fine.

Tested-by: David Gow <[email protected]>

And whoops -- forgot to add my Signed-off-by above.

Signed-off-by: David Gow <[email protected]>

It's identical to the version here, anyway -- I didn't make any
changes, just included it in this series to have everything in one
place:
https://lore.kernel.org/linux-kselftest/[email protected]/

-- David


Attachments:
smime.p7s (3.91 kB)
S/MIME Cryptographic Signature

2023-12-14 16:16:05

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] overflow: Replace fake root_device with kunit_device

On Thu, Dec 14, 2023 at 04:49:17PM +0800, [email protected] wrote:
> Using struct root_device to create fake devices for tests is something
> of a hack. The new struct kunit_device is meant for this purpose, so use
> it instead.
>
> Reviewed-by: Matti Vaittinen <[email protected]>
> Signed-off-by: David Gow <[email protected]>

Acked-by: Kees Cook <[email protected]>

--
Kees Cook