O_DIRECT reads and writes without i_sem
Subsystems (4)
RFC PATCH 1/1 O_DIRECT reads and writes without i_sem
Hello everyone,
Right now, O_DIRECT reads and writes on regular files have to take i_sem while reading file metadata in order to make sure we don't race with hole filling.
This patch tries to get around that by avoiding i_sem when we are doing an O_DIRECT read or write inside of i_size. Yet another rw semaphore is added to struct inode to protect against holes being filled during the O_DIRECT. direct-io.c gets another special case to be aware of the locking.
This has only been lightly tested, I'm posting here for general comments before I go much further. I'm rounding up some hardware with enough disks to benchmark it properly.
Only ext2 and reiserfs are modified to drop i_sem during O_DIRECT. ext3 needs some care around the orphan lists, and I didn't want to get into that until the rest of the patch was working.
-chris
[patch against 2.6.10-rc1-mm2]
fs/direct-io.c +10 -2
@@ -915,7 +915,7 @@ out: }/* -* Releases both i_sem and i_alloc_sem +* Releases i_sem, i_alloc_sem and i_hole_sem */ static ssize_t direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode,
@@ -1065,6 +1065,8 @@ direct_io_worker(int rw, struct kiocb *i*/ if ((rw ==READ ) && (dio->lock_type ==DIO_LOCKING ))up (&dio->inode->i_sem); +if ((dio->lock_type ==DIO_HOLE_LOCKING )) +up_read (&dio->inode->i_hole_sem);/* * OK, all BIOs are submitted, so we can decrement bio_count to truly
@@ -1199,6 +1201,9 @@ __blockdev_direct_IO(int rw, struct kioc* For regular files using DIO_OWN_LOCKING, * both readers and writers need to grab i_alloc_sem * neither readers nor writers hold i_sem on entry (nor exit) +* For regular files using DIO_OWN_LOCKING, +* same as DIO_OWN_LOCKING, but both readers/writers read lock +* i_hole_sem until after get_blocks has finished. */ dio->lock_type = dio_lock_type;if (dio_lock_type !=DIO_NO_LOCKING ) {
@@ -1214,11 +1219,14 @@ __blockdev_direct_IO(int rw, struct kioc goto out; }down_read (&inode->i_alloc_sem); -if (dio_lock_type ==DIO_OWN_LOCKING ) +if (dio_lock_type ==DIO_OWN_LOCKING || + dio_lock_type ==DIO_HOLE_LOCKING )up (&inode->i_sem); }else {down_read (&inode->i_alloc_sem); } +if (dio_lock_type ==DIO_HOLE_LOCKING ) +down_read (&inode->i_hole_sem); }/* * For file extending writes updating i_size before data
fs/ext2/inode.c +21 -3
@@ -31,6 +31,7 @@#include <linux/writeback.h> #include <linux/buffer_head.h> #include <linux/mpage.h> +#include <linux/uio.h> #include "ext2.h" #include "acl.h"
@@ -649,9 +650,26 @@ ext2_direct_IO(int rw, struct kiocb *ioc {struct file *file = iocb->ki_filp;struct inode *inode = file->f_mapping->host; - -return blockdev_direct_IO (rw, iocb, inode, inode->i_sb->s_bdev, iov, - offset, nr_segs, ext2_get_blocks,NULL ); +ssize_t ret; +loff_t count =iov_length (iov, nr_segs); +int locking =DIO_HOLE_LOCKING ; + +/* +* reads always use DIO_HOLE_LOCKING. writes use DIO_HOLE_LOCKING +* unless they are extending the file +*/ +if (rw ==WRITE ) { +if (offset + count >i_size_read (inode)) + locking =DIO_LOCKING ; +else +up (&inode->i_sem); + } + ret =__blockdev_direct_IO (rw, iocb, inode, inode->i_sb->s_bdev, iov, + offset, nr_segs, ext2_get_blocks,NULL , + locking); +if (rw ==WRITE && locking ==DIO_HOLE_LOCKING ) +down (&inode->i_sem); +return ret; } static int
fs/inode.c +1 -0
@@ -197,6 +197,7 @@ void inode_init_once(struct inode *inodeINIT_LIST_HEAD (&inode->i_devices);sema_init (&inode->i_sem,1 );init_rwsem (&inode->i_alloc_sem); +init_rwsem (&inode->i_hole_sem);INIT_RADIX_TREE (&inode->i_data.page_tree,GFP_ATOMIC );rwlock_init (&inode->i_data.tree_lock);spin_lock_init (&inode->i_data.i_mmap_lock);
fs/reiserfs/file.c +2 -0
@@ -1249,6 +1249,7 @@ ssize_t reiserfs_file_write( struct fileif (res) goto out; +down_write (&inode->i_hole_sem);while ( count >0 ) {/* This is the main loop in which we running until some error occures or until we write all of the data. */
@@ -1351,6 +1352,7 @@ ssize_t reiserfs_file_write( struct file count -= write_bytes;balance_dirty_pages_ratelimited (inode->i_mapping); } +up_write (&inode->i_hole_sem);/* this is only true on error */ if (th.t_trans_id) {
fs/reiserfs/inode.c +21 -3
@@ -17,6 +17,7 @@#include <linux/mpage.h> #include <linux/writeback.h> #include <linux/quotaops.h> +#include <linux/uio.h> extern int reiserfs_default_io_size;/* default io size devuned in super.c */
@@ -2741,9 +2742,26 @@ static ssize_t reiserfs_direct_IO(int rw {struct file *file = iocb->ki_filp;struct inode *inode = file->f_mapping->host; - -return blockdev_direct_IO (rw, iocb, inode, inode->i_sb->s_bdev, iov, - offset, nr_segs, reiserfs_get_blocks_direct_io,NULL ); +ssize_t ret; +loff_t count =iov_length (iov, nr_segs); +int locking =DIO_HOLE_LOCKING ; + +/* +* reads always use DIO_HOLE_LOCKING. writes use DIO_HOLE_LOCKING +* unless they are extending the file +*/ +if (rw ==WRITE ) { +if (offset + count >i_size_read (inode)) + locking =DIO_LOCKING ; +else +up (&inode->i_sem); + } + ret =__blockdev_direct_IO (rw, iocb, inode, inode->i_sb->s_bdev, + iov, offset, nr_segs, + reiserfs_get_blocks_direct_io,NULL , locking); +if (rw ==WRITE && locking ==DIO_HOLE_LOCKING ) +down (&inode->i_sem); +return ret; }int reiserfs_setattr (struct dentry *dentry,struct iattr *attr) {
include/linux/fs.h +2 -0
@@ -455,6 +455,7 @@ struct inode {spinlock_t i_lock;/* i_blocks, i_bytes, maybe i_size */ struct semaphore i_sem;struct rw_semaphore i_alloc_sem; +struct rw_semaphore i_hole_sem;struct inode_operations *i_op;struct file_operations *i_fop;/* former ->i_op->default_file_ops */ struct super_block *i_sb;
@@ -1575,6 +1576,7 @@ enum {DIO_LOCKING =1 ,/* need locking between buffered and direct access */ DIO_NO_LOCKING ,/* bdev; no locking at all between buffered/direct */ DIO_OWN_LOCKING ,/* filesystem locks buffered and direct internally */ + DIO_HOLE_LOCKING, /* use i_hole_sem to lock against hole fillers */ }; static inline ssize_t blockdev_direct_IO(int rw, struct kiocb *iocb,
mm/filemap.c +3 -0
@@ -78,6 +78,7 @@* * ->i_sem * ->i_alloc_sem (various) +* ->i_hole_sem (various) * * ->inode_lock * ->sb_lock (fs/fs-writeback.c)
@@ -1927,6 +1928,7 @@ generic_file_buffered_write(struct kiocbpagevec_init (&lru_pvec,0 ); +down_write (&inode->i_hole_sem); buf = iov->iov_base + written;/* handle partial DIO write */ do { unsigned long index;
@@ -2000,6 +2002,7 @@ generic_file_buffered_write(struct kiocbcond_resched (); }while (count); *ppos = pos; +up_write (&inode->i_hole_sem);if (cached_page)page_cache_release (cached_page);
Quoted 12 lines · Chris Mason
Hello everyone,
Right now,
O_DIRECTreads and writes on regular files have to takei_semwhile reading file metadata in order to make sure we don't race with hole filling.This patch tries to get around that by avoiding
i_semwhen we are doing anO_DIRECTread or write inside ofi_size. Yet another rw semaphore is added to struct inode to protect against holes being filled during theO_DIRECT.direct-io.cgets another special case to be aware of the locking.
Quoted 7 lines
This has only been lightly tested, I'm posting here for general comments before I go much further. I'm rounding up some hardware with enough disks to benchmark it properly.
Only ext2 and reiserfs are modified to drop
i_semduringO_DIRECT. ext3 needs some care around the orphan lists, and I didn't want to get into that until the rest of the patch was working.
This gets too complicated for it's own sake. What about going down the XFS route and making i_sem a r/w semaphore that's taken only shared during read and write I/O, but exclusive while setting up write I/O outside of i_size? Alternatively just move the I/O locking into the filesytem.
Quoted 5 lines · Christoph Hellwig
Quoted 3 lines · Chris Mason
Right now,
O_DIRECTreads and writes on regular files have to takei_semwhile reading file metadata in order to make sure we don't race with hole filling.
[ ... ]
Quoted 6 lines
This gets too complicated for it's own sake. What about going down the XFS route and making
i_sema r/w semaphore that's taken only shared during read and write I/O, but exclusive while setting up write I/O outside ofi_size? Alternatively just move the I/O locking into the filesytem.
Nod, it is too complex, this is why I posted early ;) The only place in the FS specific code for the locking I added to fs/direct-io.c would be in each filesystem get_blocks call. I went for direct-io.c because that's where all the other locking already was. shrug.
If we do down_read(i_rw_sem) for all cases except growing the file, then we still have no locking for O_DIRECT while we are filling holes in the file. The filesystem write function could do this:
down_read(i_rw_sem)
read fs metadata
if (filling hole) {
up_read(i_rw_sem)
down_write(i_rw_sem)
goto retry
}But it also gets nasty in a hurry. The second alternative is just to make O_DIRECT lock pages (or stub pages if they aren't in cache) in the filesystem address space, but this might add significant overhead in radix tree operations during O_DIRECT.
-chris
Quoted 13 lines · Christoph Hellwig
Quoted 12 lines · Chris Mason
Hello everyone,
Right now,
O_DIRECTreads and writes on regular files have to takei_semwhile reading file metadata in order to make sure we don't race with hole filling.This patch tries to get around that by avoiding
i_semwhen we are doing anO_DIRECTread or write inside ofi_size. Yet another rw semaphore is added to struct inode to protect against holes being filled during theO_DIRECT.direct-io.cgets another special case to be aware of the locking.
Quoted 3 lines
Quoted 3 lines
This has only been lightly tested, I'm posting here for general comments before I go much further. I'm rounding up some hardware with enough disks to benchmark it properly.
Looks like there's a long line to get to the big test machines here, so I did some more local performance tests. I've got two scsi drives striped into a lvm2 target (64k stripe size). FS is reiser. I did testing on writes because more of the io is done under i_sem for writes, so it should be easiest to show the benefit there.
The test is a 16k random write test on a 15GB file. 50MB of io is done by each random writer, and everyone is writing to the same preallocated file.
1 random writer = .64MB/s
2 random writers (unpatched) = .71MB/s
2 random writers (patched) = 1.02MB/s
In the patched kernel, the two writer procs finished at roughly the same time (< 1sec difference). In the unpatched kernel, writer #1 finished in about 1/2 the time as writer #2.
-chris