On Tue, 20 Dec 2011 17:16:15 +0100
Michal Hocko <mhocko@xxxxxxx> wrote:
> On Wed 14-12-11 16:52:26, KAMEZAWA Hiroyuki wrote:
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> >
> > Now, at LRU handling, memory cgroup needs to do complicated works
> > to see valid pc->mem_cgroup, which may be overwritten.
> >
> > This patch is for relaxing the protocol. This patch guarantees
> > - when pc->mem_cgroup is overwritten, page must not be on LRU.
>
> How the patch guarantees that? I do not see any enforcement. In fact we
> depend on the previous patches, don't we.
>
Ah, yes. We depends on previous patch series.
> >
> > By this, LRU routine can believe pc->mem_cgroup and don't need to
> > check bits on pc->flags. This new rule may adds small overheads to
> > swapin. But in most case, lru handling gets faster.
> >
> > After this patch, PCG_ACCT_LRU bit is obsolete and removed.
>
> It makes things much more simpler. I just think it needs a better
> description.
>
O.K.
99% of memcg charging are done by following call path.
- alloc_page() -> charge() -> map/enter radix-tree -> add to LRU.
We need some special case cares.
- SwapCache - newly allocated/fully unmapped pages are added to LRU
before charge.
=> handled by previous patch.
- FUSE - unused pages are reused.
=> handled by previous patch.
- move_account
=> we do isolate_page().
Now, we can guarantee pc->mem_cgroup is set when page is not added to
LRU or under zone->lru_lock + isolate from LRU.
I'll add some Documenation to...memcg_debug.txt
Thanks,
-Kame
> >
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> > ---
> > include/linux/page_cgroup.h | 8 -----
> > mm/memcontrol.c | 72 ++++++++++--------------------------------
> > 2 files changed, 17 insertions(+), 63 deletions(-)
> >
> > diff --git a/include/linux/page_cgroup.h b/include/linux/page_cgroup.h
> > index aaa60da..2cddacf 100644
> > --- a/include/linux/page_cgroup.h
> > +++ b/include/linux/page_cgroup.h
> > @@ -10,8 +10,6 @@ enum {
> > /* flags for mem_cgroup and file and I/O status */
> > PCG_MOVE_LOCK, /* For race between move_account v.s. following bits */
> > PCG_FILE_MAPPED, /* page is accounted as "mapped" */
> > - /* No lock in page_cgroup */
> > - PCG_ACCT_LRU, /* page has been accounted for (under lru_lock) */
> > __NR_PCG_FLAGS,
> > };
> >
> > @@ -75,12 +73,6 @@ TESTPCGFLAG(Used, USED)
> > CLEARPCGFLAG(Used, USED)
> > SETPCGFLAG(Used, USED)
> >
> > -SETPCGFLAG(AcctLRU, ACCT_LRU)
> > -CLEARPCGFLAG(AcctLRU, ACCT_LRU)
> > -TESTPCGFLAG(AcctLRU, ACCT_LRU)
> > -TESTCLEARPCGFLAG(AcctLRU, ACCT_LRU)
> > -
> > -
> > SETPCGFLAG(FileMapped, FILE_MAPPED)
> > CLEARPCGFLAG(FileMapped, FILE_MAPPED)
> > TESTPCGFLAG(FileMapped, FILE_MAPPED)
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 2ae973d..d5e21e7 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -974,30 +974,8 @@ struct lruvec *mem_cgroup_lru_add_list(struct zone *zone, struct page *page,
> > return &zone->lruvec;
> >
> > pc = lookup_page_cgroup(page);
> > - VM_BUG_ON(PageCgroupAcctLRU(pc));
> > - /*
> > - * putback: charge:
> > - * SetPageLRU SetPageCgroupUsed
> > - * smp_mb smp_mb
> > - * PageCgroupUsed && add to memcg LRU PageLRU && add to memcg LRU
> > - *
> > - * Ensure that one of the two sides adds the page to the memcg
> > - * LRU during a race.
> > - */
> > - smp_mb();
> > - /*
> > - * If the page is uncharged, it may be freed soon, but it
> > - * could also be swap cache (readahead, swapoff) that needs to
> > - * be reclaimable in the future. root_mem_cgroup will babysit
> > - * it for the time being.
> > - */
> > - if (PageCgroupUsed(pc)) {
> > - /* Ensure pc->mem_cgroup is visible after reading PCG_USED. */
> > - smp_rmb();
> > - memcg = pc->mem_cgroup;
> > - SetPageCgroupAcctLRU(pc);
> > - } else
> > - memcg = root_mem_cgroup;
> > + memcg = pc->mem_cgroup;
> > + VM_BUG_ON(!memcg);
> > mz = page_cgroup_zoneinfo(memcg, page);
> > /* compound_order() is stabilized through lru_lock */
> > MEM_CGROUP_ZSTAT(mz, lru) += 1 << compound_order(page);
> > @@ -1024,18 +1002,8 @@ void mem_cgroup_lru_del_list(struct page *page, enum lru_list lru)
> > return;
> >
> > pc = lookup_page_cgroup(page);
> > - /*
> > - * root_mem_cgroup babysits uncharged LRU pages, but
> > - * PageCgroupUsed is cleared when the page is about to get
> > - * freed. PageCgroupAcctLRU remembers whether the
> > - * LRU-accounting happened against pc->mem_cgroup or
> > - * root_mem_cgroup.
> > - */
> > - if (TestClearPageCgroupAcctLRU(pc)) {
> > - VM_BUG_ON(!pc->mem_cgroup);
> > - memcg = pc->mem_cgroup;
> > - } else
> > - memcg = root_mem_cgroup;
> > + memcg = pc->mem_cgroup;
> > + VM_BUG_ON(!memcg);
> > mz = page_cgroup_zoneinfo(memcg, page);
> > /* huge page split is done under lru_lock. so, we have no races. */
> > MEM_CGROUP_ZSTAT(mz, lru) -= 1 << compound_order(page);
> > @@ -2377,6 +2345,7 @@ static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg,
> >
> > mem_cgroup_charge_statistics(memcg, PageCgroupCache(pc), nr_pages);
> > unlock_page_cgroup(pc);
> > + WARN_ON_ONCE(PageLRU(page));
> > /*
> > * "charge_statistics" updated event counter. Then, check it.
> > * Insert ancestor (and ancestor's ancestors), to softlimit RB-tree.
> > @@ -2388,7 +2357,7 @@ static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg,
> > #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> >
> > #define PCGF_NOCOPY_AT_SPLIT ((1 << PCG_LOCK) | (1 << PCG_MOVE_LOCK) |\
> > - (1 << PCG_ACCT_LRU) | (1 << PCG_MIGRATION))
> > + (1 << PCG_MIGRATION))
> > /*
> > * Because tail pages are not marked as "used", set it. We're under
> > * zone->lru_lock, 'splitting on pmd' and compound_lock.
> > @@ -2399,6 +2368,8 @@ void mem_cgroup_split_huge_fixup(struct page *head)
> > {
> > struct page_cgroup *head_pc = lookup_page_cgroup(head);
> > struct page_cgroup *pc;
> > + struct mem_cgroup_per_zone *mz;
> > + enum lru_list lru;
> > int i;
> >
> > if (mem_cgroup_disabled())
> > @@ -2407,23 +2378,15 @@ void mem_cgroup_split_huge_fixup(struct page *head)
> > pc = head_pc + i;
> > pc->mem_cgroup = head_pc->mem_cgroup;
> > smp_wmb();/* see __commit_charge() */
> > - /*
> > - * LRU flags cannot be copied because we need to add tail
> > - * page to LRU by generic call and our hooks will be called.
> > - */
> > pc->flags = head_pc->flags & ~PCGF_NOCOPY_AT_SPLIT;
> > }
> > -
> > - if (PageCgroupAcctLRU(head_pc)) {
> > - enum lru_list lru;
> > - struct mem_cgroup_per_zone *mz;
> > - /*
> > - * We hold lru_lock, then, reduce counter directly.
> > - */
> > - lru = page_lru(head);
> > - mz = page_cgroup_zoneinfo(head_pc->mem_cgroup, head);
> > - MEM_CGROUP_ZSTAT(mz, lru) -= HPAGE_PMD_NR - 1;
> > - }
> > + /*
> > + * Tail pages will be added to LRU.
> > + * We hold lru_lock,then,reduce counter directly.
> > + */
> > + lru = page_lru(head);
> > + mz = page_cgroup_zoneinfo(head_pc->mem_cgroup, head);
> > + MEM_CGROUP_ZSTAT(mz, lru) -= HPAGE_PMD_NR - 1;
> > }
> > #endif
> >
> > @@ -2656,10 +2619,9 @@ int mem_cgroup_cache_charge(struct page *page, struct mm_struct *mm,
> > if (!page_is_file_cache(page))
> > type = MEM_CGROUP_CHARGE_TYPE_SHMEM;
> >
> > - if (!PageSwapCache(page)) {
> > + if (!PageSwapCache(page))
> > ret = mem_cgroup_charge_common(page, mm, gfp_mask, type);
> > - WARN_ON_ONCE(PageLRU(page));
> > - } else { /* page is swapcache/shmem */
> > + else { /* page is swapcache/shmem */
> > ret = mem_cgroup_try_charge_swapin(mm, page, gfp_mask, &memcg);
> > if (!ret)
> > __mem_cgroup_commit_charge_swapin(page, memcg, type);
> > --
> > 1.7.4.1
> >
> >
> > --
> > 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/ .
> > Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
> > Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
>
> --
> Michal Hocko
> SUSE Labs
> SUSE LINUX s.r.o.
> Lihovarska 1060/12
> 190 00 Praha 9
> Czech Republic
>
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>