Skip to contentSkip to conversation

512-byte alignment for O_DIRECT I/O

PATCH In review Andrew Morton 3 participants 4 messages Oct 7, 2002 – Oct 8, 2002

Subsystems (4)
VFS XFS filesystem ext2 file system JFS filesystem
lore
Share & tools
lore

Andrew Morton

author
#0 lore raw

PATCH 1/1 512-byte alignment for O_DIRECT I/O

This patch from Badari is passing all testing now.

Traditionally we have only supported O_DIRECT I/O at an alignment and granularity which matches the underlying filesystem. That typically means that all IO must be 4k-aligned and a multiple of 4k in size.

Here, we relax that so that direct I/O happens with (typically) 512-byte alignment and multiple-of-512-byte size.

The tricky part is when a write starts and/or ends partway through a filesystem block which has just been added. We need to zero out the parts of that block which lie outside the written region.

We handle that by putting appropriately-sized parts of the ZERO_PAGE into sepatate BIOs.

The generic_direct_IO() function has been changed so that the filesystem must pass in the address of the block_device against which the IO is to be performed. I'd have preferred to not do this, but we do need that info at that time so that alignment checks can be performed.

If the filesystem passes in a NULL block_device pointer then we fall back to the old behaviour - must align with the fs blocksize.

There is no trivial way for userspace to know what the minimum alignment is - it depends on what bdev_hardsect_size() says about the device.
It is _usually_ 512 bytes, but not always. This introduces the risk
that someone will develop and test applications which work fine on their hardware, but will fail on someone else's hardware.

It is possible to query the hardsect size using the BLKSSZGET ioctl against the backing block device. This can be performed at runtime or at application installation time.

The XFS patch has not yet been tested.

 fs/block_dev.c          |    2 
 fs/direct-io.c          |  124 +++++++++++++++++++++++++++++++++++++++++++-----
 fs/ext2/inode.c         |    2 
 fs/ext3/inode.c         |    2 
 fs/jfs/inode.c          |    2 
 fs/xfs/linux/xfs_aops.c |    4 -
 include/linux/fs.h      |    4 -
 7 files changed, 120 insertions(+), 20 deletions(-)
