| Message ID | 20211208165343.22349-1-arbn@yandex-team.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: mempolicy: fix THP allocations escaping mempolicy restrictions | expand |
On Wed 08-12-21 19:53:43, Andrey Ryabinin wrote: > alloc_pages_vma() may try to allocate THP page on the local > NUMA node first: > page = __alloc_pages_node(hpage_node, > gfp | __GFP_THISNODE | __GFP_NORETRY, order); > > And if the allocation fails it retries allowing remote memory: > > if (!page && (gfp & __GFP_DIRECT_RECLAIM)) > page = __alloc_pages_node(hpage_node, > gfp, order); > > However, this retry allocation completely ignores memory policy > nodemask allowing allocation to escape restrictions. > > The first appearance of this bug seems to be the commit ac5b2c18911f > ("mm: thp: relax __GFP_THISNODE for MADV_HUGEPAGE mappings") > The bug disappeared later in the commit 89c83fb539f9 > ("mm, thp: consolidate THP gfp handling into alloc_hugepage_direct_gfpmask") > and reappeared again in slightly different form in the commit 76e654cc91bb > ("mm, page_alloc: allow hugepage fallback to remote nodes when madvised") > > Fix this by passing correct nodemask to the __alloc_pages() call. > > The demonstration/reproducer of the problem: > $ mount -oremount,size=4G,huge=always /dev/shm/ > $ echo always > /sys/kernel/mm/transparent_hugepage/defrag > $ cat mbind_thp.c > #include <unistd.h> > #include <sys/mman.h> > #include <sys/stat.h> > #include <fcntl.h> > #include <assert.h> > #include <stdlib.h> > #include <stdio.h> > #include <numaif.h> > > #define SIZE 2ULL << 30 > int main(int argc, char **argv) > { > int fd; > unsigned long long i; > char *addr; > pid_t pid; > char buf[100]; > unsigned long nodemask = 1; > > fd = open("/dev/shm/test", O_RDWR|O_CREAT); > assert(fd > 0); > assert(ftruncate(fd, SIZE) == 0); > > addr = mmap(NULL, SIZE, PROT_READ|PROT_WRITE, > MAP_SHARED, fd, 0); > > assert(mbind(addr, SIZE, MPOL_BIND, &nodemask, 2, MPOL_MF_STRICT|MPOL_MF_MOVE)==0); > for (i = 0; i < SIZE; i+=4096) { > addr[i] = 1; > } > pid = getpid(); > snprintf(buf, sizeof(buf), "grep shm /proc/%d/numa_maps", pid); > system(buf); > sleep(10000); > > return 0; > } > $ gcc mbind_thp.c -o mbind_thp -lnuma > $ numactl -H > available: 2 nodes (0-1) > node 0 cpus: 0 2 > node 0 size: 1918 MB > node 0 free: 1595 MB > node 1 cpus: 1 3 > node 1 size: 2014 MB > node 1 free: 1731 MB > node distances: > node 0 1 > 0: 10 20 > 1: 20 10 > $ rm -f /dev/shm/test; taskset -c 0 ./mbind_thp > 7fd970a00000 bind:0 file=/dev/shm/test dirty=524288 active=0 N0=396800 N1=127488 kernelpagesize_kB=4 > > Fixes: ac5b2c18911f ("mm: thp: relax __GFP_THISNODE for MADV_HUGEPAGE mappings") > Signed-off-by: Andrey Ryabinin <arbn@yandex-team.com> > Cc: <stable@vger.kernel.org> > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: Michal Hocko <mhocko@suse.com> > Cc: Mel Gorman <mgorman@techsingularity.net> > Cc: David Rientjes <rientjes@google.com> Looks good to me. Acked-by: Michal Hocko <mhocko@suse.com> Thanks! > --- > mm/mempolicy.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index 10e9c87260ed..f6248affaf38 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -2140,8 +2140,7 @@ struct page *alloc_pages_vma(gfp_t gfp, int order, struct vm_area_struct *vma, > * memory with both reclaim and compact as well. > */ > if (!page && (gfp & __GFP_DIRECT_RECLAIM)) > - page = __alloc_pages_node(hpage_node, > - gfp, order); > + page = __alloc_pages(gfp, order, hpage_node, nmask); > > goto out; > } > -- > 2.32.0
On Wed, Dec 08, 2021 at 07:53:43PM +0300, Andrey Ryabinin wrote: > alloc_pages_vma() may try to allocate THP page on the local > NUMA node first: > page = __alloc_pages_node(hpage_node, > gfp | __GFP_THISNODE | __GFP_NORETRY, order); > > And if the allocation fails it retries allowing remote memory: > > if (!page && (gfp & __GFP_DIRECT_RECLAIM)) > page = __alloc_pages_node(hpage_node, > gfp, order); > > However, this retry allocation completely ignores memory policy > nodemask allowing allocation to escape restrictions. > > The first appearance of this bug seems to be the commit ac5b2c18911f > ("mm: thp: relax __GFP_THISNODE for MADV_HUGEPAGE mappings") > The bug disappeared later in the commit 89c83fb539f9 > ("mm, thp: consolidate THP gfp handling into alloc_hugepage_direct_gfpmask") > and reappeared again in slightly different form in the commit 76e654cc91bb > ("mm, page_alloc: allow hugepage fallback to remote nodes when madvised") > > Fix this by passing correct nodemask to the __alloc_pages() call. > > The demonstration/reproducer of the problem: > $ mount -oremount,size=4G,huge=always /dev/shm/ > $ echo always > /sys/kernel/mm/transparent_hugepage/defrag > $ cat mbind_thp.c > #include <unistd.h> > #include <sys/mman.h> > #include <sys/stat.h> > #include <fcntl.h> > #include <assert.h> > #include <stdlib.h> > #include <stdio.h> > #include <numaif.h> > > #define SIZE 2ULL << 30 > int main(int argc, char **argv) > { > int fd; > unsigned long long i; > char *addr; > pid_t pid; > char buf[100]; > unsigned long nodemask = 1; > > fd = open("/dev/shm/test", O_RDWR|O_CREAT); > assert(fd > 0); > assert(ftruncate(fd, SIZE) == 0); > > addr = mmap(NULL, SIZE, PROT_READ|PROT_WRITE, > MAP_SHARED, fd, 0); > > assert(mbind(addr, SIZE, MPOL_BIND, &nodemask, 2, MPOL_MF_STRICT|MPOL_MF_MOVE)==0); > for (i = 0; i < SIZE; i+=4096) { > addr[i] = 1; > } > pid = getpid(); > snprintf(buf, sizeof(buf), "grep shm /proc/%d/numa_maps", pid); > system(buf); > sleep(10000); > > return 0; > } > $ gcc mbind_thp.c -o mbind_thp -lnuma > $ numactl -H > available: 2 nodes (0-1) > node 0 cpus: 0 2 > node 0 size: 1918 MB > node 0 free: 1595 MB > node 1 cpus: 1 3 > node 1 size: 2014 MB > node 1 free: 1731 MB > node distances: > node 0 1 > 0: 10 20 > 1: 20 10 > $ rm -f /dev/shm/test; taskset -c 0 ./mbind_thp > 7fd970a00000 bind:0 file=/dev/shm/test dirty=524288 active=0 N0=396800 N1=127488 kernelpagesize_kB=4 > > Fixes: ac5b2c18911f ("mm: thp: relax __GFP_THISNODE for MADV_HUGEPAGE mappings") > Signed-off-by: Andrey Ryabinin <arbn@yandex-team.com> > Cc: <stable@vger.kernel.org> > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: Michal Hocko <mhocko@suse.com> > Cc: Mel Gorman <mgorman@techsingularity.net> > Cc: David Rientjes <rientjes@google.com> Acked-by: Mel Gorman <mgorman@techsingularity.net>
On Wed, 8 Dec 2021, Andrey Ryabinin wrote: > alloc_pages_vma() may try to allocate THP page on the local > NUMA node first: > page = __alloc_pages_node(hpage_node, > gfp | __GFP_THISNODE | __GFP_NORETRY, order); > > And if the allocation fails it retries allowing remote memory: > > if (!page && (gfp & __GFP_DIRECT_RECLAIM)) > page = __alloc_pages_node(hpage_node, > gfp, order); > > However, this retry allocation completely ignores memory policy > nodemask allowing allocation to escape restrictions. > > The first appearance of this bug seems to be the commit ac5b2c18911f > ("mm: thp: relax __GFP_THISNODE for MADV_HUGEPAGE mappings") > The bug disappeared later in the commit 89c83fb539f9 > ("mm, thp: consolidate THP gfp handling into alloc_hugepage_direct_gfpmask") > and reappeared again in slightly different form in the commit 76e654cc91bb > ("mm, page_alloc: allow hugepage fallback to remote nodes when madvised") > > Fix this by passing correct nodemask to the __alloc_pages() call. > > The demonstration/reproducer of the problem: > $ mount -oremount,size=4G,huge=always /dev/shm/ > $ echo always > /sys/kernel/mm/transparent_hugepage/defrag > $ cat mbind_thp.c > #include <unistd.h> > #include <sys/mman.h> > #include <sys/stat.h> > #include <fcntl.h> > #include <assert.h> > #include <stdlib.h> > #include <stdio.h> > #include <numaif.h> > > #define SIZE 2ULL << 30 > int main(int argc, char **argv) > { > int fd; > unsigned long long i; > char *addr; > pid_t pid; > char buf[100]; > unsigned long nodemask = 1; > > fd = open("/dev/shm/test", O_RDWR|O_CREAT); > assert(fd > 0); > assert(ftruncate(fd, SIZE) == 0); > > addr = mmap(NULL, SIZE, PROT_READ|PROT_WRITE, > MAP_SHARED, fd, 0); > > assert(mbind(addr, SIZE, MPOL_BIND, &nodemask, 2, MPOL_MF_STRICT|MPOL_MF_MOVE)==0); > for (i = 0; i < SIZE; i+=4096) { > addr[i] = 1; > } > pid = getpid(); > snprintf(buf, sizeof(buf), "grep shm /proc/%d/numa_maps", pid); > system(buf); > sleep(10000); > > return 0; > } > $ gcc mbind_thp.c -o mbind_thp -lnuma > $ numactl -H > available: 2 nodes (0-1) > node 0 cpus: 0 2 > node 0 size: 1918 MB > node 0 free: 1595 MB > node 1 cpus: 1 3 > node 1 size: 2014 MB > node 1 free: 1731 MB > node distances: > node 0 1 > 0: 10 20 > 1: 20 10 > $ rm -f /dev/shm/test; taskset -c 0 ./mbind_thp > 7fd970a00000 bind:0 file=/dev/shm/test dirty=524288 active=0 N0=396800 N1=127488 kernelpagesize_kB=4 > > Fixes: ac5b2c18911f ("mm: thp: relax __GFP_THISNODE for MADV_HUGEPAGE mappings") > Signed-off-by: Andrey Ryabinin <arbn@yandex-team.com> > Cc: <stable@vger.kernel.org> > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: Michal Hocko <mhocko@suse.com> > Cc: Mel Gorman <mgorman@techsingularity.net> > Cc: David Rientjes <rientjes@google.com> Acked-by: David Rientjes <rientjes@google.com>
diff --git a/mm/mempolicy.c b/mm/mempolicy.c index 10e9c87260ed..f6248affaf38 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -2140,8 +2140,7 @@ struct page *alloc_pages_vma(gfp_t gfp, int order, struct vm_area_struct *vma, * memory with both reclaim and compact as well. */ if (!page && (gfp & __GFP_DIRECT_RECLAIM)) - page = __alloc_pages_node(hpage_node, - gfp, order); + page = __alloc_pages(gfp, order, hpage_node, nmask); goto out; }
alloc_pages_vma() may try to allocate THP page on the local NUMA node first: page = __alloc_pages_node(hpage_node, gfp | __GFP_THISNODE | __GFP_NORETRY, order); And if the allocation fails it retries allowing remote memory: if (!page && (gfp & __GFP_DIRECT_RECLAIM)) page = __alloc_pages_node(hpage_node, gfp, order); However, this retry allocation completely ignores memory policy nodemask allowing allocation to escape restrictions. The first appearance of this bug seems to be the commit ac5b2c18911f ("mm: thp: relax __GFP_THISNODE for MADV_HUGEPAGE mappings") The bug disappeared later in the commit 89c83fb539f9 ("mm, thp: consolidate THP gfp handling into alloc_hugepage_direct_gfpmask") and reappeared again in slightly different form in the commit 76e654cc91bb ("mm, page_alloc: allow hugepage fallback to remote nodes when madvised") Fix this by passing correct nodemask to the __alloc_pages() call. The demonstration/reproducer of the problem: $ mount -oremount,size=4G,huge=always /dev/shm/ $ echo always > /sys/kernel/mm/transparent_hugepage/defrag $ cat mbind_thp.c #include <unistd.h> #include <sys/mman.h> #include <sys/stat.h> #include <fcntl.h> #include <assert.h> #include <stdlib.h> #include <stdio.h> #include <numaif.h> #define SIZE 2ULL << 30 int main(int argc, char **argv) { int fd; unsigned long long i; char *addr; pid_t pid; char buf[100]; unsigned long nodemask = 1; fd = open("/dev/shm/test", O_RDWR|O_CREAT); assert(fd > 0); assert(ftruncate(fd, SIZE) == 0); addr = mmap(NULL, SIZE, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); assert(mbind(addr, SIZE, MPOL_BIND, &nodemask, 2, MPOL_MF_STRICT|MPOL_MF_MOVE)==0); for (i = 0; i < SIZE; i+=4096) { addr[i] = 1; } pid = getpid(); snprintf(buf, sizeof(buf), "grep shm /proc/%d/numa_maps", pid); system(buf); sleep(10000); return 0; } $ gcc mbind_thp.c -o mbind_thp -lnuma $ numactl -H available: 2 nodes (0-1) node 0 cpus: 0 2 node 0 size: 1918 MB node 0 free: 1595 MB node 1 cpus: 1 3 node 1 size: 2014 MB node 1 free: 1731 MB node distances: node 0 1 0: 10 20 1: 20 10 $ rm -f /dev/shm/test; taskset -c 0 ./mbind_thp 7fd970a00000 bind:0 file=/dev/shm/test dirty=524288 active=0 N0=396800 N1=127488 kernelpagesize_kB=4 Fixes: ac5b2c18911f ("mm: thp: relax __GFP_THISNODE for MADV_HUGEPAGE mappings") Signed-off-by: Andrey Ryabinin <arbn@yandex-team.com> Cc: <stable@vger.kernel.org> Cc: Andrea Arcangeli <aarcange@redhat.com> Cc: Michal Hocko <mhocko@suse.com> Cc: Mel Gorman <mgorman@techsingularity.net> Cc: David Rientjes <rientjes@google.com> --- mm/mempolicy.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)