I tried to follow the blog post on multicore communication: https://www.freertos.org/2020/02/simple-multicore-core-to-core-communication-using-freertos-message-buffers.html
So far it seems to work, but the usage of a shared underlying StreamBufferHandle_t seems strange to me. The send/receive functions in stream_buffer.c protect the access using a critical section (taskENTER_CRITICAL and taskEXIT_CRITICAL)
The critical sections will prevent other tasks on the same core from modifying the structure, but a task on a different core will not be prevented from running!
So my question: is the method in the blog post really safe to use?
The stream buffer is implemented as a lockless circular buffer - as per documented restrictions it is safe provided there is only a single reader and a single writer - from the website:
" IMPORTANT NOTE : Uniquely among FreeRTOS objects, the stream buffer implementation (so also the message buffer implementation, as message buffers are built on top of stream buffers) assumes there is only one task or interrupt that will write to the buffer (the writer), and only one task or interrupt that will read from the buffer (the reader). It is safe for the writer and reader to be different tasks or interrupts, but, unlike other FreeRTOS objects, it is not safe to have multiple different writers or multiple different readers. If there are to be multiple different writers then the application writer must place each call to a writing API function (such as xStreamBufferSend()) inside a critical section and use a send block time of 0. Likewise, if there are to be multiple different readers then the application writer must place each call to a reading API function (such as xStreamBufferReceive()) inside a critical section and use a receive block time of 0."
Please let us know with some examples if you think there is an issue with the implementation.
Yes, it is safe with a single reader and a single writer, because there are critical sections which protect it from having the reader interfering with the writer.
On a multicore setup, the critical sections will have no influence on the other cores, so I’m worried that a reader-task on core 2 will interfere with the writer-task on core 1.
For example when writing a message on core 1, the function xStreamBufferSpacesAvailable will be called inside a critical-section, and do some computation on the “Tail” and “Head” offsets. In the meantime, nothing prevents the core 2 from reading from the buffer and modifying the “Tail” offset.
You ask a good question, so I had a look at the use of xStreamBufferSpacesAvailable(). Here it is called inside a critical section https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/master/stream_buffer.c#L598 - but I don’t think the critical section is protecting xStreamBufferSpacesAvailable() as that uses a pessimistic approach, by which I mean, if two tasks access the stream buffer at the same time it is possible for xStreamBufferSpacesAvailable() to return that no space is available when space actually is available by the time the function returns, but it should not be possible for it to say there is space available when there isn’t. xStreamBufferSapcesAvailable() is also called outside of critical sections https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/master/stream_buffer.c#L629
So then the question is, “what is the critical section for?” It appears to be making the determination that there is not enough space in the buffer and the clearing of the task notification state atomic - as per your post it won’t be atomic if the stream buffer is being accessed from another core. Will need to dig into this more deeply but on first look I wonder if the clearing of the notification state should be before checking if there is enough space, or if there should be an additional check for space before blocking on the task notification - those options may actually enable the critical section to be removed altogether.
I see now what you mean about the pessimistic approach on the buffer space calculations, so there is no problem there.
Then I spent some more time thinking about the critical sections. It seems to me that they are not just for the clearing of the notification state, but mostly to ensure the setting of the “xTaskWaitingToSend” TaskHandle is atomic with the buffer space check. Otherwise the receiving task would not send a notification if “xTaskWaitingToSend” was still NULL, but the sending task might alread have done the space check.
Regarding the multicore use-case, the critical-sections seem OK because they only need to cover xStreamBufferReceiveCompletedFromISR() which runs on the same core as xStreamBufferSend(). (and the same goes for xStreamBufferSendCompletedFromISR and xMessageBufferReceive which are both running on the same (“other”) core as well…)
Regarding your last remark, I also believe the critical sections could be removed altogether. Maybe it would be enough to set the task handles xTaskWaitingToSend / xTaskWaitingToReceive before doing the buffer size checks. This would ensure no notifications getting lost. At worst it would lead to a spurious return from xTaskNotifyWait(), if the notification was pending since from before checking the buffer size.
But a return from Wait has to be followed in any case by a repeated buffer size check, to cover the cases if too little space was freed, to this case is already covered.