Page MenuHomeFreeBSD
Differential D7151

Add more documentation regarding unsafe AIO requests.
ClosedPublic
Actions

Authored by jhb on Jul 7 2016, 11:17 PM.
Tags
None
Referenced Files
F103294859: D7151.diff
Sat, Nov 23, 4:37 AM
Unknown Object (File)
Tue, Nov 19, 6:47 PM
Unknown Object (File)
Tue, Nov 12, 4:07 AM
Unknown Object (File)
Tue, Nov 12, 12:08 AM
Unknown Object (File)
Mon, Nov 11, 5:41 PM
Unknown Object (File)
Sun, Nov 10, 8:53 AM
Unknown Object (File)
Sat, Nov 9, 2:10 PM
Unknown Object (File)
Sat, Nov 2, 6:09 AM
View All Files
Subscribers
imp

Details

Reviewers
kib
emaste
jmg
wblock
Group Reviewers
manpages
Commits
rS303164: Add more documentation regarding unsafe AIO requests.
Summary

Add more documentation regarding unsafe AIO requests.

The asynchronous I/O changes made previously result in different
behavior out of the box. Previously all AIO requests failed with
ENOSYS / SIGSYS unless aio.ko was explicitly loaded. Now, some AIO
requests complete and others ("unsafe" requests) fail with EOPNOTSUPP.

Reword the introductory paragraph in aio(4) to add a general
description of AIO before describing the vfs.aio.enable_unsafe sysctl.

Remove the ENOSYS error description from aio_fsync(2), aio_read(2),
and aio_write(2) and replace it with a description of EOPNOTSUPP.

Remove the ENOSYS error description from aio_mlock(2).

Log a message to the system log the first time a process requests an
"unsafe" AIO request that fails with EOPNOTSUPP. This is modeled on
the log message used for processes using the legacy pty devices.

Test Plan
  • Hack one of the aio tests to not check the unsafe sysctl and verify it triggers the log message in /var/log/messages.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 4558
Build 4610: arc lint + arc unit

Event Timeline

jhb updated this revision to Diff 18211.Jul 7 2016, 11:17 PM
jhb retitled this revision from to Add more documentation regarding unsafe AIO requests..
jhb updated this object.
jhb edited the test plan for this revision. (Show Details)
jhb added reviewers: wblock, jmg.
Herald added a reviewer: manpages. · View Herald TranscriptJul 7 2016, 11:17 PM
Herald added a subscriber: imp. · View Herald Transcript
jhb added reviewers: emaste, kib.Jul 18 2016, 7:17 PM
kib added inline comments.Jul 19 2016, 5:46 AM
lib/libc/sys/aio_write.2
135

s/unsafe/aio-unsafe/

(and perhaps directly reference .Xr aio 4).

It is actually not the filedescriptor which is unsafe, but our aio implementation.

share/man/man4/aio.4
55

May be, add a term there, 'aio-unsafe' filedescriptor, and provide the definition.

sys/kern/vfs_aio.c
1699

Should this be atomic_cmpxchg_int() loop ? On somebody' bad day this would race and underflow.

jhb marked 2 inline comments as done.Jul 20 2016, 6:37 PM
jhb added inline comments.
lib/libc/sys/aio_write.2
135

Yes, the language is odd. I've reworded these to say that the specific operation is unsafe.

share/man/man4/aio.4
55

I've explicitly defined the operations that can block a daemon as unsafe which I think achieves this.

sys/kern/vfs_aio.c
1699

We don't use atomics in other places (the code I copied this from in particular). The user could fix it by writing to the sysctl at least.

I think if we had some sort of 'atomic_dec_nonneg' or something that returned the new value then I would be fine using that here (and fixing the other instance in the pty code).

jhb updated this revision to Diff 18608.Jul 20 2016, 6:46 PM
jhb marked 2 inline comments as done.
  • Define "unsafe" operations and treat those as unsafe rather than fd's.
Herald edited edge metadata. · View Herald TranscriptJul 20 2016, 6:46 PM
jhb updated this revision to Diff 18614.Jul 20 2016, 10:17 PM
jhb edited edge metadata.

Rebase.

Herald edited edge metadata. · View Herald TranscriptJul 20 2016, 10:17 PM
kib accepted this revision.Jul 21 2016, 7:02 AM
kib edited edge metadata.

I looked at the unsafe stuff once more, and I think that it is too restrictive. My main point is that typical io operation on the VREG/VDIR file is almost always safe. The only exception I can think about is the NFS vnode on wedged server, and even this probably matter only for intr mounts.

Really aio-unsafe files are fifos/pipes, after sockets were fixed by your work.

sys/kern/vfs_aio.c
1699

We do not need a new atomic op there, some routine similar to pps_ratecheck() would be enough. I will code it up.

This revision is now accepted and ready to land.Jul 21 2016, 7:02 AM
kib added inline comments.Jul 21 2016, 10:16 AM
sys/kern/vfs_aio.c
1699

See D7270.

jhb added a comment.Jul 21 2016, 3:55 PM
In D7151#151193, @kib wrote:

I looked at the unsafe stuff once more, and I think that it is too restrictive. My main point is that typical io operation on the VREG/VDIR file is almost always safe. The only exception I can think about is the NFS vnode on wedged server, and even this probably matter only for intr mounts.

Really aio-unsafe files are fifos/pipes, after sockets were fixed by your work.

I erred on the side of caution in terms of only declaring things I know are safe. I think a patch to adjust the AIO code to not treat local filesystems as unsafe (assuming they really are) would be great, but also orthogonal to this change.

jhb updated this revision to Diff 18645.Jul 21 2016, 10:46 PM
jhb edited edge metadata.

Rebase and use counted_warning().

This revision now requires review to proceed.Jul 21 2016, 10:46 PM
Herald edited edge metadata. · View Herald TranscriptJul 21 2016, 10:46 PM
Closed by commit rS303164: Add more documentation regarding unsafe AIO requests. (authored by jhb). · Explain WhyJul 21 2016, 10:50 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
lib/
libc/
sys/
8 lines
4 lines
8 lines
8 lines
share/
man/
man4/
20 lines
sys/
kern/
11 lines

Diff 18645

View Options

lib/libc/sys/aio_fsync.2

Loading...
View Options

lib/libc/sys/aio_mlock.2

Loading...
View Options

lib/libc/sys/aio_read.2

Loading...
View Options

lib/libc/sys/aio_write.2

Loading...
View Options

share/man/man4/aio.4

Loading...
View Options

sys/kern/vfs_aio.c

Loading...