On Sat, Nov 22, 2014 at 1:22 AM, Taylor R Campbell <campbell+netbsd-tech-k...@mumble.net> wrote: > Date: Sat, 22 Nov 2014 01:03:58 +0900 > From: Masao Uebayashi <uebay...@gmail.com> > > The problem of TAILQ_INSERT_*() macros with respect to pserialize(9) > is that, as you are pointing out, they update new elm's next pointer > and prev elm's next pointer almost simultaneously. So you have to > ensure those ordering in the reader's code path. > > Dennis's point was that on most CPUs this ordering is guaranteed > already. My point was that that is not true on all CPUs, hence my > proposal for a membar_datadep_consumer which is a no-op on most CPUs. > > pserialize_read_enter(); > TAILQ_FOREACH(...) { > if (...) { > /* Ensure that E_p.next and generation are sync'ed. */ > mutex_spin_enter(); > > It matters what's in the ...'s. Are you doing this? > > TAILQ_FOREACH(e, head, next) { > if (e->key == key) { > mutex_spin_enter(&e->lock); > ... > > If so, the reader may see a stale e->key -- the membar_enter implied > by mutex_spin_enter is too late.
The promise there is that, e->key is constant while it's read with pserialize_read_{enter,leave}(9). If those members have to be changed, they have to be once removed from there, and re-added. > The reader needs a memory barrier > after loading e, and before loading anything e points to: > > TAILQ_FOREACH(e, head, next) { > membar_datadep_consumer(); > if (e->key == key) { > mutex_spin_enter(&e->lock); > ... > > I'm not clear on what you're trying to solve by versioning the tailqs, > but I don't think it will be necessary in the end for the purposes we > have in mind. I added because I think it is necessary.