sendfile: stop abusing kern_writev()

[kib]

Summary

Provide convenient wrapper kern_filewrite() around fo_write().
Switch to use it in vn_sendfile().  This allows to avoid duplicate
fget() when we already have the reference to the file, which creates a
correctness race with the userspace.  Also td_retval[0] clearing hack
can be removed.

[markj]

sys/kern/kern_sendfile.c

1171

The second sentence is stale.

sys/kern/sys_generic.c

547

Rather than splitting this into two functions, can we just call it kern_write() and push the td_retval handling into sendfile/kern_writev()/kern_pwritev()?

In the existing scheme, the naming is a bit confusing, and it's not clear why kern_filewrite() is not responsible for raising SIGPIPE for non-sockets.

[kib]

sys/kern/sys_generic.c

547

Intent of kern_filewrite() is exactly what it's name says: it is a convenience wrapper around fo_write(), intended to be used for (rare) cases when kernel needs to write to file. I had an impression, from a talk with Gleb, that there is a need for such function for more wide use.

dofilewrite() is setting up the userspace context for the file write result. This includes both setting td_retval[0] and delivering SIGPIPE as needed.

IMO the split is reasonable. I am not quite sure why EPIPE requires handling at the socket layer instead of the syscall layer, but this is for the other day.

[kib]

Update the comment.

[markj]

sys/kern/sys_generic.c

547

I am not quite sure why EPIPE requires handling at the socket layer instead of the syscall layer

Maybe it simplifies things for AIO, I'm not sure.

It's still a weird split: the helper handles ktrace but not SIGPIPE (except sometimes).

586

Probably you'll get a warning about unused fd if KTRACE is not defined.

[kib]

atomic_store_int(comm, -1);

sys/kern/sys_generic.c

586

I am not sure. it seems that our settings forgive the unused parameters.
As an example, the whole series of badfo_XXX() functions does not add __unused to obviously unused parameters.

[markj]

In D58035#1330852, @kib wrote:

atomic_store_int(comm, -1);

What is this from?

[kib]

In D58035#1331000, @markj wrote:

In D58035#1330852, @kib wrote:

atomic_store_int(comm, -1);

What is this from?

Apparently this is from the test program for pddupfd(2), pasted in error.