On Thu, Dec 10, 2020 at 11:42:59AM -0600, Bob Pearson wrote:
> @@ -76,26 +56,56 @@ static inline int next_index(struct rxe_queue *q, int index)
>
> static inline int queue_empty(struct rxe_queue *q)
> {
> - return ((q->buf->producer_index - q->buf->consumer_index)
> - & q->index_mask) == 0;
> + u32 prod;
> + u32 cons;
> +
> + /* make sure all changes to queue complete before
> + * testing queue empty
> + */
> + prod = smp_load_acquire(&q->buf->producer_index);
> + /* same */
> + cons = smp_load_acquire(&q->buf->consumer_index);
This is not so sensible. The one written by the kernel should be just
a normal load and there must be some lock held here to protect that
The one written by user space should be just READ_ONCE
acquire only has meaning if you go on to read additional data based on
the result of the acquire - then acquire ensures the additional reads
can't cross writes that occured before the matching release.
Jason
