diff for duplicates of <20110830151449.GA28136@cmpxchg.org> diff --git a/a/1.txt b/N1/1.txt index f5c0ec1..ec8b533 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -5,14 +5,14 @@ On Tue, Aug 30, 2011 at 12:07:07AM -0700, Ying Han wrote: > >> > On Mon, Aug 29, 2011 at 12:22:02AM -0700, Ying Han wrote: > >> > > > @@ -888,19 +888,21 @@ void mem_cgroup_del_lru_list(struct page *page, > >> > > > enum lru_list lru) -> >> > > > { -> >> > > > >------struct page_cgroup *pc; -> >> > > > >------struct mem_cgroup_per_zone *mz; +> >> > > > { +> >> > > > >------struct page_cgroup *pc; +> >> > > > >------struct mem_cgroup_per_zone *mz; > >> > > > +>------struct mem_cgroup *mem; -> >> > > > · -> >> > > > >------if (mem_cgroup_disabled()) -> >> > > > >------>-------return; -> >> > > > >------pc = lookup_page_cgroup(page); +> >> > > > . +> >> > > > >------if (mem_cgroup_disabled()) +> >> > > > >------>-------return; +> >> > > > >------pc = lookup_page_cgroup(page); > >> > > > ->------/* can happen while we handle swapcache. */ > >> > > > ->------if (!TestClearPageCgroupAcctLRU(pc)) > >> > > > ->------>-------return; @@ -26,7 +26,7 @@ On Tue, Aug 30, 2011 at 12:07:07AM -0700, Ying Han wrote: > >> > > > + > >> > > > +>------if (TestClearPageCgroupAcctLRU(pc) || PageCgroupUsed(pc)) { > >> > -> >> > This PageCgroupUsed part confuses me. A page that is being isolated +> >> > This PageCgroupUsed part confuses me. A page that is being isolated > >> > shortly after being charged while on the LRU may reach here, and then > >> > it is unaccounted from pc->mem_cgroup, which it never was accounted > >> > to. @@ -56,7 +56,7 @@ On Tue, Aug 30, 2011 at 12:07:07AM -0700, Ying Han wrote: > >> > >> what this flag tells me is that whether or not the page is on a PRIVATE lru > >> and being accounted, i used private here to differentiate from the per zone -> >> lru, where it also has PageLRU flag. The two flags are separate since pages +> >> lru, where it also has PageLRU flag. The two flags are separate since pages > >> could be on one lru not the other ( I guess ) , but this is changed after > >> having the root cgroup lru back. For example, AcctLRU is used to keep track > >> of the accounted lru pages, especially for root ( we didn't account the @@ -65,20 +65,20 @@ On Tue, Aug 30, 2011 at 12:07:07AM -0700, Ying Han wrote: > >> w/ AcctLRU flag. > >> > >> So in general, i am wondering we should be able to replace that eventually -> >> with existing Used and LRU bit. Sorry this seems to be something we like to +> >> with existing Used and LRU bit. Sorry this seems to be something we like to > >> consider later, not necessarily now :) > > > > I have now the following comment in mem_cgroup_lru_del_list(): > > -> > /* -> > * 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. -> > */ +> > /* +> > * 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. +> > */ > > -> > Does that answer your question? If not, please tell me, so I can fix +> > Does that answer your question? If not, please tell me, so I can fix > > the comment :-) > > Sorry, not clear to me yet :( @@ -98,14 +98,14 @@ with the page being lru-accounted to root_mem_cgroup (swap readahead, swapoff) or cleared with the page being lru-accounted to a different memcg (truncate/invalidate, unmap) -> >> > Always. Which also means that before_commit now ensures an LRU +> >> > Always. Which also means that before_commit now ensures an LRU > >> > page is moved to root_mem_cgroup for babysitting during the > >> > charge, so that concurrent isolations/putbacks are always -> >> > accounted correctly. Is this what you had in mind? Did I miss +> >> > accounted correctly. Is this what you had in mind? Did I miss > >> > something? > >> > >> In my tree, the before->commit->after protocol is folded into one function. -> >> I didn't post it since I know you also have patch doing that. So guess I +> >> I didn't post it since I know you also have patch doing that. So guess I > >> don't understand why we need to move the page to root while it is gonna be > >> charged to a memcg by commit_charge shortly after. > > @@ -115,12 +115,12 @@ memcg (truncate/invalidate, unmap) > > > > Consider the following scenario: > > -> > 1. page with multiple mappings swapped out. +> > 1. page with multiple mappings swapped out. > > -> > 2. one memcg faults the page, then unmaps it. The page is -> > uncharged, but swap-freeing fails due to the other ptes, and -> > the page stays lru-accounted on the memcg it's no longer -> > charged to. +> > 2. one memcg faults the page, then unmaps it. The page is +> > uncharged, but swap-freeing fails due to the other ptes, and +> > the page stays lru-accounted on the memcg it's no longer +> > charged to. > > I agree that a page could be ending up on a memcg-lru (AcctLRU) but > not charged (!Used). But not sure @@ -142,26 +142,26 @@ Yes, this scenario has this window where PageCgroupAcctLRU is cleared in before_commit and reclaim could race and isolate the page, unaccounting it from root_mem_cgroup which it was never charged to. -> > 3. another memcg faults the page. before_commit must -> > lru-unaccount from pc->mem_cgroup before pc->mem_cgroup is -> > overwritten. +> > 3. another memcg faults the page. before_commit must +> > lru-unaccount from pc->mem_cgroup before pc->mem_cgroup is +> > overwritten. > > -> > 4. the page is charged. after_commit does the fixup. +> > 4. the page is charged. after_commit does the fixup. > > -> > Between 3. and 4., a reclaimer can isolate the page. The old +> > Between 3. and 4., a reclaimer can isolate the page. The old > > lru-accounting is undone and mem_cgroup_lru_del() does this: > > -> > if (TestClearPageCgroupAcctLRU(pc)) { -> > VM_BUG_ON(!pc->mem_cgroup); -> > mem = pc->mem_cgroup; -> > } else -> > mem = root_mem_cgroup; -> > mz = page_cgroup_zoneinfo(mem, page); -> > /* huge page split is done under lru_lock. so, we have no races. */ -> > MEM_CGROUP_ZSTAT(mz, lru) -= 1 << compound_order(page); +> > if (TestClearPageCgroupAcctLRU(pc)) { +> > VM_BUG_ON(!pc->mem_cgroup); +> > mem = pc->mem_cgroup; +> > } else +> > mem = root_mem_cgroup; +> > mz = page_cgroup_zoneinfo(mem, page); +> > /* huge page split is done under lru_lock. so, we have no races. */ +> > MEM_CGROUP_ZSTAT(mz, lru) -= 1 << compound_order(page); > > > > The rule is that !PageCgroupAcctLRU means that the page is -> > lru-accounted to root_mem_cgroup. So when charging, the page has to +> > lru-accounted to root_mem_cgroup. So when charging, the page has to > > be moved to root_mem_cgroup until a new memcg is responsible for it. > > So here we are saying that isolating a page which has be @@ -186,3 +186,10 @@ Calling it on a page not on the LRU is a bug. > Wonder if clearing the PageLRU after before_commit is helpful here. How would after_commit detect whether the page needs relinking or not? + +-- +To unsubscribe, send a message with 'unsubscribe linux-mm' in +the body to majordomo@kvack.org. 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@kvack.org"> email@kvack.org </a> diff --git a/a/content_digest b/N1/content_digest index 1f39ae6..65d238d 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -35,14 +35,14 @@ "> >> > On Mon, Aug 29, 2011 at 12:22:02AM -0700, Ying Han wrote:\n" "> >> > > > @@ -888,19 +888,21 @@ void mem_cgroup_del_lru_list(struct page *page,\n" "> >> > > > enum lru_list lru)\n" - "> >> > > > \302\240{\n" - "> >> > > > \302\240>------struct page_cgroup *pc;\n" - "> >> > > > \302\240>------struct mem_cgroup_per_zone *mz;\n" + "> >> > > > {\n" + "> >> > > > >------struct page_cgroup *pc;\n" + "> >> > > > >------struct mem_cgroup_per_zone *mz;\n" "> >> > > > +>------struct mem_cgroup *mem;\n" - "> >> > > > \302\267\n" - "> >> > > > \302\240>------if (mem_cgroup_disabled())\n" - "> >> > > > \302\240>------>-------return;\n" - "> >> > > > \302\240>------pc = lookup_page_cgroup(page);\n" + "> >> > > > .\n" + "> >> > > > >------if (mem_cgroup_disabled())\n" + "> >> > > > >------>-------return;\n" + "> >> > > > >------pc = lookup_page_cgroup(page);\n" "> >> > > > ->------/* can happen while we handle swapcache. */\n" "> >> > > > ->------if (!TestClearPageCgroupAcctLRU(pc))\n" "> >> > > > ->------>-------return;\n" @@ -56,7 +56,7 @@ "> >> > > > +\n" "> >> > > > +>------if (TestClearPageCgroupAcctLRU(pc) || PageCgroupUsed(pc)) {\n" "> >> >\n" - "> >> > This PageCgroupUsed part confuses me. \302\240A page that is being isolated\n" + "> >> > This PageCgroupUsed part confuses me. A page that is being isolated\n" "> >> > shortly after being charged while on the LRU may reach here, and then\n" "> >> > it is unaccounted from pc->mem_cgroup, which it never was accounted\n" "> >> > to.\n" @@ -86,7 +86,7 @@ "> >>\n" "> >> what this flag tells me is that whether or not the page is on a PRIVATE lru\n" "> >> and being accounted, i used private here to differentiate from the per zone\n" - "> >> lru, where it also has PageLRU flag. \302\240The two flags are separate since pages\n" + "> >> lru, where it also has PageLRU flag. The two flags are separate since pages\n" "> >> could be on one lru not the other ( I guess ) , but this is changed after\n" "> >> having the root cgroup lru back. For example, AcctLRU is used to keep track\n" "> >> of the accounted lru pages, especially for root ( we didn't account the\n" @@ -95,20 +95,20 @@ "> >> w/ AcctLRU flag.\n" "> >>\n" "> >> So in general, i am wondering we should be able to replace that eventually\n" - "> >> with existing Used and LRU bit. \302\240Sorry this seems to be something we like to\n" + "> >> with existing Used and LRU bit. Sorry this seems to be something we like to\n" "> >> consider later, not necessarily now :)\n" "> >\n" "> > I have now the following comment in mem_cgroup_lru_del_list():\n" "> >\n" - "> > \302\240 \302\240 \302\240 \302\240/*\n" - "> > \302\240 \302\240 \302\240 \302\240 * root_mem_cgroup babysits uncharged LRU pages, but\n" - "> > \302\240 \302\240 \302\240 \302\240 * PageCgroupUsed is cleared when the page is about to get\n" - "> > \302\240 \302\240 \302\240 \302\240 * freed. \302\240PageCgroupAcctLRU remembers whether the\n" - "> > \302\240 \302\240 \302\240 \302\240 * LRU-accounting happened against pc->mem_cgroup or\n" - "> > \302\240 \302\240 \302\240 \302\240 * root_mem_cgroup.\n" - "> > \302\240 \302\240 \302\240 \302\240 */\n" + "> > /*\n" + "> > * root_mem_cgroup babysits uncharged LRU pages, but\n" + "> > * PageCgroupUsed is cleared when the page is about to get\n" + "> > * freed. PageCgroupAcctLRU remembers whether the\n" + "> > * LRU-accounting happened against pc->mem_cgroup or\n" + "> > * root_mem_cgroup.\n" + "> > */\n" "> >\n" - "> > Does that answer your question? \302\240If not, please tell me, so I can fix\n" + "> > Does that answer your question? If not, please tell me, so I can fix\n" "> > the comment :-)\n" "> \n" "> Sorry, not clear to me yet :(\n" @@ -128,14 +128,14 @@ "swapoff) or cleared with the page being lru-accounted to a different\n" "memcg (truncate/invalidate, unmap)\n" "\n" - "> >> > Always. \302\240Which also means that before_commit now ensures an LRU\n" + "> >> > Always. Which also means that before_commit now ensures an LRU\n" "> >> > page is moved to root_mem_cgroup for babysitting during the\n" "> >> > charge, so that concurrent isolations/putbacks are always\n" - "> >> > accounted correctly. \302\240Is this what you had in mind? \302\240Did I miss\n" + "> >> > accounted correctly. Is this what you had in mind? Did I miss\n" "> >> > something?\n" "> >>\n" "> >> In my tree, the before->commit->after protocol is folded into one function.\n" - "> >> I didn't post it since I know you also have patch doing that. \302\240So guess I\n" + "> >> I didn't post it since I know you also have patch doing that. So guess I\n" "> >> don't understand why we need to move the page to root while it is gonna be\n" "> >> charged to a memcg by commit_charge shortly after.\n" "> >\n" @@ -145,12 +145,12 @@ "> >\n" "> > Consider the following scenario:\n" "> >\n" - "> > \302\240 \302\240 \302\240 \302\2401. page with multiple mappings swapped out.\n" + "> > 1. page with multiple mappings swapped out.\n" "> >\n" - "> > \302\240 \302\240 \302\240 \302\2402. one memcg faults the page, then unmaps it. \302\240The page is\n" - "> > \302\240 \302\240 \302\240 \302\240uncharged, but swap-freeing fails due to the other ptes, and\n" - "> > \302\240 \302\240 \302\240 \302\240the page stays lru-accounted on the memcg it's no longer\n" - "> > \302\240 \302\240 \302\240 \302\240charged to.\n" + "> > 2. one memcg faults the page, then unmaps it. The page is\n" + "> > uncharged, but swap-freeing fails due to the other ptes, and\n" + "> > the page stays lru-accounted on the memcg it's no longer\n" + "> > charged to.\n" "> \n" "> I agree that a page could be ending up on a memcg-lru (AcctLRU) but\n" "> not charged (!Used). But not sure\n" @@ -172,26 +172,26 @@ "in before_commit and reclaim could race and isolate the page,\n" "unaccounting it from root_mem_cgroup which it was never charged to.\n" "\n" - "> > \302\240 \302\240 \302\240 \302\2403. another memcg faults the page. \302\240before_commit must\n" - "> > \302\240 \302\240 \302\240 \302\240lru-unaccount from pc->mem_cgroup before pc->mem_cgroup is\n" - "> > \302\240 \302\240 \302\240 \302\240overwritten.\n" + "> > 3. another memcg faults the page. before_commit must\n" + "> > lru-unaccount from pc->mem_cgroup before pc->mem_cgroup is\n" + "> > overwritten.\n" "> >\n" - "> > \302\240 \302\240 \302\240 \302\2404. the page is charged. \302\240after_commit does the fixup.\n" + "> > 4. the page is charged. after_commit does the fixup.\n" "> >\n" - "> > Between 3. and 4., a reclaimer can isolate the page. \302\240The old\n" + "> > Between 3. and 4., a reclaimer can isolate the page. The old\n" "> > lru-accounting is undone and mem_cgroup_lru_del() does this:\n" "> >\n" - "> > \302\240 \302\240 \302\240 \302\240if (TestClearPageCgroupAcctLRU(pc)) {\n" - "> > \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240VM_BUG_ON(!pc->mem_cgroup);\n" - "> > \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240mem = pc->mem_cgroup;\n" - "> > \302\240 \302\240 \302\240 \302\240} else\n" - "> > \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240mem = root_mem_cgroup;\n" - "> > \302\240 \302\240 \302\240 mz = page_cgroup_zoneinfo(mem, page);\n" - "> > \302\240 \302\240 \302\240 \302\240/* huge page split is done under lru_lock. so, we have no races. */\n" - "> > \302\240 \302\240 \302\240 \302\240MEM_CGROUP_ZSTAT(mz, lru) -= 1 << compound_order(page);\n" + "> > if (TestClearPageCgroupAcctLRU(pc)) {\n" + "> > VM_BUG_ON(!pc->mem_cgroup);\n" + "> > mem = pc->mem_cgroup;\n" + "> > } else\n" + "> > mem = root_mem_cgroup;\n" + "> > mz = page_cgroup_zoneinfo(mem, page);\n" + "> > /* huge page split is done under lru_lock. so, we have no races. */\n" + "> > MEM_CGROUP_ZSTAT(mz, lru) -= 1 << compound_order(page);\n" "> >\n" "> > The rule is that !PageCgroupAcctLRU means that the page is\n" - "> > lru-accounted to root_mem_cgroup. \302\240So when charging, the page has to\n" + "> > lru-accounted to root_mem_cgroup. So when charging, the page has to\n" "> > be moved to root_mem_cgroup until a new memcg is responsible for it.\n" "> \n" "> So here we are saying that isolating a page which has be\n" @@ -215,6 +215,13 @@ "> \n" "> Wonder if clearing the PageLRU after before_commit is helpful here.\n" "\n" - How would after_commit detect whether the page needs relinking or not? + "How would after_commit detect whether the page needs relinking or not?\n" + "\n" + "--\n" + "To unsubscribe, send a message with 'unsubscribe linux-mm' in\n" + "the body to majordomo@kvack.org. For more info on Linux MM,\n" + "see: http://www.linux-mm.org/ .\n" + "Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/\n" + "Don't email: <a href=mailto:\"dont@kvack.org\"> email@kvack.org </a>" -ae9092805c72f9b4da2ab0ed3b5aaebb9748fdf3547ac89999d687dc8a7a4267 +dc1892ea10697b7c1978d74752de58a7be5c6a005efadb268df9db02eed50641
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.