[PATCH] mm/migrate: Split only transparent huge pages when allocation fails

[PATCH] mm/migrate: Split only transparent huge pages when allocation fails

[Anshuman Khandual]

When unmap_and_move[_huge_page] function fails due to lack of memory, the
splitting should happen only for transparent huge pages not for HugeTLB
pages. PageTransHuge() returns true for both THP and HugeTLB pages. Hence
the conditonal check should test PagesHuge() flag to make sure that given
pages is not a HugeTLB one.

Fixes: 94723aafb9 ("mm: unclutter THP migration")
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 mm/migrate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index d6a2e89..d2297fe 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1411,7 +1411,7 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page,
 				 * we encounter them after the rest of the list
 				 * is processed.
 				 */
-				if (PageTransHuge(page)) {
+				if (PageTransHuge(page) && !PageHuge(page)) {
 					lock_page(page);
 					rc = split_huge_page_to_list(page, from);
 					unlock_page(page);
-- 
2.7.4

Re: [PATCH] mm/migrate: Split only transparent huge pages when allocation fails

[Michal Hocko]

On Mon 24-09-18 19:44:55, Anshuman Khandual wrote:
> When unmap_and_move[_huge_page] function fails due to lack of memory, the
> splitting should happen only for transparent huge pages not for HugeTLB
> pages. PageTransHuge() returns true for both THP and HugeTLB pages. Hence
> the conditonal check should test PagesHuge() flag to make sure that given
> pages is not a HugeTLB one.

Well spotted! Have you actually seen this happening or this is review
driven? I am wondering what would be the real effect of this mismatch?
I have tried to follow to code path but I suspect
split_huge_page_to_list would fail for hugetlbfs pages. If there is a
more serious effect then we should mark the patch for stable as well.

> 
> Fixes: 94723aafb9 ("mm: unclutter THP migration")
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/migrate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index d6a2e89..d2297fe 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1411,7 +1411,7 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page,
>  				 * we encounter them after the rest of the list
>  				 * is processed.
>  				 */
> -				if (PageTransHuge(page)) {
> +				if (PageTransHuge(page) && !PageHuge(page)) {
>  					lock_page(page);
>  					rc = split_huge_page_to_list(page, from);
>  					unlock_page(page);
> -- 
> 2.7.4

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm/migrate: Split only transparent huge pages when allocation fails

[Anshuman Khandual]

On 09/24/2018 08:00 PM, Michal Hocko wrote:
> On Mon 24-09-18 19:44:55, Anshuman Khandual wrote:
>> When unmap_and_move[_huge_page] function fails due to lack of memory, the
>> splitting should happen only for transparent huge pages not for HugeTLB
>> pages. PageTransHuge() returns true for both THP and HugeTLB pages. Hence
>> the conditonal check should test PagesHuge() flag to make sure that given
>> pages is not a HugeTLB one.
> 
> Well spotted! Have you actually seen this happening or this is review
> driven? I am wondering what would be the real effect of this mismatch?
> I have tried to follow to code path but I suspect
> split_huge_page_to_list would fail for hugetlbfs pages. If there is a
> more serious effect then we should mark the patch for stable as well.

split_huge_page_to_list() fails on HugeTLB pages. I was experimenting around
moving 32MB contig HugeTLB pages on arm64 (with a debug patch applied) hit
the following stack trace when the kernel crashed.

[ 3732.462797] Call trace:
[ 3732.462835]  split_huge_page_to_list+0x3b0/0x858
[ 3732.462913]  migrate_pages+0x728/0xc20
[ 3732.462999]  soft_offline_page+0x448/0x8b0
[ 3732.463097]  __arm64_sys_madvise+0x724/0x850
[ 3732.463197]  el0_svc_handler+0x74/0x110
[ 3732.463297]  el0_svc+0x8/0xc
[ 3732.463347] Code: d1000400 f90b0e60 f2fbd5a2 a94982a1 (f9000420)

Re: [PATCH] mm/migrate: Split only transparent huge pages when allocation fails

[Michal Hocko]

On Mon 24-09-18 22:10:30, Anshuman Khandual wrote:
> 
> 
> On 09/24/2018 08:00 PM, Michal Hocko wrote:
> > On Mon 24-09-18 19:44:55, Anshuman Khandual wrote:
> >> When unmap_and_move[_huge_page] function fails due to lack of memory, the
> >> splitting should happen only for transparent huge pages not for HugeTLB
> >> pages. PageTransHuge() returns true for both THP and HugeTLB pages. Hence
> >> the conditonal check should test PagesHuge() flag to make sure that given
> >> pages is not a HugeTLB one.
> > 
> > Well spotted! Have you actually seen this happening or this is review
> > driven? I am wondering what would be the real effect of this mismatch?
> > I have tried to follow to code path but I suspect
> > split_huge_page_to_list would fail for hugetlbfs pages. If there is a
> > more serious effect then we should mark the patch for stable as well.
> 
> split_huge_page_to_list() fails on HugeTLB pages. I was experimenting around
> moving 32MB contig HugeTLB pages on arm64 (with a debug patch applied) hit
> the following stack trace when the kernel crashed.
> 
> [ 3732.462797] Call trace:
> [ 3732.462835]  split_huge_page_to_list+0x3b0/0x858
> [ 3732.462913]  migrate_pages+0x728/0xc20
> [ 3732.462999]  soft_offline_page+0x448/0x8b0
> [ 3732.463097]  __arm64_sys_madvise+0x724/0x850
> [ 3732.463197]  el0_svc_handler+0x74/0x110
> [ 3732.463297]  el0_svc+0x8/0xc
> [ 3732.463347] Code: d1000400 f90b0e60 f2fbd5a2 a94982a1 (f9000420)

Please make sure this makes it to the changelog and mark the patch for
stable.

-- 
Michal Hocko
SUSE Labs