Re: [PATCH 1/2] fs: remove duplicated iovec checking code v8

2007-03-19 Thread Christoph Hellwig
On Mon, Mar 19, 2007 at 10:49:01AM +0300, Dmitriy Monakhov wrote:
> 
> Where are several places where the same code used for iovec checks.
> This patch just move this code to separate helper function, and replace
> duplicated code with it. IMHO it is better because these are checks that
> we want for all filesystems/drivers that use vectored I/O.

Please move this into the common code path, so it's checked before
entering the filesystem.  This won't cover the calculating count
until we have an iodesc/uio strcuture to pass it down, so feel free
to add a tiny helper for that temporaily.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/2] fs: remove duplicated iovec checking code v8

2007-03-19 Thread Dmitriy Monakhov

Where are several places where the same code used for iovec checks.
This patch just move this code to separate helper function, and replace
duplicated code with it. IMHO it is better because these are checks that
we want for all filesystems/drivers that use vectored I/O.

Signed-off-by: Dmitriy Monakhov <[EMAIL PROTECTED]>
---
 fs/ntfs/file.c |   21 +++--
 fs/read_write.c|   40 
 fs/xfs/linux-2.6/xfs_lrw.c |   22 +++---
 include/linux/fs.h |3 +++
 mm/filemap.c   |   43 ++-
 5 files changed, 55 insertions(+), 74 deletions(-)

diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index dbbac55..2c672a4 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -2129,28 +2129,13 @@ static ssize_t ntfs_file_aio_write_nolock(struct kiocb 
*iocb,
struct address_space *mapping = file->f_mapping;
struct inode *inode = mapping->host;
loff_t pos;
-   unsigned long seg;
size_t count;   /* after file limit checks */
ssize_t written, err;
 
count = 0;
-   for (seg = 0; seg < nr_segs; seg++) {
-   const struct iovec *iv = [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_READ, iv->iov_base, iv->iov_len))
-   continue;
-   if (!seg)
-   return -EFAULT;
-   nr_segs = seg;
-   count -= iv->iov_len;   /* This segment is no good */
-   break;
-   }
+   err = generic_iovec_checks(iov, _segs, , VERIFY_READ);
+   if (err)
+   return err;
pos = *ppos;
vfs_check_frozen(inode->i_sb, SB_FREEZE_WRITE);
/* We can write back this queue in page reclaim. */
diff --git a/fs/read_write.c b/fs/read_write.c
index 4d03008..22ec324 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -217,6 +217,46 @@ Einval:
return -EINVAL;
 }
 
