Add aio_writev and aio_readv

[asomers]

Summary

POSIX AIO is great, but it lacks vectored I/O functions. This commit fixes
that shortcoming by adding aio_writev and aio_readv. They aren't part of
the standard, but they're an obvious extension. They work just like their
synchronous equivalents writev and readv.

It isn't yet possible to use vectored aiocbs with lio_listio, but that could
be added in the future.

Test Plan

ATF test cases added

[asomers]

sys/kern/vfs_aio.c

1203

I'm not sure about these SDT probes. They were useful for me, but they might not be useful for anybody else. Think I should delete them?

sys/sys/aio.h

100

AFAICT, the only reason why the standard mandates volatile here is so the user can manipulate the aiocb's data while it's being processed by the kernel or vice versa. But that's dumb. I don't think it's necessary, so I didn't include it on aio_iov.

104

Should I add some kind of padding field for LP64 arches?

141

It turns out that the bp, pbuf, and pages fields were redundant. pbuf and pages were already stored in the bio, and bp is available during aio_biowakeup. So I could just delete those fields.

[tmunro]

Great feature. +1 for allowing it in lio_listio() for batched submissions in later work (I'd also like to see LIO_FSYNC allowed in there too). I wonder if it is OK to use C11 anon union syntax in a system header; I see that sys/ucred.h does that.

[bcr]

lib/libc/sys/aio_write.2

88

No need for an extra line break here, continue to write after "system call".

tests/sys/aio/aio_test.c

1369

Is that s/DOS/DoS/ here?

[asomers]

Formatting changes suggested by bcr

[bcr]

The man page is OK now, thanks.
Remember to bump the .Dd for this content change when you do the commit.

[jhb]

I didn't look at the tests in detail.

The one larger suggestion I have is that I would not deal with vectored vs non-vectored in all the backends, instead I would do what the kernel does for regular read/write which is to always allocate a uio (it can live in struct kaiocb, and perhaps struct kaiocb should also contain a single struct iovec to minimize allocations for the non-vectored case) and have the I/O always be vectored in the backends. This would reduce some duplicated code to handle this in the socket and process_rw cases. I can take care of updating the Chelsio bits to cope (I think those are the only aio handlers you haven't already touched).

lib/libc/sys/aio_read.2

232

Maybe Xr readv(2) here for the description of iovec?

lib/libc/sys/aio_write.2

69

I think you can Xr writev(2) here for the description of struct iovec instead of duplicating it. Regardless of which approach, aio_readv(2) and aio_writev(2) should be consistent.

80

Maybe "write operations append to the file..."?

sys/kern/vfs_aio.c

818

Style: buddy the { up to the previous line.

1203

I'm not sure about these SDT probes. They were useful for me, but they might not be useful for anybody else. Think I should delete them?

I think they are fine, but perhaps a blank line before the static int?

2441

We don't use // comments in the kernel generally, and add a blank line before comments.

tests/sys/aio/aio_test.c

1331

Style: // comments vs /*.

[asomers]

• Fix some whitespace in aio.h
• Fix a memory leak in an error path

Respond to jhb's comments:

• style
• Create the struct uio in aio_aqueue, instead of in each separate backend.

Note that the Chelsio implementation for sockets will probably still work (I
haven't tested it), but it isn't taking advantage of the already-created uio).

[asomers]

Delete some errant commented code

[kib]

lib/libc/sys/Symbol.map

419

Should be alphabetically ordered.

sys/kern/vfs_aio.c

797

I believe the continuation line is wrongly indented.

813

Fix style while there:
error != 0
remove unneded ()

817

Extra () again.

1224

You can fill lines much more compact.

1239

style: != 0

1270

This check together with the last arg of vm_fault_quick_hold_pages() still left as atop(maxphys) + 1 makes me somewhat worried. I think that it would be better/safer to track number of slots in pages array and pass the right number to fault_quick_hold.

1304

void * casts are not needed.

1584

Style: we split lines after binary op, not before.

sys/sys/aio.h

141

I think this change should be extracted and reviewed/committed alone in advance.

223

Should be under BSD_VISIBLE guard, the functions are not POSIX.

[asomers]

Respond to kib's comments:

• style
• Guard the aio_writev and aio_readv declarations by BSD_VISIBLE

sys/kern/vfs_aio.c

1270

But the final argument to vm_fault_quick_hold_pages doesn't really do anything. It's just there for a sanity check. Given that, I think that passing a constant makes sense.

sys/sys/aio.h

141

Ok, I'll open another review for that.

[asomers]

Replaced by D27743