Memory barriers about buf_ring(9)

[rhenjau]

Hi, Ian
     Thank you for your information.

On 2015/5/7 22:20, Ian Lepore wrote:
> 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
>