Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
(3/27/13 9:00 AM), Michal Hocko wrote: > On Tue 26-03-13 16:35:35, Naoya Horiguchi wrote: > [...] >> The differences is that migrate_huge_page() has one hugepage as an argument, >> and migrate_pages() has a pagelist with multiple hugepages. >> I already told this before and I'm not sure it's enough to answer the >> question, >> so I explain another point about why this patch do like it. > > OK, I am blind. It is > + list_move(>lru, ); > + ret = migrate_pages(, new_page, MPOL_MF_MOVE_ALL, > + MIGRATE_SYNC, MR_MEMORY_FAILURE); > > which moves it from active_list and so you have to put it back. > >> I think that we must do putback_*pages() for source pages whether migration >> succeeds or not. >> But when we call migrate_pages() with a pagelist, >> the caller can't access to the successfully migrated source pages >> after migrate_pages() returns, because they are no longer on the pagelist. >> So putback of the successfully migrated source pages should be done *in* >> unmap_and_move() and/or unmap_and_move_huge_page(). > > If the migration succeeds then the page becomes unused and free after > its last reference drops. So I do not see any reason to put it back to > active list and free it right afterwards. > On the other hand unmap_and_move does the same thing (although page > reference counting is a bit more complicated in that case) so it would > be good to keep in sync with regular pages case. Even if pages are isolated from lists, there are several page count increasing path. So, putback_pages() close a race when page count != 1. I'm not sure, but I guess follow_hugepage() can make the same race. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
(3/27/13 9:00 AM), Michal Hocko wrote: On Tue 26-03-13 16:35:35, Naoya Horiguchi wrote: [...] The differences is that migrate_huge_page() has one hugepage as an argument, and migrate_pages() has a pagelist with multiple hugepages. I already told this before and I'm not sure it's enough to answer the question, so I explain another point about why this patch do like it. OK, I am blind. It is + list_move(hpage-lru, pagelist); + ret = migrate_pages(pagelist, new_page, MPOL_MF_MOVE_ALL, + MIGRATE_SYNC, MR_MEMORY_FAILURE); which moves it from active_list and so you have to put it back. I think that we must do putback_*pages() for source pages whether migration succeeds or not. But when we call migrate_pages() with a pagelist, the caller can't access to the successfully migrated source pages after migrate_pages() returns, because they are no longer on the pagelist. So putback of the successfully migrated source pages should be done *in* unmap_and_move() and/or unmap_and_move_huge_page(). If the migration succeeds then the page becomes unused and free after its last reference drops. So I do not see any reason to put it back to active list and free it right afterwards. On the other hand unmap_and_move does the same thing (although page reference counting is a bit more complicated in that case) so it would be good to keep in sync with regular pages case. Even if pages are isolated from lists, there are several page count increasing path. So, putback_pages() close a race when page count != 1. I'm not sure, but I guess follow_hugepage() can make the same race. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Mon 01-04-13 10:43:14, Aneesh Kumar K.V wrote: > Michal Hocko writes: > > > On Tue 26-03-13 16:59:40, Aneesh Kumar K.V wrote: > >> Naoya Horiguchi writes: > > [...] > >> > diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c > >> > index df0694c..4e01082 100644 > >> > --- v3.9-rc3.orig/mm/memory-failure.c > >> > +++ v3.9-rc3/mm/memory-failure.c > >> > @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page > >> > *page, int flags) > >> > int ret; > >> > unsigned long pfn = page_to_pfn(page); > >> > struct page *hpage = compound_head(page); > >> > +LIST_HEAD(pagelist); > >> > > >> > /* > >> > * This double-check of PageHWPoison is to avoid the race with > >> > @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page > >> > *page, int flags) > >> > unlock_page(hpage); > >> > > >> > /* Keep page count to indicate a given hugepage is isolated. */ > >> > -ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL, > >> > -MIGRATE_SYNC); > >> > -put_page(hpage); > >> > +list_move(>lru, ); > >> > >> we use hpage->lru to add the hpage to h->hugepage_activelist. This will > >> break a hugetlb cgroup removal isn't it ? > > > > This particular part will not break removal because > > hugetlb_cgroup_css_offline loops until hugetlb_cgroup_have_usage is 0. > > > > But we still need to hold hugetlb_lock around that right ? Right. Racing hugetlb_cgroup_move_parent and hugetlb_cgroup_migrate could lead to newpage pointing to NULL cgroup. That could be fixed by checking old page cgroup for NULL inside hugetlb_lock and using list_for_each_safe in hugetlb_cgroup_css_offline no? -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Mon 01-04-13 10:43:14, Aneesh Kumar K.V wrote: Michal Hocko mho...@suse.cz writes: On Tue 26-03-13 16:59:40, Aneesh Kumar K.V wrote: Naoya Horiguchi n-horigu...@ah.jp.nec.com writes: [...] diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c index df0694c..4e01082 100644 --- v3.9-rc3.orig/mm/memory-failure.c +++ v3.9-rc3/mm/memory-failure.c @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page *page, int flags) int ret; unsigned long pfn = page_to_pfn(page); struct page *hpage = compound_head(page); +LIST_HEAD(pagelist); /* * This double-check of PageHWPoison is to avoid the race with @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page *page, int flags) unlock_page(hpage); /* Keep page count to indicate a given hugepage is isolated. */ -ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL, -MIGRATE_SYNC); -put_page(hpage); +list_move(hpage-lru, pagelist); we use hpage-lru to add the hpage to h-hugepage_activelist. This will break a hugetlb cgroup removal isn't it ? This particular part will not break removal because hugetlb_cgroup_css_offline loops until hugetlb_cgroup_have_usage is 0. But we still need to hold hugetlb_lock around that right ? Right. Racing hugetlb_cgroup_move_parent and hugetlb_cgroup_migrate could lead to newpage pointing to NULL cgroup. That could be fixed by checking old page cgroup for NULL inside hugetlb_lock and using list_for_each_safe in hugetlb_cgroup_css_offline no? -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
Michal Hocko writes: > On Tue 26-03-13 16:59:40, Aneesh Kumar K.V wrote: >> Naoya Horiguchi writes: > [...] >> > diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c >> > index df0694c..4e01082 100644 >> > --- v3.9-rc3.orig/mm/memory-failure.c >> > +++ v3.9-rc3/mm/memory-failure.c >> > @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page *page, >> > int flags) >> >int ret; >> >unsigned long pfn = page_to_pfn(page); >> >struct page *hpage = compound_head(page); >> > + LIST_HEAD(pagelist); >> > >> >/* >> > * This double-check of PageHWPoison is to avoid the race with >> > @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page >> > *page, int flags) >> >unlock_page(hpage); >> > >> >/* Keep page count to indicate a given hugepage is isolated. */ >> > - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL, >> > - MIGRATE_SYNC); >> > - put_page(hpage); >> > + list_move(>lru, ); >> >> we use hpage->lru to add the hpage to h->hugepage_activelist. This will >> break a hugetlb cgroup removal isn't it ? > > This particular part will not break removal because > hugetlb_cgroup_css_offline loops until hugetlb_cgroup_have_usage is 0. > But we still need to hold hugetlb_lock around that right ? -aneesh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
Michal Hocko mho...@suse.cz writes: On Tue 26-03-13 16:59:40, Aneesh Kumar K.V wrote: Naoya Horiguchi n-horigu...@ah.jp.nec.com writes: [...] diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c index df0694c..4e01082 100644 --- v3.9-rc3.orig/mm/memory-failure.c +++ v3.9-rc3/mm/memory-failure.c @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page *page, int flags) int ret; unsigned long pfn = page_to_pfn(page); struct page *hpage = compound_head(page); + LIST_HEAD(pagelist); /* * This double-check of PageHWPoison is to avoid the race with @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page *page, int flags) unlock_page(hpage); /* Keep page count to indicate a given hugepage is isolated. */ - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL, - MIGRATE_SYNC); - put_page(hpage); + list_move(hpage-lru, pagelist); we use hpage-lru to add the hpage to h-hugepage_activelist. This will break a hugetlb cgroup removal isn't it ? This particular part will not break removal because hugetlb_cgroup_css_offline loops until hugetlb_cgroup_have_usage is 0. But we still need to hold hugetlb_lock around that right ? -aneesh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page() (fwd)
Forwarding off-list email On Fri 29-03-13 18:24:01, Aneesh Kumar K.V wrote: > Michal Hocko writes: > > > On Fri 29-03-13 10:56:00, Aneesh Kumar K.V wrote: > >> Michal Hocko writes: > > [...] > >> > Little bit offtopic: > >> > Btw. hugetlb migration breaks to charging even before this patchset > >> > AFAICS. The above put_page should remove the last reference and then it > >> > will uncharge it but I do not see anything that would charge a new page. > >> > This is all because regula LRU pages are uncharged when they are > >> > unmapped. But this a different story not related to this series. > >> > >> > >> But when we call that put_page, we would have alreayd move the cgroup > >> information to the new page. We have > >> > >>h_cg = hugetlb_cgroup_from_page(oldhpage); > >>set_hugetlb_cgroup(oldhpage, NULL); > >> > >>/* move the h_cg details to new cgroup */ > >>set_hugetlb_cgroup(newhpage, h_cg); > >> > >> > >> in hugetlb_cgroup_migrate > > > > Yes but the res counter charge would be missing for the newpage after > > put_page > > > > Moving this to private as i didn't get what you meant here. > > We unchage hugepage via > > __put_compound_page -> free_huge_page -> hugetlb_cgroup_uncharge_page > > That does > > h_cg = hugetlb_cgroup_from_page(page); > if (unlikely(!h_cg)) > return; > > During migrate we do > set_hugetlb_cgroup(oldhpage, NULL); > > /* move the h_cg details to new cgroup */ > set_hugetlb_cgroup(newhpage, h_cg); > > So when we do a put_page(oldhpage), we won't be finding any cgroup > attached to it and hence won't be updating the cgroup res counter value. > > So not sure what you mean by res counter change would be missing for the > newpage after put_page. I may be missing something here. Can you explain > ? Dang! You are right of course. I have missed the !h_cg check in hugetlb_cgroup_uncharge_page. Sorry about the confusion. I am so distracted by many issues these days... Thanks! -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Fri 29-03-13 10:56:00, Aneesh Kumar K.V wrote: > Michal Hocko writes: [...] > > Little bit offtopic: > > Btw. hugetlb migration breaks to charging even before this patchset > > AFAICS. The above put_page should remove the last reference and then it > > will uncharge it but I do not see anything that would charge a new page. > > This is all because regula LRU pages are uncharged when they are > > unmapped. But this a different story not related to this series. > > > But when we call that put_page, we would have alreayd move the cgroup > information to the new page. We have > > h_cg = hugetlb_cgroup_from_page(oldhpage); > set_hugetlb_cgroup(oldhpage, NULL); > > /* move the h_cg details to new cgroup */ > set_hugetlb_cgroup(newhpage, h_cg); > > > in hugetlb_cgroup_migrate Yes but the res counter charge would be missing for the newpage after put_page -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Fri 29-03-13 10:56:00, Aneesh Kumar K.V wrote: Michal Hocko mho...@suse.cz writes: [...] Little bit offtopic: Btw. hugetlb migration breaks to charging even before this patchset AFAICS. The above put_page should remove the last reference and then it will uncharge it but I do not see anything that would charge a new page. This is all because regula LRU pages are uncharged when they are unmapped. But this a different story not related to this series. But when we call that put_page, we would have alreayd move the cgroup information to the new page. We have h_cg = hugetlb_cgroup_from_page(oldhpage); set_hugetlb_cgroup(oldhpage, NULL); /* move the h_cg details to new cgroup */ set_hugetlb_cgroup(newhpage, h_cg); in hugetlb_cgroup_migrate Yes but the res counter charge would be missing for the newpage after put_page -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page() (fwd)
Forwarding off-list email On Fri 29-03-13 18:24:01, Aneesh Kumar K.V wrote: Michal Hocko mho...@suse.cz writes: On Fri 29-03-13 10:56:00, Aneesh Kumar K.V wrote: Michal Hocko mho...@suse.cz writes: [...] Little bit offtopic: Btw. hugetlb migration breaks to charging even before this patchset AFAICS. The above put_page should remove the last reference and then it will uncharge it but I do not see anything that would charge a new page. This is all because regula LRU pages are uncharged when they are unmapped. But this a different story not related to this series. But when we call that put_page, we would have alreayd move the cgroup information to the new page. We have h_cg = hugetlb_cgroup_from_page(oldhpage); set_hugetlb_cgroup(oldhpage, NULL); /* move the h_cg details to new cgroup */ set_hugetlb_cgroup(newhpage, h_cg); in hugetlb_cgroup_migrate Yes but the res counter charge would be missing for the newpage after put_page Moving this to private as i didn't get what you meant here. We unchage hugepage via __put_compound_page - free_huge_page - hugetlb_cgroup_uncharge_page That does h_cg = hugetlb_cgroup_from_page(page); if (unlikely(!h_cg)) return; During migrate we do set_hugetlb_cgroup(oldhpage, NULL); /* move the h_cg details to new cgroup */ set_hugetlb_cgroup(newhpage, h_cg); So when we do a put_page(oldhpage), we won't be finding any cgroup attached to it and hence won't be updating the cgroup res counter value. So not sure what you mean by res counter change would be missing for the newpage after put_page. I may be missing something here. Can you explain ? Dang! You are right of course. I have missed the !h_cg check in hugetlb_cgroup_uncharge_page. Sorry about the confusion. I am so distracted by many issues these days... Thanks! -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
Michal Hocko writes: > On Tue 26-03-13 16:59:40, Aneesh Kumar K.V wrote: >> Naoya Horiguchi writes: > [...] >> > diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c >> > index df0694c..4e01082 100644 >> > --- v3.9-rc3.orig/mm/memory-failure.c >> > +++ v3.9-rc3/mm/memory-failure.c >> > @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page *page, >> > int flags) >> >int ret; >> >unsigned long pfn = page_to_pfn(page); >> >struct page *hpage = compound_head(page); >> > + LIST_HEAD(pagelist); >> > >> >/* >> > * This double-check of PageHWPoison is to avoid the race with >> > @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page >> > *page, int flags) >> >unlock_page(hpage); >> > >> >/* Keep page count to indicate a given hugepage is isolated. */ >> > - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL, >> > - MIGRATE_SYNC); >> > - put_page(hpage); >> > + list_move(>lru, ); >> >> we use hpage->lru to add the hpage to h->hugepage_activelist. This will >> break a hugetlb cgroup removal isn't it ? > > This particular part will not break removal because > hugetlb_cgroup_css_offline loops until hugetlb_cgroup_have_usage is 0. > > Little bit offtopic: > Btw. hugetlb migration breaks to charging even before this patchset > AFAICS. The above put_page should remove the last reference and then it > will uncharge it but I do not see anything that would charge a new page. > This is all because regula LRU pages are uncharged when they are > unmapped. But this a different story not related to this series. But when we call that put_page, we would have alreayd move the cgroup information to the new page. We have h_cg = hugetlb_cgroup_from_page(oldhpage); set_hugetlb_cgroup(oldhpage, NULL); /* move the h_cg details to new cgroup */ set_hugetlb_cgroup(newhpage, h_cg); in hugetlb_cgroup_migrate -aneesh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Wed 27-03-13 15:19:24, Naoya Horiguchi wrote: > On Wed, Mar 27, 2013 at 02:52:50PM +0100, Michal Hocko wrote: > > On Tue 26-03-13 16:59:40, Aneesh Kumar K.V wrote: > > > Naoya Horiguchi writes: > > [...] > > > > diff --git v3.9-rc3.orig/mm/memory-failure.c > > > > v3.9-rc3/mm/memory-failure.c > > > > index df0694c..4e01082 100644 > > > > --- v3.9-rc3.orig/mm/memory-failure.c > > > > +++ v3.9-rc3/mm/memory-failure.c > > > > @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page > > > > *page, int flags) > > > > int ret; > > > > unsigned long pfn = page_to_pfn(page); > > > > struct page *hpage = compound_head(page); > > > > + LIST_HEAD(pagelist); > > > > > > > > /* > > > > * This double-check of PageHWPoison is to avoid the race with > > > > @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page > > > > *page, int flags) > > > > unlock_page(hpage); > > > > > > > > /* Keep page count to indicate a given hugepage is isolated. */ > > > > - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL, > > > > - MIGRATE_SYNC); > > > > - put_page(hpage); > > > > + list_move(>lru, ); > > > > > > we use hpage->lru to add the hpage to h->hugepage_activelist. This will > > > break a hugetlb cgroup removal isn't it ? > > > > This particular part will not break removal because > > hugetlb_cgroup_css_offline loops until hugetlb_cgroup_have_usage is 0. > > Right. > > > Little bit offtopic: > > Btw. hugetlb migration breaks to charging even before this patchset > > AFAICS. The above put_page should remove the last reference and then it > > will uncharge it but I do not see anything that would charge a new page. > > This is all because regula LRU pages are uncharged when they are > > unmapped. But this a different story not related to this series. > > It seems to me that alloc_huge_page_node() needs to call > hugetlb_cgroup_charge_cgroup() before dequeuing a new hugepage. This is not that easy because the new page has to be charged to the same group as the original one but the migration process might be running in the context of a different group. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
Michal Hocko mho...@suse.cz writes: On Tue 26-03-13 16:59:40, Aneesh Kumar K.V wrote: Naoya Horiguchi n-horigu...@ah.jp.nec.com writes: [...] diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c index df0694c..4e01082 100644 --- v3.9-rc3.orig/mm/memory-failure.c +++ v3.9-rc3/mm/memory-failure.c @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page *page, int flags) int ret; unsigned long pfn = page_to_pfn(page); struct page *hpage = compound_head(page); + LIST_HEAD(pagelist); /* * This double-check of PageHWPoison is to avoid the race with @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page *page, int flags) unlock_page(hpage); /* Keep page count to indicate a given hugepage is isolated. */ - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL, - MIGRATE_SYNC); - put_page(hpage); + list_move(hpage-lru, pagelist); we use hpage-lru to add the hpage to h-hugepage_activelist. This will break a hugetlb cgroup removal isn't it ? This particular part will not break removal because hugetlb_cgroup_css_offline loops until hugetlb_cgroup_have_usage is 0. Little bit offtopic: Btw. hugetlb migration breaks to charging even before this patchset AFAICS. The above put_page should remove the last reference and then it will uncharge it but I do not see anything that would charge a new page. This is all because regula LRU pages are uncharged when they are unmapped. But this a different story not related to this series. But when we call that put_page, we would have alreayd move the cgroup information to the new page. We have h_cg = hugetlb_cgroup_from_page(oldhpage); set_hugetlb_cgroup(oldhpage, NULL); /* move the h_cg details to new cgroup */ set_hugetlb_cgroup(newhpage, h_cg); in hugetlb_cgroup_migrate -aneesh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Wed 27-03-13 15:19:24, Naoya Horiguchi wrote: On Wed, Mar 27, 2013 at 02:52:50PM +0100, Michal Hocko wrote: On Tue 26-03-13 16:59:40, Aneesh Kumar K.V wrote: Naoya Horiguchi n-horigu...@ah.jp.nec.com writes: [...] diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c index df0694c..4e01082 100644 --- v3.9-rc3.orig/mm/memory-failure.c +++ v3.9-rc3/mm/memory-failure.c @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page *page, int flags) int ret; unsigned long pfn = page_to_pfn(page); struct page *hpage = compound_head(page); + LIST_HEAD(pagelist); /* * This double-check of PageHWPoison is to avoid the race with @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page *page, int flags) unlock_page(hpage); /* Keep page count to indicate a given hugepage is isolated. */ - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL, - MIGRATE_SYNC); - put_page(hpage); + list_move(hpage-lru, pagelist); we use hpage-lru to add the hpage to h-hugepage_activelist. This will break a hugetlb cgroup removal isn't it ? This particular part will not break removal because hugetlb_cgroup_css_offline loops until hugetlb_cgroup_have_usage is 0. Right. Little bit offtopic: Btw. hugetlb migration breaks to charging even before this patchset AFAICS. The above put_page should remove the last reference and then it will uncharge it but I do not see anything that would charge a new page. This is all because regula LRU pages are uncharged when they are unmapped. But this a different story not related to this series. It seems to me that alloc_huge_page_node() needs to call hugetlb_cgroup_charge_cgroup() before dequeuing a new hugepage. This is not that easy because the new page has to be charged to the same group as the original one but the migration process might be running in the context of a different group. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Wed, Mar 27, 2013 at 02:52:50PM +0100, Michal Hocko wrote: > On Tue 26-03-13 16:59:40, Aneesh Kumar K.V wrote: > > Naoya Horiguchi writes: > [...] > > > diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c > > > index df0694c..4e01082 100644 > > > --- v3.9-rc3.orig/mm/memory-failure.c > > > +++ v3.9-rc3/mm/memory-failure.c > > > @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page > > > *page, int flags) > > > int ret; > > > unsigned long pfn = page_to_pfn(page); > > > struct page *hpage = compound_head(page); > > > + LIST_HEAD(pagelist); > > > > > > /* > > >* This double-check of PageHWPoison is to avoid the race with > > > @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page > > > *page, int flags) > > > unlock_page(hpage); > > > > > > /* Keep page count to indicate a given hugepage is isolated. */ > > > - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL, > > > - MIGRATE_SYNC); > > > - put_page(hpage); > > > + list_move(>lru, ); > > > > we use hpage->lru to add the hpage to h->hugepage_activelist. This will > > break a hugetlb cgroup removal isn't it ? > > This particular part will not break removal because > hugetlb_cgroup_css_offline loops until hugetlb_cgroup_have_usage is 0. Right. > Little bit offtopic: > Btw. hugetlb migration breaks to charging even before this patchset > AFAICS. The above put_page should remove the last reference and then it > will uncharge it but I do not see anything that would charge a new page. > This is all because regula LRU pages are uncharged when they are > unmapped. But this a different story not related to this series. It seems to me that alloc_huge_page_node() needs to call hugetlb_cgroup_charge_cgroup() before dequeuing a new hugepage. Thanks, Naoya -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Tue 26-03-13 16:59:40, Aneesh Kumar K.V wrote: > Naoya Horiguchi writes: [...] > > diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c > > index df0694c..4e01082 100644 > > --- v3.9-rc3.orig/mm/memory-failure.c > > +++ v3.9-rc3/mm/memory-failure.c > > @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page *page, > > int flags) > > int ret; > > unsigned long pfn = page_to_pfn(page); > > struct page *hpage = compound_head(page); > > + LIST_HEAD(pagelist); > > > > /* > > * This double-check of PageHWPoison is to avoid the race with > > @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page > > *page, int flags) > > unlock_page(hpage); > > > > /* Keep page count to indicate a given hugepage is isolated. */ > > - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL, > > - MIGRATE_SYNC); > > - put_page(hpage); > > + list_move(>lru, ); > > we use hpage->lru to add the hpage to h->hugepage_activelist. This will > break a hugetlb cgroup removal isn't it ? This particular part will not break removal because hugetlb_cgroup_css_offline loops until hugetlb_cgroup_have_usage is 0. Little bit offtopic: Btw. hugetlb migration breaks to charging even before this patchset AFAICS. The above put_page should remove the last reference and then it will uncharge it but I do not see anything that would charge a new page. This is all because regula LRU pages are uncharged when they are unmapped. But this a different story not related to this series. [...] -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Tue 26-03-13 16:35:35, Naoya Horiguchi wrote: [...] > The differences is that migrate_huge_page() has one hugepage as an argument, > and migrate_pages() has a pagelist with multiple hugepages. > I already told this before and I'm not sure it's enough to answer the > question, > so I explain another point about why this patch do like it. OK, I am blind. It is + list_move(>lru, ); + ret = migrate_pages(, new_page, MPOL_MF_MOVE_ALL, + MIGRATE_SYNC, MR_MEMORY_FAILURE); which moves it from active_list and so you have to put it back. > I think that we must do putback_*pages() for source pages whether migration > succeeds or not. > But when we call migrate_pages() with a pagelist, > the caller can't access to the successfully migrated source pages > after migrate_pages() returns, because they are no longer on the pagelist. > So putback of the successfully migrated source pages should be done *in* > unmap_and_move() and/or unmap_and_move_huge_page(). If the migration succeeds then the page becomes unused and free after its last reference drops. So I do not see any reason to put it back to active list and free it right afterwards. On the other hand unmap_and_move does the same thing (although page reference counting is a bit more complicated in that case) so it would be good to keep in sync with regular pages case. > And when we used migrate_huge_page(), we passed a hugepage to be migrated > as an argument, so the caller can still access to the page even if the > migration succeeds. [...] -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Tue 26-03-13 16:35:35, Naoya Horiguchi wrote: [...] The differences is that migrate_huge_page() has one hugepage as an argument, and migrate_pages() has a pagelist with multiple hugepages. I already told this before and I'm not sure it's enough to answer the question, so I explain another point about why this patch do like it. OK, I am blind. It is + list_move(hpage-lru, pagelist); + ret = migrate_pages(pagelist, new_page, MPOL_MF_MOVE_ALL, + MIGRATE_SYNC, MR_MEMORY_FAILURE); which moves it from active_list and so you have to put it back. I think that we must do putback_*pages() for source pages whether migration succeeds or not. But when we call migrate_pages() with a pagelist, the caller can't access to the successfully migrated source pages after migrate_pages() returns, because they are no longer on the pagelist. So putback of the successfully migrated source pages should be done *in* unmap_and_move() and/or unmap_and_move_huge_page(). If the migration succeeds then the page becomes unused and free after its last reference drops. So I do not see any reason to put it back to active list and free it right afterwards. On the other hand unmap_and_move does the same thing (although page reference counting is a bit more complicated in that case) so it would be good to keep in sync with regular pages case. And when we used migrate_huge_page(), we passed a hugepage to be migrated as an argument, so the caller can still access to the page even if the migration succeeds. [...] -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Tue 26-03-13 16:59:40, Aneesh Kumar K.V wrote: Naoya Horiguchi n-horigu...@ah.jp.nec.com writes: [...] diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c index df0694c..4e01082 100644 --- v3.9-rc3.orig/mm/memory-failure.c +++ v3.9-rc3/mm/memory-failure.c @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page *page, int flags) int ret; unsigned long pfn = page_to_pfn(page); struct page *hpage = compound_head(page); + LIST_HEAD(pagelist); /* * This double-check of PageHWPoison is to avoid the race with @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page *page, int flags) unlock_page(hpage); /* Keep page count to indicate a given hugepage is isolated. */ - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL, - MIGRATE_SYNC); - put_page(hpage); + list_move(hpage-lru, pagelist); we use hpage-lru to add the hpage to h-hugepage_activelist. This will break a hugetlb cgroup removal isn't it ? This particular part will not break removal because hugetlb_cgroup_css_offline loops until hugetlb_cgroup_have_usage is 0. Little bit offtopic: Btw. hugetlb migration breaks to charging even before this patchset AFAICS. The above put_page should remove the last reference and then it will uncharge it but I do not see anything that would charge a new page. This is all because regula LRU pages are uncharged when they are unmapped. But this a different story not related to this series. [...] -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Wed, Mar 27, 2013 at 02:52:50PM +0100, Michal Hocko wrote: On Tue 26-03-13 16:59:40, Aneesh Kumar K.V wrote: Naoya Horiguchi n-horigu...@ah.jp.nec.com writes: [...] diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c index df0694c..4e01082 100644 --- v3.9-rc3.orig/mm/memory-failure.c +++ v3.9-rc3/mm/memory-failure.c @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page *page, int flags) int ret; unsigned long pfn = page_to_pfn(page); struct page *hpage = compound_head(page); + LIST_HEAD(pagelist); /* * This double-check of PageHWPoison is to avoid the race with @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page *page, int flags) unlock_page(hpage); /* Keep page count to indicate a given hugepage is isolated. */ - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL, - MIGRATE_SYNC); - put_page(hpage); + list_move(hpage-lru, pagelist); we use hpage-lru to add the hpage to h-hugepage_activelist. This will break a hugetlb cgroup removal isn't it ? This particular part will not break removal because hugetlb_cgroup_css_offline loops until hugetlb_cgroup_have_usage is 0. Right. Little bit offtopic: Btw. hugetlb migration breaks to charging even before this patchset AFAICS. The above put_page should remove the last reference and then it will uncharge it but I do not see anything that would charge a new page. This is all because regula LRU pages are uncharged when they are unmapped. But this a different story not related to this series. It seems to me that alloc_huge_page_node() needs to call hugetlb_cgroup_charge_cgroup() before dequeuing a new hugepage. Thanks, Naoya -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Tue, Mar 26, 2013 at 10:49:50AM +0100, Michal Hocko wrote:
> On Tue 26-03-13 00:34:40, Naoya Horiguchi wrote:
> > On Mon, Mar 25, 2013 at 01:31:28PM +0100, Michal Hocko wrote:
> > > On Fri 22-03-13 16:23:48, Naoya Horiguchi wrote:
> [...]
> > > > @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page
> > > > *page, int flags)
> > > > unlock_page(hpage);
> > > >
> > > > /* Keep page count to indicate a given hugepage is isolated. */
> > > > - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL,
> > > > - MIGRATE_SYNC);
> > > > - put_page(hpage);
> > > > + list_move(>lru, );
> > > > + ret = migrate_pages(, new_page, MPOL_MF_MOVE_ALL,
> > > > + MIGRATE_SYNC, MR_MEMORY_FAILURE);
> > > > if (ret) {
> > > > pr_info("soft offline: %#lx: migration failed %d, type
> > > > %lx\n",
> > > > pfn, ret, page->flags);
> > > > + /*
> > > > +* We know that soft_offline_huge_page() tries to
> > > > migrate
> > > > +* only one hugepage pointed to by hpage, so we need not
> > > > +* run through the pagelist here.
> > > > +*/
> > > > + putback_active_hugepage(hpage);
> > >
> > > Maybe I am missing something but why we didn't need to call this before
> > > when using migrate_huge_page?
> >
> > migrate_huge_page() does not need list handling before/after the call,
> > because it's defined to migrate only one hugepage, and it has a page as
> > an argument, not list_head.
>
> I do not understand this reasoning. migrate_huge_page calls
> unmap_and_move_huge_page and migrate_pages does the same + accounting.
> So what is the difference here?
My previous comment missed the point, sorry.
Let me restate for your original question:
> > > Maybe I am missing something but why we didn't need to call this before
> > > when using migrate_huge_page?
Before 189ebff28, there was no hugepage_activelist and in-use hugepages are
not linked to any pagelist, so put_page was used instead.
And present question:
> I do not understand this reasoning. migrate_huge_page calls
> unmap_and_move_huge_page and migrate_pages does the same + accounting.
> So what is the difference here?
The differences is that migrate_huge_page() has one hugepage as an argument,
and migrate_pages() has a pagelist with multiple hugepages.
I already told this before and I'm not sure it's enough to answer the question,
so I explain another point about why this patch do like it.
I think that we must do putback_*pages() for source pages whether migration
succeeds or not. But when we call migrate_pages() with a pagelist,
the caller can't access to the successfully migrated source pages
after migrate_pages() returns, because they are no longer on the pagelist.
So putback of the successfully migrated source pages should be done *in*
unmap_and_move() and/or unmap_and_move_huge_page().
And when we used migrate_huge_page(), we passed a hugepage to be migrated
as an argument, so the caller can still access to the page even if the
migration succeeds.
> I suspect that putback_active_hugepage
> was simply missing in this code path.
Commit 189ebff28 moved put_page() out of the if-block with removing put_page()
in unmap_and_move_huge_page(). As I wrote above, this is correct only when
migrate_huge_page() handles only one hugepage.
But this patch makes us back to pagelist implementation, so we should cancel
this change.
> > > > + if (ret > 0)
> > > > + ret = -EIO;
> > > > } else {
> > > > set_page_hwpoison_huge_page(hpage);
> > > > dequeue_hwpoisoned_huge_page(hpage);
> > > > diff --git v3.9-rc3.orig/mm/migrate.c v3.9-rc3/mm/migrate.c
> > > > index f69f354..66030b6 100644
> > > > --- v3.9-rc3.orig/mm/migrate.c
> > > > +++ v3.9-rc3/mm/migrate.c
> > > > @@ -981,6 +981,8 @@ static int unmap_and_move_huge_page(new_page_t
> > > > get_new_page,
> > > >
> > > > unlock_page(hpage);
> > > > out:
> > > > + if (rc != -EAGAIN)
> > > > + putback_active_hugepage(hpage);
> > >
> > > And why do you put it here? If it is called from migrate_pages then the
> > > caller already does the clean-up (putback_lru_pages).
> >
> > What the caller of migrate_pages() cleans up is the (huge)pages which failed
> > to be migrated. And what the above code cleans up is the source hugepage
> > after the migration succeeds.
>
> Why should you want to add successfully migrated page? /me confused.
When hugepage migration succeeds, the source hugepage is freed back to
free hugepage pool (just after copy of data and mapping ended,
refcount of the source hugepage should be 1, so free_huge_page() is called
in this putback_active_hugepage().)
As I stated above, the caller cannot access to the source page, so we
need to do this here.
Thanks,
Naoya
--
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
Naoya Horiguchi writes:
> Currently migrate_huge_page() takes a pointer to a hugepage to be
> migrated as an argument, instead of taking a pointer to the list of
> hugepages to be migrated. This behavior was introduced in commit
> 189ebff28 ("hugetlb: simplify migrate_huge_page()"), and was OK
> because until now hugepage migration is enabled only for soft-offlining
> which takes only one hugepage in a single call.
>
> But the situation will change in the later patches in this series
> which enable other users of page migration to support hugepage migration.
> They can kick migration for both of normal pages and hugepages
> in a single call, so we need to go back to original implementation
> of using linked lists to collect the hugepages to be migrated.
>
> Signed-off-by: Naoya Horiguchi
> ---
> mm/memory-failure.c | 15 ---
> mm/migrate.c| 2 ++
> 2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c
> index df0694c..4e01082 100644
> --- v3.9-rc3.orig/mm/memory-failure.c
> +++ v3.9-rc3/mm/memory-failure.c
> @@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page *page,
> int flags)
> int ret;
> unsigned long pfn = page_to_pfn(page);
> struct page *hpage = compound_head(page);
> + LIST_HEAD(pagelist);
>
> /*
>* This double-check of PageHWPoison is to avoid the race with
> @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page *page,
> int flags)
> unlock_page(hpage);
>
> /* Keep page count to indicate a given hugepage is isolated. */
> - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL,
> - MIGRATE_SYNC);
> - put_page(hpage);
> + list_move(>lru, );
we use hpage->lru to add the hpage to h->hugepage_activelist. This will
break a hugetlb cgroup removal isn't it ?
> + ret = migrate_pages(, new_page, MPOL_MF_MOVE_ALL,
> + MIGRATE_SYNC, MR_MEMORY_FAILURE);
> if (ret) {
> pr_info("soft offline: %#lx: migration failed %d, type %lx\n",
> pfn, ret, page->flags);
> + /*
> + * We know that soft_offline_huge_page() tries to migrate
> + * only one hugepage pointed to by hpage, so we need not
> + * run through the pagelist here.
> + */
> + putback_active_hugepage(hpage);
> + if (ret > 0)
> + ret = -EIO;
> } else {
> set_page_hwpoison_huge_page(hpage);
> dequeue_hwpoisoned_huge_page(hpage);
> diff --git v3.9-rc3.orig/mm/migrate.c v3.9-rc3/mm/migrate.c
> index f69f354..66030b6 100644
> --- v3.9-rc3.orig/mm/migrate.c
> +++ v3.9-rc3/mm/migrate.c
> @@ -981,6 +981,8 @@ static int unmap_and_move_huge_page(new_page_t
> get_new_page,
>
> unlock_page(hpage);
> out:
> + if (rc != -EAGAIN)
> + putback_active_hugepage(hpage);
> put_page(new_hpage);
> if (result) {
> if (rc)
> --
-aneesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Tue 26-03-13 00:34:40, Naoya Horiguchi wrote:
> On Mon, Mar 25, 2013 at 01:31:28PM +0100, Michal Hocko wrote:
> > On Fri 22-03-13 16:23:48, Naoya Horiguchi wrote:
[...]
> > > @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page
> > > *page, int flags)
> > > unlock_page(hpage);
> > >
> > > /* Keep page count to indicate a given hugepage is isolated. */
> > > - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL,
> > > - MIGRATE_SYNC);
> > > - put_page(hpage);
> > > + list_move(>lru, );
> > > + ret = migrate_pages(, new_page, MPOL_MF_MOVE_ALL,
> > > + MIGRATE_SYNC, MR_MEMORY_FAILURE);
> > > if (ret) {
> > > pr_info("soft offline: %#lx: migration failed %d, type %lx\n",
> > > pfn, ret, page->flags);
> > > + /*
> > > + * We know that soft_offline_huge_page() tries to migrate
> > > + * only one hugepage pointed to by hpage, so we need not
> > > + * run through the pagelist here.
> > > + */
> > > + putback_active_hugepage(hpage);
> >
> > Maybe I am missing something but why we didn't need to call this before
> > when using migrate_huge_page?
>
> migrate_huge_page() does not need list handling before/after the call,
> because it's defined to migrate only one hugepage, and it has a page as
> an argument, not list_head.
I do not understand this reasoning. migrate_huge_page calls
unmap_and_move_huge_page and migrate_pages does the same + accounting.
So what is the difference here? I suspect that putback_active_hugepage
was simply missing in this code path.
> > > + if (ret > 0)
> > > + ret = -EIO;
> > > } else {
> > > set_page_hwpoison_huge_page(hpage);
> > > dequeue_hwpoisoned_huge_page(hpage);
> > > diff --git v3.9-rc3.orig/mm/migrate.c v3.9-rc3/mm/migrate.c
> > > index f69f354..66030b6 100644
> > > --- v3.9-rc3.orig/mm/migrate.c
> > > +++ v3.9-rc3/mm/migrate.c
> > > @@ -981,6 +981,8 @@ static int unmap_and_move_huge_page(new_page_t
> > > get_new_page,
> > >
> > > unlock_page(hpage);
> > > out:
> > > + if (rc != -EAGAIN)
> > > + putback_active_hugepage(hpage);
> >
> > And why do you put it here? If it is called from migrate_pages then the
> > caller already does the clean-up (putback_lru_pages).
>
> What the caller of migrate_pages() cleans up is the (huge)pages which failed
> to be migrated. And what the above code cleans up is the source hugepage
> after the migration succeeds.
Why should you want to add successfully migrated page? /me confused.
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Tue 26-03-13 00:34:40, Naoya Horiguchi wrote:
On Mon, Mar 25, 2013 at 01:31:28PM +0100, Michal Hocko wrote:
On Fri 22-03-13 16:23:48, Naoya Horiguchi wrote:
[...]
@@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page
*page, int flags)
unlock_page(hpage);
/* Keep page count to indicate a given hugepage is isolated. */
- ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL,
- MIGRATE_SYNC);
- put_page(hpage);
+ list_move(hpage-lru, pagelist);
+ ret = migrate_pages(pagelist, new_page, MPOL_MF_MOVE_ALL,
+ MIGRATE_SYNC, MR_MEMORY_FAILURE);
if (ret) {
pr_info(soft offline: %#lx: migration failed %d, type %lx\n,
pfn, ret, page-flags);
+ /*
+ * We know that soft_offline_huge_page() tries to migrate
+ * only one hugepage pointed to by hpage, so we need not
+ * run through the pagelist here.
+ */
+ putback_active_hugepage(hpage);
Maybe I am missing something but why we didn't need to call this before
when using migrate_huge_page?
migrate_huge_page() does not need list handling before/after the call,
because it's defined to migrate only one hugepage, and it has a page as
an argument, not list_head.
I do not understand this reasoning. migrate_huge_page calls
unmap_and_move_huge_page and migrate_pages does the same + accounting.
So what is the difference here? I suspect that putback_active_hugepage
was simply missing in this code path.
+ if (ret 0)
+ ret = -EIO;
} else {
set_page_hwpoison_huge_page(hpage);
dequeue_hwpoisoned_huge_page(hpage);
diff --git v3.9-rc3.orig/mm/migrate.c v3.9-rc3/mm/migrate.c
index f69f354..66030b6 100644
--- v3.9-rc3.orig/mm/migrate.c
+++ v3.9-rc3/mm/migrate.c
@@ -981,6 +981,8 @@ static int unmap_and_move_huge_page(new_page_t
get_new_page,
unlock_page(hpage);
out:
+ if (rc != -EAGAIN)
+ putback_active_hugepage(hpage);
And why do you put it here? If it is called from migrate_pages then the
caller already does the clean-up (putback_lru_pages).
What the caller of migrate_pages() cleans up is the (huge)pages which failed
to be migrated. And what the above code cleans up is the source hugepage
after the migration succeeds.
Why should you want to add successfully migrated page? /me confused.
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
Naoya Horiguchi n-horigu...@ah.jp.nec.com writes:
Currently migrate_huge_page() takes a pointer to a hugepage to be
migrated as an argument, instead of taking a pointer to the list of
hugepages to be migrated. This behavior was introduced in commit
189ebff28 (hugetlb: simplify migrate_huge_page()), and was OK
because until now hugepage migration is enabled only for soft-offlining
which takes only one hugepage in a single call.
But the situation will change in the later patches in this series
which enable other users of page migration to support hugepage migration.
They can kick migration for both of normal pages and hugepages
in a single call, so we need to go back to original implementation
of using linked lists to collect the hugepages to be migrated.
Signed-off-by: Naoya Horiguchi n-horigu...@ah.jp.nec.com
---
mm/memory-failure.c | 15 ---
mm/migrate.c| 2 ++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git v3.9-rc3.orig/mm/memory-failure.c v3.9-rc3/mm/memory-failure.c
index df0694c..4e01082 100644
--- v3.9-rc3.orig/mm/memory-failure.c
+++ v3.9-rc3/mm/memory-failure.c
@@ -1467,6 +1467,7 @@ static int soft_offline_huge_page(struct page *page,
int flags)
int ret;
unsigned long pfn = page_to_pfn(page);
struct page *hpage = compound_head(page);
+ LIST_HEAD(pagelist);
/*
* This double-check of PageHWPoison is to avoid the race with
@@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page *page,
int flags)
unlock_page(hpage);
/* Keep page count to indicate a given hugepage is isolated. */
- ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL,
- MIGRATE_SYNC);
- put_page(hpage);
+ list_move(hpage-lru, pagelist);
we use hpage-lru to add the hpage to h-hugepage_activelist. This will
break a hugetlb cgroup removal isn't it ?
+ ret = migrate_pages(pagelist, new_page, MPOL_MF_MOVE_ALL,
+ MIGRATE_SYNC, MR_MEMORY_FAILURE);
if (ret) {
pr_info(soft offline: %#lx: migration failed %d, type %lx\n,
pfn, ret, page-flags);
+ /*
+ * We know that soft_offline_huge_page() tries to migrate
+ * only one hugepage pointed to by hpage, so we need not
+ * run through the pagelist here.
+ */
+ putback_active_hugepage(hpage);
+ if (ret 0)
+ ret = -EIO;
} else {
set_page_hwpoison_huge_page(hpage);
dequeue_hwpoisoned_huge_page(hpage);
diff --git v3.9-rc3.orig/mm/migrate.c v3.9-rc3/mm/migrate.c
index f69f354..66030b6 100644
--- v3.9-rc3.orig/mm/migrate.c
+++ v3.9-rc3/mm/migrate.c
@@ -981,6 +981,8 @@ static int unmap_and_move_huge_page(new_page_t
get_new_page,
unlock_page(hpage);
out:
+ if (rc != -EAGAIN)
+ putback_active_hugepage(hpage);
put_page(new_hpage);
if (result) {
if (rc)
--
-aneesh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Tue, Mar 26, 2013 at 10:49:50AM +0100, Michal Hocko wrote:
On Tue 26-03-13 00:34:40, Naoya Horiguchi wrote:
On Mon, Mar 25, 2013 at 01:31:28PM +0100, Michal Hocko wrote:
On Fri 22-03-13 16:23:48, Naoya Horiguchi wrote:
[...]
@@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page
*page, int flags)
unlock_page(hpage);
/* Keep page count to indicate a given hugepage is isolated. */
- ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL,
- MIGRATE_SYNC);
- put_page(hpage);
+ list_move(hpage-lru, pagelist);
+ ret = migrate_pages(pagelist, new_page, MPOL_MF_MOVE_ALL,
+ MIGRATE_SYNC, MR_MEMORY_FAILURE);
if (ret) {
pr_info(soft offline: %#lx: migration failed %d, type
%lx\n,
pfn, ret, page-flags);
+ /*
+* We know that soft_offline_huge_page() tries to
migrate
+* only one hugepage pointed to by hpage, so we need not
+* run through the pagelist here.
+*/
+ putback_active_hugepage(hpage);
Maybe I am missing something but why we didn't need to call this before
when using migrate_huge_page?
migrate_huge_page() does not need list handling before/after the call,
because it's defined to migrate only one hugepage, and it has a page as
an argument, not list_head.
I do not understand this reasoning. migrate_huge_page calls
unmap_and_move_huge_page and migrate_pages does the same + accounting.
So what is the difference here?
My previous comment missed the point, sorry.
Let me restate for your original question:
Maybe I am missing something but why we didn't need to call this before
when using migrate_huge_page?
Before 189ebff28, there was no hugepage_activelist and in-use hugepages are
not linked to any pagelist, so put_page was used instead.
And present question:
I do not understand this reasoning. migrate_huge_page calls
unmap_and_move_huge_page and migrate_pages does the same + accounting.
So what is the difference here?
The differences is that migrate_huge_page() has one hugepage as an argument,
and migrate_pages() has a pagelist with multiple hugepages.
I already told this before and I'm not sure it's enough to answer the question,
so I explain another point about why this patch do like it.
I think that we must do putback_*pages() for source pages whether migration
succeeds or not. But when we call migrate_pages() with a pagelist,
the caller can't access to the successfully migrated source pages
after migrate_pages() returns, because they are no longer on the pagelist.
So putback of the successfully migrated source pages should be done *in*
unmap_and_move() and/or unmap_and_move_huge_page().
And when we used migrate_huge_page(), we passed a hugepage to be migrated
as an argument, so the caller can still access to the page even if the
migration succeeds.
I suspect that putback_active_hugepage
was simply missing in this code path.
Commit 189ebff28 moved put_page() out of the if-block with removing put_page()
in unmap_and_move_huge_page(). As I wrote above, this is correct only when
migrate_huge_page() handles only one hugepage.
But this patch makes us back to pagelist implementation, so we should cancel
this change.
+ if (ret 0)
+ ret = -EIO;
} else {
set_page_hwpoison_huge_page(hpage);
dequeue_hwpoisoned_huge_page(hpage);
diff --git v3.9-rc3.orig/mm/migrate.c v3.9-rc3/mm/migrate.c
index f69f354..66030b6 100644
--- v3.9-rc3.orig/mm/migrate.c
+++ v3.9-rc3/mm/migrate.c
@@ -981,6 +981,8 @@ static int unmap_and_move_huge_page(new_page_t
get_new_page,
unlock_page(hpage);
out:
+ if (rc != -EAGAIN)
+ putback_active_hugepage(hpage);
And why do you put it here? If it is called from migrate_pages then the
caller already does the clean-up (putback_lru_pages).
What the caller of migrate_pages() cleans up is the (huge)pages which failed
to be migrated. And what the above code cleans up is the source hugepage
after the migration succeeds.
Why should you want to add successfully migrated page? /me confused.
When hugepage migration succeeds, the source hugepage is freed back to
free hugepage pool (just after copy of data and mapping ended,
refcount of the source hugepage should be 1, so free_huge_page() is called
in this putback_active_hugepage().)
As I stated above, the caller cannot access to the source page, so we
need to do this here.
Thanks,
Naoya
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Mon, Mar 25, 2013 at 01:31:28PM +0100, Michal Hocko wrote:
> On Fri 22-03-13 16:23:48, Naoya Horiguchi wrote:
> > Currently migrate_huge_page() takes a pointer to a hugepage to be
> > migrated as an argument, instead of taking a pointer to the list of
> > hugepages to be migrated. This behavior was introduced in commit
> > 189ebff28 ("hugetlb: simplify migrate_huge_page()"), and was OK
> > because until now hugepage migration is enabled only for soft-offlining
> > which takes only one hugepage in a single call.
> >
> > But the situation will change in the later patches in this series
> > which enable other users of page migration to support hugepage migration.
> > They can kick migration for both of normal pages and hugepages
> > in a single call, so we need to go back to original implementation
> > of using linked lists to collect the hugepages to be migrated.
>
> If the purpose of this patch is to reduce code duplication then you
> should remove migrate_huge_page as it doesn't have any caller anymore.
Yes, that makes sense. I'll do this.
> [...]
> > @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page
> > *page, int flags)
> > unlock_page(hpage);
> >
> > /* Keep page count to indicate a given hugepage is isolated. */
> > - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL,
> > - MIGRATE_SYNC);
> > - put_page(hpage);
> > + list_move(>lru, );
> > + ret = migrate_pages(, new_page, MPOL_MF_MOVE_ALL,
> > + MIGRATE_SYNC, MR_MEMORY_FAILURE);
> > if (ret) {
> > pr_info("soft offline: %#lx: migration failed %d, type %lx\n",
> > pfn, ret, page->flags);
> > + /*
> > +* We know that soft_offline_huge_page() tries to migrate
> > +* only one hugepage pointed to by hpage, so we need not
> > +* run through the pagelist here.
> > +*/
> > + putback_active_hugepage(hpage);
>
> Maybe I am missing something but why we didn't need to call this before
> when using migrate_huge_page?
migrate_huge_page() does not need list handling before/after the call,
because it's defined to migrate only one hugepage, and it has a page as
an argument, not list_head.
> > + if (ret > 0)
> > + ret = -EIO;
> > } else {
> > set_page_hwpoison_huge_page(hpage);
> > dequeue_hwpoisoned_huge_page(hpage);
> > diff --git v3.9-rc3.orig/mm/migrate.c v3.9-rc3/mm/migrate.c
> > index f69f354..66030b6 100644
> > --- v3.9-rc3.orig/mm/migrate.c
> > +++ v3.9-rc3/mm/migrate.c
> > @@ -981,6 +981,8 @@ static int unmap_and_move_huge_page(new_page_t
> > get_new_page,
> >
> > unlock_page(hpage);
> > out:
> > + if (rc != -EAGAIN)
> > + putback_active_hugepage(hpage);
>
> And why do you put it here? If it is called from migrate_pages then the
> caller already does the clean-up (putback_lru_pages).
What the caller of migrate_pages() cleans up is the (huge)pages which failed
to be migrated. And what the above code cleans up is the source hugepage
after the migration succeeds.
The latter clean-up code originally existed, but removed in 189ebff28
("hugetlb: simplify migrate_huge_page()").
This commit cleans up the code based on that there was only one user
of hugepage migration, but that's not true any more.
So the above hunk is a part of revert of the commit.
But it's not a simple revert, because there's one difference between
now and before 189ebff28 that we link hugepages in-use to hugepage_activelist.
Then we finally come to the above change.
Thanks,
Naoya
> > put_page(new_hpage);
> > if (result) {
> > if (rc)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Fri 22-03-13 16:23:48, Naoya Horiguchi wrote:
> Currently migrate_huge_page() takes a pointer to a hugepage to be
> migrated as an argument, instead of taking a pointer to the list of
> hugepages to be migrated. This behavior was introduced in commit
> 189ebff28 ("hugetlb: simplify migrate_huge_page()"), and was OK
> because until now hugepage migration is enabled only for soft-offlining
> which takes only one hugepage in a single call.
>
> But the situation will change in the later patches in this series
> which enable other users of page migration to support hugepage migration.
> They can kick migration for both of normal pages and hugepages
> in a single call, so we need to go back to original implementation
> of using linked lists to collect the hugepages to be migrated.
If the purpose of this patch is to reduce code duplication then you
should remove migrate_huge_page as it doesn't have any caller anymore.
[...]
> @@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page *page,
> int flags)
> unlock_page(hpage);
>
> /* Keep page count to indicate a given hugepage is isolated. */
> - ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL,
> - MIGRATE_SYNC);
> - put_page(hpage);
> + list_move(>lru, );
> + ret = migrate_pages(, new_page, MPOL_MF_MOVE_ALL,
> + MIGRATE_SYNC, MR_MEMORY_FAILURE);
> if (ret) {
> pr_info("soft offline: %#lx: migration failed %d, type %lx\n",
> pfn, ret, page->flags);
> + /*
> + * We know that soft_offline_huge_page() tries to migrate
> + * only one hugepage pointed to by hpage, so we need not
> + * run through the pagelist here.
> + */
> + putback_active_hugepage(hpage);
Maybe I am missing something but why we didn't need to call this before
when using migrate_huge_page?
> + if (ret > 0)
> + ret = -EIO;
> } else {
> set_page_hwpoison_huge_page(hpage);
> dequeue_hwpoisoned_huge_page(hpage);
> diff --git v3.9-rc3.orig/mm/migrate.c v3.9-rc3/mm/migrate.c
> index f69f354..66030b6 100644
> --- v3.9-rc3.orig/mm/migrate.c
> +++ v3.9-rc3/mm/migrate.c
> @@ -981,6 +981,8 @@ static int unmap_and_move_huge_page(new_page_t
> get_new_page,
>
> unlock_page(hpage);
> out:
> + if (rc != -EAGAIN)
> + putback_active_hugepage(hpage);
And why do you put it here? If it is called from migrate_pages then the
caller already does the clean-up (putback_lru_pages).
> put_page(new_hpage);
> if (result) {
> if (rc)
> --
> 1.7.11.7
>
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Fri 22-03-13 16:23:48, Naoya Horiguchi wrote:
Currently migrate_huge_page() takes a pointer to a hugepage to be
migrated as an argument, instead of taking a pointer to the list of
hugepages to be migrated. This behavior was introduced in commit
189ebff28 (hugetlb: simplify migrate_huge_page()), and was OK
because until now hugepage migration is enabled only for soft-offlining
which takes only one hugepage in a single call.
But the situation will change in the later patches in this series
which enable other users of page migration to support hugepage migration.
They can kick migration for both of normal pages and hugepages
in a single call, so we need to go back to original implementation
of using linked lists to collect the hugepages to be migrated.
If the purpose of this patch is to reduce code duplication then you
should remove migrate_huge_page as it doesn't have any caller anymore.
[...]
@@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page *page,
int flags)
unlock_page(hpage);
/* Keep page count to indicate a given hugepage is isolated. */
- ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL,
- MIGRATE_SYNC);
- put_page(hpage);
+ list_move(hpage-lru, pagelist);
+ ret = migrate_pages(pagelist, new_page, MPOL_MF_MOVE_ALL,
+ MIGRATE_SYNC, MR_MEMORY_FAILURE);
if (ret) {
pr_info(soft offline: %#lx: migration failed %d, type %lx\n,
pfn, ret, page-flags);
+ /*
+ * We know that soft_offline_huge_page() tries to migrate
+ * only one hugepage pointed to by hpage, so we need not
+ * run through the pagelist here.
+ */
+ putback_active_hugepage(hpage);
Maybe I am missing something but why we didn't need to call this before
when using migrate_huge_page?
+ if (ret 0)
+ ret = -EIO;
} else {
set_page_hwpoison_huge_page(hpage);
dequeue_hwpoisoned_huge_page(hpage);
diff --git v3.9-rc3.orig/mm/migrate.c v3.9-rc3/mm/migrate.c
index f69f354..66030b6 100644
--- v3.9-rc3.orig/mm/migrate.c
+++ v3.9-rc3/mm/migrate.c
@@ -981,6 +981,8 @@ static int unmap_and_move_huge_page(new_page_t
get_new_page,
unlock_page(hpage);
out:
+ if (rc != -EAGAIN)
+ putback_active_hugepage(hpage);
And why do you put it here? If it is called from migrate_pages then the
caller already does the clean-up (putback_lru_pages).
put_page(new_hpage);
if (result) {
if (rc)
--
1.7.11.7
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()
On Mon, Mar 25, 2013 at 01:31:28PM +0100, Michal Hocko wrote:
On Fri 22-03-13 16:23:48, Naoya Horiguchi wrote:
Currently migrate_huge_page() takes a pointer to a hugepage to be
migrated as an argument, instead of taking a pointer to the list of
hugepages to be migrated. This behavior was introduced in commit
189ebff28 (hugetlb: simplify migrate_huge_page()), and was OK
because until now hugepage migration is enabled only for soft-offlining
which takes only one hugepage in a single call.
But the situation will change in the later patches in this series
which enable other users of page migration to support hugepage migration.
They can kick migration for both of normal pages and hugepages
in a single call, so we need to go back to original implementation
of using linked lists to collect the hugepages to be migrated.
If the purpose of this patch is to reduce code duplication then you
should remove migrate_huge_page as it doesn't have any caller anymore.
Yes, that makes sense. I'll do this.
[...]
@@ -1482,12 +1483,20 @@ static int soft_offline_huge_page(struct page
*page, int flags)
unlock_page(hpage);
/* Keep page count to indicate a given hugepage is isolated. */
- ret = migrate_huge_page(hpage, new_page, MPOL_MF_MOVE_ALL,
- MIGRATE_SYNC);
- put_page(hpage);
+ list_move(hpage-lru, pagelist);
+ ret = migrate_pages(pagelist, new_page, MPOL_MF_MOVE_ALL,
+ MIGRATE_SYNC, MR_MEMORY_FAILURE);
if (ret) {
pr_info(soft offline: %#lx: migration failed %d, type %lx\n,
pfn, ret, page-flags);
+ /*
+* We know that soft_offline_huge_page() tries to migrate
+* only one hugepage pointed to by hpage, so we need not
+* run through the pagelist here.
+*/
+ putback_active_hugepage(hpage);
Maybe I am missing something but why we didn't need to call this before
when using migrate_huge_page?
migrate_huge_page() does not need list handling before/after the call,
because it's defined to migrate only one hugepage, and it has a page as
an argument, not list_head.
+ if (ret 0)
+ ret = -EIO;
} else {
set_page_hwpoison_huge_page(hpage);
dequeue_hwpoisoned_huge_page(hpage);
diff --git v3.9-rc3.orig/mm/migrate.c v3.9-rc3/mm/migrate.c
index f69f354..66030b6 100644
--- v3.9-rc3.orig/mm/migrate.c
+++ v3.9-rc3/mm/migrate.c
@@ -981,6 +981,8 @@ static int unmap_and_move_huge_page(new_page_t
get_new_page,
unlock_page(hpage);
out:
+ if (rc != -EAGAIN)
+ putback_active_hugepage(hpage);
And why do you put it here? If it is called from migrate_pages then the
caller already does the clean-up (putback_lru_pages).
What the caller of migrate_pages() cleans up is the (huge)pages which failed
to be migrated. And what the above code cleans up is the source hugepage
after the migration succeeds.
The latter clean-up code originally existed, but removed in 189ebff28
(hugetlb: simplify migrate_huge_page()).
This commit cleans up the code based on that there was only one user
of hugepage migration, but that's not true any more.
So the above hunk is a part of revert of the commit.
But it's not a simple revert, because there's one difference between
now and before 189ebff28 that we link hugepages in-use to hugepage_activelist.
Then we finally come to the above change.
Thanks,
Naoya
put_page(new_hpage);
if (result) {
if (rc)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
