Date: Fri, 08 May 2015 12:35:54 +0800 From: rhenjau <rhenjau@gmail.com> To: Ian Lepore <ian@freebsd.org> Cc: freebsd-hackers@freebsd.org Subject: Re: Memory barriers about buf_ring(9) Message-ID: <554C3D2A.7030909@gmail.com> In-Reply-To: <1431008458.6170.165.camel@freebsd.org> References: <CA%2BO7MXzRuy33tWQRWAnh3Z6ZsdLOsVvm0-1p8%2BjrfFOannjZbQ@mail.gmail.com> <1431008458.6170.165.camel@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?554C3D2A.7030909>