Re: [PATCH v11 7/8] iio: osf: add core KUnit tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] kunit: missing mutex_destroy() for dynamically allocated locks
- [Low] kconfig: OSF KUnit tests unnecessarily restricted to built-in modularity
--

commit 168e51876dbdd13bb34af3a59ad653d5632d1c07
Author: Jinseob Kim <kimjinseob88@gmail.com>

iio: osf: add core KUnit tests

This commit adds focused unit tests for the Open Sensor Fusion core, exercising
capability discovery, inventories, exact lengths, and reserved wire bytes. It
also covers sample-cache admission and deferred cleanup routines.

> diff --git a/drivers/iio/sensorhubs/osf/Kconfig b/drivers/iio/sensorhubs/osf/Kconfig
> index f955a1f2993d2..316f33be1327c 100644
> --- a/drivers/iio/sensorhubs/osf/Kconfig
> +++ b/drivers/iio/sensorhubs/osf/Kconfig
> @@ -13,3 +13,15 @@ config OPEN_SENSOR_FUSION
[ ... ]
> +config OPEN_SENSOR_FUSION_KUNIT_TEST
> +	bool "KUnit tests for Open Sensor Fusion" if !KUNIT_ALL_TESTS
> +	depends on OPEN_SENSOR_FUSION=y && KUNIT=y

[Severity: Low]
Does this unnecessarily restrict test coverage when the driver is built as a
loadable module?

Since the test object is appended to open-sensor-fusion-y in the Makefile,
it seems it can be successfully built and executed inside the loadable
module open-sensor-fusion.ko.

While it makes sense to avoid OPEN_SENSOR_FUSION=y with KUNIT=m to prevent
link errors, forcing the driver to be built-in reduces coverage on standard
distribution kernels. Could a dependency like depends on OPEN_SENSOR_FUSION
&& !(OPEN_SENSOR_FUSION=y && KUNIT=m) work better here to allow modular
test execution?

> diff --git a/drivers/iio/sensorhubs/osf/osf_core_test.c b/drivers/iio/sensorhubs/osf/osf_core_test.c
> new file mode 100644
> index 0000000000000..fabf19018b200
> --- /dev/null
> +++ b/drivers/iio/sensorhubs/osf/osf_core_test.c
> @@ -0,0 +1,1056 @@
[ ... ]
> +static void osf_test_unregister_pending(void *data)
> +{
> +	osf_core_unregister_iio(data);
> +}

[Severity: Low]
Does this custom teardown path miss a call to mutex_destroy() for the
pending device's latest_lock initialized during osf_core_init()?

Since KUnit will automatically free the allocated memory for the pending
device at the end of the test, leaving the active mutex undestroyed can
leave stale pointers in the lockdep tracking structures, causing debug
splats when CONFIG_DEBUG_MUTEXES is enabled.

[ ... ]
> +static void osf_session_fault_buffer_race_test(struct kunit *test)
> +{
> +	struct osf_test_context *ctx = test->priv;
[ ... ]
> +	race = kunit_kzalloc(test, sizeof(*race), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_NULL(test, race);
> +	race->ctx = ctx;
> +	mutex_init(&race->rx_lock);
> +	init_completion(&race->started);

[Severity: Low]
Are we missing a corresponding mutex_destroy() for the dynamically
allocated race->rx_lock before the test finishes?

Similar to the issue above, the race object is managed by kunit_kzalloc()
and will be automatically freed. Freeing active mutex memory without
destroying it could trigger lockdep warnings.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011231340.42885-1-kimjinseob88@gmail.com?part=7