On Sat, Sep 18, 2021 at 8:13 AM Roman Gushchin <guro@xxxxxx> wrote: > > On Fri, Sep 17, 2021 at 06:49:21PM +0800, Muchun Song wrote: > > On Fri, Sep 17, 2021 at 9:29 AM Roman Gushchin <guro@xxxxxx> wrote: > > > > > > Hi Muchun! > > > > > > On Thu, Sep 16, 2021 at 09:47:35PM +0800, Muchun Song wrote: > > > > This version is rebased over linux 5.15-rc1, because Shakeel has asked me > > > > if I could do that. I rework some code suggested by Roman as well in this > > > > version. I have not removed the Acked-by tags which are from Roman, because > > > > this version is not based on the folio relevant. If Roman wants me to > > > > do this, please let me know, thanks. > > > > > > I'm fine with this, thanks for clarifying. > > > > > > > > > > > Since the following patchsets applied. All the kernel memory are charged > > > > with the new APIs of obj_cgroup. > > > > > > > > [v17,00/19] The new cgroup slab memory controller[1] > > > > [v5,0/7] Use obj_cgroup APIs to charge kmem pages[2] > > > > > > > > But user memory allocations (LRU pages) pinning memcgs for a long time - > > > > it exists at a larger scale and is causing recurring problems in the real > > > > world: page cache doesn't get reclaimed for a long time, or is used by the > > > > second, third, fourth, ... instance of the same job that was restarted into > > > > a new cgroup every time. Unreclaimable dying cgroups pile up, waste memory, > > > > and make page reclaim very inefficient. > > > > > > I've an idea: what if we use struct list_lru_memcg as an intermediate object > > > between an individual page and struct mem_cgroup? > > > > > > It could contain a pointer to a memory cgroup structure (not even sure if a > > > reference is needed), and a lru page can contain a pointer to the lruvec instead > > > of memcg/objcg. > > lruvec_memcg I mean. Thanks for your clarification. > > > > > Hi Roman, > > > > If I understand properly, here you mean the struct page has a pointer > > to the struct lruvec not struct list_lru_memcg. What's the functionality > > of the struct list_lru_memcg? Would you mind exposing more details? > > So the basic idea is simple: a lru page charged to a memcg is associated with > a per-memcg lruvec (list_lru_memcg), which is associated with a memory cgroup. > And after your patches there is a second link of associations: page to objcg > to memcg: > > 1) page->objcg->memcg > 2) page->list_lru_memcg->memcg > > (those are not necessarily direct pointers, but generally speaking, relations). > > My gut feeling is that if we can merge them into just 2) and use list_lru_memcg > as an intermediate object between pages and memory cgroups, the whole thing can > be more efficient and beautiful. > > Yes, on reparenting we'd need to scan over all pages in the lru list, but > hopefully we can do it from a worker context. And it's not such a big deal as > with slab objects, where we simple had no list of all objects. struct list_lru_memcg seems to be redundant, it just contains a pointer to struct mem_cgroup. We need to update each page->lruvec_memcg, why not update page->memcg_data directly to its parent memcg? The update of page->lruvec_memcg should be under both child and parent's lruvec lock, right? I suppose scanning over all pages may be a problem if there are many pages. Thanks. > > Again, I'm not 100% sure if it's possible and worth it, so it shouldn't block > your patchset if everybody else like it. > > Thanks
