Does clip_start require entry simplification?

[dougm]

Summary

This change removes a call to vm_map_simplify_entry from _vm_map_clip_start. Testing has shown this call to be effective previously, but since recent changes to vm_map_protect, those same tests no longer show it to have any effect.

Test Plan

Peter Holm has tested a version that wrote a message when vm_map_simplify_entry would do something here, and it produces no message.

[pho]

I'm building a kernel with this patch right now.

[pho]

panic: _vm_map_clip_begin: entry can be simplified
cpuid = 1
time = 1560522673
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe02cf87a7b0
vpanic() at vpanic+0x19d/frame 0xfffffe02cf87a800
panic() at panic+0x43/frame 0xfffffe02cf87a860
_vm_map_clip_start() at _vm_map_clip_start+0x10a/frame 0xfffffe02cf87a8a0
vm_map_delete() at vm_map_delete+0x99/frame 0xfffffe02cf87a920
kern_munmap() at kern_munmap+0x115/frame 0xfffffe02cf87a990
amd64_syscall() at amd64_syscall+0x291/frame 0xfffffe02cf87aab0
fast_syscall_common() at fast_syscall_common+0x101/frame 0xfffffe02cf87aab0

https://people.freebsd.org/~pho/stress/log/dougm032.txt

[dougm]

Write information about the cases in which clip_start requires entry simplification, with the hope that the origins of those cases can be identified and that simplification happen elsewhere.

[pho]

[root@mercat1 /usr/src/sys/amd64/compile/PHO]# cc -c -O0  -pipe -fno-strict-aliasing  -g -nostdinc  -I. -I../../.. -I../../../contrib/ck/include -I../../../contrib/libfdt -D_KERNEL -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h    -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -MD  -MF.depend.vm_map.o -MTvm_map.o -fdebug-prefix-map=./machine=/usr/src/sys/amd64/include -fdebug-prefix-map=./x86=/usr/src/sys/x86/include -mcmodel=kernel -mno-red-zone -mno-mmx -mno-sse -msoft-float  -fno-asynchronous-unwind-tables -ffreestanding -fwrapv -fstack-protector -gdwarf-2 -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wcast-qual -Wundef -Wno-pointer-sign -D__printf__=__freebsd_kprintf__ -Wmissing-include-dirs -fdiagnostics-show-option -Wno-unknown-pragmas -Wno-error-tautological-compare -Wno-error-empty-body -Wno-error-parentheses-equality -Wno-error-unused-function -Wno-error-pointer-sign -Wno-error-shift-negative-value -Wno-address-of-packed-member  -mno-aes -mno-avx  -std=iso9899:1999 -Werror  ../../../vm/vm_map.c
../../../vm/vm_map.c:2203:36: error: member reference type 'union vm_map_object' is not a pointer; did you mean to use '.'?
                    entry->object ? entry->object->type : -1);
                                    ~~~~~~~~~~~~~^~
                                                 .
../../../vm/vm_map.c:2203:38: error: no member named 'type' in 'union vm_map_object'
                    entry->object ? entry->object->type : -1);
                                    ~~~~~~~~~~~~~  ^
2 errors generated.
[root@mercat1 /usr/src/sys/amd64/compile/PHO]#

[dougm]

Make it compile.

[pho]

The mmap11.sh test triggered these :

_vm_map_clip_start: simplifying entry start 206000 end 217000 next_read 203000 max_free 7ff7ddff9000 eflags 24 object-type 0
_vm_map_clip_start: simplifying entry start 20d000 end 217000 next_read 203000 max_free 7ff7ddff9000 eflags 24 object-type 0
_vm_map_clip_start: simplifying entry start 800e00000 end 802014000 next_read 800e00000 max_free 7ff7d1df9000 eflags 0 object-type 0
_vm_map_clip_start: simplifying entry start 800e00000 end 80b0fb000 next_read 800e00000 max_free 7ff7d1df9000 eflags 0 object-type 0

[pho]

I ran all of the mmap(2) tests I have + a buildworld / installworld.
No problems seen.

[dougm]

Just to be clear, I understand you to be saying that the map11 test produces the four lines of output above and no other tests, including buildworld/installworld, produce any such output. Is that correct?

[pho]

No sorry, other tests also produced the printout.
Here's the full console log: https://people.freebsd.org/~pho/stress/log/dougm033.txt

[dougm]

I've identified a case where vm_map_protect leaves an entry un-simplified, and attempted to address it. I hope to discover whether the number of simplify_entry calls logged as necessary in clip_start is reduced by this change.

[pho]

I see lots of _vm_map_clip_start on i386 with D20633.58825.diff.
https://people.freebsd.org/~pho/stress/log/dougm034.txt

[dougm]

In D20633#447540, @pho wrote:

I see lots of _vm_map_clip_start on i386 with D20633.58825.diff.
https://people.freebsd.org/~pho/stress/log/dougm034.txt

