On Fri, Nov 01, 2013 at 04:25:42PM +0200, Victor Kaplansky wrote:

"Paul E. McKenney" [off-list ref] wrote on 10/31/2013
08:40:15 AM:

quoted

quoted

void ubuf_read(void)
{
   u64 head, tail;

   tail = ACCESS_ONCE(ubuf->tail);
   head = ACCESS_ONCE(ubuf->head);

   /*
    * Ensure we read the buffer boundaries before the actual buffer
    * data...
    */
   smp_rmb(); /* C, matches with B */

   while (tail != head) {
      obj = ubuf->data + tail;
      /* process obj */
      tail += obj->size;
      tail %= ubuf->size;
   }

   /*
    * Ensure all data reads are complete before we issue the
    * ubuf->tail update; once that update hits, kbuf_write() can
    * observe and overwrite data.
    */
   smp_mb(); /* D, matches with A */

   ubuf->tail = tail;
}

quoted

quoted

Could we replace A and C with an smp_read_barrier_depends()?

C, yes, given that you have ACCESS_ONCE() on the fetch from ->tail
and that the value fetch from ->tail feeds into the address used for
the "obj =" assignment.

No! You must to have a full smp_rmb() at C. The race on the reader side
is not between fetch of @tail and read from address pointed by @tail.
The real race here is between a fetch of @head and read of obj from
memory pointed by @tail.

I believe you are in fact correct, good catch.

							Thanx, Paul