On Thu, Sep 25, 2014 at 09:43:42AM -0400, Johannes Weiner wrote:
> > > + if (next_css == &root->css ||
> > > + css_tryget_online(next_css)) {
> > > + struct mem_cgroup *memcg;
> > > +
> > > + memcg = mem_cgroup_from_css(next_css);
> > > + if (memcg->initialized) {
> > > + /*
> > > + * Make sure the caller's accesses to
> > > + * the memcg members are issued after
> > > + * we see this flag set.
> >
> > I usually prefer if the comment points to the exact location that the
> > matching memory barriers live. Sometimes it's difficult to locate the
> > partner barrier even w/ the functional explanation.
That is indeed good practise! :-)
> > > + */
> > > + smp_rmb();
> > > + return memcg;
> >
> > In an unlikely event this rmb becomes an issue, a self-pointing
> > pointer which is set/read using smp_store_release() and
> > smp_load_acquire() respectively can do with plain barrier() on the
> > reader side on archs which don't need data dependency barrier
> > (basically everything except alpha). Not sure whether that'd be more
> > or less readable than this tho.
> So as far as I understand memory-barriers.txt we do not even need a
> data dependency here to use store_release and load_acquire:
>
> mem_cgroup_css_online():
> <initialize memcg>
> smp_store_release(&memcg->initialized, 1);
>
> mem_cgroup_iter():
> <look up maybe-initialized memcg>
> if (smp_load_acquire(&memcg->initialized))
> return memcg;
>
> So while I doubt that the smp_rmb() will become a problem in this
> path, it would be neat to annotate the state flag around which we
> synchronize like this, rather than have an anonymous barrier.
>
> Peter, would you know if this is correct, or whether these primitives
> actually do require a data dependency?
I'm fairly sure you do not. load_acquire() has the same barrier in on
Alpha that read_barrier_depends() does, and that's the only arch that
matters.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>