Refactoring asynchronous I/O

[John Baldwin]

On Wednesday, February 10, 2016 03:19:12 PM John Baldwin wrote:
> On Wednesday, February 10, 2016 11:58:44 PM Jilles Tjoelker wrote:
> > On Mon, Feb 08, 2016 at 11:22:25AM -0800, John Baldwin wrote:
> > > On Friday, February 05, 2016 10:32:12 PM Jilles Tjoelker wrote:
> > > > On Fri, Feb 05, 2016 at 12:21:52PM -0800, John Baldwin wrote:
> > > > > On Monday, February 01, 2016 12:02:14 AM Jilles Tjoelker wrote:
> > > > > > On Tue, Jan 26, 2016 at 05:39:03PM -0800, John Baldwin wrote:
> > > > > > > Note that binding the AIO support to a new fileop does mean that
> > > > > > > the AIO code now becomes mandatory (rather than optional).  We
> > > > > > > could perhaps make the system calls continue to be optional if
> > > > > > > people really need that, but the guts of the code will now need to
> > > > > > > always be in the kernel.
> > 
> > > > > > Enabling this by default is OK with me as long as the easy ways to get a
> > > > > > stuck process are at least disabled by default. Currently, a process
> > > > > > gets stuck forever if it has an AIO request from or to a pipe that will
> > > > > > never complete. An AIO daemon should not be allowed to perform an
> > > > > > unbounded sleep such as for a pipe (NFS server should be OK).
> > 
> > > > > One thing I could do is split vfs_aio.c into two files: kern_aio.c that
> > > > > holds the "library" such as aio_aqueue() / aio_complete(), etc. and a
> > > > > sys_aio.c that is just the system calls.  kern_aio.c would be standard,
> > > > > but sys_aio.c could still be optional and aio.ko would contain it.
> > > > > This would still make AIO optional, though aio.ko would be fairly small,
> > > > > so not having it probably wouldn't save much in terms of size.
> > 
> > > > > Does this seem reasonable or is a trivial aio.ko not worth it?
> > 
> > > > It is one possible option. Another option is to refuse AIO for kinds of
> > > > file that have not been vetted to avoid blocking an AIO daemon for too
> > > > long. "Kinds of file" is about the code that will be executed to do I/O,
> > > > so that, for example, /dev/klog and /dev/ttyv0 are different kinds.
> > > > Depending on whether this restriction breaks existing code, it may need
> > > > to be a sysctl.
> > 
> > > This doesn't seem to be very easy to implement, especially if it should
> > > vary by character device.  It sounds like what you would do today is
> > > to allow mlock() and AIO on sockets and maybe the fast physio path
> > > and disable everything else by default for now?  This would be slightly
> > > more feasible than something more fine-grained.  A sysctl would allow
> > > "full" AIO use that would disable these checks?
> > 
> > It does not seem that complicated to me. A check can be inserted before
> > placing a job into aio_jobs (in aio_queue_file() in the patched code).
> > If the code can detect some safe cases, they could be allowed;
> > otherwise, the AIO daemons will not be used. I'm assuming that the code
> > in the new fo_aio_queue file ops behaves properly and will not cause AIO
> > daemons to get stuck.
> > 
> > Note that this means there is no easy way to do kernel AIO on a kind of
> > file without specific support in the code for that kind of file. I think
> > the risk of a stuck process (that may even cause shutdown to hang
> > indefinitely) is bad enough to forbid it by default.
> 
> Ok.  One issue is that some software may assume that aio will "work" if
> modfind("aio") works and might be surprised by it not working.  I'm not sure
> how real that is.  I don't plan on merging this to 10 so we will have some
> testing time in 11 to figure that out.  If it does prove problematic we can
> revert to splitting the syscalls out into aio.ko.  For now I think the two
> cases to enable by default would be sockets (which use a fo_aio_queue method)
> and physio.  We could let the VFS_AIO option change the sysctl's default
> value to allow all AIO for compat, though that seems a bit clumsy as the
> name doesn't really make sense for that.
> 
> (I also still don't like the vfs_aio.c filename as aio is not VFS-specific.)

I've pushed a new version with a vfs.aio.enable_unsafe sysctl (defaults to
off).  If you think this is ok then I'll work on cleaning up some of the
module bits (removing the unload support mostly).  I figure marking the
syscalls as STD and removing all the helper stuff can be done as a followup
commit to reduce extra noise in this diff (which is large enough on its own).

https://github.com/freebsd/freebsd/compare/master...bsdjhb:aio_rework

(Also, for the inital commit I will leave out the socket protocol hook.  That
can be added later.)

-- 
John Baldwin