In D60281#1386059, @glebius wrote:

Wait... Maybe we should understand why userland_sysctl() does this and is it needed at all today? It originates from 7a69d9230f834f562d2c5c36c837e67eacdf5ab3. @phk maybe you can comment on this one?

Also in af80b2c90182cbbc0580d237bf3049090d511897 @kib definitely also answered the question "why?" to himself and maybe Kostik can explain.

Besides the formatting bug, I think that the algorithmic change is fine.

Calculate mp by VOP_GETWRITEMOUNT()
Do not leak VI_FOPENING

This is a forward compatibility, which we do not support. You must run the kernel which is newer than the userspace, and there were a lot of time between PD_NOWAITPID introduction and the first use.
But I do not intend to block this.

Rename flags.

Apply even more atomicity to loads.
Drop the mp != NULL check.

Same request to run the full stress2 with Peter' help.

Please ask Peter to do the full stress2 run

Add assert in addition to stating the locking mode for mnt_susp_owner

Elaborate more about locking.

In D58851#1384275, @rmacklem wrote:

I do have a question w.r.t. read on the NFS server.
I am guessing that the NFS code needs to VOP_GETATTR()
and use va_size to determine where the file ends in the last
page.
Is that correct?

Add comment.

Ensure that mnt_susp_owner is not NULL.

In D60283#1383798, @rick_sloservers.com wrote:

I have a question about vn_open_nosuspend.

It reads mnt_kern_flag and mnt_susp_owner without MNT_ILOCK, is it possible for a core dump to start and see (MNTK_SUSPEND | MNTK_SUSPENDED) before mnt_susp_owner is set (a possible but unlikely NULL ptr dereference here)?

If a core dump and a fs snapshot start at the same time, for example?

There is also D60285 somewhat related to this.

Use dvp for nosuspend check.

Push suspend check into vn_open_cred().

There is a global issue with the restart apparently, and this change only highlights it. If the ABI pass failed, ndp is reinitialized with namei_getpath() which re-reads the userspace path. In fact it is more, the same re-reading might happen due to the cached lookup. I think eventually we should fix it.

I think this is fine, I only have two relatively formal notes about the code structure.

In D60280#1383380, @kib wrote:

We can compare the old and new uio_resid then.

We can compare the old and new uio_resid then.

I generally do not like this version, it adds a lot of complexity for something that is hard to test.

In D60255#1382883, @imp wrote:

So is there some reason this can't be just

/* This file is in the public domain */
#pragma once
#include <sys/_floatingpoint.h>

where sys/_floatingpoint.h is

/* This file is in the public domain */
#pragma once
#include <sys/cdefs.h>
#include <machine/ieeefp.h>

I think this is fine. I suggest asking Peter to do the full stress2 run with the patch applied.

Might be LINUXKPI_SOURCES?

In D60207#1381711, @emaste wrote:

In the end I'll do the s///g to whatever people think is best.

Unfortunately I don't have a great idea of a better name, it just looked a little odd. So I suspect you'll have to pick something and nobody will object. If you want to leave it as is that's fine with me too. It looks a bit strange but it's obvious in context that we're not literally adding "YES" to CFLAGS.

I was going to say that e.g. LINUXKPI_CFLAGS=YES would be my suggestion before discovering that most of the literal CFLAGS additions use that form already. I do like that it's in a LINUXKPI namespace though.

Change -1 to FCHALTROOT_DISABLE
Check VACCESS(EXEC) for altroot
Man page editings.

In D60167#1380633, @olce wrote:

I'm wondering whether to put #include <sys/cdefs.h> in all headers, and add the missing includes to make all these headers standalone.