sys/{tsrb, oorb}: Thread-safe ringbuffers#15103
sys/{tsrb, oorb}: Thread-safe ringbuffers#15103maribu wants to merge 3 commits intoRIOT-OS:masterfrom
Conversation
The thread-safe ringbuffer assumes that only one thread writes to it and only one thread reads from it. This rename makes it more obvious and allows to additionally add a (less efficient) thread-safe ringbuffer implementation that works with multiple readers and/or writers.
Based on the tsrb code just renamed to oorb, implement a thread-safe ring buffer that works for multiple readers and/or writers by disabling IRQs during critical phases.
maribu
added
Area: sys
benpicco
added
the
CI: ready for build
benpicco
left a comment
benpicco
left a comment
There was a problem hiding this comment.
I wonder if the TSRB could just do e.g.
static inline int tsrb_empty(const oorb_t *rb)
{
int retval;
unsigned status = irq_disable();
retval = oorb_empty(rb);
irq_restore(status);
return retval;
}to avoid code duplication
| volatile unsigned reads; /**< total number of reads */ | ||
| volatile unsigned writes; /**< total number of writes */ |
There was a problem hiding this comment.
Do those need to be volatie?`
There was a problem hiding this comment.
No, those should be atomic. Or more precisely, semi-atomic :-D
(As this is the two-threads-one-buffer mode, only the reader will write to reads and only the writer will write to writes. So as long as the read and the write access to them is atomically, everything is fine. This means explicitly that a read-modify-write operation could consist of three operations with only the read and write being atomic. Real atomic accesses would make the whole read-modify-write atomically. The atomic utils helper functions introduce just the semi-atomic functions this needs - which at least for word sized variables should be lock free.)
But I intentionally left the implementation of oorb unchanged. Fixing it efficiently would require the atomic utils PR being merged. And fixing it inefficiently is done in tsrb.
|
Btw: As boths contributions have been split out, there is no need to keep this open. |
2 participants
Contribution description
tsrbimplementation tooorbtsrbcode, but insertirq_disable()...irq_restore()where needed to provide thready-safety for multiple readers and writersoorbTesting procedure
E.g. stdio over UART on STM32 should still work. And now concurrent stdio should not result in corrupting the output (only in mixing it).
Issues/PRs references
Addresses #9882. (But the same issues still apply to the new
oorb. However, with the documentation pointing those issues out, I think we can consider them as features :-D. And those "features" should be addressed in a follow up soonish.)