[PATCH] read(v)/write(v) fixes

Clean up vfs_readv/writev() interface and avoid code duplication.

Make kNFSd use the cleaned-up interfaces, and disable the code that
accesses the low-level readpage() function of the exported filesystem
(not allowed - many filesystems need extra setup, which is why we have
a separate ->sendpage() routine for that).

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 3414a439f24046a2e3612af6ac1562f3a5ae9af1..b22b34d7ba1eba6a99dca8ae25ee393849f1a6e3 100644 (file)
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -572,6 +572,7 @@ found:
        return ra;
 }
 
+#if 0 /* don't poke into fs code directly */
 /*
  * Grab and keep cached pages assosiated with a file in the svc_rqst
  * so that they can be passed to the netowork sendmsg/sendpage routines
@@ -626,6 +627,7 @@ nfsd_getpages(struct file *filp, struct svc_rqst *rqstp, unsigned long count)
                retval = desc.error;
        return retval;
 }
+#endif
 
 /*
  * Read data from a file. count must contain the requested read count
@@ -658,13 +660,16 @@ nfsd_read(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset,
        if (ra)
                file.f_ra = ra->p_ra;
 
+#if 0 /* don't poke into fs code directly */
        if (inode->i_mapping->a_ops->readpage) {
                file.f_pos = offset;
                err = nfsd_getpages(&file, rqstp, *count);
-       } else {
+       } else
+#endif
+       {
                oldfs = get_fs();
                set_fs(KERNEL_DS);
-               err = vfs_readv(&file, vec, vlen, *count, &offset);
+               err = vfs_readv(&file, vec, vlen, &offset);
                set_fs(oldfs);
        }
 
@@ -740,7 +745,7 @@ nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset,
 
        /* Write the data. */
        oldfs = get_fs(); set_fs(KERNEL_DS);
-       err = vfs_writev(&file, vec, vlen, cnt, &offset);
+       err = vfs_writev(&file, vec, vlen, &offset);
        if (err >= 0)
                nfsdstats.io_write += cnt;
        set_fs(oldfs);

diff --git a/fs/read_write.c b/fs/read_write.c
index ac9610241a66bdc75c34d962b143db21411c44d8..a335041c98b895b8d6c0cd1362011c890022d75d 100644 (file)
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -207,53 +207,6 @@ ssize_t vfs_read(struct file *file, char *buf, size_t count, loff_t *pos)
        return ret;
 }
 
-ssize_t vfs_readv(struct file *file, struct iovec *vec, int vlen, size_t count, loff_t *pos)
-{
-       struct inode *inode = file->f_dentry->d_inode;
-       ssize_t ret;
-
-       if (!(file->f_mode & FMODE_READ))
-               return -EBADF;
-       if (!file->f_op || (!file->f_op->read && !file->f_op->aio_read))
-               return -EINVAL;
-
-       ret = locks_verify_area(FLOCK_VERIFY_READ, inode, file, *pos, count);
-       if (!ret) {
-               ret = security_ops->file_permission (file, MAY_READ);
-               if (!ret) {
-                       if (file->f_op->readv)
-                               ret = file->f_op->readv(file, vec, vlen, pos);
-                       else {
-                               /* do it by hand */
-                               struct iovec *vector = vec;
-                               ret = 0;
-                               while (vlen > 0) {
-                                       void * base =  vector->iov_base;
-                                       size_t len = vector->iov_len;
-                                       ssize_t nr;
-                                       vector++;
-                                       vlen--;
-                                       if (file->f_op->read)
-                                               nr = file->f_op->read(file, base, len, pos);
-                                       else
-                                               nr = do_sync_read(file, base, len, pos);
-                                       if (nr < 0) {
-                                               if (!ret) ret = nr;
-                                               break;
-                                       }
-                                       ret += nr;
-                                       if (nr != len)
-                                               break;
-                               }
-                       }
-                       if (ret > 0)
-                               dnotify_parent(file->f_dentry, DN_ACCESS);
-               }
-       }
-
-       return ret;
-}
-
 ssize_t do_sync_write(struct file *filp, const char *buf, size_t len, loff_t *ppos)
 {
        struct kiocb kiocb;
@@ -294,53 +247,6 @@ ssize_t vfs_write(struct file *file, const char *buf, size_t count, loff_t *pos)
        return ret;
 }
 
