Messages in this thread | | | Date | Mon, 18 Nov 2013 10:32:47 +0000 | From | Mel Gorman <> | Subject | Re: [v3][PATCH 2/2] mm: thp: give transparent hugepage code a separate copy_page |
| |
On Fri, Nov 15, 2013 at 02:55:53PM -0800, Dave Hansen wrote: > > Changes from v2: > * > Changes from v1: > * removed explicit might_sleep() in favor of the one that we > get from the cond_resched(); > > -- > > From: Dave Hansen <dave.hansen@linux.intel.com> > > Right now, the migration code in migrate_page_copy() uses > copy_huge_page() for hugetlbfs and thp pages: > > if (PageHuge(page) || PageTransHuge(page)) > copy_huge_page(newpage, page); > > So, yay for code reuse. But: > > void copy_huge_page(struct page *dst, struct page *src) > { > struct hstate *h = page_hstate(src); > > and a non-hugetlbfs page has no page_hstate(). This works 99% of > the time because page_hstate() determines the hstate from the > page order alone. Since the page order of a THP page matches the > default hugetlbfs page order, it works. > > But, if you change the default huge page size on the boot > command-line (say default_hugepagesz=1G), then we might not even > *have* a 2MB hstate so page_hstate() returns null and > copy_huge_page() oopses pretty fast since copy_huge_page() > dereferences the hstate: > > void copy_huge_page(struct page *dst, struct page *src) > { > struct hstate *h = page_hstate(src); > if (unlikely(pages_per_huge_page(h) > MAX_ORDER_NR_PAGES)) { > ... > > Mel noticed that the migration code is really the only user of > these functions. This moves all the copy code over to migrate.c > and makes copy_huge_page() work for THP by checking for it > explicitly. > > I believe the bug was introduced in b32967ff101: > Author: Mel Gorman <mgorman@suse.de> > Date: Mon Nov 19 12:35:47 2012 +0000 > mm: numa: Add THP migration for the NUMA working set scanning fault case. > > Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Acked-by: Mel Gorman <mgorman@suse.de>
-- Mel Gorman SUSE Labs
| |