ring buffer concurrency

[xhpohanka]

Hi,

there is this sentence in the zephyr ring buffer docs

For the trivial case of one producer and one consumer, concurrency control shouldn’t be needed.

I have a simple case where data received over UART are filled into a ring buffer using ring_buf_put_claim() in an interrupt routine. Then there is a single thread which periodically access the data with ring_buf_get_claim(). I see very rare occurrences when the received data are wrong. After some time I have started to be suspicious about the ring buffer concurrency issue. Can it be the case here? Shouldn't I do ring_buf_get_claim() inside a critical section?

best regards
Jan

[rhaberkorn]

Actually, I just wanted to open another topic about the same potential problem.

Reading through the code, I also have my doubts about that same statement in the documentation. The reason is that ring_buf_put_finish() (i.e. the producer) increases buf->put_tail while ring_buf_size_get() reads put_tail and is called from ring_buf_get_claim() (i.e. the consumer). This is only safe if the increase is actually compiled into an atomic operation, or at least an atomic store. This appears to be the case on 32-bit ARM.
However, there is no guarantee this is the case on all processor architectures around, nor that GCC won't emit two e.g. STRH instructions for whatever reason. C does not guarantee anything with regard to += and the correctness of code must not depend on "common sense" or implementation details. They could change at any time.

I am not suggesting to rewrite ring_buf, as there already is a lock free ring buffer implementation, which I would always recommend to use in situations like this. However, the misleading sentence should perhaps be removed from the documentation, pointing people to spsc_lockfree instead.

[ldomaigne]

I agree with your observation.

Concurrency:

The ring buffer APIs do not provide any concurrency control. Depending on usage (particularly with respect to number of concurrent readers/writers) applications may need to protect the ring buffer with mutexes and/or use semaphores to notify consumers that there is data to read.

For the trivial case of one producer and one consumer, concurrency control shouldn’t be needed.

In my opinion, that last sentence of the "concurrency" paragraph is misleading. While it is true that on some architecture/compiler combination no concurrency control might be needed for the trivial case one producer/one consumer, But I don't think we can really rely on it (to be sure, we have to check the produced assembly code).

Perhaps it would be simpler to remove that last sentence. Please report an issue against the documentation. Happy to take care of it, if you want to.

[ldomaigne]

There's already an open issue on the topic: Issue 69403.