On Wed, Jul 07, 2021 at 04:41:05PM -0400, Johannes Weiner wrote: > 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. OK, so in the prerequisite series to this patch, lock_page() becomes folio_lock(). This series turns lock_page_memcg() into folio_memcg_lock(). As a minimum, then, this needs to turn into __folio_memcg_unlock(). > 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. Unfortunately, it's still not clear. I've answered questions from people who think that they have the page locked because they called lock_page_memcg() ;-(
