add support for vectored and async I/O to all simple filesystems
Subsystems (11)
PATCH 1/1 add support for vectored and async I/O to all simple filesystems
Every filesystem using generic_file_read/generic_file_write directly can easily support vectored and async (well at least the API, it's not async quite yet in mainline) I/O. This patch adds the proper vectors to all filesystems and switches them to do_sync_read/do_sync_write. This we we can get rid of plain generic_file_read/generic_file_write soon and remove one of the many variants of those functions. Long term I plan to unify aio and vectored support into a single operation so we can cut down filemap.c to a sane set of these routines.
drivers/char/raw.c +1 -1
@@ -262,7 +262,7 @@static struct file_operations raw_fops = { - .read = generic_file_read, + .read = do_sync_read, .aio_read = generic_file_aio_read, .write = raw_file_write, .aio_write = raw_file_aio_write,
fs/adfs/file.c +7 -2
@@ -31,11 +31,16 @@struct file_operations adfs_file_operations = { .llseek = generic_file_llseek, - .read = generic_file_read, + .read = do_sync_read, + .write = do_sync_write, + .readv = generic_file_readv, + .writev = generic_file_writev, + .aio_read = generic_file_aio_read, + .aio_write = generic_file_aio_write, .mmap = generic_file_mmap, .fsync = file_fsync, - .write = generic_file_write, .sendfile = generic_file_sendfile, + };struct inode_operations adfs_file_inode_operations = {
fs/bfs/file.c +6 -2
@@ -19,8 +19,12 @@struct file_operations bfs_file_operations = { .llseek = generic_file_llseek, - .read = generic_file_read, - .write = generic_file_write, + .read = do_sync_read, + .write = do_sync_write, + .readv = generic_file_readv, + .writev = generic_file_writev, + .aio_read = generic_file_aio_read, + .aio_write = generic_file_aio_write, .mmap = generic_file_mmap, .sendfile = generic_file_sendfile, };
fs/ext2/file.c +2 -2
@@ -41,8 +41,8 @@*/ struct file_operations ext2_file_operations = { .llseek = generic_file_llseek, - .read = generic_file_read, - .write = generic_file_write, + .read = do_sync_read, + .write = do_sync_write, .aio_read = generic_file_aio_read, .aio_write = generic_file_aio_write, .ioctl = ext2_ioctl,
fs/fuse/file.c +6 -2
@@ -523,8 +523,12 @@static struct file_operations fuse_file_operations = { .llseek = generic_file_llseek, - .read = generic_file_read, - .write = generic_file_write, + .read = do_sync_read, + .write = do_sync_write, + .readv = generic_file_readv, + .writev = generic_file_writev, + .aio_read = generic_file_aio_read, + .aio_write = generic_file_aio_write, .mmap = fuse_file_mmap, .open = fuse_open, .flush = fuse_flush,
fs/hfs/inode.c +6 -2
@@ -616,8 +616,12 @@static struct file_operations hfs_file_operations = { .llseek = generic_file_llseek, - .read = generic_file_read, - .write = generic_file_write, + .read = do_sync_read, + .write = do_sync_write, + .readv = generic_file_readv, + .writev = generic_file_writev, + .aio_read = generic_file_aio_read, + .aio_write = generic_file_aio_write, .mmap = generic_file_mmap, .sendfile = generic_file_sendfile, .fsync = file_fsync,
fs/hfsplus/inode.c +6 -2
@@ -302,8 +302,12 @@static struct file_operations hfsplus_file_operations = { .llseek = generic_file_llseek, - .read = generic_file_read, - .write = generic_file_write, + .read = do_sync_read, + .write = do_sync_write, + .readv = generic_file_readv, + .writev = generic_file_writev, + .aio_read = generic_file_aio_read, + .aio_write = generic_file_aio_write, .mmap = generic_file_mmap, .sendfile = generic_file_sendfile, .fsync = file_fsync,
fs/hostfs/hostfs_kern.c +2 -2
@@ -388,13 +388,13 @@static struct file_operations hostfs_file_fops = { .llseek = generic_file_llseek, - .read = generic_file_read, + .read = do_sync_read, .sendfile = generic_file_sendfile, .aio_read = generic_file_aio_read, .aio_write = generic_file_aio_write, .readv = generic_file_readv, .writev = generic_file_writev, - .write = generic_file_write, + .write = do_sync_write, .mmap = generic_file_mmap, .open = hostfs_file_open, .release =NULL ,
fs/jffs/inode-v23.c +6 -2
@@ -1633,8 +1633,12 @@ { .open = generic_file_open, .llseek = generic_file_llseek, - .read = generic_file_read, - .write = generic_file_write, + .read = do_sync_read, + .write = do_sync_write, + .readv = generic_file_readv, + .writev = generic_file_writev, + .aio_read = generic_file_aio_read, + .aio_write = generic_file_aio_write, .ioctl = jffs_ioctl, .mmap = generic_file_readonly_mmap, .fsync = jffs_fsync,
fs/jffs2/file.c +6 -2
@@ -42,8 +42,12 @@ { .llseek = generic_file_llseek, .open = generic_file_open, - .read = generic_file_read, - .write = generic_file_write, + .read = do_sync_read, + .write = do_sync_write, + .readv = generic_file_readv, + .writev = generic_file_writev, + .aio_read = generic_file_aio_read, + .aio_write = generic_file_aio_write, .ioctl = jffs2_ioctl, .mmap = generic_file_readonly_mmap, .fsync = jffs2_fsync,
fs/jfs/file.c +2 -2
@@ -103,8 +103,8 @@struct file_operations jfs_file_operations = { .open = jfs_open, .llseek = generic_file_llseek, - .write = generic_file_write, - .read = generic_file_read, + .write = do_sync_write, + .read = do_sync_read, .aio_read = generic_file_aio_read, .aio_write = generic_file_aio_write, .mmap = generic_file_mmap,
fs/minix/file.c +6 -2
@@ -17,8 +17,12 @@struct file_operations minix_file_operations = { .llseek = generic_file_llseek, - .read = generic_file_read, - .write = generic_file_write, + .read = do_sync_read, + .write = do_sync_write, + .readv = generic_file_readv, + .writev = generic_file_writev, + .aio_read = generic_file_aio_read, + .aio_write = generic_file_aio_write, .mmap = generic_file_mmap, .fsync = minix_sync_file, .sendfile = generic_file_sendfile,
fs/ramfs/inode.c +6 -2
@@ -149,8 +149,12 @@ };struct file_operations ramfs_file_operations = { - .read = generic_file_read, - .write = generic_file_write, + .read = do_sync_read, + .write = do_sync_write, + .readv = generic_file_readv, + .writev = generic_file_writev, + .aio_read = generic_file_aio_read, + .aio_write = generic_file_aio_write, .mmap = generic_file_mmap, .fsync = simple_sync_file, .sendfile = generic_file_sendfile,
fs/read_write.c +3 -1
@@ -20,7 +20,9 @@struct file_operations generic_ro_fops = { .llseek = generic_file_llseek, - .read = generic_file_read, + .read = do_sync_read, + .readv = generic_file_readv, + .aio_read = generic_file_aio_read, .mmap = generic_file_readonly_mmap, .sendfile = generic_file_sendfile, };
fs/sysv/file.c +6 -2
@@ -21,8 +21,12 @@*/ struct file_operations sysv_file_operations = { .llseek = generic_file_llseek, - .read = generic_file_read, - .write = generic_file_write, + .read = do_sync_read, + .write = do_sync_write, + .readv = generic_file_readv, + .writev = generic_file_writev, + .aio_read = generic_file_aio_read, + .aio_write = generic_file_aio_write, .mmap = generic_file_mmap, .fsync = sysv_sync_file, .sendfile = generic_file_sendfile,
fs/ufs/file.c +6 -2
@@ -43,8 +43,12 @@struct file_operations ufs_file_operations = { .llseek = generic_file_llseek, - .read = generic_file_read, - .write = generic_file_write, + .read = do_sync_read, + .write = do_sync_write, + .readv = generic_file_readv, + .writev = generic_file_writev, + .aio_read = generic_file_aio_read, + .aio_write = generic_file_aio_write, .mmap = generic_file_mmap, .open = generic_file_open, .sendfile = generic_file_sendfile,
fs/reiserfs/file.c +4 -8
@@ -1541,22 +1541,18 @@return res; } -static ssize_t reiserfs_aio_write (struct kiocb *iocb,const char __user * buf, -size_t count,loff_t pos) -{ -return generic_file_aio_write (iocb, buf, count, pos); -} -struct file_operations reiserfs_file_operations = { - .read = generic_file_read, + .read = do_sync_read, .write = reiserfs_file_write, + .readv = generic_file_readv, + .writev = generic_file_writev, .ioctl = reiserfs_ioctl, .mmap = generic_file_mmap, .release = reiserfs_file_release, .fsync = reiserfs_sync_file, .sendfile = generic_file_sendfile, .aio_read = generic_file_aio_read, - .aio_write = reiserfs_aio_write, + .aio_write = generic_file_aio_write, };struct inode_operations reiserfs_file_inode_operations = {
fs/block_dev.c +1 -1
@@ -796,7 +796,7 @@ .open = blkdev_open, .release = blkdev_close, .llseek = block_llseek, - .read = generic_file_read, + .read = do_sync_read, .write = blkdev_file_write, .aio_read = generic_file_aio_read, .aio_write = blkdev_file_aio_write,
fs/ntfs/file.c +1 -1
@@ -2305,7 +2305,7 @@struct file_operations ntfs_file_ops = { .llseek = generic_file_llseek,/* Seek inside file. */ - .read = generic_file_read,/* Read from file. */ + .read = do_sync_read,/* Read from file. */ .aio_read = generic_file_aio_read,/* Async read from file. */ .readv = generic_file_readv,/* Read from file. */ #ifdef NTFS_RW
fs/qnx4/file.c +6 -2
@@ -25,11 +25,15 @@struct file_operations qnx4_file_operations = { .llseek = generic_file_llseek, - .read = generic_file_read, + .read = do_sync_read, + .readv = generic_file_readv, + .aio_read = generic_file_aio_read, .mmap = generic_file_mmap, .sendfile = generic_file_sendfile, #ifdef CONFIG_QNX4FS_RW - .write = generic_file_write, + .write = do_sync_write, + .writev = generic_file_writev, + .aio_write = generic_file_aio_write, .fsync = qnx4_sync_file, #endif };
Miklos Szeredi
maintainerQuoted 4 lines
Every filesystem using generic_file_read/generic_file_write directly can easily support vectored and async (well at least the API, it's not async quite yet in mainline) I/O. This patch adds the proper vectors to all filesystems and switches them to do_sync_read/do_sync_write.
Do you really need to replace generic_file_read/write with do_sync_read/write? That seems to only add overhead.
The rest of the patch I agree with.
Miklos
Quoted 7 lines · Miklos Szeredi
Quoted 4 lines
Every filesystem using generic_file_read/generic_file_write directly can easily support vectored and async (well at least the API, it's not async quite yet in mainline) I/O. This patch adds the proper vectors to all filesystems and switches them to do_sync_read/do_sync_write.
Do you really need to replace generic_file_read/write with do_sync_read/write? That seems to only add overhead.
do_sync_read/write are doing the same thing as generic_file_read/write, just above the method vectors. Once I have all filesystems support proper vectored and aio methods do_sync_read/write will go away completly and upper code will always call the complex methods ( or hopefull just one cmobined aio/vectored ops with all fancies) directly.
Quoted 4 lines
The rest of the patch I agree with.
Miklos
---end quoted text---
Miklos Szeredi
maintainerQuoted 9 lines · Miklos Szeredi
Quoted 7 lines
Quoted 4 lines
Every filesystem using generic_file_read/generic_file_write directly can easily support vectored and async (well at least the API, it's not async quite yet in mainline) I/O. This patch adds the proper vectors to all filesystems and switches them to do_sync_read/do_sync_write.
Do you really need to replace generic_file_read/write with do_sync_read/write? That seems to only add overhead.
Quoted 6 lines
do_sync_read/write are doing the same thing as generic_file_read/write, just above the method vectors. Once I have all filesystems support proper vectored and aio methods do_sync_read/write will go away completly and upper code will always call the complex methods ( or hopefull just one cmobined aio/vectored ops with all fancies) directly.
You mean f_op->read, f_op->write going away completely? That's quite a bit of work yet.
Until then you are throwing away some performance in the hottest read/write paths. Is it worth it? I'm not sure. Did you benchmark the change?
Miklos
Quoted 3 lines · Christoph Hellwig
Every filesystem using generic_file_read/generic_file_write directly can easily support vectored and async (well at least the API, it's not async quite yet in mainline) I/O.
Does this change mean aio system calls will now succeed, but not actually be asynchronous?
-- Jamie
Quoted 7 lines · Jamie Lokier
Quoted 3 lines · Christoph Hellwig
Every filesystem using generic_file_read/generic_file_write directly can easily support vectored and async (well at least the API, it's not async quite yet in mainline) I/O.
Does this change mean aio system calls will now succeed, but not actually be asynchronous?
Yes, there doesn't seem to be the will to merge the buffered filesystem aio patches.
-ben
Quoted 11 lines · Benjamin LaHaise
Quoted 7 lines · Jamie Lokier
Quoted 3 lines · Christoph Hellwig
Every filesystem using generic_file_read/generic_file_write directly can easily support vectored and async (well at least the API, it's not async quite yet in mainline) I/O.
Does this change mean aio system calls will now succeed, but not actually be asynchronous?
Yes, there doesn't seem to be the will to merge the buffered filesystem aio patches.
So it means that any program that mustn't block, must now have a stupid kernel version check to make sure it avoids even trying aio system calls? I was under the impression that the right thing to do so far was try them, and when EINVAL is returned, use threads instead.
-- Jamie
Quoted 4 lines · Jamie Lokier
So it means that any program that mustn't block, must now have a stupid kernel version check to make sure it avoids even trying aio system calls? I was under the impression that the right thing to do so far was try them, and when EINVAL is returned, use threads instead.
Yes, that is correct.
-ben
Signature · 1 line
"Time is what keeps everything from happening all at once." -- John Wheeler
Quoted 7 lines · Benjamin LaHaise
Quoted 4 lines · Jamie Lokier
So it means that any program that mustn't block, must now have a stupid kernel version check to make sure it avoids even trying aio system calls? I was under the impression that the right thing to do so far was try them, and when EINVAL is returned, use threads instead.
Yes, that is correct.
To be fair, the aio system calls were never _guaranteed_ to not block, were they? ISTR there were various corner cases that would still get your task blocking while doing an aio submission.
Quoted 3 lines · Matthew Wilcox
To be fair, the aio system calls were never _guaranteed_ to not block, were they? ISTR there were various corner cases that would still get your task blocking while doing an aio submission.
They're suppose to not block except for memory allocation, that is the definition of how the aio api is supposed to work. The patches to fix that always seem to get met with a "woe! complexity!" response. I'm of the opinion now that the only way to get the aio api to a usable state of implementation is to use threads so that the impact on the rest of the kernel isn't there for anything other than fast paths.
-ben
Signature · 1 line
"Time is what keeps everything from happening all at once." -- John Wheeler
Quoted 7 lines · Matthew Wilcox
Quoted 6 lines · Benjamin LaHaise
Quoted 4 lines · Jamie Lokier
So it means that any program that mustn't block, must now have a stupid kernel version check to make sure it avoids even trying aio system calls? I was under the impression that the right thing to do so far was try them, and when EINVAL is returned, use threads instead.
Quoted 5 lines
Quoted 1 line
Yes, that is correct.
To be fair, the aio system calls were never _guaranteed_ to not block, were they? ISTR there were various corner cases that would still get your task blocking while doing an aio submission.
Could we have some documentation of when those corner cases occur?
The main point of aio, as far as I'm aware, is to avoid the need for threads (or reduce the number of threads) in programs using I/O that shouldn't block, particularly when they are latency sensitive too.
If aio has a habit of blocking from time to time, then it may still be useful, but it would be helpful to know that multiple threads are still needed to ensure a program (e.g. such as a HTTP or SMB server) can continue to make progress - and more helpful to know when.
One particular question is: can aio calls block for a long time due to network delays (e.g. over NFS) and I/O delays (e.g. slow disk or CD), or are the corner cases restricted to things like paging during memory allocation, which is unavoidable one way or another anyway?
-- Jamie
Quoted 25 lines · Jamie Lokier
Quoted 7 lines · Matthew Wilcox
Quoted 6 lines · Benjamin LaHaise
Quoted 4 lines · Jamie Lokier
So it means that any program that mustn't block, must now have a stupid kernel version check to make sure it avoids even trying aio system calls? I was under the impression that the right thing to do so far was try them, and when EINVAL is returned, use threads instead.
Quoted 5 lines
Quoted 1 line
Yes, that is correct.
To be fair, the aio system calls were never _guaranteed_ to not block, were they? ISTR there were various corner cases that would still get your task blocking while doing an aio submission.
Could we have some documentation of when those corner cases occur?
The main point of aio, as far as I'm aware, is to avoid the need for threads (or reduce the number of threads) in programs using I/O that shouldn't block, particularly when they are latency sensitive too.
If aio has a habit of blocking from time to time, then it may still be useful, but it would be helpful to know that multiple threads are still needed to ensure a program (e.g. such as a HTTP or SMB server) can continue to make progress - and more helpful to know when.
Quoted 4 lines
One particular question is: can aio calls block for a long time due to network delays (e.g. over NFS) and I/O delays (e.g. slow disk or CD), or are the corner cases restricted to things like paging during memory allocation, which is unavoidable one way or another anyway?
Yes, of course aio can block and in fact will block arbitrarily for arbitrary lengths of time. At least at present the implementations of ->aio_read and ->aio_write in the file systems will block left right and center.
For a start, i_sem is downed which can block.
Then when we get inside readpage or the relevant file write function, buffers may be allocated for the current page which can block.
Then the filesystem needs to map the buffers if they are not mapped already and it is possible the filesystem needs to obtain other locks (again can block here) and even worse the filesystem may need to read data from disk to determine where mapping information for the buffers. This obviously is a slow and blocking operation unless your device is a ram disk.
And in the write case the filesystem may need to allocate blocks on disk first, which in turn will involve taking locks (and possibly blocking) in addition to reading/writing metadata to find free blocks that can be allocated and marking them as allocated. And that of course can involve on-disk access and hence again blocking.
I am not sure we need documentation for all that. It is kind of obvious once you sit and think about what a read and a write actually implies.
The only way you can _really_ have guaranteed async io is to queue the io to a kernel thread work queue and return immediately to the caller. The only thing you will then block on potentially is allocating memory for the "queue entry item" and on waiting for the lock to the "queue" so it is safe to write to it.
And if you do that, it then becomes easy to be truly non-blocking. Just allocate with GFP_ATOMIC (and perhaps add __GFP_NORETRY and __GFP_NORECLAIM?) and do a try lock for the queue lock. And if either of those fails, punt the reqest and return immediately to the user with error -EWOULDBLOCK or whatever...
You could even optimise away the queue lock by using an atomic compare and exchange based queue addition function but that may not be worth the extra complexity, don't know. I guess the big smp folks may see contention on that lock... You could at least do the queues and hence their locks per superblock or something...
Best regards,
Anton
Signature · 4 lines
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @) Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/
Quoted 6 lines · Anton Altaparmakov
Yes, of course aio can block and in fact will block arbitrarily for arbitrary lengths of time. At least at present the implementations of ->aio_read and ->aio_write in the file systems will block left right and center.
[examples...]
That's a shame. I was hoping it would offer similar properties to non-blocking I/O on sockets: something which can indicate that the resource is unavailable, but allows the application to continue with other things without needing parallel threads for that.
Quoted 5 lines
The only way you can _really_ have guaranteed async io is to queue the io to a kernel thread work queue and return immediately to the caller. The only thing you will then block on potentially is allocating memory for the "queue entry item" and on waiting for the lock to the "queue" so it is safe to write to it.
Since threads with well-defined blocking points can be mechanically transformed to state machines, it is possible to guarantee async I/O without needing extra threads. I had wondered if the Linux AIO implementation did something like that.
However, it is too complex to convert all filesystem code from blocking to state machines (by hand), and unnecessary. I've thought about possible AIO implementations many times, and always return to the idea of having AIOs be state machines which are handled synchronously and by interrupts, up until a point where a complex blocking point is reached in a filesystem (which shouldn't happen for the common read/write cases, but may happen for the uncommon ones), and then the AIOs convert to work handled by a worker thread spawned from a pool on demand.
In other words, there are plenty of ways to guarantee properly async I/O. I'm surprised Linux AIO isn't - that seems to defeat the whole point of AIO.
-- Jamie
Quoted 7 lines · Jamie Lokier
Quoted 3 lines · Christoph Hellwig
Every filesystem using generic_file_read/generic_file_write directly can easily support vectored and async (well at least the API, it's not async quite yet in mainline) I/O.
Does this change mean aio system calls will now succeed, but not actually be asynchronous?
for buffered I/O: yes. meaning the behaviour on all these obscure filesystems is the same as on all the ones everyone uses. Whether that hehaviour is good or not is a different question and can easily tweaked in a single file (fs/aio.c) for all filesystems now, whereas previously we wetter utterly inconsistent.
Quoted 9 lines · Miklos Szeredi
Quoted 6 lines
do_sync_read/write are doing the same thing as generic_file_read/write, just above the method vectors. Once I have all filesystems support proper vectored and aio methods do_sync_read/write will go away completly and upper code will always call the complex methods ( or hopefull just one cmobined aio/vectored ops with all fancies) directly.
You mean
f_op->read,f_op->writegoing away completely? That's quite a bit of work yet.
No, they will have to stay at least until character drivers get their own set of operation vectors. I just want to make them optional and not need to implement them in all (or at least most) of the filesystems.
Quoted 3 lines
Until then you are throwing away some performance in the hottest read/write paths. Is it worth it? I'm not sure. Did you benchmark the change?
there's not difference in dbench and tiobench runs on ppc64. the only majuor architecture that could support is ia64 because of it's horribly weak branch prediction for indirect calls.