From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> To: Michal Hocko <mhocko@suse.cz> Cc: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>, Mel Gorman <mel@csn.ul.ie>, Hugh Dickins <hughd@google.com>, KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>, Andi Kleen <andi@firstfloor.org>, Hillf Danton <dhillf@gmail.com>, linux-kernel@vger.kernel.org Subject: Re: [PATCH 06/10] migrate: add hugepage migration code to move_pages() Date: Tue, 26 Mar 2013 03:06:18 -0400 [thread overview] Message-ID: <1364281578-4bs50rjv-mutt-n-horiguchi@ah.jp.nec.com> (raw) In-Reply-To: <20130325133644.GY2154@dhcp22.suse.cz> On Mon, Mar 25, 2013 at 02:36:44PM +0100, Michal Hocko wrote: > On Fri 22-03-13 16:23:51, Naoya Horiguchi wrote: > > This patch extends move_pages() to handle vma with VM_HUGETLB set. > > We will be able to migrate hugepage with move_pages(2) after > > applying the enablement patch which comes later in this series. > > > > We avoid getting refcount on tail pages of hugepage, because unlike thp, > > hugepage is not split and we need not care about races with splitting. > > > > And migration of larger (1GB for x86_64) hugepage are not enabled. > > > > ChangeLog v2: > > - updated description and renamed patch title > > > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > > --- > > mm/memory.c | 6 ++++-- > > mm/migrate.c | 26 +++++++++++++++++++------- > > 2 files changed, 23 insertions(+), 9 deletions(-) > > > > diff --git v3.9-rc3.orig/mm/memory.c v3.9-rc3/mm/memory.c > > index 494526a..3b6ad3d 100644 > > --- v3.9-rc3.orig/mm/memory.c > > +++ v3.9-rc3/mm/memory.c > > @@ -1503,7 +1503,8 @@ struct page *follow_page_mask(struct vm_area_struct *vma, > > if (pud_none(*pud)) > > goto no_page_table; > > if (pud_huge(*pud) && vma->vm_flags & VM_HUGETLB) { > > - BUG_ON(flags & FOLL_GET); > > + if (flags & FOLL_GET) > > + goto out; > > > > page = follow_huge_pud(mm, address, pud, flags & FOLL_WRITE); > > goto out; > > } > > @@ -1514,8 +1515,9 @@ struct page *follow_page_mask(struct vm_area_struct *vma, > > if (pmd_none(*pmd)) > > goto no_page_table; > > if (pmd_huge(*pmd) && vma->vm_flags & VM_HUGETLB) { > > - BUG_ON(flags & FOLL_GET); > > page = follow_huge_pmd(mm, address, pmd, flags & FOLL_WRITE); > > + if (flags & FOLL_GET && PageHead(page)) > > + get_page_foll(page); > > Hmm, so the caller gets a non-null page without elevated ref counted > even when he asked for it. This means that all callers have to check > PageTail && hugetlb and put_page according to that. That is _really_ > fragile. I agree. And refcounting of tail pages are already very fragile, because get_page_foll() does something very tricky on tail pages, where we use page->_mapcount for refcount. This seems to be to handle some thp splitting problem, and is never intended to be used for hugepage. So I just avoid calling it for tail pages of hugepage in caller's side. > I think that returning NULL would make more sense in this case. Sounds nice. I'll do this with some comment. > > goto out; > > } > > if ((flags & FOLL_NUMA) && pmd_numa(*pmd)) > > @@ -1164,6 +1175,12 @@ static int do_move_page_to_node_array(struct mm_struct *mm, > [...] > > !migrate_all) > > goto put_and_set; > > > > + if (PageHuge(page)) { > > + get_page(page); > > + list_move_tail(&page->lru, &pagelist); > > + goto put_and_set; > > + } > > Why do you take an additional reference here? You have one from > follow_page already. For normal pages, follow_page(FOLL_GET) takes a refcount and isolate_lru_page() takes another one, so I think the same should be done for hugepages. Refcounting of this function looks tricky, and I'm not sure why existing code does like that. Thanks, Naoya
WARNING: multiple messages have this Message-ID (diff)
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> To: Michal Hocko <mhocko@suse.cz> Cc: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>, Mel Gorman <mel@csn.ul.ie>, Hugh Dickins <hughd@google.com>, KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>, Andi Kleen <andi@firstfloor.org>, Hillf Danton <dhillf@gmail.com>, linux-kernel@vger.kernel.org Subject: Re: [PATCH 06/10] migrate: add hugepage migration code to move_pages() Date: Tue, 26 Mar 2013 03:06:18 -0400 [thread overview] Message-ID: <1364281578-4bs50rjv-mutt-n-horiguchi@ah.jp.nec.com> (raw) In-Reply-To: <20130325133644.GY2154@dhcp22.suse.cz> On Mon, Mar 25, 2013 at 02:36:44PM +0100, Michal Hocko wrote: > On Fri 22-03-13 16:23:51, Naoya Horiguchi wrote: > > This patch extends move_pages() to handle vma with VM_HUGETLB set. > > We will be able to migrate hugepage with move_pages(2) after > > applying the enablement patch which comes later in this series. > > > > We avoid getting refcount on tail pages of hugepage, because unlike thp, > > hugepage is not split and we need not care about races with splitting. > > > > And migration of larger (1GB for x86_64) hugepage are not enabled. > > > > ChangeLog v2: > > - updated description and renamed patch title > > > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > > --- > > mm/memory.c | 6 ++++-- > > mm/migrate.c | 26 +++++++++++++++++++------- > > 2 files changed, 23 insertions(+), 9 deletions(-) > > > > diff --git v3.9-rc3.orig/mm/memory.c v3.9-rc3/mm/memory.c > > index 494526a..3b6ad3d 100644 > > --- v3.9-rc3.orig/mm/memory.c > > +++ v3.9-rc3/mm/memory.c > > @@ -1503,7 +1503,8 @@ struct page *follow_page_mask(struct vm_area_struct *vma, > > if (pud_none(*pud)) > > goto no_page_table; > > if (pud_huge(*pud) && vma->vm_flags & VM_HUGETLB) { > > - BUG_ON(flags & FOLL_GET); > > + if (flags & FOLL_GET) > > + goto out; > > > > page = follow_huge_pud(mm, address, pud, flags & FOLL_WRITE); > > goto out; > > } > > @@ -1514,8 +1515,9 @@ struct page *follow_page_mask(struct vm_area_struct *vma, > > if (pmd_none(*pmd)) > > goto no_page_table; > > if (pmd_huge(*pmd) && vma->vm_flags & VM_HUGETLB) { > > - BUG_ON(flags & FOLL_GET); > > page = follow_huge_pmd(mm, address, pmd, flags & FOLL_WRITE); > > + if (flags & FOLL_GET && PageHead(page)) > > + get_page_foll(page); > > Hmm, so the caller gets a non-null page without elevated ref counted > even when he asked for it. This means that all callers have to check > PageTail && hugetlb and put_page according to that. That is _really_ > fragile. I agree. And refcounting of tail pages are already very fragile, because get_page_foll() does something very tricky on tail pages, where we use page->_mapcount for refcount. This seems to be to handle some thp splitting problem, and is never intended to be used for hugepage. So I just avoid calling it for tail pages of hugepage in caller's side. > I think that returning NULL would make more sense in this case. Sounds nice. I'll do this with some comment. > > goto out; > > } > > if ((flags & FOLL_NUMA) && pmd_numa(*pmd)) > > @@ -1164,6 +1175,12 @@ static int do_move_page_to_node_array(struct mm_struct *mm, > [...] > > !migrate_all) > > goto put_and_set; > > > > + if (PageHuge(page)) { > > + get_page(page); > > + list_move_tail(&page->lru, &pagelist); > > + goto put_and_set; > > + } > > Why do you take an additional reference here? You have one from > follow_page already. For normal pages, follow_page(FOLL_GET) takes a refcount and isolate_lru_page() takes another one, so I think the same should be done for hugepages. Refcounting of this function looks tricky, and I'm not sure why existing code does like that. Thanks, Naoya -- 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>
next prev parent reply other threads:[~2013-03-26 7:07 UTC|newest] Thread overview: 132+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-03-22 20:23 [PATCH v2 0/10] extend hugepage migration Naoya Horiguchi 2013-03-22 20:23 ` Naoya Horiguchi 2013-03-22 20:23 ` [PATCH 01/10] migrate: add migrate_entry_wait_huge() Naoya Horiguchi 2013-03-22 20:23 ` Naoya Horiguchi 2013-03-23 15:55 ` Rik van Riel 2013-03-23 15:55 ` Rik van Riel 2013-03-25 10:13 ` Michal Hocko 2013-03-25 10:13 ` Michal Hocko 2013-03-26 4:25 ` Naoya Horiguchi 2013-03-26 4:25 ` Naoya Horiguchi 2013-04-05 20:33 ` KOSAKI Motohiro 2013-04-05 20:33 ` KOSAKI Motohiro 2013-04-08 20:00 ` Naoya Horiguchi 2013-04-08 20:00 ` Naoya Horiguchi 2013-04-05 20:33 ` KOSAKI Motohiro 2013-04-05 20:33 ` KOSAKI Motohiro 2013-03-22 20:23 ` [PATCH 02/10] migrate: make core migration code aware of hugepage Naoya Horiguchi 2013-03-22 20:23 ` Naoya Horiguchi 2013-03-25 10:57 ` Michal Hocko 2013-03-25 10:57 ` Michal Hocko 2013-03-26 4:33 ` Naoya Horiguchi 2013-03-26 4:33 ` Naoya Horiguchi 2013-03-26 8:49 ` Michal Hocko 2013-03-26 8:49 ` Michal Hocko 2013-04-05 20:41 ` KOSAKI Motohiro 2013-04-05 20:41 ` KOSAKI Motohiro 2013-03-22 20:23 ` [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page() Naoya Horiguchi 2013-03-22 20:23 ` Naoya Horiguchi 2013-03-25 12:31 ` Michal Hocko 2013-03-25 12:31 ` Michal Hocko 2013-03-26 4:34 ` Naoya Horiguchi 2013-03-26 4:34 ` Naoya Horiguchi 2013-03-26 9:49 ` Michal Hocko 2013-03-26 9:49 ` Michal Hocko 2013-03-26 20:35 ` Naoya Horiguchi 2013-03-26 20:35 ` Naoya Horiguchi 2013-03-27 13:00 ` Michal Hocko 2013-03-27 13:00 ` Michal Hocko 2013-04-05 21:11 ` KOSAKI Motohiro 2013-04-05 21:11 ` KOSAKI Motohiro 2013-03-26 11:29 ` Aneesh Kumar K.V 2013-03-26 11:29 ` Aneesh Kumar K.V 2013-03-27 13:52 ` Michal Hocko 2013-03-27 13:52 ` Michal Hocko 2013-03-27 19:19 ` Naoya Horiguchi 2013-03-27 19:19 ` Naoya Horiguchi 2013-03-28 8:53 ` Michal Hocko 2013-03-28 8:53 ` Michal Hocko 2013-03-29 5:26 ` Aneesh Kumar K.V 2013-03-29 5:26 ` Aneesh Kumar K.V 2013-03-29 9:36 ` Michal Hocko 2013-03-29 9:36 ` Michal Hocko 2013-04-01 5:13 ` Aneesh Kumar K.V 2013-04-01 5:13 ` Aneesh Kumar K.V 2013-04-02 9:45 ` Michal Hocko 2013-04-02 9:45 ` Michal Hocko 2013-03-22 20:23 ` [PATCH 04/10] migrate: clean up migrate_huge_page() Naoya Horiguchi 2013-03-22 20:23 ` Naoya Horiguchi 2013-04-05 21:13 ` KOSAKI Motohiro 2013-04-05 21:13 ` KOSAKI Motohiro 2013-03-22 20:23 ` [PATCH 05/10] migrate: add hugepage migration code to migrate_pages() Naoya Horiguchi 2013-03-22 20:23 ` Naoya Horiguchi 2013-03-25 13:04 ` Michal Hocko 2013-03-25 13:04 ` Michal Hocko 2013-03-26 5:13 ` Naoya Horiguchi 2013-03-26 5:13 ` Naoya Horiguchi 2013-03-26 8:55 ` Michal Hocko 2013-03-26 8:55 ` Michal Hocko 2013-04-05 21:17 ` KOSAKI Motohiro 2013-04-05 21:17 ` KOSAKI Motohiro 2013-04-08 20:21 ` Naoya Horiguchi 2013-04-08 20:21 ` Naoya Horiguchi 2013-03-22 20:23 ` [PATCH 06/10] migrate: add hugepage migration code to move_pages() Naoya Horiguchi 2013-03-22 20:23 ` Naoya Horiguchi 2013-03-25 13:36 ` Michal Hocko 2013-03-25 13:36 ` Michal Hocko 2013-03-26 7:06 ` Naoya Horiguchi [this message] 2013-03-26 7:06 ` Naoya Horiguchi 2013-03-26 10:02 ` Michal Hocko 2013-03-26 10:02 ` Michal Hocko 2013-03-26 20:37 ` Naoya Horiguchi 2013-03-26 20:37 ` Naoya Horiguchi 2013-03-22 20:23 ` [PATCH 07/10] mbind: add hugepage migration code to mbind() Naoya Horiguchi 2013-03-22 20:23 ` Naoya Horiguchi 2013-03-25 13:49 ` Michal Hocko 2013-03-25 13:49 ` Michal Hocko 2013-04-05 22:23 ` KOSAKI Motohiro 2013-04-05 22:23 ` KOSAKI Motohiro 2013-04-06 7:04 ` Michal Hocko 2013-04-06 7:04 ` Michal Hocko 2013-04-05 22:18 ` KOSAKI Motohiro 2013-04-05 22:18 ` KOSAKI Motohiro 2013-04-08 20:25 ` Naoya Horiguchi 2013-04-08 20:25 ` Naoya Horiguchi 2013-03-22 20:23 ` [PATCH 08/10] migrate: remove VM_HUGETLB from vma flag check in vma_migratable() Naoya Horiguchi 2013-03-22 20:23 ` Naoya Horiguchi 2013-03-22 20:23 ` [PATCH 09/10] memory-hotplug: enable memory hotplug to handle hugepage Naoya Horiguchi 2013-03-22 20:23 ` Naoya Horiguchi 2013-03-25 15:09 ` Michal Hocko 2013-03-25 15:09 ` Michal Hocko 2013-03-26 18:23 ` Naoya Horiguchi 2013-03-26 18:23 ` Naoya Horiguchi 2013-03-27 14:19 ` Michal Hocko 2013-03-27 14:19 ` Michal Hocko 2013-03-27 21:29 ` Naoya Horiguchi 2013-03-27 21:29 ` Naoya Horiguchi 2013-03-27 21:58 ` Naoya Horiguchi 2013-03-27 21:58 ` Naoya Horiguchi 2013-03-27 22:55 ` Michal Hocko 2013-03-27 22:55 ` Michal Hocko 2013-03-26 12:01 ` Aneesh Kumar K.V 2013-03-26 12:01 ` Aneesh Kumar K.V 2013-03-27 19:28 ` Naoya Horiguchi 2013-03-27 19:28 ` Naoya Horiguchi 2013-04-06 0:13 ` KOSAKI Motohiro 2013-04-06 0:13 ` KOSAKI Motohiro 2013-04-09 20:07 ` Naoya Horiguchi 2013-04-09 20:07 ` Naoya Horiguchi 2013-04-09 21:27 ` KOSAKI Motohiro 2013-04-09 21:27 ` KOSAKI Motohiro 2013-04-09 22:43 ` Naoya Horiguchi 2013-04-09 22:43 ` Naoya Horiguchi 2013-04-10 1:56 ` KOSAKI Motohiro 2013-04-10 1:56 ` KOSAKI Motohiro 2013-04-10 2:24 ` Naoya Horiguchi 2013-04-10 2:24 ` Naoya Horiguchi 2013-03-22 20:23 ` [PATCH 10/10] prepare to remove /proc/sys/vm/hugepages_treat_as_movable Naoya Horiguchi 2013-03-22 20:23 ` Naoya Horiguchi 2013-03-25 15:12 ` Michal Hocko 2013-03-25 15:12 ` Michal Hocko 2013-04-06 0:15 ` KOSAKI Motohiro 2013-04-06 0:15 ` KOSAKI Motohiro
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1364281578-4bs50rjv-mutt-n-horiguchi@ah.jp.nec.com \ --to=n-horiguchi@ah.jp.nec.com \ --cc=akpm@linux-foundation.org \ --cc=andi@firstfloor.org \ --cc=dhillf@gmail.com \ --cc=hughd@google.com \ --cc=kosaki.motohiro@jp.fujitsu.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mel@csn.ul.ie \ --cc=mhocko@suse.cz \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.