Re: [PATCH v5 1/8] drivers: kunit: Generic helpers for test device creation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


Hi Maxime, all

On 3/23/23 12:21, Greg Kroah-Hartman wrote:
On Thu, Mar 23, 2023 at 11:12:16AM +0100, Maxime Ripard wrote:
On Wed, Mar 22, 2023 at 07:57:10PM +0100, Greg Kroah-Hartman wrote:
+ * test_kunit_helper_alloc_device - Allocate a mock device for a KUnit test
+ * @test: The test context object
+ *
+ * This allocates a fake struct &device to create a mock for a KUnit
+ * test. The device will also be bound to a fake driver. It will thus be
+ * able to leverage the usual infrastructure and most notably the
+ * device-managed resources just like a "real" device.

What specific "usual infrastructure" are you wanting to access here?

And again, if you want a fake device, make a virtual one, by just
calling device_create().

Or are you wanting to do "more" with that device pointer than
device_create() can give you?

Personally, I was (am) only interested in devm_ unwinding. I guess the
device_create(), device_add(), device_remove()... (didn't study this
sequence in details so sorry if there is errors) could've been sufficient
for me. I haven't looked how much of the code that there is for 'platform
devices' should be duplicated to support that sequence for testability

Any device can access devm_ code, there's no need for it to be a
platform device at all.

Sure but the resources are only released if the device is part of a bus,
so it can't be a root_device (or bare device) either

The resources are not cleaned up when the device is freed no matter if
it's on a bus or not?  If so, then that's a bug that needs to be fixed,
and tested :)

This is strange. I just ran a test on a beaglebone black using Linux 6.3.0-rc2 + the IIO patches we se here (but the IIO test patch modified to use the root_device_register() and root_device_unregister().

I passed the device pointer from root_device_register() to the devm_iio_init_iio_gts()

// snip
        dev = root_device_register(IIO_GTS_TEST_DEV);
        KUNIT_EXPECT_NOT_ERR_OR_NULL(test, dev);
        if (IS_ERR_OR_NULL(dev))
                return NULL;

        ret = devm_iio_init_iio_gts(dev, TEST_SCALE_1X, 0, g_table, num_g,
                                    i_table, num_i, gts);

 - and saw the tables for available scales allocated:

 if (gts.num_avail_all_scales)
                pr_info("GTS: table allocation succeeded\n");
                pr_info("GTS: table allocation failed\n");

pr_info("gts: num_avail_all_scales %d\n", gts.num_avail_all_scales);

(this printed:
[   52.132966]     # Subtest: iio-gain-time-scale
[   52.132982]     1..7
[   52.157455] GTS: table allocation succeeded
[   52.164077] gts: num_avail_all_scales 16

Next I unregister the root-device and check if the unwinding code which frees the tables and zeroes the scale count was ran:

pr_info("gts: num_avail_all_scales %d\n", gts.num_avail_all_scales);

        if (gts.num_avail_all_scales)
                pr_info("devm unwinding not done\n");
                pr_info("devm unwinding succeeded\n");

Which printed:
[   52.168101] gts: num_avail_all_scales 0
[   52.171957] devm unwinding succeeded

I can send patch(es) just for testing this on other machines if someone want's to see if the lack of devm unwinding is somehow architecture specific (which sounds very strange to me) - although using this IIO series just for checking the unwinding is a bit of an overkill. I just happened to have these tests at my hands / in my tree for testing.

In any case, devm unwinding using root_device_[un]register() seems to "work on my machine".

Naxime, what was the environment where you observed lack of unwinding? (Huh, I am so afraid of sending this post out - I've experienced too many "Oh, boy - how I didn't notice THAT" moments in the past and maybe I am again overlooking something...)

	-- Matti

Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     []

  Powered by Linux