Skip to content

Commit

Permalink
drivers: base: Add basic devm tests for platform devices
Browse files Browse the repository at this point in the history 
Platform devices show some inconsistencies with how devm resources are
released when the device has been probed and when it hasn't.

In particular, we can register device-managed actions no matter if the
device has be bound to a driver or not, but devres_release_all() will
only be called if it was bound to a driver or if there's no reference
held to it anymore.

If it wasn't bound to a driver and we still have a reference,
devres_release_all() will never get called. This is surprising
considering that if the driver isn't bound but doesn't have any
reference to it anymore, that function will get called, and if it was
bound to a driver but still has references, that function will get
called as well.

Even if that case is fairly unusual, it can easily lead to memory leaks.

The plan is, with the next patch, to make it consistent and enforce that
devres_release_all() is called no matter what situation we're in. For
now, it just tests for the current behaviour and skips over the
inconsistencies.

Reviewed-by: David Gow <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
Link: https://lore.kernel.org/r/20230720-kunit-devm-inconsistencies-test-v3-2-6aa7e074f373@kernel.org
Signed-off-by: Greg Kroah-Hartman <[email protected]>
  • Loading branch information
@mripard @gregkh
mripard authored and gregkh committed Aug 12, 2023
1 parent 06188bc commit b4cc443
Show file tree
Hide file tree
Showing 2 changed files with 223 additions and 0 deletions.
1 change: 1 addition & 0 deletions drivers/base/test/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
obj-$(CONFIG_TEST_ASYNC_DRIVER_PROBE) += test_async_driver_probe.o

obj-$(CONFIG_DM_KUNIT_TEST) += root-device-test.o
obj-$(CONFIG_DM_KUNIT_TEST) += platform-device-test.o

obj-$(CONFIG_DRIVER_PE_KUNIT_TEST) += property-entry-test.o
CFLAGS_property-entry-test.o += $(DISABLE_STRUCTLEAK_PLUGIN)
222 changes: 222 additions & 0 deletions drivers/base/test/platform-device-test.c
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
// SPDX-License-Identifier: GPL-2.0

#include <kunit/resource.h>

#include <linux/device.h>
#include <linux/platform_device.h>

#define DEVICE_NAME "test"

struct test_priv {
bool probe_done;
bool release_done;
wait_queue_head_t probe_wq;
wait_queue_head_t release_wq;
struct device *dev;
};

static int platform_device_devm_init(struct kunit *test)
{
struct test_priv *priv;

priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
init_waitqueue_head(&priv->probe_wq);
init_waitqueue_head(&priv->release_wq);

test->priv = priv;

return 0;
}

static void devm_device_action(void *ptr)
{
struct test_priv *priv = ptr;

priv->release_done = true;
wake_up_interruptible(&priv->release_wq);
}

static void devm_put_device_action(void *ptr)
{
struct test_priv *priv = ptr;

put_device(priv->dev);
priv->release_done = true;
wake_up_interruptible(&priv->release_wq);
}

#define RELEASE_TIMEOUT_MS 100

/*
* Tests that a platform bus, non-probed device will run its
* device-managed actions when unregistered.
*/
static void platform_device_devm_register_unregister_test(struct kunit *test)
{
struct platform_device *pdev;
struct test_priv *priv = test->priv;
int ret;

pdev = platform_device_alloc(DEVICE_NAME, PLATFORM_DEVID_NONE);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);

ret = platform_device_add(pdev);
KUNIT_ASSERT_EQ(test, ret, 0);

priv->dev = &pdev->dev;

ret = devm_add_action_or_reset(priv->dev, devm_device_action, priv);
KUNIT_ASSERT_EQ(test, ret, 0);

platform_device_unregister(pdev);

ret = wait_event_interruptible_timeout(priv->release_wq, priv->release_done,
msecs_to_jiffies(RELEASE_TIMEOUT_MS));
KUNIT_EXPECT_GT(test, ret, 0);
}

/*
* Tests that a platform bus, non-probed device will run its
* device-managed actions when unregistered, even if someone still holds
* a reference to it.
*/
static void platform_device_devm_register_get_unregister_with_devm_test(struct kunit *test)
{
struct platform_device *pdev;
struct test_priv *priv = test->priv;
int ret;

kunit_skip(test, "This needs to be fixed in the core.");

pdev = platform_device_alloc(DEVICE_NAME, PLATFORM_DEVID_NONE);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);

