Note that for devfs vnodes the vnode lock is often not held when writes are started. This is esp. true when metadata io is initiated by UFS.

Style.

Please fix two style issues, no need to re-post the updated patch.

In D21060#463035, @jhb wrote:

FWIW, gcc4.2.1 does get installed as /usr/bin/gcc on platforms that use it as /usr/bin/cc, and the base/gcc port that installs gcc6 as /usr/bin/cc also installs it as /usr/bin/gcc. I think gcc may be slightly different though in that autoconf scripts tend to look specifically for gcc sometimes instead of cc. I haven't run into similar configure scripts that look specifically for 'clang'. In my experience they look for 'cc' which happens to be clang.

I suspect the diff generation missed vm_phys.c.

I noted on IRC that roundup2 on line 1058 might result in overflow as well, at least theoretically.

In D21060#462930, @imp wrote:

I'd personally rather see us bring back something from the past . We had WITH_CLANG_AS_CC which controlled creating cc->clang links. What if we had WITH_CLANG_LINKS or similar that would create clang->cc links and have it default to off? This isn't blocking if there's no support for it, though. I didn't click request changes for this :).

See below.

In D21272#462891, @ray wrote:

In D21272#462874, @kib wrote:

In D21272#462601, @markj wrote:

Can't we simply test paddr < rounddown2(seg->end, VM_LEVEL_0_SIZE)?

Is seg->end 0xffffffff or 0 for the problematic case ?

No, it is 0xfff00000. But problem with paddr, which is 32bits and paddr + 0x100000 will be 0x00000000.
It satisfy check:
0xffe00000 <= 0xfff00000 - true
0xfff00000 <= 0xfff00000 - true
0x00000000 <= 0xfff00000 - true
BTW, I set MTX_NEW to mtx_init for test, and it just go around.

Anyway, I prefer the explicit check for overflow, it makes the things absolutely clear for reader.
uint64_t cast is no-go of course.

cast to uint64_t will do nothing on 64bits machines and will have impact on 32bits machines only.
cast to intmax_t will add "double-math" for all, but save code from modifications when 64->128 era begin :)
and since it is affect only boot process, think it acceptable.

vM is coded using (somewhat) abstracted types. We do not assume that everything is ILP32 or LP64, at least not in places where it is easy to avoid and which would create one more portability bug for future system porting.

In D21272#462601, @markj wrote:

Can't we simply test paddr < rounddown2(seg->end, VM_LEVEL_0_SIZE)?

Is seg->end 0xffffffff or 0 for the problematic case ? Anyway, I prefer the explicit check for overflow, it makes the things absolutely clear for reader.

Peter, could you please, test the patch ? The only interesting scenarios are OOMs. It should allow the system to unwind from low memory situations much quicker, or sometimes it should make out of problems previously causing hang.

Add sysctls descriptions, slightly edited from the wording provided by Mark.

In D13671#462633, @markj wrote:

I think this is ok. Just some thoughts below.
In head, if a process sleeps for a long time in vm_waitpfault(), the page daemon can deactivate and reclaim most of the process' mapped pages. If it reclaims enough pages to wake up the process, the process will run and immediately fault a number of times, adding pressure to the pool of free pages. So I can see why the OOM killer would fail to make progress: with many large processes we will just reclaim pages from one process to satisfy PF allocations for another, and this can go on indefinitely.
With your patch this is less likely to happen since a starved process can eventually trigger OOM kills, but in principle it can still be a problem if the page daemon reclaims user-mapped pages quickly enough. We can tune the sysctls you added, but I wonder if there is a way to recognize this pattern in general.

In D21060#462643, @bdrewery wrote:

Which package provides clang? I don't have a /usr/local/bin/clang on my system.

(assuming Mark and John notes are handled)

In D21272#462558, @ray wrote:

In D21272#462554, @markj wrote:

In D21272#462545, @ray wrote:

In D21272#462526, @markj wrote:

Don't we then have the same problem if seg->end < VM_LEVEL_0_SIZE?

only if you want to add some really small seg at 0x00000000 :)

Why does it have to be at 0x00000000? With the segment below we will test 0x100000 <= 0x9d000 - 0x100000, and the variables are unsigned.

SEGMENT 0:  
start:     0x10000                 
end:       0x9d000
domain:    0                       
free list: 0xffffffff81233cb0

no, start and end will be truncated and rounded up.
So you will get segment 0x00000000 with size 0x00100000.
First iteration will look that:
0x000000 <= 0x100000 - 0x100000, which is true and you will initialize one super page.

In D21206#462415, @mjg wrote:

I had an extra look and there is an extremely worrisome comment: apparently the routine is expected to by async-signal safe. This means both malloc and realloc are off the table. This can be worked around with having a statically sized buffer and requesting the bitmap in chunks which fit the buffer, i.e. pass the buffer and an offset (size of the buffer * iteration counter). See https://gitlab.gnome.org/GNOME/glib/blob/master/glib/gspawn.c#L1182

Eliminate vm_oom_ratelim_count.

Update the patch to the latest merge, trying to revive the review process.

Remove confusing comments in linuxolator that mention COMPAT_43.

In D21200#460959, @imp wrote:

In D21200#460958, @jhb wrote:

I think a related question on the mail thread was if COMPAT_43 should be in any non-x86 config files at all. E.g. it shouldn't be in sys/conf/notes but should move to sys/x86/conf/NOTES once we start that.

Let's see, these changes would mean COMPAT_43 is just a normal 'include old code' stuff.
sparc64 wouldn't need it, since it didn't do a.out But it's frankly irrelevant...
powerpc I'm unsure about, but I don't think it needs it, despite it being in several kernels
powerpc64 I'm pretty sure doesn't need it since it doesn't support a.out.
mips doesn't need it since binary compatibility has been busted several times and we've never supported a.out.
I'm pretty sure it's close to a no op on 32-bit arm. There's no really old binaries that matter: we broke compatibility with EABI changes, and then again with the move to v6/v7, so compat for anything before FreeBSD 9 (or was it 10) is a don't care.
arm64 doesn't need it if arm doesn't need it.
riscv doesn't need it since it's too new.
I'd be willing to do the legwork unless kib@ knows some reason you and I are overlooking.

In D21200#460954, @imp wrote:

I think these are good. There's comments in the Linux emulator that says they do different things because of this ifdef:
compat/linux/linux_misc.c: * td->td_retval[1] when COMPAT_43 is defined. This clobbers registers that
compat/linux/linux_uid16.c: * when COMPAT_43 is defined. This clobbers registers that are assumed to
maybe that means there can be a simplification?

Ultimately we want some other sysctl to get the list of filedescs, because kern.proc.filedesc performs a huge amount of the work that is discarded.

Add comment hinting why we do not stop iteration on asprintf (malloc) failure.

1. Can you rewrite the review summary according to the new code ? I want to see the commit message for the change.
2. Looking at proc_to_reap() and you initial note about kern_wait6(), I wonder should we check for p_procdesc for all idtype cases in proc_to_reap() ? In other words, isn't there somewhat larger problem with kern_wait6() reaping procdesc zombies at all ?

Add checks for ENOMEM.

Do not leak pwbuf.

Fix bugs pointed out by ian. I did not found other instances of it.