fs/block_dev.c renamed from fs/block_dev.c~dio-fine-alignment +1 -1
@@ -121,7 +121,7 @@ blkdev_direct_IO(int rw, struct file *fi
 {
 	struct inode *inode = file->f_dentry->d_inode->i_mapping->host;
 
-	return generic_direct_IO(rw, inode, iov, offset,
+	return generic_direct_IO(rw, inode, inode->i_bdev, iov, offset,
 				nr_segs, blkdev_get_blocks);
 }
fs/direct-io.c renamed from fs/direct-io.c~dio-fine-alignment +112 -12

206 lines not on this page · raw message

fs/ext2/inode.c renamed from fs/ext2/inode.c~dio-fine-alignment +1 -1
@@ -624,7 +624,7 @@ ext2_direct_IO(int rw, struct file *file
 {
 	struct inode *inode = file->f_dentry->d_inode->i_mapping->host;
 
-	return generic_direct_IO(rw, inode, iov,
+	return generic_direct_IO(rw, inode, inode->i_sb->s_bdev, iov,
 				offset, nr_segs, ext2_get_blocks);
 }
fs/ext3/inode.c renamed from fs/ext3/inode.c~dio-fine-alignment +1 -1
@@ -1431,7 +1431,7 @@ static int ext3_direct_IO(int rw, struct
 		}
 	}
 
-	ret = generic_direct_IO(rw, inode, iov, offset,
+	ret = generic_direct_IO(rw, inode, inode->i_sb->s_bdev, iov, offset,
 				nr_segs, ext3_direct_io_get_blocks);
 
 out_stop:
fs/jfs/inode.c renamed from fs/jfs/inode.c~dio-fine-alignment +1 -1
@@ -315,7 +315,7 @@ static int jfs_direct_IO(int rw, struct 
 {
 	struct inode *inode = file->f_dentry->d_inode->i_mapping->host;
 
-	return generic_direct_IO(rw, inode, iov,
+	return generic_direct_IO(rw, inode, inode->i_sb->s_bdev, iov,
 				offset, nr_segs, jfs_get_blocks);
 }
include/linux/fs.h renamed from include/linux/fs.h~dio-fine-alignment +2 -2
@@ -1245,8 +1245,8 @@ extern ssize_t generic_file_sendfile(str
 extern void do_generic_file_read(struct file *, loff_t *, read_descriptor_t *, read_actor_t);
 extern ssize_t generic_file_direct_IO(int rw, struct file *file,
 	const struct iovec *iov, loff_t offset, unsigned long nr_segs);
-extern int generic_direct_IO(int rw, struct inode *inode, const struct iovec 
-	*iov, loff_t offset, unsigned long nr_segs, get_blocks_t *get_blocks);
+extern int generic_direct_IO(int rw, struct inode *inode, struct block_device *bdev,
+	const struct iovec *iov, loff_t offset, unsigned long nr_segs, get_blocks_t *get_blocks);
 extern ssize_t generic_file_readv(struct file *filp, const struct iovec *iov, 
 	unsigned long nr_segs, loff_t *ppos);
 ssize_t generic_file_writev(struct file *filp, const struct iovec *iov,
fs/xfs/linux/xfs_aops.c renamed from fs/xfs/linux/xfs_aops.c~dio-fine-alignment +2 -2
@@ -688,8 +688,8 @@ linvfs_direct_IO(
 {
 	struct inode *inode = file->f_dentry->d_inode->i_mapping->host;
 
-        return generic_direct_IO(rw, inode, iov, offset, nr_segs,
-					linvfs_get_blocks_direct);
+        return generic_direct_IO(rw, inode, inode->i_sb->s_bdev,
+			iov, offset, nr_segs, linvfs_get_blocks_direct);
 }
 
 STATIC int

.

#1 lore raw
Quoted 3 lines · Andrew Morton

This patch from Badari is passing all testing now.

.....

fs/xfs/linux/xfs_aops.c linvfs_direct_IO( · line 695

Quoted 13 lines

+++ 2.5.41-akpm/fs/xfs/linux/xfs_aops.c Mon Oct 7 15:50:21 2002

fs/xfs/linux/xfs_aops.c renamed from fs/xfs/linux/xfs_aops.c~dio-fine-alignment +2 -2
@@ -688,8 +688,8 @@ linvfs_direct_IO(
 {
 	struct inode *inode = file->f_dentry->d_inode->i_mapping->host;
 
-        return generic_direct_IO(rw, inode, iov, offset, nr_segs,
-					linvfs_get_blocks_direct);
+        return generic_direct_IO(rw, inode, inode->i_sb->s_bdev,
+			iov, offset, nr_segs, linvfs_get_blocks_direct);
 }
 
 STATIC int

Actually this part is broken for XFS - it will work for most cases, but not for realtime files, in this case there is another bdev involved. I just have to work out how to get to it from here...... the getblock code knows enough to set it in the bh, but at this level we do not.

Steve

Signature · 2 lines
Steve Lord                                      voice: +1-651-683-3511
Principal Engineer, Filesystem Software         email: [email protected]
author
#2 lore raw
Quoted 17 lines · Steve Lord
Quoted 3 lines · Andrew Morton

This patch from Badari is passing all testing now.

.....

Quoted 11 lines

+++ 2.5.41-akpm/fs/xfs/linux/xfs_aops.c Mon Oct 7 15:50:21 2002

fs/xfs/linux/xfs_aops.c renamed from fs/xfs/linux/xfs_aops.c~dio-fine-alignment +2 -2
@@ -688,8 +688,8 @@ linvfs_direct_IO(
 {
      struct inode *inode = file->f_dentry->d_inode->i_mapping->host;
 
-        return generic_direct_IO(rw, inode, iov, offset, nr_segs,
-                                     linvfs_get_blocks_direct);
+        return generic_direct_IO(rw, inode, inode->i_sb->s_bdev,
+                     iov, offset, nr_segs, linvfs_get_blocks_direct);
 }
Quoted 8 lines

fs/xfs/linux/xfs_aops.c linvfs_direct_IO( · line 695

Quoted 2 lines
fs/xfs/linux/xfs_aops.c renamed from fs/xfs/linux/xfs_aops.c~dio-fine-alignment +0 -0
 STATIC int

Actually this part is broken for XFS - it will work for most cases, but not for realtime files, in this case there is another bdev involved. I just have to work out how to get to it from here...... the getblock code knows enough to set it in the bh, but at this level we do not.

Well we can pass in NULL for the while, get the old behaviour.

But yes, I'd prefer to only ever use the get_block() value. It's really messy though - things like deferring the check of the aligment of all the iovec segments until we've run get_block...

Maybe we should ask the caller to pass in the alignment itself, just 512, 2048, etc?

#3 lore raw
Quoted 28 lines
Quoted 17 lines · Steve Lord
Quoted 3 lines · Andrew Morton

This patch from Badari is passing all testing now.

.....

Quoted 11 lines

+++ 2.5.41-akpm/fs/xfs/linux/xfs_aops.c Mon Oct 7 15:50:21 2002

fs/xfs/linux/xfs_aops.c renamed from fs/xfs/linux/xfs_aops.c~dio-fine-alignment +2 -2
@@ -688,8 +688,8 @@ linvfs_direct_IO(
 {
      struct inode *inode = file->f_dentry->d_inode->i_mapping->host;
 
-        return generic_direct_IO(rw, inode, iov, offset, nr_segs,
-                                     linvfs_get_blocks_direct);
+        return generic_direct_IO(rw, inode, inode->i_sb->s_bdev,
+                     iov, offset, nr_segs, linvfs_get_blocks_direct);
 }
Quoted 8 lines

fs/xfs/linux/xfs_aops.c linvfs_direct_IO( · line 695

Quoted 2 lines
fs/xfs/linux/xfs_aops.c renamed from fs/xfs/linux/xfs_aops.c~dio-fine-alignment +0 -0
 STATIC int

Actually this part is broken for XFS - it will work for most cases, but not for realtime files, in this case there is another bdev involved. I just have to work out how to get to it from here...... the getblock code knows enough to set it in the bh, but at this level we do not.

Quoted 8 lines

Well we can pass in NULL for the while, get the old behaviour.

But yes, I'd prefer to only ever use the get_block() value. It's really messy though - things like deferring the check of the aligment of all the iovec segments until we've run get_block...

Maybe we should ask the caller to pass in the alignment itself, just 512, 2048, etc?

In XFS case, if it does not know what the bdev is, how would the caller know the alignment ?

If we really *really* want to use "bdev" from get_block(), we can do a dummy get_block(..., 0, READ) upfront and use the "bdev". Its UGLY but ..

- Badari