Skip to content

rtio: Callback chaining and testing #72765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

teburd
Copy link
Contributor

@teburd teburd commented May 14, 2024

Callbacks were a bit neglected in terms of test coverage, especially when used in chains. It was clear from the code that chained callbacks may not actually work, and callback ordering then was hard to verify. Test callbacks chained to transactions work as expected.

The test iodev had built up some cruft over time and in the process showed a few bugs once callback chaining was fixed so the test iodev now better matches typical iodev implementations at this point.

Fixes #72740

@zephyrbot zephyrbot requested a review from yperess May 14, 2024 19:48
@teburd teburd force-pushed the rtio_callback_chaining branch from 28307bb to 1666c2e Compare May 14, 2024 19:55
@teburd teburd requested review from ubieda and MaureenHelm May 14, 2024 19:55
@teburd teburd marked this pull request as draft May 14, 2024 19:57
@teburd
Copy link
Contributor Author

teburd commented May 14, 2024

Converted back to draft as twister revealed a few issues still on certain platforms

@teburd teburd force-pushed the rtio_callback_chaining branch from 1666c2e to 2f6b31e Compare May 14, 2024 20:14
@teburd teburd marked this pull request as ready for review May 14, 2024 20:32
@teburd
Copy link
Contributor Author

teburd commented May 14, 2024

Update: ok I believe I've corrected the issue, though the behavior is as it was before, I'm wondering if this is the right behavior @yperess

Imagine I have 3 chained submissions and cancel the one in the middle before submit is called, the first one should report success, the next two should be cancelled, should they report anything?

I guess more critically, why is cancelling before submit a special case, can you help clarify this @yperess? Thanks!

Callbacks were a bit neglected in terms of test coverage, especially
when used in chains. It was clear from the code that chained callbacks
may not actually work, and callback ordering then was hard to verify.
Test callbacks chained to transactions work as expected.

The test iodev had built up some cruft over time and in the process
showed a few bugs once callback chaining was fixed so the test iodev now
better matches typical iodev implementations at this point.

Cancellation testing now includes an added case for cancelling a the
second submission in the chain prior to calling submit noting that no
completions notifications should be given back for those.

Signed-off-by: Tom Burdick <[email protected]>
@teburd teburd force-pushed the rtio_callback_chaining branch from 2f6b31e to a2b8360 Compare May 15, 2024 14:19
@teburd teburd added the bug The issue is a bug, or the PR is fixing a bug label May 16, 2024
@henrikbrixandersen henrikbrixandersen merged commit 054f453 into zephyrproject-rtos:main May 29, 2024
@teburd teburd deleted the rtio_callback_chaining branch May 29, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: RTIO bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rtio: Possibly missed callback when linking
5 participants