Messages in this thread | | | Date | Sun, 14 Aug 2011 20:01:36 -0700 | Subject | Re: [patch 8/8] mm: make per-memcg lru lists exclusive | From | Ying Han <> |
| |
On Fri, Aug 12, 2011 at 12:17 PM, Johannes Weiner <hannes@cmpxchg.org> wrote: > On Fri, Aug 12, 2011 at 10:08:18AM -0700, Ying Han wrote: >> On Fri, Aug 12, 2011 at 1:34 AM, Johannes Weiner <hannes@cmpxchg.org> wrote: >> > And in reality, we only care about properly memcg-unaccounting the >> > old lru state before we change pc->mem_cgroup, so this becomes >> > >> > if (!PageLRU(page)) >> > return; >> > spin_lock_irqsave(&zone->lru_lock, flags); >> > if (!PageCgroupUsed(pc)) >> > mem_cgroup_lru_del(page); >> > spin_unlock_irqrestore(&zone->lru_lock, flags); >> > >> > I don't see why we should care if the page stays physically linked >> > to the list. >> >> Can you clarify that? > > Well, I don't see anything wrong with leaving it on the LRU. We just > need to unaccount the page from pc->mem_cgroup's lru stats before the > page is charged, pc->mem_cgroup overwritten, and the account lost. > >> > The handling after committing the charge becomes this: >> > >> > - if (likely(!PageLRU(page))) >> > - return; >> > spin_lock_irqsave(&zone->lru_lock, flags); >> > lru = page_lru(page); >> > if (PageLRU(page) && !PageCgroupAcctLRU(pc)) { >> > del_page_from_lru_list(zone, page, lru); >> > add_page_to_lru_list(zone, page, lru); >> > } >> > >> > If the page is not on the LRU, someone else will put it there and link >> > it up properly. If it is on the LRU and already memcg-accounted then >> > it must be on the right lruvec as setting pc->mem_cgroup and PCG_USED >> > is properly ordered. Otherwise, it has to be physically moved to the >> > correct lruvec and memcg-accounted for. >> >> While working on the zone->lru_lock patch, i have been questioning myself on >> the PageLRU and PageCgroupAcctLRU bit. Here is my question: >> >> It looks to me that PageLRU indicates the page is linked to per-zone lru >> list, and PageCgroupAcctLRU indicates the page is charged to a memcg and >> also linked to memcg's private lru list. All of these work nicely when we >> have both global and private (per-memcg) lru list, but i can not put them >> together after this patch. >> >> Now page is linked to private lru always either memcg or root. While linked >> to either lru list, the page could be uncharged (like swapcache). No matter >> what, i am thinking whether or not we can get rid of the AcctLRU bit from pc >> and use LRU bit only here. > > As I said above: if after the commit the page is on the LRU (PageLRU > set), pc->mem_cgroup's lru stats may or may not include the page, and > the page may or may not be on the right lruvec. > > If someone had the page isolated (reclaim?) while we charge it and put > it back, the page may either be charged or uncharged at the time of > putback.
Thank you and this is a good example.
So PageLRU bit is consistent w/ whether or not the page is linked to a lru list (root or memcg), and AcctLRU indicates more on the memcg charge/uncharge.
Here I am trying to summarize the possibilities of different flags of a page linked to a lru list ( based on the implementation after this patch series). please help to correct :
root lru: 1. PageLRU, Used, AcctLRU: page charged to root and linked to root lru list. ( ext: page allocated under root cgroup )
2. PageLRU, !Used, !AcctLRU: page not charged and linked to root lru list. ( ext: page uncharged before free, or like readahead swapcache)
3. PageLRU, Used, !AcctLRU: page del from root lru before uncharge, or charged before add to root lru
4. PageLRU, !Used, AcctLRU: page uncharged before del from root lru (ext: swapcache)
non-root lru:
1. PageLRU, Used, AcctLRU: page charged to memcg and linked to memcg lru list
2. PageLRU, !Used, !AcctLRU: not sure if this is possible
3. PageLRU, Used, !AcctLRU: page del from memcg lru before uncharge, or charged before add to memcg lru
4. PageLRU, !Used, AcctLRU: page uncharged before del from memcg lru (ext: swapcache)
> > unused: PageLRU is set, but page possibly on the wrong lruvec > (root_mem_cgroup's per default, see mem_cgroup_lru_add_list) > and not properly accounted for. We can detect this case by > seeing AcctLRU cleared.
This fits the case 2 above.
> > used: PageLRU is set, page on the right lruvec and properly > accounted. We can detect this case by seeing that > mem_cgroup_lru_add_list() set AcctLRU.
This fits the case 1 above.
Thanks
--Ying -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |