Patch "mm/hugetlb: take page table lock in follow_huge_pmd()" has been added to the 3.19-stable tree
Subsystems (6)
This is a note to let you know that I've just added the patch titled
mm/hugetlb: take page table lock in follow_huge_pmd()
to the 3.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
mm-hugetlb-take-page-table-lock-in-follow_huge_pmd.patch
and it can be found in the queue-3.19 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
Quoted 1 line
From
e66f17ff71772b209eed39de35aaa99ba819c93dMon Sep 17 00:00:00 2001
From: Naoya Horiguchi <[email protected]>
Date: Wed, 11 Feb 2015 15:25:22 -0800
Subject: mm/hugetlb: take page table lock in follow_huge_pmd()
From: Naoya Horiguchi <[email protected]>
commit e66f17ff71772b209eed39de35aaa99ba819c93d upstream.
We have a race condition between move_pages() and freeing hugepages, where move_pages() calls follow_page(FOLL_GET) for hugepages internally and tries to get its refcount without preventing concurrent freeing. This race crashes the kernel, so this patch fixes it by moving FOLL_GET code for hugepages into follow_huge_pmd() with taking the page table lock.
This patch intentionally removes page==NULL check after pte_page. This is justified because pte_page() never returns NULL for any architectures or configurations.
This patch changes the behavior of follow_huge_pmd() for tail pages and then tail pages can be pinned/returned. So the caller must be changed to properly handle the returned tail pages.
We could have a choice to add the similar locking to follow_huge_(addr|pud) for consistency, but it's not necessary because currently these functions don't support FOLL_GET flag, so let's leave it for future development.
Here is the reproducer:
$ cat movepages.c #include <stdio.h> #include <stdlib.h> #include <numaif.h>
#define ADDR_INPUT 0x700000000000UL #define HPS 0x200000 #define PS 0x1000
int main(int argc, char *argv[]) {
int i;
int nr_hp = strtol(argv[1], NULL, 0);
int nr_p = nr_hp * HPS / PS;
int ret;
void **addrs;
int *status;
int *nodes;
pid_t pid; pid = strtol(argv[2], NULL, 0);
addrs = malloc(sizeof(char *) * nr_p + 1);
status = malloc(sizeof(char *) * nr_p + 1);
nodes = malloc(sizeof(char *) * nr_p + 1);
while (1) {
for (i = 0; i < nr_p; i++) {
addrs[i] = (void *)ADDR_INPUT + i * PS;
nodes[i] = 1;
status[i] = 0;
}
ret = numa_move_pages(pid, nr_p, addrs, nodes, status,
MPOL_MF_MOVE_ALL);
if (ret == -1)
err("move_pages");
for (i = 0; i < nr_p; i++) {
addrs[i] = (void *)ADDR_INPUT + i * PS;
nodes[i] = 0;
status[i] = 0;
}
ret = numa_move_pages(pid, nr_p, addrs, nodes, status,
MPOL_MF_MOVE_ALL);
if (ret == -1)
err("move_pages");
}
return 0;}
$ cat hugepage.c #include <stdio.h> #include <sys/mman.h> #include <string.h>
#define ADDR_INPUT 0x700000000000UL
#define HPS 0x200000
int main(int argc, char *argv[]) {
int nr_hp = strtol(argv[1], NULL, 0);
char *p; while (1) {
p = mmap((void *)ADDR_INPUT, nr_hp * HPS, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
if (p != (void *)ADDR_INPUT) {
perror("mmap");
break;
}
memset(p, 0, nr_hp * HPS);
munmap(p, nr_hp * HPS);
}}
$ sysctl vm.nr_hugepages=40 $ ./hugepage 10 & $ ./movepages 10 $(pgrep -f hugepage)
--- include/linux/hugetlb.h | 8 ++++---- include/linux/swapops.h | 4 ++++ mm/gup.c | 25 ++++++++----------------- mm/hugetlb.c | 48 ++++++++++++++++++++++++++++++++++-------------- mm/migrate.c | 5 +++-- 5 files changed, 53 insertions(+), 37 deletions(-)
include/linux/hugetlb.h +4 -4
@@ -99,9 +99,9 @@ int huge_pmd_unshare(struct mm_struct *mstruct page *follow_huge_addr (struct mm_struct *mm,unsigned long address,int write);struct page *follow_huge_pmd (struct mm_struct *mm,unsigned long address, -pmd_t *pmd,int write); +pmd_t *pmd,int flags);struct page *follow_huge_pud (struct mm_struct *mm,unsigned long address, -pud_t *pud,int write); +pud_t *pud,int flags);int pmd_huge (pmd_t pmd);int pud_huge (pud_t pmd); unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
@@ -133,8 +133,8 @@ static inline void hugetlb_report_meminfstatic inline void hugetlb_show_meminfo (void ) { } -#define follow_huge_pmd (mm, addr, pmd, write) NULL -#define follow_huge_pud (mm, addr, pud, write) NULL +#define follow_huge_pmd (mm, addr, pmd, flags) NULL +#define follow_huge_pud (mm, addr, pud, flags) NULL#define prepare_hugepage_range (file, addr, len) (-EINVAL)#define pmd_huge (x) 0#define pud_huge (x) 0
include/linux/swapops.h +4 -0
@@ -137,6 +137,8 @@ static inline void make_migration_entry_ *entry =swp_entry (SWP_MIGRATION_READ ,swp_offset (*entry)); } +extern void __migration_entry_wait (struct mm_struct *mm,pte_t *ptep, +spinlock_t *ptl);extern void migration_entry_wait (struct mm_struct *mm,pmd_t *pmd,unsigned long address); extern void migration_entry_wait_huge(struct vm_area_struct *vma,
@@ -150,6 +152,8 @@ static inline int is_migration_entry(swp }#define migration_entry_to_page (swp) NULLstatic inline void make_migration_entry_read (swp_entry_t *entryp) { } +static inline void __migration_entry_wait (struct mm_struct *mm,pte_t *ptep, +spinlock_t *ptl) { }static inline void migration_entry_wait (struct mm_struct *mm,pmd_t *pmd,unsigned long address) { } static inline void migration_entry_wait_huge(struct vm_area_struct *vma,
mm/gup.c +8 -17
@@ -167,10 +167,10 @@ struct page *follow_page_mask(struct vm_if (pud_none (*pud))return no_page_table (vma, flags);if (pud_huge (*pud) && vma->vm_flags &VM_HUGETLB ) { -if (flags &FOLL_GET ) -return NULL ; - page =follow_huge_pud (mm, address, pud, flags &FOLL_WRITE ); -return page; + page =follow_huge_pud (mm, address, pud, flags); +if (page) +return page; +return no_page_table (vma, flags); }if (unlikely (pud_bad (*pud)))return no_page_table (vma, flags);
@@ -179,19 +179,10 @@ struct page *follow_page_mask(struct vm_if (pmd_none (*pmd))return no_page_table (vma, flags);if (pmd_huge (*pmd) && vma->vm_flags &VM_HUGETLB ) { - page =follow_huge_pmd (mm, address, pmd, flags &FOLL_WRITE ); -if (flags &FOLL_GET ) { -/* -* Refcount on tail pages are not well-defined and -* shouldn't be taken. The caller should handle a NULL -* return when trying to follow tail pages. -*/ -if (PageHead (page)) -get_page (page); -else - page =NULL ; - } -return page; + page =follow_huge_pmd (mm, address, pmd, flags); +if (page) +return page; +return no_page_table (vma, flags); }if ((flags &FOLL_NUMA ) &&pmd_numa (*pmd))return no_page_table (vma, flags);
mm/hugetlb.c +34 -14
62 lines not on this page · raw message
mm/migrate.c +3 -2
@@ -229,7 +229,7 @@ static void remove_migration_ptes(struct* get to the page and wait until migration is finished. * When we return from this function the fault will be retried. */ -static void __migration_entry_wait (struct mm_struct *mm,pte_t *ptep, +void __migration_entry_wait (struct mm_struct *mm,pte_t *ptep,spinlock_t *ptl) {pte_t pte;
@@ -1268,7 +1268,8 @@ static int do_move_page_to_node_array(st goto put_and_set;if (PageHuge (page)) { -isolate_huge_page (page, &pagelist); +if (PageHead (page)) +isolate_huge_page (page, &pagelist); goto put_and_set; }
Patches currently in stable-queue which might be from [email protected] are
queue-3.19/mm-hugetlb-take-page-table-lock-in-follow_huge_pmd.patch queue-3.19/mm-hugetlb-reduce-arch-dependent-code-around-follow_huge_.patch