rtio: Transactional request support #55382
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
teburd
force-pushed
the
rtio_iodev_transact
branch
4 times, most recently
from
e4124f9 to
0193165
Compare
teburd
force-pushed
the
rtio_iodev_transact
branch
from
0193165 to
886dbd0
Compare
yperess
previously approved these changes
Mar 16, 2023
Contributor
yperess
left a comment
yperess
left a comment
There was a problem hiding this comment.
Now that I understand this a bit better I'm happy to approve :)
Tests were failing when the sqe was allocated on the stack for some architectures. Initialization of variables on the stack is not guaranteed to be zeroed like static data. This caused undefined behavior. Secondly if the sqe is recycled and had a flag set, there would unexpected behavior as well. Signed-off-by: Tom Burdick <[email protected]>
Race was possible though very unlikely between the atomic cas and queue push/pop operations. The outcome of the race had it shown up would have been a submission not worked on due to the timer never being started. A small critical section fixes this and clarifies where the single conumer part of the mpsc queue comes in despite there being multiple contexts which may enter that section. Signed-off-by: Tom Burdick <[email protected]>
The test suites have grown to cover different units really and having them in one file was becoming a bit much to scroll around in. Coincidentally found a accidental reuse of a define between the spsc and mpsc tests. Signed-off-by: Tom Burdick <[email protected]>
Transactional submissions treat a sequence of sqes as a single atomic submission given to a single iodev and expect as a reply a single completion. This is useful for scatter gather like APIs that exist in Zephyr already for I2C and SPI. Signed-off-by: Tom Burdick <[email protected]>
teburd
force-pushed
the
rtio_iodev_transact
branch
from
886dbd0 to
918cac9
Compare
The pending_sqe logic to track where in the ring queue the concurrent executor had left off was slightly flawed. It didn't account for starting all sqes in the queue and ending back up at the beginning. Instead track the last SQE in the queue, from which the next one in the queue will the one to start next. If we happen to sweep the last known SQE in the queue, reset it to NULL so the next time prepare is called we start at the beginning of the queue again. Signed-off-by: Tom Burdick <[email protected]>
C++ requires casting void * as the implicit cast isn't enough and the C++ sample app fails to build without duplicating type information here. Do that, and wrap it in extern C allowing the C++ to include rtio_mpsc.h directly or indirectly. Signed-off-by: Tom Burdick <[email protected]>
Ability to cancel all pending requests in the queue turns out to be useful and shareable. Signed-off-by: Tom Burdick <[email protected]>
When the consume semaphore is enabled always give and take from the semaphore. It's expected that rtio_cqe_consume and rtio_cqe_consume_block will be used exclusively rather than directly poking at the SPSC queues. Signed-off-by: Tom Burdick <[email protected]>
teburd
force-pushed
the
rtio_iodev_transact
branch
from
3670287 to
5eafe25
Compare
Contributor
Author
|
Includes a stack of bug fixes now so CI hopefully remains happy |
MaureenHelm
approved these changes
Mar 17, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds the ability to mark submissions as being transactional which has the unique semantics of allowing multiple receive/send type operations to be viewed by the submitted to iodev as a single unit of work to do.
This is needed for SPI where a sequence of reads/writes with chip select are part of a transaction.