You're right. A write memory barrier is needed there. Thanks
On Thu, Sep 22, 2011 at 12:43 AM, Arnaud Lacombe <[email protected]> wrote: > Hi, > > On Mon, Sep 19, 2011 at 8:46 AM, K. Macy <[email protected]> wrote: >> If the value lags next by one then it is ours. This rule applies to >> all callers so the rule holds consistently. >> > I think you do not understand what I mean, which is that the following: > > 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(); > > at runtime, can be seen, 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. Yes, `br_prod_tail' is marked > `volatile', but there is no guarantee that it will not be re-ordered > wrt. non-volatile write (to `br_prod_bufs' and `br_prod_bytes'). > > - Arnaud > >> On Mon, Sep 19, 2011 at 5:53 AM, Arnaud Lacombe <[email protected]> wrote: >>> Hi, >>> >>> On Fri, Sep 16, 2011 at 10:41 AM, K. Macy <[email protected]> wrote: >>>> On Fri, Sep 16, 2011 at 3:02 AM, Arnaud Lacombe <[email protected]> wrote: >>>>> Hi, >>>>> >>>>> On Wed, Sep 14, 2011 at 10:53 PM, Arnaud Lacombe <[email protected]> >>>>> wrote: >>>>>> Hi Kip, >>>>>> >>>>>> I've got a few question about the buf_ring(9) API. >>>>>> >>>>>> 1) what means the 'drbr_' prefix. I can guess the two last letter, 'b' >>>>>> and 'r', for Buffer Ring, but what about 'd' and 'r' ? >>>>>> >>>>>> 2) in `sys/sys/buf_ring.h', you defined 'struct buf_ring' as: >>>>>> >>>>>> struct buf_ring { >>>>>> volatile uint32_t br_prod_head; >>>>>> volatile uint32_t br_prod_tail; >>>>>> int br_prod_size; >>>>>> int br_prod_mask; >>>>>> uint64_t br_drops; >>>>>> uint64_t br_prod_bufs; >>>>>> uint64_t br_prod_bytes; >>>>> shouldn't those 3 fields be updated atomically, especially on 32bits >>>>> platforms ? That might pose a problem as, AFAIK, FreeBSD do not have >>>>> MI 64bits atomics operations... >>>> >>>> Between the point at which br_prod_tail == prod_head and when we >>>> update br_prod_tail to point to prod_next we are the exclusive owners >>>> of the fields in buf_ring. That is why we wait for any other >>>> enqueueing threads to update br_prod_tail to point to prod_head before >>>> continuing. >>>> >>> How do you enforce ordering ? I do not see anything particular >>> forbidding the `br->br_prod_tail' to be committed first, leading other >>> thread to believe they have access to the statistics, while the other >>> thread has not yet committed its change. >>> >>> Thanks, >>> - Arnaud >>> >>>> Cheers >>>> >>>> /* >>>> * 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(); >>>> >>> >> > _______________________________________________ [email protected] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[email protected]"
