[GFS2] Alter direct I/O path
Steven Whitehouse [Tue, 25 Jul 2006 21:24:12 +0000 (17:24 -0400)]
As per comments received, alter the GFS2 direct I/O path so that
it uses the standard read functions "out of the box". Needs a
small change to one of the VFS functions. This reduces the size
of the code quite a lot and also removes the need for one new export.

Some more work remains to be done, but this is the bones of the
thing.

Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>

fs/gfs2/ops_address.c
fs/gfs2/ops_file.c
mm/filemap.c

index 031270a..d33f6aa 100644 (file)
@@ -589,8 +589,9 @@ static void gfs2_invalidatepage(struct page *page, unsigned long offset)
        return;
 }
 
-static ssize_t gfs2_direct_IO_write(struct kiocb *iocb, const struct iovec *iov,
-                                   loff_t offset, unsigned long nr_segs)
+static ssize_t gfs2_direct_IO(int rw, struct kiocb *iocb,
+                             const struct iovec *iov, loff_t offset,
+                             unsigned long nr_segs)
 {
        struct file *file = iocb->ki_filp;
        struct inode *inode = file->f_mapping->host;
@@ -598,8 +599,10 @@ static ssize_t gfs2_direct_IO_write(struct kiocb *iocb, const struct iovec *iov,
        struct gfs2_holder gh;
        int rv;
 
+       if (rw == READ)
+               mutex_lock(&inode->i_mutex);
        /*
-        * Shared lock, even though its write, since we do no allocation
+        * Shared lock, even if its a write, since we do no allocation
         * on this path. All we need change is atime.
         */
        gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME, &gh);
@@ -607,6 +610,9 @@ static ssize_t gfs2_direct_IO_write(struct kiocb *iocb, const struct iovec *iov,
        if (rv)
                goto out;
 
+       if (offset > i_size_read(inode))
+               goto out;
+
        /*
         * Should we return an error here? I can't see that O_DIRECT for
         * a journaled file makes any sense. For now we'll silently fall
@@ -619,48 +625,20 @@ static ssize_t gfs2_direct_IO_write(struct kiocb *iocb, const struct iovec *iov,
        if (gfs2_is_stuffed(ip))
                goto out;
 
-       rv = __blockdev_direct_IO(WRITE, iocb, inode, inode->i_sb->s_bdev,
-                                 iov, offset, nr_segs, gfs2_get_block,
-                                 NULL, DIO_OWN_LOCKING);
+       rv = blockdev_direct_IO_own_locking(rw, iocb, inode,
+                                           inode->i_sb->s_bdev,
+                                           iov, offset, nr_segs,
+                                           gfs2_get_block, NULL);
 out:
        gfs2_glock_dq_m(1, &gh);
        gfs2_holder_uninit(&gh);
+       if (rw == READ)
+               mutex_unlock(&inode->i_mutex);
 
        return rv;
 }
 
 /**
- * gfs2_direct_IO
- *
- * This is called with a shared lock already held for the read path.
- * Currently, no locks are held when the write path is called.
- */
-static ssize_t gfs2_direct_IO(int rw, struct kiocb *iocb,
-                             const struct iovec *iov, loff_t offset,
-                             unsigned long nr_segs)
-{
-       struct file *file = iocb->ki_filp;
-       struct inode *inode = file->f_mapping->host;
-       struct gfs2_inode *ip = GFS2_I(inode);
-       struct gfs2_sbd *sdp = GFS2_SB(inode);
-       int ret;
-
-       if (rw == WRITE)
-               return gfs2_direct_IO_write(iocb, iov, offset, nr_segs);
-
-       if (gfs2_assert_warn(sdp, gfs2_glock_is_locked_by_me(ip->i_gl)) ||
-           gfs2_assert_warn(sdp, !gfs2_is_stuffed(ip)))
-               return -EINVAL;
-
-       mutex_lock(&inode->i_mutex);
-       ret = __blockdev_direct_IO(READ, iocb, inode, inode->i_sb->s_bdev, iov,
-                                  offset, nr_segs, gfs2_get_block, NULL,
-                                  DIO_OWN_LOCKING);
-       mutex_unlock(&inode->i_mutex);
-       return ret;
-}
-
-/**
  * stuck_releasepage - We're stuck in gfs2_releasepage().  Print stuff out.
  * @bh: the buffer we're stuck on
  *
@@ -765,7 +743,7 @@ int gfs2_releasepage(struct page *page, gfp_t gfp_mask)
        }
        while (bh != head);
 
- out:
+out:
        return try_to_free_buffers(page);
 }
 
index 6764ca6..d13e04e 100644 (file)
@@ -148,170 +148,6 @@ static loff_t gfs2_llseek(struct file *file, loff_t offset, int origin)
        return error;
 }
 
-
-static ssize_t gfs2_direct_IO_read(struct kiocb *iocb, const struct iovec *iov,
-                                  loff_t offset, unsigned long nr_segs)
-{
-       struct file *file = iocb->ki_filp;
-       struct address_space *mapping = file->f_mapping;
-       ssize_t retval;
-
-       retval = filemap_write_and_wait(mapping);
-       if (retval == 0) {
-               retval = mapping->a_ops->direct_IO(READ, iocb, iov, offset,
-                                                  nr_segs);
-       }
-       return retval;
-}
-
-/**
- * __gfs2_file_aio_read - The main GFS2 read function
- * 
- * N.B. This is almost, but not quite the same as __generic_file_aio_read()
- * the important subtle different being that inode->i_size isn't valid
- * unless we are holding a lock, and we do this _only_ on the O_DIRECT
- * path since otherwise locking is done entirely at the page cache
- * layer.
- */
-static ssize_t __gfs2_file_aio_read(struct kiocb *iocb,
-                                   const struct iovec *iov,
-                                   unsigned long nr_segs, loff_t *ppos)
-{
-       struct file *filp = iocb->ki_filp;
-       struct gfs2_inode *ip = GFS2_I(filp->f_mapping->host);
-       struct gfs2_holder gh;
-       ssize_t retval;
-       unsigned long seg;
-       size_t count;
-
-       count = 0;
-       for (seg = 0; seg < nr_segs; seg++) {
-               const struct iovec *iv = &iov[seg];
-
-               /*
-                * If any segment has a negative length, or the cumulative
-                * length ever wraps negative then return -EINVAL.
-                */
-               count += iv->iov_len;
-               if (unlikely((ssize_t)(count|iv->iov_len) < 0))
-                       return -EINVAL;
-               if (access_ok(VERIFY_WRITE, iv->iov_base, iv->iov_len))
-                       continue;
-               if (seg == 0)
-                       return -EFAULT;
-               nr_segs = seg;
-               count -= iv->iov_len;   /* This segment is no good */
-               break;
-       }
-
-       /* coalesce the iovecs and go direct-to-BIO for O_DIRECT */
-       if (filp->f_flags & O_DIRECT) {
-               loff_t pos = *ppos, size;
-               struct address_space *mapping;
-               struct inode *inode;
-
-               mapping = filp->f_mapping;
-               inode = mapping->host;
-               retval = 0;
-               if (!count)
-                       goto out; /* skip atime */
-
-               gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME, &gh);
-               retval = gfs2_glock_nq_m_atime(1, &gh);
-               if (retval)
-                       goto out;
-               if (gfs2_is_stuffed(ip)) {
-                       gfs2_glock_dq_m(1, &gh);
-                       gfs2_holder_uninit(&gh);
-                       goto fallback_to_normal;
-               }
-               size = i_size_read(inode);
-               if (pos < size) {
-                       retval = gfs2_direct_IO_read(iocb, iov, pos, nr_segs);
-                       if (retval > 0 && !is_sync_kiocb(iocb))
-                               retval = -EIOCBQUEUED;
-                       if (retval > 0)
-                               *ppos = pos + retval;
-               }
-               file_accessed(filp);
-               gfs2_glock_dq_m(1, &gh);
-               gfs2_holder_uninit(&gh);
-               goto out;
-       }
-
-fallback_to_normal:
-       retval = 0;
-       if (count) {
-               for (seg = 0; seg < nr_segs; seg++) {
-                       read_descriptor_t desc;
-
-                       desc.written = 0;
-                       desc.arg.buf = iov[seg].iov_base;
-                       desc.count = iov[seg].iov_len;
-                       if (desc.count == 0)
-                               continue;
-                       desc.error = 0;
-                       do_generic_file_read(filp,ppos,&desc,file_read_actor);
-                       retval += desc.written;
-                       if (desc.error) {
-                               retval = retval ?: desc.error;
-                                break;
-                       }
-               }
-       }
-out:
-       return retval;
-}
-
-/**
- * gfs2_read - Read bytes from a file
- * @file: The file to read from
- * @buf: The buffer to copy into
- * @size: The amount of data requested
- * @offset: The current file offset
- *
- * Outputs: Offset - updated according to number of bytes read
- *
- * Returns: The number of bytes read, errno on failure
- */
-
-static ssize_t gfs2_read(struct file *filp, char __user *buf, size_t size,
-                        loff_t *offset)
-{
-       struct iovec local_iov = { .iov_base = buf, .iov_len = size };
-       struct kiocb kiocb;
-       ssize_t ret;
-
-       init_sync_kiocb(&kiocb, filp);
-       ret = __gfs2_file_aio_read(&kiocb, &local_iov, 1, offset);
-       if (-EIOCBQUEUED == ret)
-               ret = wait_on_sync_kiocb(&kiocb);
-       return ret;
-}
-
-static ssize_t gfs2_file_readv(struct file *filp, const struct iovec *iov,
-                              unsigned long nr_segs, loff_t *ppos)
-{
-       struct kiocb kiocb;
-       ssize_t ret;
-
-       init_sync_kiocb(&kiocb, filp);
-       ret = __gfs2_file_aio_read(&kiocb, iov, nr_segs, ppos);
-       if (-EIOCBQUEUED == ret)
-               ret = wait_on_sync_kiocb(&kiocb);
-       return ret;
-}
-
-static ssize_t gfs2_file_aio_read(struct kiocb *iocb, char __user *buf,
-                                 size_t count, loff_t pos)
-{
-        struct iovec local_iov = { .iov_base = buf, .iov_len = count };
-
-        BUG_ON(iocb->ki_pos != pos);
-        return __gfs2_file_aio_read(iocb, &local_iov, 1, &iocb->ki_pos);
-}
-
-
 /**
  * filldir_reg_func - Report a directory entry to the caller of gfs2_dir_read()
  * @opaque: opaque data used by the function
@@ -949,9 +785,9 @@ static int gfs2_flock(struct file *file, int cmd, struct file_lock *fl)
 
 const struct file_operations gfs2_file_fops = {
        .llseek = gfs2_llseek,
-       .read = gfs2_read,
-       .readv = gfs2_file_readv,
-       .aio_read = gfs2_file_aio_read,
+       .read = generic_file_read,
+       .readv = generic_file_readv,
+       .aio_read = generic_file_aio_read,
        .write = generic_file_write,
        .writev = generic_file_writev,
        .aio_write = generic_file_aio_write,
index b9c91ab..a92d690 100644 (file)
@@ -1122,7 +1122,6 @@ success:
        desc->arg.buf += size;
        return size;
 }
-EXPORT_SYMBOL_GPL(file_read_actor);
 
 /**
  * __generic_file_aio_read - generic filesystem read routine
@@ -1184,7 +1183,8 @@ __generic_file_aio_read(struct kiocb *iocb, const struct iovec *iov,
                                *ppos = pos + retval;
                }
                file_accessed(filp);
-               goto out;
+               if (retval != 0)
+                       goto out;
        }
 
        retval = 0;