rtio: Correctly handle completion count wraps (2a046004) · Commits · Wenxi XU / zephyr

include/zephyr/rtio/rtio.h

+35 −18

Original line number	Diff line number	Diff line
		@@ -1259,7 +1259,16 @@ static inline void rtio_cqe_submit(struct rtio r, int result, void userdata, u
		rtio_cqe_produce(r, cqe);
		}

		atomic_inc(&r->cq_count);
		/* atomic_t isn't guaranteed to wrap correctly as it could be signed, so
		* we must resort to a cas loop.
		*/
		atomic_t val, new_val;

		do {
		val = atomic_get(&r->cq_count);
		new_val = (atomic_t)((uintptr_t)val + 1);
		} while (!atomic_cas(&r->cq_count, val, new_val));

		#ifdef CONFIG_RTIO_SUBMIT_SEM
		if (r->submit_count > 0) {
		r->submit_count--;
		@@ -1510,6 +1519,8 @@ static inline int z_impl_rtio_cqe_copy_out(struct rtio *r,
		* submission chain, freeing submission queue events when done, and
		* producing completion queue events as submissions are completed.
		*
		* @warning It is undefined behavior to have re-entrant calls to submit
		*
		* @param r RTIO context
		* @param wait_count Number of submissions to wait for completion of.
		*
		@@ -1517,13 +1528,11 @@ static inline int z_impl_rtio_cqe_copy_out(struct rtio *r,
		*/
		__syscall int rtio_submit(struct rtio *r, uint32_t wait_count);

		#ifdef CONFIG_RTIO_SUBMIT_SEM
		static inline int z_impl_rtio_submit(struct rtio *r, uint32_t wait_count)
		{
		int res = 0;

		#ifdef CONFIG_RTIO_SUBMIT_SEM
		/* TODO undefined behavior if another thread calls submit of course
		*/
		if (wait_count > 0) {
		__ASSERT(!k_is_in_isr(),
		"expected rtio submit with wait count to be called from a thread");
		@@ -1531,35 +1540,43 @@ static inline int z_impl_rtio_submit(struct rtio *r, uint32_t wait_count)
		k_sem_reset(r->submit_sem);
		r->submit_count = wait_count;
		}
		#else
		uintptr_t cq_count = (uintptr_t)atomic_get(&r->cq_count) + wait_count;
		#endif

		/* Submit the queue to the executor which consumes submissions
		* and produces completions through ISR chains or other means.
		*/
		rtio_executor_submit(r);


		/* TODO could be nicer if we could suspend the thread and not
		* wake up on each completion here.
		*/
		#ifdef CONFIG_RTIO_SUBMIT_SEM

		if (wait_count > 0) {
		res = k_sem_take(r->submit_sem, K_FOREVER);
		__ASSERT(res == 0,
		"semaphore was reset or timed out while waiting on completions!");
		}

		return res;
		}
		#else
		while ((uintptr_t)atomic_get(&r->cq_count) < cq_count) {
		static inline int z_impl_rtio_submit(struct rtio *r, uint32_t wait_count)
		{

		int res = 0;
		uintptr_t cq_count = (uintptr_t)atomic_get(&r->cq_count);
		uintptr_t cq_complete_count = cq_count + wait_count;
		bool wraps = cq_complete_count < cq_count;

		rtio_executor_submit(r);

		if (wraps) {
		while ((uintptr_t)atomic_get(&r->cq_count) >= cq_count) {
		Z_SPIN_DELAY(10);
		k_yield();
		}
		}

		while ((uintptr_t)atomic_get(&r->cq_count) < cq_complete_count) {
		Z_SPIN_DELAY(10);
		k_yield();
		}
		#endif

		return res;
		}
		#endif /* CONFIG_RTIO_SUBMIT_SEM */

		/**
		* @}

tests/subsys/rtio/rtio_api/src/test_rtio_api.c

+18 −3

Original line number	Diff line number	Diff line
		@@ -618,8 +618,23 @@ ZTEST(rtio_api, test_rtio_transaction)

		ZTEST(rtio_api, test_rtio_cqe_count_overflow)
		{
		/* atomic_t is signed, but RTIO uses cq_count as `uintptr_t` */
		const atomic_t max_val = (1ULL << (ATOMIC_BITS)) - 1;
		/* atomic_t max value as `uintptr_t` */
		const atomic_t max_uval = UINTPTR_MAX;

		/* atomic_t max value as if it were a signed word `intptr_t` */
		const atomic_t max_sval = UINTPTR_MAX >> 1;

		TC_PRINT("initializing iodev test devices\n");

		for (int i = 0; i < 2; i++) {
		rtio_iodev_test_init(iodev_test_transaction[i]);
		}

		TC_PRINT("rtio transaction CQE overflow\n");
		atomic_set(&r_transaction.cq_count, max_uval - 3);
		for (int i = 0; i < TEST_REPEATS; i++) {
		test_rtio_transaction_(&r_transaction);
		}

		TC_PRINT("initializing iodev test devices\n");

		@@ -628,7 +643,7 @@ ZTEST(rtio_api, test_rtio_cqe_count_overflow)
		}

		TC_PRINT("rtio transaction CQE overflow\n");
		atomic_set(&r_transaction.cq_count, max_val - 3);
		atomic_set(&r_transaction.cq_count, max_sval - 3);
		for (int i = 0; i < TEST_REPEATS; i++) {
		test_rtio_transaction_(&r_transaction);
		}

Admin message