kern/160992: buf_ring(9) statistics accounting not MPSAFE

[Arnaud Lacombe]

>Number:         160992
>Category:       kern
>Synopsis:       buf_ring(9) statistics accounting not MPSAFE
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sat Sep 24 20:20:10 UTC 2011
>Closed-Date:
>Last-Modified:
>Originator:     Arnaud Lacombe
>Release:        9-CURRENT
>Organization:
n/a
>Environment:
>Description:
The following block of code, in `sys/sys/buf_ring.h':


       /*
        * If there are other enqueues in progress
        * that preceeded us, we need to wait for them
        * to complete
        */
       while (br->br_prod_tail != prod_head)
               cpu_spinwait();
       br->br_prod_bufs++;
       br->br_prod_bytes += nbytes;
       br->br_prod_tail = prod_next;
       critical_exit();


can be seen at runtime, memory-wise as:

      while (br->br_prod_tail != prod_head)
              cpu_spinwait();
      br->br_prod_tail = prod_next;
      br->br_prod_bufs++;
      br->br_prod_bytes += nbytes;
      critical_exit();


That is, there is no memory barrier to enforce completion of the
load/increment/store/load/load/addition/store operations before
updating what other thread spin on. 

Even if `br_prod_tail' is marked `volatile', there is no guarantee that it will not be re-ordered wrt. non-volatile write (to `br_prod_bufs' and `br_prod_bytes').


Confirmed by Kip Macy (kmacy@) in http://lists.freebsd.org/pipermail/freebsd-hackers/2011-September/036454.html.
>How-To-Repeat:
code review.
>Fix:


>Release-Note:
>Audit-Trail:
>Unformatted: