Messages in this thread | | | Date | Fri, 24 Aug 2007 16:34:57 -0400 | From | Chris Snook <> | Subject | Re: [PATCH] i386: Fix a couple busy loops in mach_wakecpu.h:wait_for_init_deassert() |
| |
Denys Vlasenko wrote: > On Friday 24 August 2007 18:06, Christoph Lameter wrote: >> On Fri, 24 Aug 2007, Satyam Sharma wrote: >>> But if people do seem to have a mixed / confused notion of atomicity >>> and barriers, and if there's consensus, then as I'd said earlier, I >>> have no issues in going with the consensus (eg. having API variants). >>> Linus would be more difficult to convince, however, I suspect :-) >> The confusion may be the result of us having barrier semantics in >> atomic_read. If we take that out then we may avoid future confusions. > > I think better name may help. Nuke atomic_read() altogether. > > n = atomic_value(x); // doesnt hint as strongly at reading as "atomic_read" > n = atomic_fetch(x); // yes, we _do_ touch RAM > n = atomic_read_uncached(x); // or this > > How does that sound?
atomic_value() vs. atomic_fetch() should be rather unambiguous. atomic_read_uncached() begs the question of precisely which cache we are avoiding, and could itself cause confusion.
So, if I were writing atomic.h from scratch, knowing what I know now, I think I'd use atomic_value() and atomic_fetch(). The problem is that there are a lot of existing users of atomic_read(), and we can't write a script to correctly guess their intent. I'm not sure auditing all uses of atomic_read() is really worth the comparatively miniscule benefits.
We could play it safe and convert them all to atomic_fetch(), or we could acknowledge that changing the semantics 8 months ago was not at all disastrous, and make them all atomic_value(), allowing people to use atomic_fetch() where they really care.
-- Chris - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |