On 01/21/2013 06:49 PM, Michal Hocko wrote: > On Mon 21-01-13 15:13:31, Glauber Costa wrote: >> After the preparation work done in earlier patches, the cgroup_lock can >> be trivially replaced with a memcg-specific lock. This is an automatic >> translation in every site the values involved were queried. >> >> The sites were values are written, however, used to be naturally called >> under cgroup_lock. This is the case for instance of the css_online >> callback. For those, we now need to explicitly add the memcg_lock. >> >> Also, now that the memcg_mutex is available, there is no need to abuse >> the set_limit mutex in kmemcg value setting. The memcg_mutex will do a >> better job, and we now resort to it. > > You will hate me for this because I should have said that in the > previous round already (but I will use "I shown a mercy on you and > that blinded me" for my defense). > I am not so sure it will do a better job (it is only kmem that uses both > locks). I thought that memcg_mutex is just a first step and that we move > to a more finer grained locking later (a too general documentation of > the lock even asks for it). So I would keep the limit mutex and figure > whether memcg_mutex could be split up even further. > > Other than that the patch looks good to me > By now I have more than enough reasons to hate you, so this one won't add much. Even then, don't worry. Beer resets it all. That said, I disagree with you. As you noted yourself, kmem needs both locks: 1) cgroup_lock, because we need to prevent creation of sub-groups. 2) set_limit lock, because we need one - any one - memcg global lock be held while we are manipulating the kmem-specific data structures, and we would like to spread cgroup_lock all around for that. I now regret not having created the memcg_mutex for that: I'd be now just extending it to other users, instead of trying a replacement. So first of all, if the limit mutex is kept, we would *still* need to hold the memcg mutex to avoid children appearing. If we *ever* switch to a finer-grained lock(*), we will have to hold that lock anyway. So why hold set_limit_mutex?? (*) None of the operations protected by this mutex are fast paths... >> With this, all the calls to cgroup_lock outside cgroup core are gone. > > OK, Tejun will be happy ;) > He paid me ice cream. -- 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>