Date: Wed, 10 Feb 2016 15:19:12 -0800 From: John Baldwin <jhb@freebsd.org> To: Jilles Tjoelker <jilles@stack.nl> Cc: arch@freebsd.org Subject: Re: Refactoring asynchronous I/O Message-ID: <1897781.JF7dmmKAT1@ralph.baldwin.cx> In-Reply-To: <20160210225844.GA89743@stack.nl> References: <2793494.0Z1kBV82mT@ralph.baldwin.cx> <2544692.uejYAA75dx@ralph.baldwin.cx> <20160210225844.GA89743@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
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.)
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1897781.JF7dmmKAT1>