On Thu 06-06-13 17:52:42, Tejun Heo wrote:
> Hello,
>
> On Thu, Jun 06, 2013 at 01:50:31PM +0200, Michal Hocko wrote:
> > > Also, do we need to hold a lock? It doesn't have to be completely
> > > strict, so we might as well get away with something like,
> > >
> > > for_each_cached_pos() {
> > > if (hint == me) {
> > > /* simple clearing implementation, we prolly wanna push it forward */
> > > cached = xchg(hint, NULL);
> > > if (cached)
> > > css_put(cached);
> > > }
> > > }
> >
> > This would be racy:
> > mem_cgroup_iter
> > rcu_read_lock
> > __mem_cgroup_iter_next cgroup_destroy_locked
> > css_tryget(memcg)
> > atomic_add(CSS_DEACT_BIAS)
> > offline_css(memcg)
> > xchg(memcg, NULL)
> > mem_cgroup_iter_update
> > iter->last_visited = memcg
> > rcy_read_unlock
> >
> > But if it was called from call_rcu the we should be safe AFAICS.
>
> Oh yeah, it is racy. That's what I meant by "not having to be
> completely strict". The race window is small enough and it's not like
> we're messing up refcnt or may end up with use-after-free.
But it would potentially pin (aka leak) the memcg for ever.
> Doing it from RCU would make the race go away but I'm not sure whether
> the extra RCU bouncing is worthwhile. I don't know. Maybe.
>
> Thanks.
>
> --
> tejun
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
