Re: [PATCH] Introduce buflock, a one-to-many circular buffer mechanism (rev2)
From: Henrik Rydberg <hidden>
Date: 2010-06-22 13:55:53
Also in:
lkml
From: Henrik Rydberg <hidden>
Date: 2010-06-22 13:55:53
Also in:
lkml
Andrew Morton wrote: [...]
I don't understand why this has "lock" in its name. The API itself is a mixture of "bufwrite_foo" and "bufread_foo". It's all a bit chaotic. I'd suggest picking a sane name for the whole subsytem - perhaps "mrbuf" for "multi reader buffer"? Then consistently name all interface functions as "mrbuf_foo". mrbuf.h, mrbuf_write_lock(), etc.
Point taken.
quoted
+static __always_inline bool __must_check bufread_retry(struct buflock_reader *br, const struct buflock_writer *bw) +{ + smp_rmb(); + if (unlikely(((br->tail - br->last) & bw->page) < bw->next - br->last)) + return true; + ++br->tail; + if (unlikely(br->head - br->tail > bw->page)) + br->tail = br->head; + return false; +}This looks too large to be inlined. What's the __always_inline for? Was gcc uninlining this within separate compilation units?
As you say, the function is large, and I am uncertain about the rules regarding compiler reordering across general function calls. Starting a general function with a memory barrier feels weird. Perhaps the function should be split? Thanks, Henrik