On Wed, Jul 07, 2021 at 08:28:39PM +0100, Matthew Wilcox wrote: > On Wed, Jul 07, 2021 at 01:08:51PM -0400, Johannes Weiner wrote: > > On Wed, Jun 30, 2021 at 05:00:29AM +0100, Matthew Wilcox (Oracle) wrote: > > > -static void __unlock_page_memcg(struct mem_cgroup *memcg) > > > +static void __memcg_unlock(struct mem_cgroup *memcg) > > > > This is too generic a name. There are several locks in the memcg, and > > this one only locks the page->memcg bindings in the group. > > Fair. __memcg_move_unlock looks like the right name to me? Could you please elaborate what the problem with the current name is? mem_cgroup_move_account() does this: lock_page_memcg(page); page->memcg = to; __unlock_page_memcg(from); It locks and unlocks the page->memcg binding which can be done coming from the page or the memcg. The current names are symmetrical to reflect that it's the same lock. We could switch them both to move_lock, but as per the other email, lock_page_memcg() was chosen to resemble lock_page(). Because from a memcg POV they're interchangeable - the former is just a more narrowly scoped version for contexts that don't hold the page lock. It used to be called something else and we had several contexts taking redundant locks on accident because this hierarchy wasn't clear. I don't mind fixing poorly chosen or misleading naming schemes, but I think we need better explanations to overcome the reasoning behind the existing names, not just the assumption that there weren't any.
