Memory barriers about buf_ring(9)

[Ian Lepore]

On Thu, 2015-05-07 at 18:10 +0800, rhenjau wrote:
> Hi, hackers
>     I'm reading buf_ring(9) and have a question about the following
> function. Is a memory barrier needed before br_cons_tail to prevent the
> loading of br_ring[cons_head] load is reordered after br_cons_tail is set?
> Producers may overwrite the area.
> 
> /*
>  * single-consumer dequeue
>  * use where dequeue is protected by a lock
>  * e.g. a network driver's tx queue lock
>  */
> static __inline void *
> buf_ring_dequeue_sc(struct buf_ring *br)
> {
>         uint32_t cons_head, cons_next;
> #ifdef PREFETCH_DEFINED
>         uint32_t cons_next_next;
> #endif
>         uint32_t prod_tail;
>         void *buf;
> 
>         cons_head = br->br_cons_head;
>         prod_tail = br->br_prod_tail;
> 
>         cons_next = (cons_head + 1) & br->br_cons_mask;
> #ifdef PREFETCH_DEFINED
>         cons_next_next = (cons_head + 2) & br->br_cons_mask;
> #endif
> 
>         if (cons_head == prod_tail)
>                 return (NULL);
> 
> #ifdef PREFETCH_DEFINED
>         if (cons_next != prod_tail) {
>                 prefetch(br->br_ring[cons_next]);
>                 if (cons_next_next != prod_tail)
>                         prefetch(br->br_ring[cons_next_next]);
>         }
> #endif
>         br->br_cons_head = cons_next;
>         buf = br->br_ring[cons_head];
> 
> #ifdef DEBUG_BUFRING
>         br->br_ring[cons_head] = NULL;
>         if (!mtx_owned(br->br_lock))
>                 panic("lock not held on single consumer dequeue");
>         if (br->br_cons_tail != cons_head)
>                 panic("inconsistent list cons_tail=%d cons_head=%d",
>                     br->br_cons_tail, cons_head);
> #endif
>         ----------------------------------------------------- need memory
> barrier?
>         br->br_cons_tail = cons_next;
>         return (buf);
> }
> 

There was some discussion and changes relating to barriers in buf_ring,
but it looks like the changes never got committed (I don't know why).

https://reviews.freebsd.org/D1945

-- Ian