New: mprotect got stuck when THP is "always" enabled

Bugme-new Andrew Morton 4 participants 5 messages Apr 19, 2011 – Apr 20, 2011 1 message not in the archive

Subsystems (3)

Memory management MM core THP

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]/

Andrew Morton

author maintainer

2011-04-19 06:03 UTC #1 lore raw

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Quoted 124 lines · On Tue, 19 Apr 2011 05:25:41 GMT [email protected] wrote:

https://bugzilla.kernel.org/show_bug.cgi?id=33682
Summary: mprotect got stuck when THP is "always" enabled
Product: Memory Management
Version: 2.5
Kernel Version: 2.6.38-r1
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Other
AssignedTo: [email protected]
ReportedBy: [email protected]
Regression: No
Created an attachment (id=54662)
--> (https://bugzilla.kernel.org/attachment.cgi?id=54662)
mprotect test program
Description of problem:
see attached test program. This program can be run like this:
./mprotect <times> <length> <flag>
times: how many times the mprotect() function execute;
length: same as the "length" option in mprotect() function;
flag: when flag is set to 1, the program would touch every page within the range [addr, addr+length-1] before it calls mprotect().
to reproduce the stuck, execute: ./mprotect 50 128 1
Note that the stuck only happens when the following conditions are all satisfied:
flag == 1, i.e. touch page before mprotect()
proto = PROT_NONE in mprotect()
THP is enabled with "always" option
Version-Release number of selected component (if applicable):
Linux version 2.6.39-rc3 (caspar@caspar-gentoo) (gcc version 4.5.2 (Gentoo 4.5.2 p1.0, pie-0.4.5) ) #1 SMP Tue Apr 19 12:32:20 CST 2011
How reproducible:
very often
Actual results:
test program got stuck when touching pages + THP always enabled:
caspar-gentoo tmp # echo always > /sys/kernel/mm/transparent_hugepage/enabled caspar-gentoo tmp # ./mprotect 50 128 1
^C <- stuck
caspar-gentoo tmp # ./mprotect 50 128 1
^C
caspar-gentoo tmp # ./mprotect 50 128 1
^C
caspar-gentoo tmp # ./mprotect 50 128 1
^C
caspar-gentoo tmp # echo madvise > /sys/kernel/mm/transparent_hugepage/enabled caspar-gentoo tmp # ./mprotect 50 128 1
done caspar-gentoo tmp # ./mprotect 50 128 1
done caspar-gentoo tmp # ./mprotect 50 128 1
done caspar-gentoo tmp # ./mprotect 50 128 1
done caspar-gentoo tmp # echo never > /sys/kernel/mm/transparent_hugepage/enabled
caspar-gentoo tmp # ./mprotect 50 128 1
done caspar-gentoo tmp # ./mprotect 50 128 1
done caspar-gentoo tmp # ./mprotect 50 128 1
done caspar-gentoo tmp # ./mprotect 50 128 1
done caspar-gentoo tmp # ./mprotect 50 128 1
done caspar-gentoo tmp # ./mprotect 50 128 1
done caspar-gentoo tmp # ./mprotect 50 128 1
Expected results:
test program exit normally
Additional info:
This reproducer was similar to a test program in upstream test suite: libMicro
(http://hub.opensolaris.org/bin/view/Project+libmicro/)
strace ouput:
# strace ./mprotect 50 128 1
execve("./mprotect", ["./mprotect", "50", "128", "1"], [/* 35 vars */]) = 0
brk(0)                                  = 0x16ad000
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7ffa0c266000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY)      = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=163815, ...}) = 0
mmap(NULL, 163815, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7ffa0c23e000
close(3)                                = 0
open("/lib64/libc.so.6", O_RDONLY)      = 3
read(3,
"\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\320\357\1\0\0\0\0\0"..., 832) =
832
fstat(3, {st_mode=S_IFREG|0755, st_size=1608912, ...}) = 0
mmap(NULL, 3718152, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) =
0x7ffa0bcbc000
mprotect(0x7ffa0be3f000, 2093056, PROT_NONE) = 0
mmap(0x7ffa0c03e000, 20480, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x182000) = 0x7ffa0c03e000
mmap(0x7ffa0c043000, 19464, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7ffa0c043000
close(3)                                = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7ffa0c23d000
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7ffa0c23c000
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7ffa0c23b000
arch_prctl(ARCH_SET_FS, 0x7ffa0c23c700) = 0
mprotect(0x7ffa0c03e000, 16384, PROT_READ) = 0
mprotect(0x600000, 4096, PROT_READ)     = 0
mprotect(0x7ffa0c267000, 4096, PROT_READ) = 0
munmap(0x7ffa0c23e000, 163815)          = 0
open("/dev/zero", O_RDWR)               = 3
mmap(NULL, 6553600, PROT_READ|PROT_WRITE, MAP_PRIVATE, 3, 0) = 0x7ffa0b67c000
mprotect(0x7ffa0b67c000, 131072, PROT_NONE) = 0

Quoted 9 lines

mprotect(0x7ffa0b69c000, 131072, PROT_NONE) = 0
mprotect(0x7ffa0b6bc000, 131072, PROT_NONE) = 0
mprotect(0x7ffa0b6dc000, 131072, PROT_NONE) = 0
mprotect(0x7ffa0b6fc000, 131072, PROT_NONE) = 0
mprotect(0x7ffa0b71c000, 131072, PROT_NONE) = 0
mprotect(0x7ffa0b73c000, 131072, PROT_NONE) = 0
mprotect(0x7ffa0b75c000, 131072, PROT_NONE) = 0
<repeated random times, snip>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Andrea Arcangeli

2011-04-19 11:26 UTC #2 lore raw

Quoted 3 lines · Andrew Morton

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Ok.

/dev/zero mapped with MAP_PRIVATE is special as it doesn't set vm_ops but is vm_file backed. The page fault code only checks the vm_ops being NULL to decide if to use THP or not (it doesn't check vm_file). But then the huge_memory.c code also checks vm_file (like in vma_adjust_trans_huge). So I think what's going on in the special case of /dev/zero MAP_PRIVATE is that we could have THP enabled on a filebacked vma (but no vm_ops backed) that doesn't fully enable all operations necessary to be safe like vma_adjust_trans_huge because vm_file being set. To let /dev/zero to use THP safely I guess we can remove the vm_file checks and only use the vm_ops checks across huge_memory.c/huge_mm.h or to add add "!vma->vm_ops && !vma->vm_file" to the page fault so THP isn't used. The former is more risky than adding the vm_file check in the page fault path but it'd match more closely what do_anonymous_page does, so I'm thinking what are the implications of removing vm_file across the huge_memory.c/huge_mm.h. I didn't manage to reboot into a fixed kernel to test yet... I'll let you know the results after some testing.

Andrea Arcangeli

2011-04-19 13:52 UTC #3 lore raw

Hi,

this should fix bug
https://bugzilla.kernel.org/show_bug.cgi?id=33682 .

====
Subject: thp: fix /dev/zero MAP_PRIVATE and vm_flags cleanups

From: Andrea Arcangeli <[email protected]>

The huge_memory.c THP page fault was allowed to run if vm_ops was null (which would succeed for /dev/zero MAP_PRIVATE, as the f_op->mmap wouldn't setup a special vma->vm_ops and it would fallback to regular anonymous memory) but other THP logics weren't fully activated for vmas with vm_file not NULL (/dev/zero has a not NULL vma->vm_file).

So this removes the vm_file checks so that /dev/zero also can safely use THP (the other albeit safer approach to fix this bug would have been to prevent the THP initial page fault to run if vm_file was set).

After removing the vm_file checks, this also makes huge_memory.c stricter in khugepaged for the DEBUG_VM=y case. It doesn't replace the vm_file check with a is_pfn_mapping check (but it keeps checking for VM_PFNMAP under VM_BUG_ON) because for a is_cow_mapping() mapping VM_PFNMAP should only be allowed to exist before the first page fault, and in turn when vma->anon_vma is null (so preventing khugepaged registration). So I tend to think the previous comment saying if vm_file was set, VM_PFNMAP might have been set and we could still be registered in khugepaged (despite anon_vma was not NULL to be registered in khugepaged) was too paranoid. The is_linear_pfn_mapping check is also I think superfluous (as described by comment) but under DEBUG_VM it is safe to stay.

Signed-off-by Andrea Arcangeli <[email protected]>

---

include/linux/huge_mm.h +1 -1

index df29c8f..8847c8c 100644
@@ -117,7 +117,7 @@ static inline void vma_adjust_trans_huge(struct vm_area_struct *vma,
 					 unsigned long end,
 					 long adjust_next)
 {
-	if (!vma->anon_vma || vma->vm_ops || vma->vm_file)
+	if (!vma->anon_vma || vma->vm_ops)
 		return;
 	__vma_adjust_trans_huge(vma, start, end, adjust_next);
 }

include/linux/mm.h +2 -1

index 692dbae..2348db2 100644
@@ -137,7 +137,8 @@ extern unsigned int kobjsize(const void *objp);
 #define VM_RandomReadHint(v)		((v)->vm_flags & VM_RAND_READ)
 
 /*
- * special vmas that are non-mergable, non-mlock()able
+ * Special vmas that are non-mergable, non-mlock()able.
+ * Note: mm/huge_memory.c VM_NO_THP depends on this definition.
  */
 #define VM_SPECIAL (VM_IO | VM_DONTEXPAND | VM_RESERVED | VM_PFNMAP)

mm/huge_memory.c +24 ~~-19~~

87 lines not on this page · raw message

Rik van Riel

Acked-by

2011-04-19 19:00 UTC #4 lore raw

Quoted 10 lines · Andrea Arcangeli

Hi,
this should fix bug
https://bugzilla.kernel.org/show_bug.cgi?id=33682 .
====
Subject: thp: fix /dev/zero MAP_PRIVATE and vm_flags cleanups
From: Andrea Arcangeli<[email protected]>

Quoted 7 lines

The huge_memory.c THP page fault was allowed to run if vm_ops was null (which would succeed for /dev/zero MAP_PRIVATE, as the f_op->mmap wouldn't setup a special vma->vm_ops and it would fallback to regular anonymous memory) but other THP logics weren't fully activated for vmas with vm_file not NULL (/dev/zero has a not NULL vma->vm_file).
Signed-off-by Andrea Arcangeli<[email protected]>

Acked-by Rik van Riel <[email protected]>

Mel Gorman

Acked-by

2011-04-20 10:01 UTC #5 lore raw

In reply to Andrea Arcangeli

Quoted 33 lines · Andrea Arcangeli

Hi,
this should fix bug
https://bugzilla.kernel.org/show_bug.cgi?id=33682 .
====
Subject: thp: fix /dev/zero MAP_PRIVATE and vm_flags cleanups
From: Andrea Arcangeli <[email protected]>
The huge_memory.c THP page fault was allowed to run if vm_ops was null (which would succeed for /dev/zero MAP_PRIVATE, as the f_op->mmap wouldn't setup a special vma->vm_ops and it would fallback to regular anonymous memory) but other THP logics weren't fully activated for vmas with vm_file not NULL (/dev/zero has a not NULL vma->vm_file).
So this removes the vm_file checks so that /dev/zero also can safely use THP (the other albeit safer approach to fix this bug would have been to prevent the THP initial page fault to run if vm_file was set).
After removing the vm_file checks, this also makes huge_memory.c stricter in khugepaged for the DEBUG_VM=y case. It doesn't replace the vm_file check with a is_pfn_mapping check (but it keeps checking for VM_PFNMAP under VM_BUG_ON) because for a is_cow_mapping() mapping VM_PFNMAP should only be allowed to exist before the first page fault, and in turn when vma->anon_vma is null (so preventing khugepaged registration). So I tend to think the previous comment saying if vm_file was set, VM_PFNMAP might have been set and we could still be registered in khugepaged (despite anon_vma was not NULL to be registered in khugepaged) was too paranoid. The is_linear_pfn_mapping check is also I think superfluous (as described by comment) but under DEBUG_VM it is safe to stay.

Quoted 1 line

Signed-off-by Andrea Arcangeli <[email protected]>

Acked-by Mel Gorman <[email protected]>

Signature · 9 lines

Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>