-ssize_t vfs_writev(struct file *file, const struct iovec *vec, int vlen, size_t count, loff_t *pos)
-{
-       struct inode *inode = file->f_dentry->d_inode;
-       ssize_t ret;
-
-       if (!(file->f_mode & FMODE_WRITE))
-               return -EBADF;
-       if (!file->f_op || (!file->f_op->write && !file->f_op->aio_write))
-               return -EINVAL;
-
-       ret = locks_verify_area(FLOCK_VERIFY_WRITE, inode, file, *pos, count);
-       if (!ret) {
-               ret = security_ops->file_permission (file, MAY_WRITE);
-               if (!ret) {
-                       if (file->f_op->writev)
-                               ret = file->f_op->writev(file, vec, vlen, pos);
-                       else {
-                               /* do it by hand */
-                               const struct iovec *vector = vec;
-                               ret = 0;
-                               while (vlen > 0) {
-                                       void * base = vector->iov_base;
-                                       size_t len = vector->iov_len;
-                                       ssize_t nr;
-                                       vector++;
-                                       vlen--;
-                                       if (file->f_op->write)
-                                               nr = file->f_op->write(file, base, len, pos);
-                                       else
-                                               nr = do_sync_write(file, base, len, pos);
-                                       if (nr < 0) {
-                                               if (!ret) ret = nr;
-                                               break;
-                                       }
-                                       ret += nr;
-                                       if (nr != len)
-                                               break;
-                               }
-                       }
-                       if (ret > 0)
-                               dnotify_parent(file->f_dentry, DN_MODIFY);
-               }
-       }
-
-       return ret;
-}
-
 asmlinkage ssize_t sys_read(unsigned int fd, char * buf, size_t count)
 {
        struct file *file;
@@ -427,7 +333,7 @@ unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to)
 
 static ssize_t do_readv_writev(int type, struct file *file,
                               const struct iovec * vector,
-                              unsigned long nr_segs)
+                              unsigned long nr_segs, loff_t *pos)
 {
        typedef ssize_t (*io_fn_t)(struct file *, char *, size_t, loff_t *);
        typedef ssize_t (*iov_fn_t)(struct file *, const struct iovec *, unsigned long, loff_t *);
@@ -496,7 +402,7 @@ static ssize_t do_readv_writev(int type, struct file *file,
        /* VERIFY_WRITE actually means a read, as we write to user space */
        ret = locks_verify_area((type == READ 
                                 ? FLOCK_VERIFY_READ : FLOCK_VERIFY_WRITE),
-                               inode, file, file->f_pos, tot_len);
+                               inode, file, *pos, tot_len);
        if (ret)
                goto out;
 
@@ -509,7 +415,7 @@ static ssize_t do_readv_writev(int type, struct file *file,
                fnv = file->f_op->writev;
        }
        if (fnv) {
-               ret = fnv(file, iov, nr_segs, &file->f_pos);
+               ret = fnv(file, iov, nr_segs, pos);
                goto out;
        }
 
@@ -526,7 +432,7 @@ static ssize_t do_readv_writev(int type, struct file *file,
                vector++;
                nr_segs--;
 
-               nr = fn(file, base, len, &file->f_pos);
+               nr = fn(file, base, len, pos);
 
                if (nr < 0) {
                        if (!ret) ret = nr;
@@ -545,50 +451,56 @@ out:
        return ret;
 }
 
+ssize_t vfs_readv(struct file *file, const struct iovec *vec,
+                 unsigned long vlen, loff_t *pos)
+{
+       if (!(file->f_mode & FMODE_READ))
+               return -EBADF;
+       if (!file->f_op || (!file->f_op->readv && !file->f_op->read))
+               return -EINVAL;
 
-asmlinkage ssize_t
-sys_readv(unsigned long fd, const struct iovec *vector, unsigned long nr_segs)
+       return do_readv_writev(READ, file, vec, vlen, pos);
+}
+
+ssize_t vfs_writev(struct file *file, const struct iovec *vec,
+                  unsigned long vlen, loff_t *pos)
 {
-       struct file * file;
-       ssize_t ret;
+       if (!(file->f_mode & FMODE_WRITE))
+               return -EBADF;
+       if (!file->f_op || (!file->f_op->writev && !file->f_op->write))
+               return -EINVAL;
+
+       return do_readv_writev(WRITE, file, vec, vlen, pos);
+}
+
 
+asmlinkage ssize_t
+sys_readv(unsigned long fd, const struct iovec *vec, unsigned long vlen)
+{
+       struct file *file;
+       ssize_t ret = -EBADF;
 
-       ret = -EBADF;
        file = fget(fd);
-       if (!file)
-               goto bad_file;
-       if (file->f_op && (file->f_mode & FMODE_READ) &&
-           (file->f_op->readv || file->f_op->read)) {
-               ret = security_ops->file_permission (file, MAY_READ);
-               if (!ret)
-                       ret = do_readv_writev(READ, file, vector, nr_segs);
+       if (file) {
+               ret = vfs_readv(file, vec, vlen, &file->f_pos);
+               fput(file);
        }
-       fput(file);
 
-bad_file:
        return ret;
 }
 
 asmlinkage ssize_t
-sys_writev(unsigned long fd, const struct iovec * vector, unsigned long nr_segs)
+sys_writev(unsigned long fd, const struct iovec *vec, unsigned long vlen)
 {
-       struct file * file;
-       ssize_t ret;
-
+       struct file *file;
+       ssize_t ret = -EBADF;
 
-       ret = -EBADF;
        file = fget(fd);
-       if (!file)
-               goto bad_file;
-       if (file->f_op && (file->f_mode & FMODE_WRITE) &&
-           (file->f_op->writev || file->f_op->write)) {
-               ret = security_ops->file_permission (file, MAY_WRITE);
-               if (!ret)
-                       ret = do_readv_writev(WRITE, file, vector, nr_segs);
+       if (file) {
+               ret = vfs_writev(file, vec, vlen, &file->f_pos);
+               fput(file);
        }
-       fput(file);
 
-bad_file:
        return ret;
 }
 

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4e0e4bd005a66e0ae9a89f05539174c4277d7b22..b71df992cad79dfc290017bab59e00095fd59961 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -793,8 +793,10 @@ struct seq_file;
 
 extern ssize_t vfs_read(struct file *, char *, size_t, loff_t *);
 extern ssize_t vfs_write(struct file *, const char *, size_t, loff_t *);
-extern ssize_t vfs_readv(struct file *, struct iovec *, int, size_t, loff_t *);
-extern ssize_t vfs_writev(struct file *, const struct iovec *, int, size_t, loff_t *);
+extern ssize_t vfs_readv(struct file *, const struct iovec *,
+               unsigned long, loff_t *);
+extern ssize_t vfs_writev(struct file *, const struct iovec *,
+               unsigned long, loff_t *);
 
 /*
  * NOTE: write_inode, delete_inode, clear_inode, put_inode can be called