sendfile: don't panic when VOP_GETPAGES_ASYNC returns an error

[asomers]

Summary

sendfile: don't panic when VOP_GETPAGES_ASYNC returns an error

This is a partial merge of 350144 from projects/fuse2

Test Plan

Test added in the fuse2 branch

[cem]

sys/kern/kern_sendfile.c

445 ↗

(On Diff #60219)

This can be written like pa[i + j], or in the style of the other loops in this function, for (j = i; j < i + count; j++) { ... }.

I'm not sure if this handles bogus_page entries correctly.

450 ↗

(On Diff #60219)

style(9): return (EIO);

807 ↗

(On Diff #60219)

style(9): if (error != 0) {

[asomers]

style changes suggested by cem

[markj]

sys/kern/kern_sendfile.c

443 ↗

(On Diff #60222)

Why add a new probe instead of using vfs:vop:vop_getpages_async:return?

445 ↗

(On Diff #60219)

Right, we don't want to put bogus_page in the page queues.

[asomers]

Remove SDT probe and handle bogus_page

[markj]

sys/kern/kern_sendfile.c

805 ↗

(On Diff #60239)

sendfile_iodone() will free this structure, and that function is sometimes called in the error case, for example if the vfs_bio_getpages() call in ffs_getpages_async() fails. In that case this would be a double free.

[markj]

sys/kern/kern_sendfile.c

805 ↗

(On Diff #60239)

BTW, it looks like vfs_bio_getpages() is always synchronous. That seems like a bug.

[asomers]

sys/kern/kern_sendfile.c

443 ↗

(On Diff #60222)

I guess we could just rely on that.

[asomers]

sys/kern/kern_sendfile.c

805 ↗

(On Diff #60239)

In fact, sendfile_iodone doesn't free the structure because of the refcount on sfio->nios. I've confirmed it with dtrace; my fusefs read:sendfile_eio test does call sendfile_iodone (from vop_stdgetpages_async). In fact, *none* of the tests in lib/libc/sys/sendfile_test ever call sendfile_iodone or any of the pru_ready methods. This looks like a bug. I'm guessing that the second argument to refcount_init in vn_sendfile should be 0 instead of 1.

[asomers]

sys/kern/kern_sendfile.c

805 ↗

(On Diff #60239)

Oh, I understand now. The refcount_init is correct. What's missing is a corresponding refcount_release at line 1013. The sendfile_test never calls sendfile_iodone because the data for those tests is always cached. sendfile_iodone will only be called if data must be retrieved from disk.

My free(sfio, M_TEMP) is correct because it precedes a goto done;, which skips the terminal free at line 1013 or sendfile_iodone at line 1021.

[markj]

sys/kern/kern_sendfile.c

805 ↗

(On Diff #60239)

I see now, thanks.

We can move the free of sfio out from under the vnode lock.