ret = platform_device_add(pdev);
KUNIT_ASSERT_EQ(test, ret, 0);

priv->dev = &pdev->dev;

get_device(priv->dev);

ret = devm_add_action_or_reset(priv->dev, devm_put_device_action, priv);
KUNIT_ASSERT_EQ(test, ret, 0);

platform_device_unregister(pdev);

ret = wait_event_interruptible_timeout(priv->release_wq, priv->release_done,
msecs_to_jiffies(RELEASE_TIMEOUT_MS));
KUNIT_EXPECT_GT(test, ret, 0);
}

static int fake_probe(struct platform_device *pdev)
{
struct test_priv *priv = platform_get_drvdata(pdev);

priv->probe_done = true;
wake_up_interruptible(&priv->probe_wq);

return 0;
}

static struct platform_driver fake_driver = {
.probe = fake_probe,
.driver = {
.name = DEVICE_NAME,
},
};

/*
* Tests that a platform bus, probed device will run its device-managed
* actions when unregistered.
*/
static void probed_platform_device_devm_register_unregister_test(struct kunit *test)
{
struct platform_device *pdev;
struct test_priv *priv = test->priv;
int ret;

ret = platform_driver_register(&fake_driver);
KUNIT_ASSERT_EQ(test, ret, 0);

pdev = platform_device_alloc(DEVICE_NAME, PLATFORM_DEVID_NONE);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);

priv->dev = &pdev->dev;
platform_set_drvdata(pdev, priv);

ret = platform_device_add(pdev);
KUNIT_ASSERT_EQ(test, ret, 0);

ret = wait_event_interruptible_timeout(priv->probe_wq, priv->probe_done,
msecs_to_jiffies(RELEASE_TIMEOUT_MS));
KUNIT_ASSERT_GT(test, ret, 0);

ret = devm_add_action_or_reset(priv->dev, devm_device_action, priv);
KUNIT_ASSERT_EQ(test, ret, 0);

platform_device_unregister(pdev);

ret = wait_event_interruptible_timeout(priv->release_wq, priv->release_done,
msecs_to_jiffies(RELEASE_TIMEOUT_MS));
KUNIT_EXPECT_GT(test, ret, 0);

platform_driver_unregister(&fake_driver);
}

/*
* Tests that a platform bus, probed device will run its device-managed
* actions when unregistered, even if someone still holds a reference to
* it.
*/
static void probed_platform_device_devm_register_get_unregister_with_devm_test(struct kunit *test)
{
struct platform_device *pdev;
struct test_priv *priv = test->priv;
int ret;

ret = platform_driver_register(&fake_driver);
KUNIT_ASSERT_EQ(test, ret, 0);

pdev = platform_device_alloc(DEVICE_NAME, PLATFORM_DEVID_NONE);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);

priv->dev = &pdev->dev;
platform_set_drvdata(pdev, priv);

ret = platform_device_add(pdev);
KUNIT_ASSERT_EQ(test, ret, 0);

ret = wait_event_interruptible_timeout(priv->probe_wq, priv->probe_done,
msecs_to_jiffies(RELEASE_TIMEOUT_MS));
KUNIT_ASSERT_GT(test, ret, 0);

get_device(priv->dev);

ret = devm_add_action_or_reset(priv->dev, devm_put_device_action, priv);
KUNIT_ASSERT_EQ(test, ret, 0);

platform_device_unregister(pdev);

ret = wait_event_interruptible_timeout(priv->release_wq, priv->release_done,
msecs_to_jiffies(RELEASE_TIMEOUT_MS));
KUNIT_EXPECT_GT(test, ret, 0);

platform_driver_unregister(&fake_driver);
}

static struct kunit_case platform_device_devm_tests[] = {
KUNIT_CASE(platform_device_devm_register_unregister_test),
KUNIT_CASE(platform_device_devm_register_get_unregister_with_devm_test),
KUNIT_CASE(probed_platform_device_devm_register_unregister_test),
KUNIT_CASE(probed_platform_device_devm_register_get_unregister_with_devm_test),
{}
};

static struct kunit_suite platform_device_devm_test_suite = {
.name = "platform-device-devm",
.init = platform_device_devm_init,
.test_cases = platform_device_devm_tests,
};

kunit_test_suite(platform_device_devm_test_suite);

0 comments on commit b4cc443

Please sign in to comment.