On Thu 08-11-12 07:29:23, Tejun Heo wrote: > Hey, Michal. > > On Thu, Nov 08, 2012 at 04:20:39PM +0100, Michal Hocko wrote: > > > So, in the above example in CPU2, (B->state & FREEZING) test and > > > freezer_apply_state(C, false) can't be interleaved with the same > > > inheritance operation from CPU1. They either happen before or after. > > > > I am not sure I understand what you mean by inheritance operation but > > The operation of looking at one's parent and inherting the state. > Here, checking parent->state and calling apply_state accordingly. > > > you are right that the race is not possible because spin_lock makes > > sure that Foo->state is done after the lock(Foo->child) and spin_unlock > > then serves as a leave barrier so the other CPUs will see the correctly > > updated value. The rest is handled by the fixed ordered tree walk. So > > there is really no interleaving going on here. > > I'm worried that this is a bit too subtle.
Dunno, it looks obvious now, I just missed the entry&leave implicit barriers by spinlocks and again sorry about the confusion. > This will work fine with a single hierarchy mutex protecting hierarchy > updates and state propagations through it and that should work for > most controllers too. But single mutex is just ugly. > I want to have at least one example of finer grained locking for > future reference and cgroup_freezer happened to be the one I started > working on. I think this is a good example because it shows how to share the state without too many implementation details. > So, it is more complicated (not necessarily in written code but the > sync rules) than necessary. I'm still not sure whether to keep it or > not. I think the locking is fine and I would keep it this way rather than a big lock. > I'll add more documentation about synchronization in > cgroup_for_each_descendant_pre() either way. more doc cannot hurt ;) Thanks -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
