On Fri, Aug 12, 2011 at 12:17 PM, Johannes Weiner <hannes@xxxxxxxxxxx> 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@xxxxxxxxxxx> 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, 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