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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] iio: osf: KUnit test hangs kernel due to invalid assumption about unregister serialization
- [Medium] iio: osf: KUnit test osf_iio_buffer_lifetime_test fails due to incorrect locking assumption
--

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

iio: osf: add IIO KUnit tests
    
This patch adds KUnit tests for the Open Sensor Fusion core sample
acceptance, direct-read cache, and IIO buffer paths. It exercises
active-scan packing, timestamp alignment, and producer quiescence
during buffer reconfiguration and unregistration.

> diff --git a/drivers/iio/sensorhubs/osf/osf_iio_test.c b/drivers/iio/sensorhubs/osf/osf_iio_test.c
> new file mode 100644
> index 0000000000000..4c3fd8cfb2b26
> --- /dev/null
> +++ b/drivers/iio/sensorhubs/osf/osf_iio_test.c
> @@ -0,0 +1,386 @@
> +// SPDX-License-Identifier: GPL-2.0-only

[ ... ]

> +static void osf_iio_expect_quiesce(struct kunit *test, struct osf_iio_race *race)
> +{
> +	struct iio_dev *indio_dev = race->indio_dev;
> +	const unsigned long *scan_mask = indio_dev->active_scan_mask;
> +	unsigned long mask;
> +	unsigned long waited;
> +	int ret;
> +
> +	KUNIT_ASSERT_NOT_NULL(test, scan_mask);
> +	mask = *scan_mask;
> +	race->config = kthread_run(osf_iio_test_configure, race, "osf-cfg-test");
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, race->config);
> +	ret = kunit_add_action_or_reset(test, osf_iio_stop_config, race);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +	KUNIT_EXPECT_NE(test, wait_for_completion_timeout(&race->config_started, HZ), 0UL);
> +	waited = wait_for_completion_timeout(&race->config_done, msecs_to_jiffies(20));
> +	KUNIT_EXPECT_EQ(test, waited, 0UL);
> +	/* The blocked OSF push owns the mode claim until its store returns. */
> +	KUNIT_EXPECT_TRUE(test, iio_buffer_enabled(indio_dev));
> +	KUNIT_EXPECT_PTR_EQ(test, indio_dev->active_scan_mask, scan_mask);

[Severity: Medium]
Will these assertions fail unconditionally?

The test assumes iio_device_unregister() blocks on mlock while the
producer is active. However, iio_device_unregister() only takes
info_exist_lock, not mlock. It proceeds directly to disable all buffers
and nullifies active_scan_mask immediately, which would cause the
timeout assertion and state assertions here to fail.

[ ... ]

> +	/* Publish the completions to the producer's fully ordered atomic_xchg(). */
> +	atomic_set_release(&race->block_store, 1);
> +	KUNIT_EXPECT_NE(test, wait_for_completion_timeout(&race->entered, HZ), 0UL);
> +	race->unregister = true;
> +	osf_iio_expect_quiesce(test, race);

[Severity: High]
Can this sequence cause a kernel soft lockup?

When race->unregister is true, the test concurrently triggers
iio_device_unregister() while the producer is actively blocked pushing a
sample. Because iio_device_unregister() does not block on mlock, it removes
the active buffer from buffer_list using list_del_init().

When the test unblocks the producer, it resumes in iio_push_to_buffers()
inside list_for_each_entry(). Since the buffer was removed with
list_del_init(), its next pointer points to itself, creating an infinite
loop.

> +	kunit_release_action(test, osf_iio_stop_producer, race);
> +	KUNIT_EXPECT_EQ(test, atomic_read(&race->bad_store), 0);
> +	KUNIT_EXPECT_GT(test, atomic_read(&race->stores), 0);
> +	kunit_release_action(test, osf_iio_restore_access, race);
> +	kunit_release_action(test, osf_iio_test_put_device, &indio_dev->dev);
> +}

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