diff for duplicates of <20110815093912.GA15136@cmpxchg.org> diff --git a/a/1.txt b/N1/1.txt index 3c4e542..5f1ad24 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -9,19 +9,19 @@ On Sun, Aug 14, 2011 at 06:34:07PM -0700, Ying Han wrote: > >> +++ b/mm/memcontrol.c > >> @@ -832,7 +832,7 @@ static void > >> mem_cgroup_lru_del_before_commit_swapcache(struct page *page) -> >> * Forget old LRU when this page_cgroup is *not* used. This Used bit -> >> * is guarded by lock_page() because the page is SwapCache. -> >> */ -> >> - if (!PageCgroupUsed(pc)) -> >> + if (PageLRU(page) && !PageCgroupUsed(pc)) -> >> del_page_from_lru(zone, page); -> >> spin_unlock_irqrestore(&zone->lru_lock, flags); +> >> * Forget old LRU when this page_cgroup is *not* used. This Used bit +> >> * is guarded by lock_page() because the page is SwapCache. +> >> */ +> >> - if (!PageCgroupUsed(pc)) +> >> + if (PageLRU(page) && !PageCgroupUsed(pc)) +> >> del_page_from_lru(zone, page); +> >> spin_unlock_irqrestore(&zone->lru_lock, flags); > > > > Yes, as the first PageLRU check is outside the lru_lock, PageLRU may -> > indeed go away before grabbing the lock. The page will already be +> > indeed go away before grabbing the lock. The page will already be > > unlinked and the LRU accounting will be off. > > -> > The deeper problem, however, is that del_page_from_lru is wrong. We +> > The deeper problem, however, is that del_page_from_lru is wrong. We > > can not keep the page off the LRU while leaving PageLRU set, or it > > won't be very meaningful after the commit, anyway. > @@ -39,15 +39,15 @@ Nope. > > 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); +> > 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. The PageLRU check outside the lock is still fine as the +> > the list. The PageLRU check outside the lock is still fine as the > > accounting has been done already if !PageLRU and a putback without > > PageCgroupUsed will not re-account to pc->mem_cgroup, as the comment > > above this code explains nicely. @@ -74,14 +74,14 @@ The before-commit function is purely about accounting. > > 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 (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); +> > } > > Is the function mem_cgroup_lru_add_after_commit() ? I don't understand > why we have del_page_from_lru_list() here?Here is how the function @@ -112,3 +112,10 @@ LRU list linking. But it returns the lruvec the page has to sit on. The reason why we need to do del_page_from_lru_list() after my series is because the page may sit on the wrong lruvec and needs to be relinked. So del, and readd. + +-- +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 a2d8035..051b8ec 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -33,19 +33,19 @@ "> >> +++ b/mm/memcontrol.c\n" "> >> @@ -832,7 +832,7 @@ static void\n" "> >> mem_cgroup_lru_del_before_commit_swapcache(struct page *page)\n" - "> >> \302\240 \302\240 \302\240 \302\240 \302\240* Forget old LRU when this page_cgroup is *not* used. This Used bit\n" - "> >> \302\240 \302\240 \302\240 \302\240 \302\240* is guarded by lock_page() because the page is SwapCache.\n" - "> >> \302\240 \302\240 \302\240 \302\240 \302\240*/\n" - "> >> - \302\240 \302\240 \302\240 if (!PageCgroupUsed(pc))\n" - "> >> + \302\240 \302\240 \302\240 if (PageLRU(page) && !PageCgroupUsed(pc))\n" - "> >> \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 del_page_from_lru(zone, page);\n" - "> >> \302\240 \302\240 \302\240 \302\240 spin_unlock_irqrestore(&zone->lru_lock, flags);\n" + "> >> * Forget old LRU when this page_cgroup is *not* used. This Used bit\n" + "> >> * is guarded by lock_page() because the page is SwapCache.\n" + "> >> */\n" + "> >> - if (!PageCgroupUsed(pc))\n" + "> >> + if (PageLRU(page) && !PageCgroupUsed(pc))\n" + "> >> del_page_from_lru(zone, page);\n" + "> >> spin_unlock_irqrestore(&zone->lru_lock, flags);\n" "> >\n" "> > Yes, as the first PageLRU check is outside the lru_lock, PageLRU may\n" - "> > indeed go away before grabbing the lock. \302\240The page will already be\n" + "> > indeed go away before grabbing the lock. The page will already be\n" "> > unlinked and the LRU accounting will be off.\n" "> >\n" - "> > The deeper problem, however, is that del_page_from_lru is wrong. \302\240We\n" + "> > The deeper problem, however, is that del_page_from_lru is wrong. We\n" "> > can not keep the page off the LRU while leaving PageLRU set, or it\n" "> > won't be very meaningful after the commit, anyway.\n" "> \n" @@ -63,15 +63,15 @@ "> > the old lru state before > we change pc->mem_cgroup, so this\n" "> > becomes\n" "> >\n" - "> > \302\240 \302\240 \302\240 \302\240if (!PageLRU(page))\n" - "> > \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240return;\n" - "> > \302\240 \302\240 \302\240 \302\240spin_lock_irqsave(&zone->lru_lock, flags);\n" - "> > \302\240 \302\240 \302\240 \302\240if (!PageCgroupUsed(pc))\n" - "> > \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240mem_cgroup_lru_del(page);\n" - "> > \302\240 \302\240 \302\240 \302\240spin_unlock_irqrestore(&zone->lru_lock, flags);\n" + "> > if (!PageLRU(page))\n" + "> > return;\n" + "> > spin_lock_irqsave(&zone->lru_lock, flags);\n" + "> > if (!PageCgroupUsed(pc))\n" + "> > mem_cgroup_lru_del(page);\n" + "> > spin_unlock_irqrestore(&zone->lru_lock, flags);\n" "> >\n" "> > I don't see why we should care if the page stays physically linked to\n" - "> > the list. \302\240The PageLRU check outside the lock is still fine as the\n" + "> > the list. The PageLRU check outside the lock is still fine as the\n" "> > accounting has been done already if !PageLRU and a putback without\n" "> > PageCgroupUsed will not re-account to pc->mem_cgroup, as the comment\n" "> > above this code explains nicely.\n" @@ -98,14 +98,14 @@ "\n" "> > The handling after committing the charge becomes this:\n" "> >\n" - "> > - \302\240 \302\240 \302\240 if (likely(!PageLRU(page)))\n" - "> > - \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 return;\n" - "> > \302\240 \302\240 \302\240 \302\240spin_lock_irqsave(&zone->lru_lock, flags);\n" - "> > \302\240 \302\240 \302\240 \302\240lru = page_lru(page);\n" - "> > \302\240 \302\240 \302\240 \302\240if (PageLRU(page) && !PageCgroupAcctLRU(pc)) {\n" - "> > \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240del_page_from_lru_list(zone, page, lru);\n" - "> > \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240add_page_to_lru_list(zone, page, lru);\n" - "> > \302\240 \302\240 \302\240 \302\240}\n" + "> > - if (likely(!PageLRU(page)))\n" + "> > - return;\n" + "> > spin_lock_irqsave(&zone->lru_lock, flags);\n" + "> > lru = page_lru(page);\n" + "> > if (PageLRU(page) && !PageCgroupAcctLRU(pc)) {\n" + "> > del_page_from_lru_list(zone, page, lru);\n" + "> > add_page_to_lru_list(zone, page, lru);\n" + "> > }\n" "> \n" "> Is the function mem_cgroup_lru_add_after_commit() ? I don't understand\n" "> why we have del_page_from_lru_list() here?Here is how the function\n" @@ -135,6 +135,13 @@ "\n" "The reason why we need to do del_page_from_lru_list() after my series\n" "is because the page may sit on the wrong lruvec and needs to be\n" - relinked. So del, and readd. + "relinked. So del, and readd.\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>" -7162403df6caf749dd2be6765c392d044018d597dbd388051e23bfac5b7089e6 +16889b0d12f677b9c3bf064f766d2ac79c66c66ff473f5ec84de3e144aa41d83
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.