+/*
+ * Performs necessary iovec checks before doing a write
+ * @iov:   io vector request
+ * @nr_segs:   number of segments in the iovec
+ * @count: number of bytes to write
+ * @access_flags: type of access: %VERIFY_READ or %VERIFY_WRITE
+ *
+ * Adjust number of segments and amount of bytes to write (nr_segs should be
+ * properly initialized first). Returns appropriate error code that caller
+ * should return or zero in case that write should be allowed.
+ */
+int generic_iovec_checks(const struct iovec *iov,
+   unsigned long *nr_segs, size_t *count,
+   unsigned long access_flags)
+{
+   unsigned long   seg;
+   size_t cnt = 0;
+   for (seg = 0; seg < *nr_segs; seg++) {
+   const struct iovec *iv = [seg];
+
+   /*
+* If any segment has a negative length, or the cumulative
+* length ever wraps negative then return -EINVAL.
+*/
+   cnt += iv->iov_len;
+   if (unlikely((ssize_t)(cnt|iv->iov_len) < 0))
+   return -EINVAL;
+   if (access_ok(access_flags, iv->iov_base, iv->iov_len))
+   continue;
+   if (seg == 0)
+   return -EFAULT;
+   *nr_segs = seg;
+   cnt -= iv->iov_len; /* This segment is no good */
+   break;
+   }
+   *count = cnt;
+   return 0;
+}
+EXPORT_SYMBOL(generic_iovec_checks);
+
 static void wait_on_retry_sync_kiocb(struct kiocb *iocb)
 {
set_current_state(TASK_UNINTERRUPTIBLE);
diff --git a/fs/xfs/linux-2.6/xfs_lrw.c b/fs/xfs/linux-2.6/xfs_lrw.c
index ff8d64e..9a11b00 100644
--- a/fs/xfs/linux-2.6/xfs_lrw.c
+++ b/fs/xfs/linux-2.6/xfs_lrw.c
@@ -639,7 +639,6 @@ xfs_write(
xfs_fsize_t isize, new_size;
xfs_iocore_t*io;
bhv_vnode_t *vp;
-   unsigned long   seg;
int iolock;
int eventsent = 0;
bhv_vrwlock_t   locktype;
@@ -652,24 +651,9 @@ xfs_write(
vp = BHV_TO_VNODE(bdp);
xip = XFS_BHVTOI(bdp);
 
-   for (seg = 0; seg < segs; seg++) {
-   const struct iovec *iv = [seg];
-
-   /*
-* If any segment has a negative length, or the cumulative
-* length ever wraps negative then return -EINVAL.
-*/
-   ocount += iv->iov_len;
-   if (unlikely((ssize_t)(ocount|iv->iov_len) < 0))
-   return -EINVAL;
-   if (access_ok(VERIFY_READ, iv->iov_base, iv->iov_len))
- 

[PATCH 1/2] fs: remove duplicated iovec checking code v8

2007-03-19 Thread Dmitriy Monakhov

Where are several places where the same code used for iovec checks.
This patch just move this code to separate helper function, and replace
duplicated code with it. IMHO it is better because these are checks that
we want for all filesystems/drivers that use vectored I/O.

Signed-off-by: Dmitriy Monakhov [EMAIL PROTECTED]
---
 fs/ntfs/file.c |   21 +++--
 fs/read_write.c|   40 
 fs/xfs/linux-2.6/xfs_lrw.c |   22 +++---
 include/linux/fs.h |3 +++
 mm/filemap.c   |   43 ++-
 5 files changed, 55 insertions(+), 74 deletions(-)

diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index dbbac55..2c672a4 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -2129,28 +2129,13 @@ static ssize_t ntfs_file_aio_write_nolock(struct kiocb 
*iocb,
struct address_space *mapping = file-f_mapping;
struct inode *inode = mapping-host;
loff_t pos;
-   unsigned long seg;
size_t count;   /* after file limit checks */
ssize_t written, err;
 
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_READ, iv-iov_base, iv-iov_len))
-   continue;
-   if (!seg)
-   return -EFAULT;
-   nr_segs = seg;
-   count -= iv-iov_len;   /* This segment is no good */
-   break;
-   }
+   err = generic_iovec_checks(iov, nr_segs, count, VERIFY_READ);
+   if (err)
+   return err;
pos = *ppos;
vfs_check_frozen(inode-i_sb, SB_FREEZE_WRITE);
/* We can write back this queue in page reclaim. */
diff --git a/fs/read_write.c b/fs/read_write.c
index 4d03008..22ec324 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -217,6 +217,46 @@ Einval:
return -EINVAL;
 }
 
+/*
+ * Performs necessary iovec checks before doing a write
+ * @iov:   io vector request
+ * @nr_segs:   number of segments in the iovec
+ * @count: number of bytes to write
+ * @access_flags: type of access: %VERIFY_READ or %VERIFY_WRITE
+ *
+ * Adjust number of segments and amount of bytes to write (nr_segs should be
+ * properly initialized first). Returns appropriate error code that caller
+ * should return or zero in case that write should be allowed.
+ */
+int generic_iovec_checks(const struct iovec *iov,
+   unsigned long *nr_segs, size_t *count,
+   unsigned long access_flags)
+{
+   unsigned long   seg;
+   size_t cnt = 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.
+*/
+   cnt += iv-iov_len;
+   if (unlikely((ssize_t)(cnt|iv-iov_len)  0))
+   return -EINVAL;
+   if (access_ok(access_flags, iv-iov_base, iv-iov_len))
+   continue;
+   if (seg == 0)
+   return -EFAULT;
+   *nr_segs = seg;
+   cnt -= iv-iov_len; /* This segment is no good */
+   break;
+   }
+   *count = cnt;
+   return 0;
+}
+EXPORT_SYMBOL(generic_iovec_checks);
+
 static void wait_on_retry_sync_kiocb(struct kiocb *iocb)
 {
set_current_state(TASK_UNINTERRUPTIBLE);
diff --git a/fs/xfs/linux-2.6/xfs_lrw.c b/fs/xfs/linux-2.6/xfs_lrw.c
index ff8d64e..9a11b00 100644
--- a/fs/xfs/linux-2.6/xfs_lrw.c
+++ b/fs/xfs/linux-2.6/xfs_lrw.c
@@ -639,7 +639,6 @@ xfs_write(
xfs_fsize_t isize, new_size;
xfs_iocore_t*io;
bhv_vnode_t *vp;
-   unsigned long   seg;
int iolock;
int eventsent = 0;
bhv_vrwlock_t   locktype;
@@ -652,24 +651,9 @@ xfs_write(
vp = BHV_TO_VNODE(bdp);
xip = XFS_BHVTOI(bdp);
 
-   for (seg = 0; seg  segs; seg++) {
-   const struct iovec *iv = iovp[seg];
-
-   /*
-* If any segment has a negative length, or the cumulative
-* length ever wraps negative then return -EINVAL.
-*/
-   ocount += iv-iov_len;
-   if (unlikely((ssize_t)(ocount|iv-iov_len)  0))
-   return -EINVAL;
-   if (access_ok(VERIFY_READ, iv-iov_base, iv-iov_len))
- 

Re: [PATCH 1/2] fs: remove duplicated iovec checking code v8

2007-03-19 Thread Christoph Hellwig
On Mon, Mar 19, 2007 at 10:49:01AM +0300, Dmitriy Monakhov wrote:
 
 Where are several places where the same code used for iovec checks.
 This patch just move this code to separate helper function, and replace
 duplicated code with it. IMHO it is better because these are checks that
 we want for all filesystems/drivers that use vectored I/O.

Please move this into the common code path, so it's checked before
entering the filesystem.  This won't cover the calculating count
until we have an iodesc/uio strcuture to pass it down, so feel free
to add a tiny helper for that temporaily.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/