But I see the number of such lines reduced from 1156 in dougm033.txt to 338 here, so that seems like progress.

[pho]

I forgot to mention that it is *only* on i386 I see this. No printfs on amd64.

[dougm]

Just to be clear, I understand that this latest patch produces no printfs on amd64, but only on i386, and the previous version produces printfs on both architectures. Is that understanding correct?

[pho]

No.
This is the first time I ran tests on i386, so I have no way of knowing if this was an issue before.
doug033.txt was on amd64 and doug034.txt on i386.
Today on amd64 I ran all of the mmap() tests I have (that is, not a full test) and observed no printfs.
I'll start a full test on amd64, just to be sure.

[pho]

I'm almost done with the amd64 tests and only found one instance of the printf:

20190621 13:21:33 all (235/646): mmap14.sh
_vm_map_clip_start: simplifying entry start 7fffdfdfe000 end 7fffdffde000 next_read 7fffdfdfd000 max_free 7ff7de9fc000 eflags 30000 object-type -1

Shall I move on to D20711?

[dougm]

In D20633#448011, @pho wrote:

I'm almost done with the amd64 tests and only found one instance of the printf:

20190621 13:21:33 all (235/646): mmap14.sh
_vm_map_clip_start: simplifying entry start 7fffdfdfe000 end 7fffdffde000 next_read 7fffdfdfd000 max_free 7ff7de9fc000 eflags 30000 object-type -1

Shall I move on to D20711?

Yes, please do.

We're going to have to track down that one case later. It looks weird.

[pho]

During a "init 6" with this kernel I got:

Stopping devd.
Waiting for PIDS: 1963.
Writing entropy file:.
Writing early boot entropy file:.
.
Terminated
Jun 23 08:19:56 panic: swap_reserved < decr
cpuid = 1
time = 1561270796
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe00adaae6a0
vpanic() at vpanic+0x19d/frame 0xfffffe00adaae6f0
panic() at panic+0x43/frame 0xfffffe00adaae750
swap_release_by_cred() at swap_release_by_cred+0x9d/frame 0xfffffe00adaae770
vm_map_entry_delete() at vm_map_entry_delete+0x1ad/frame 0xfffffe00adaae7e0
vm_map_delete() at vm_map_delete+0x2a3/frame 0xfffffe00adaae860
vm_map_remove() at vm_map_remove+0xa6/frame 0xfffffe00adaae8a0
vmspace_dofree() at vmspace_dofree+0x3f/frame 0xfffffe00adaae8d0
vmspace_exit() at vmspace_exit+0x154/frame 0xfffffe00adaae910
exit1() at exit1+0x5ad/frame 0xfffffe00adaae980
sys_sys_exit() at sys_sys_exit+0xd/frame 0xfffffe00adaae990
amd64_syscall() at amd64_syscall+0x291/frame 0xfffffe00adaaeab0
fast_syscall_common() at fast_syscall_common+0x101/frame 0xfffffe00adaaeab0

https://people.freebsd.org/~pho/stress/log/dougm036.txt

[pho]

I updated dougm036.txt with some gdb output.

[dougm]

Update after some changes that will reduce, if not eliminate, the need to simplify before clipping.

[pho]

I ran random tests for 3 hours with D20633.59145.diff.
No problems seen.

[kib]

vm_map_protect() is not the only user of the vm_map_clip_start(). More, I suspect that wire/unwire and vm_map_delete() might be more prominent users, because apps do not change protection often, while they definitely unmap enough. Did you evaluate these cases ?

[dougm]

In D20633#450353, @kib wrote:

vm_map_protect() is not the only user of the vm_map_clip_start(). More, I suspect that wire/unwire and vm_map_delete() might be more prominent users, because apps do not change protection often, while they definitely unmap enough. Did you evaluate these cases ?

I looked for places where a loop called vm_map_simplify_entry and there was a way to avoid completing the loop after starting to clip or modify entry fields.

vm_map_delete doesn't simplify, but I evaluated it, and after every clip_start, the clipped entry is deleted by this loop or by some other process so that there's no entry left to merge with.

In vm_map_unwire, an entry can avoid getting simplified if:

• • MAP_ENTRY_IN_TRANSITION. Moreover, another thread
• could be simultaneously wiring this new mapping
• entry. Detect these cases and skip any entries
• marked as in transition by us.

		 */
		if ((entry->eflags & MAP_ENTRY_IN_TRANSITION) == 0 ||
		    entry->wiring_thread != curthread) {
			KASSERT((flags & VM_MAP_WIRE_HOLESOK) != 0,
			    ("vm_map_unwire: !HOLESOK and new/changed entry"));
			continue;
		}

(I think there should be a 'not' in the last line of that comment.) So we don't simplify something that somebody else is supposed to simplify, as I understand it, but they will before we could simplify it in vm_map_clip_start.

in vm_map_wire_locked, things look the same as in vm_map_unwire.