On Thu, Jan 16, 2014 at 4:35 PM, Steven Whitehouse <swhiteho@xxxxxxxxxx> wrote: > > Following on from the "Re: [PATCH v3] vfs: fix a bug when we do some dio > reads with append dio writes" thread on linux-fsdevel, this patch is my > current version of the fix proposed as option (b) in that thread. > > Removing the i_size test from the direct i/o read path at VFS level > means that filesystems now have to deal with requests which are beyond > i_size themselves. These I've divided into three sets: > > a) Those with "no op" ->direct_IO (9p, cifs, ceph) > These are obviously not going to be an issue > > b) Those with "home brew" ->direct_IO (nfs, fuse) > I've been told that NFS should not have any problem with the larger > i_size, however I've added an extra test to FUSE to duplicate the > original behaviour just to be on the safe side. Someone who knows fuse > better maybe able to confirm whether this is actually required or not. > > c) Those using __blockdev_direct_IO() > These call through to ->get_block() which should deal with the EOF > condition correctly. I've verified that with GFS2 and I believe that > Zheng has verified it for ext4. I've also run the test on XFS and it > passes both before and after this change. > > The part of the patch in filemap.c looks a lot larger than it really is > - there are only two lines of real change. The rest is just indentation > of the contained code. > > There remains a test of i_size though, which was added for btrfs. It > doesn't cause the other filesystems a problem as the test is performed > after ->direct_IO has been called. It is possible that there is a race > that does matter to btrfs, however this patch doesn't change that, so > its still an overall improvement. > > So please have a look at this and let me know what you think. I guess > that when time comes to submit it, it should probably be via the vfs > tree. > > Signed-off-by: Steven Whitehouse <swhiteho@xxxxxxxxxx> > Reported-by: Zheng Liu <gnehzuil.liu@xxxxxxxxx> > Cc: Jan Kara <jack@xxxxxxx> > Cc: Dave Chinner <david@xxxxxxxxxxxxx> > Cc: Miklos Szeredi <miklos@xxxxxxxxxx> > Cc: Chris Mason <clm@xxxxxx> > Cc: Josef Bacik <jbacik@xxxxxx> > Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx> > Cc: Alexander Viro <viro@xxxxxxxxxxxxxxxxxx> > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index 7e70506..89fdfd1 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -2710,6 +2710,9 @@ fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov, > inode = file->f_mapping->host; > i_size = i_size_read(inode); > > + if ((rw == READ) && (offset > i_size)) > + return 0; > + Hmm, OK. It's not strictly needed, but a valid optimization. So ACK. Thanks, Miklos -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
