* [RFC PATCH] mm/vmscan: remove two un-needed mem_cgroup_page_lruvec() call @ 2014-02-16 3:30 Weijie Yang 2014-02-16 4:00 ` Hugh Dickins 0 siblings, 1 reply; 4+ messages in thread From: Weijie Yang @ 2014-02-16 3:30 UTC (permalink / raw) To: riel Cc: 'Mel Gorman', 'Andrew Morton', 'Minchan Kim', weijie.yang.kh, 'linux-kernel', 'Linux-MM' In putback_inactive_pages() and move_active_pages_to_lru(), lruvec is already an input parameter and pages are all from this lruvec, therefore there is no need to call mem_cgroup_page_lruvec() in loop. Signed-off-by: Weijie Yang <weijie.yang@samsung.com> --- mm/vmscan.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/mm/vmscan.c b/mm/vmscan.c index a9c74b4..4804fdb 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1393,8 +1393,6 @@ putback_inactive_pages(struct lruvec *lruvec, struct list_head *page_list) continue; } - lruvec = mem_cgroup_page_lruvec(page, zone); - SetPageLRU(page); lru = page_lru(page); add_page_to_lru_list(page, lruvec, lru); @@ -1602,7 +1600,6 @@ static void move_active_pages_to_lru(struct lruvec *lruvec, while (!list_empty(list)) { page = lru_to_page(list); - lruvec = mem_cgroup_page_lruvec(page, zone); VM_BUG_ON_PAGE(PageLRU(page), page); SetPageLRU(page); -- 1.7.10.4 -- 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/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] mm/vmscan: remove two un-needed mem_cgroup_page_lruvec() call 2014-02-16 3:30 [RFC PATCH] mm/vmscan: remove two un-needed mem_cgroup_page_lruvec() call Weijie Yang @ 2014-02-16 4:00 ` Hugh Dickins 2014-02-16 5:01 ` Weijie Yang 0 siblings, 1 reply; 4+ messages in thread From: Hugh Dickins @ 2014-02-16 4:00 UTC (permalink / raw) To: Weijie Yang Cc: riel, Mel Gorman, Andrew Morton, Minchan Kim, weijie.yang.kh, linux-kernel, Linux-MM On Sun, 16 Feb 2014, Weijie Yang wrote: > In putback_inactive_pages() and move_active_pages_to_lru(), > lruvec is already an input parameter and pages are all from this lruvec, > therefore there is no need to call mem_cgroup_page_lruvec() in loop. > > Signed-off-by: Weijie Yang <weijie.yang@samsung.com> Looks plausible but I believe it's incorrect. The lruvec passed in is the one we took the pages from, but there's a small but real chance that the page has become uncharged meanwhile, and should now be put back on the root_mem_cgroup's lruvec instead of the original memcg's lruvec. Hugh > --- > mm/vmscan.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index a9c74b4..4804fdb 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -1393,8 +1393,6 @@ putback_inactive_pages(struct lruvec *lruvec, struct list_head *page_list) > continue; > } > > - lruvec = mem_cgroup_page_lruvec(page, zone); > - > SetPageLRU(page); > lru = page_lru(page); > add_page_to_lru_list(page, lruvec, lru); > @@ -1602,7 +1600,6 @@ static void move_active_pages_to_lru(struct lruvec *lruvec, > > while (!list_empty(list)) { > page = lru_to_page(list); > - lruvec = mem_cgroup_page_lruvec(page, zone); > > VM_BUG_ON_PAGE(PageLRU(page), page); > SetPageLRU(page); > -- > 1.7.10.4 > > > -- > 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/ . > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> > -- 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/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] mm/vmscan: remove two un-needed mem_cgroup_page_lruvec() call 2014-02-16 4:00 ` Hugh Dickins @ 2014-02-16 5:01 ` Weijie Yang 2014-02-16 5:25 ` Hugh Dickins 0 siblings, 1 reply; 4+ messages in thread From: Weijie Yang @ 2014-02-16 5:01 UTC (permalink / raw) To: Hugh Dickins Cc: Weijie Yang, riel, Mel Gorman, Andrew Morton, Minchan Kim, linux-kernel, Linux-MM On Sun, Feb 16, 2014 at 12:00 PM, Hugh Dickins <hughd@google.com> wrote: > On Sun, 16 Feb 2014, Weijie Yang wrote: > >> In putback_inactive_pages() and move_active_pages_to_lru(), >> lruvec is already an input parameter and pages are all from this lruvec, >> therefore there is no need to call mem_cgroup_page_lruvec() in loop. >> >> Signed-off-by: Weijie Yang <weijie.yang@samsung.com> > > Looks plausible but I believe it's incorrect. The lruvec passed in > is the one we took the pages from, but there's a small but real chance > that the page has become uncharged meanwhile, and should now be put back > on the root_mem_cgroup's lruvec instead of the original memcg's lruvec. Hi Hugh, Thanks for your review. Frankly speaking, I am not very sure about it, that is why I add a RFC tag here. So, do we need update the reclaim_stat meanwhile as we change the lruvec? Regards, > Hugh > >> --- >> mm/vmscan.c | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/mm/vmscan.c b/mm/vmscan.c >> index a9c74b4..4804fdb 100644 >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -1393,8 +1393,6 @@ putback_inactive_pages(struct lruvec *lruvec, struct list_head *page_list) >> continue; >> } >> >> - lruvec = mem_cgroup_page_lruvec(page, zone); >> - >> SetPageLRU(page); >> lru = page_lru(page); >> add_page_to_lru_list(page, lruvec, lru); >> @@ -1602,7 +1600,6 @@ static void move_active_pages_to_lru(struct lruvec *lruvec, >> >> while (!list_empty(list)) { >> page = lru_to_page(list); >> - lruvec = mem_cgroup_page_lruvec(page, zone); >> >> VM_BUG_ON_PAGE(PageLRU(page), page); >> SetPageLRU(page); >> -- >> 1.7.10.4 >> >> >> -- >> 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/ . >> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> >> -- 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/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] mm/vmscan: remove two un-needed mem_cgroup_page_lruvec() call 2014-02-16 5:01 ` Weijie Yang @ 2014-02-16 5:25 ` Hugh Dickins 0 siblings, 0 replies; 4+ messages in thread From: Hugh Dickins @ 2014-02-16 5:25 UTC (permalink / raw) To: Weijie Yang Cc: Hugh Dickins, Weijie Yang, riel, Mel Gorman, Andrew Morton, Minchan Kim, linux-kernel, Linux-MM On Sun, 16 Feb 2014, Weijie Yang wrote: > On Sun, Feb 16, 2014 at 12:00 PM, Hugh Dickins <hughd@google.com> wrote: > > On Sun, 16 Feb 2014, Weijie Yang wrote: > > > >> In putback_inactive_pages() and move_active_pages_to_lru(), > >> lruvec is already an input parameter and pages are all from this lruvec, > >> therefore there is no need to call mem_cgroup_page_lruvec() in loop. > >> > >> Signed-off-by: Weijie Yang <weijie.yang@samsung.com> > > > > Looks plausible but I believe it's incorrect. The lruvec passed in > > is the one we took the pages from, but there's a small but real chance > > that the page has become uncharged meanwhile, and should now be put back > > on the root_mem_cgroup's lruvec instead of the original memcg's lruvec. > > Hi Hugh, > > Thanks for your review. > Frankly speaking, I am not very sure about it, that is why I add a RFC tag here. > So, do we need update the reclaim_stat meanwhile as we change the lruvec? No, it's not worth bothering about, it's only for stats and this is an unlikely case; whereas wrong memcg can be a significant correctness issue. Hugh -- 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/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-02-16 5:26 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-02-16 3:30 [RFC PATCH] mm/vmscan: remove two un-needed mem_cgroup_page_lruvec() call Weijie Yang 2014-02-16 4:00 ` Hugh Dickins 2014-02-16 5:01 ` Weijie Yang 2014-02-16 5:25 ` Hugh Dickins
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox