[PATCH 0/4] xfs: fs/xfs/xfs_fs_subr.c die die die

[PATCH 0/4] xfs: fs/xfs/xfs_fs_subr.c die die die

[Dave Chinner]

Hi folks,

This patch series replaces all the functions in fs/xfs/xfs_fs_subr.c
with direct calls to their VFS equivalents. This fixes bugs and
removes code at the same time.

I haven't bothered to change the ranges passed to the functions -
that requires a lot more scrutiny and testing - the ranges have now
may not be optimal but they are a known factor. The changes have
passed smoke testing on several different configurations and about 6
hours of continuous fsx testing, which is what I'd expect seeing as
I have changed the ranges at all.

The diffstat is as follows:

 fs/xfs/Makefile       |    1 -
 fs/xfs/xfs_aops.c     |    2 +-
 fs/xfs/xfs_bmap.c     |    2 +-
 fs/xfs/xfs_dfrag.c    |   13 +++----
 fs/xfs/xfs_file.c     |   23 ++++++------
 fs/xfs/xfs_fs_subr.c  |   96 ---------------------------------------------------
 fs/xfs/xfs_iops.c     |    4 +--
 fs/xfs/xfs_vnodeops.c |   46 +++++++++++++++++-------
 fs/xfs/xfs_vnodeops.h |    7 ----
 9 files changed, 54 insertions(+), 140 deletions(-)

Cheers,

Dave.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/4] xfs: remove xfs_tosspages

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

It's a buggy, unnecessary wrapper that is duplicating
truncate_pagecache_range().

When replacing the call in xfs_change_file_space(), also ensure that
the length being allocated/freed is always positive before making
any changes. These checks are done in the lower extent manipulation
functions, too, but we need to do them before any page cache
operations.

Reported-by: Andrew Dahl <adahl@sgi.com>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_dfrag.c    |    3 +--
 fs/xfs/xfs_fs_subr.c  |   12 ------------
 fs/xfs/xfs_vnodeops.c |   28 +++++++++++++++++++++++-----
 fs/xfs/xfs_vnodeops.h |    2 --
 4 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/fs/xfs/xfs_dfrag.c b/fs/xfs/xfs_dfrag.c
index b9b8646..b2c63a2 100644
--- a/fs/xfs/xfs_dfrag.c
+++ b/fs/xfs/xfs_dfrag.c
@@ -315,8 +315,7 @@ xfs_swap_extents(
 	 * are safe.  We don't really care if non-io related
 	 * fields change.
 	 */
-
-	xfs_tosspages(ip, 0, -1, FI_REMAPF);
+	truncate_pagecache_range(VFS_I(ip), 0, -1);
 
 	tp = xfs_trans_alloc(mp, XFS_TRANS_SWAPEXT);
 	if ((error = xfs_trans_reserve(tp, 0,
diff --git a/fs/xfs/xfs_fs_subr.c b/fs/xfs/xfs_fs_subr.c
index 652b875..d49de3d 100644
--- a/fs/xfs/xfs_fs_subr.c
+++ b/fs/xfs/xfs_fs_subr.c
@@ -25,18 +25,6 @@
  * note: all filemap functions return negative error codes. These
  * need to be inverted before returning to the xfs core functions.
  */
-void
-xfs_tosspages(
-	xfs_inode_t	*ip,
-	xfs_off_t	first,
-	xfs_off_t	last,
-	int		fiopt)
-{
-	/* can't toss partial tail pages, so mask them out */
-	last &= ~(PAGE_SIZE - 1);
-	truncate_inode_pages_range(VFS_I(ip)->i_mapping, first, last - 1);
-}
-
 int
 xfs_flushinval_pages(
 	xfs_inode_t	*ip,
diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index c2ddd7a..f7de578 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -2118,7 +2118,6 @@ xfs_change_file_space(
 	xfs_fsize_t	fsize;
 	int		setprealloc;
 	xfs_off_t	startoffset;
-	xfs_off_t	llen;
 	xfs_trans_t	*tp;
 	struct iattr	iattr;
 	int		prealloc_type;
@@ -2139,12 +2138,30 @@ xfs_change_file_space(
 		return XFS_ERROR(EINVAL);
 	}
 
-	llen = bf->l_len > 0 ? bf->l_len - 1 : bf->l_len;
+	/*
+	 * length of <= 0 for resv/unresv/zero is invalid.  length for
+	 * alloc/free is ignored completely and we have no idea what userspace
+	 * might have set it to, so set it to zero to allow range
+	 * checks to pass.
+	 */
+	switch (cmd) {
+	case XFS_IOC_ZERO_RANGE:
+	case XFS_IOC_RESVSP:
+	case XFS_IOC_RESVSP64:
+	case XFS_IOC_UNRESVSP:
+	case XFS_IOC_UNRESVSP64:
+		if (bf->l_len <= 0)
+			return XFS_ERROR(EINVAL);
+		break;
+	default:
+		bf->l_len = 0;
+		break;
+	}
 
 	if (bf->l_start < 0 ||
 	    bf->l_start > mp->m_super->s_maxbytes ||
-	    bf->l_start + llen < 0 ||
-	    bf->l_start + llen > mp->m_super->s_maxbytes)
+	    bf->l_start + bf->l_len < 0 ||
+	    bf->l_start + bf->l_len >= mp->m_super->s_maxbytes)
 		return XFS_ERROR(EINVAL);
 
 	bf->l_whence = 0;
@@ -2169,7 +2186,8 @@ xfs_change_file_space(
 	switch (cmd) {
 	case XFS_IOC_ZERO_RANGE:
 		prealloc_type |= XFS_BMAPI_CONVERT;
-		xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
+		truncate_pagecache_range(VFS_I(ip), startoffset,
+			 round_down(startoffset + bf->l_len, PAGE_SIZE) - 1);
 		/* FALLTHRU */
 	case XFS_IOC_RESVSP:
 	case XFS_IOC_RESVSP64:
diff --git a/fs/xfs/xfs_vnodeops.h b/fs/xfs/xfs_vnodeops.h
index 52fafc4..d48141d 100644
--- a/fs/xfs/xfs_vnodeops.h
+++ b/fs/xfs/xfs_vnodeops.h
@@ -48,8 +48,6 @@ int xfs_attr_set(struct xfs_inode *dp, const unsigned char *name,
 int xfs_attr_remove(struct xfs_inode *dp, const unsigned char *name, int flags);
 int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
 		int flags, struct attrlist_cursor_kern *cursor);
-void xfs_tosspages(struct xfs_inode *inode, xfs_off_t first,
-		xfs_off_t last, int fiopt);
 int xfs_flushinval_pages(struct xfs_inode *ip, xfs_off_t first,
 		xfs_off_t last, int fiopt);
 int xfs_flush_pages(struct xfs_inode *ip, xfs_off_t first,
-- 
1.7.10

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/4] xfs: remove xfs_wait_on_pages()

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

It's just a simple wrapper around a VFS function that is only called
by another function in xfs_fs_subr.c. Remove it and call the VFS
function directly.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_fs_subr.c  |   18 ++----------------
 fs/xfs/xfs_vnodeops.h |    1 -
 2 files changed, 2 insertions(+), 17 deletions(-)

diff --git a/fs/xfs/xfs_fs_subr.c b/fs/xfs/xfs_fs_subr.c
index d49de3d..3365823 100644
--- a/fs/xfs/xfs_fs_subr.c
+++ b/fs/xfs/xfs_fs_subr.c
@@ -62,23 +62,9 @@ xfs_flush_pages(
 				last == -1 ? LLONG_MAX : last);
 	if (flags & XBF_ASYNC)
 		return ret;
-	ret2 = xfs_wait_on_pages(ip, first, last);
+	ret2 = -filemap_fdatawait_range(mapping, first,
+				last == -1 ? XFS_ISIZE(ip) - 1 : last);
 	if (!ret)
 		ret = ret2;
 	return ret;
 }
-
-int
-xfs_wait_on_pages(
-	xfs_inode_t	*ip,
-	xfs_off_t	first,
-	xfs_off_t	last)
-{
-	struct address_space *mapping = VFS_I(ip)->i_mapping;
-
-	if (mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) {
-		return -filemap_fdatawait_range(mapping, first,
-					last == -1 ? XFS_ISIZE(ip) - 1 : last);
-	}
-	return 0;
-}
diff --git a/fs/xfs/xfs_vnodeops.h b/fs/xfs/xfs_vnodeops.h
index d48141d..c8ad48b 100644
--- a/fs/xfs/xfs_vnodeops.h
+++ b/fs/xfs/xfs_vnodeops.h
@@ -52,7 +52,6 @@ int xfs_flushinval_pages(struct xfs_inode *ip, xfs_off_t first,
 		xfs_off_t last, int fiopt);
 int xfs_flush_pages(struct xfs_inode *ip, xfs_off_t first,
 		xfs_off_t last, uint64_t flags, int fiopt);
-int xfs_wait_on_pages(struct xfs_inode *ip, xfs_off_t first, xfs_off_t last);
 
 int xfs_zero_eof(struct xfs_inode *, xfs_off_t, xfs_fsize_t);
 int xfs_free_eofblocks(struct xfs_mount *, struct xfs_inode *, bool);
-- 
1.7.10

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/4] xfs: remove xfs_flush_pages

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

It is a complex wrapper around VFS functions, but there are VFS
functions that provide exactly the same functionality. Call the VFS
functions directly and remove the unnecessary indirection and
complexity.

We don't need to care about clearing the XFS_ITRUNCATED flag, as
that is done during .writepages. Hence is cleared by the VFS
writeback path if there is anything to write back during the flush.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_aops.c     |    2 +-
 fs/xfs/xfs_bmap.c     |    2 +-
 fs/xfs/xfs_fs_subr.c  |   24 ------------------------
 fs/xfs/xfs_iops.c     |    4 ++--
 fs/xfs/xfs_vnodeops.c |    7 +++++--
 fs/xfs/xfs_vnodeops.h |    2 --
 6 files changed, 9 insertions(+), 32 deletions(-)

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index e562dd4..42ef842 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -1617,7 +1617,7 @@ xfs_vm_bmap(
 
 	trace_xfs_vm_bmap(XFS_I(inode));
 	xfs_ilock(ip, XFS_IOLOCK_SHARED);
-	xfs_flush_pages(ip, (xfs_off_t)0, -1, 0, FI_REMAPF);
+	filemap_write_and_wait(mapping);
 	xfs_iunlock(ip, XFS_IOLOCK_SHARED);
 	return generic_block_bmap(mapping, block, xfs_get_blocks);
 }
diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
index 83d0cf3..a60f3d1 100644
--- a/fs/xfs/xfs_bmap.c
+++ b/fs/xfs/xfs_bmap.c
@@ -5599,7 +5599,7 @@ xfs_getbmap(
 	xfs_ilock(ip, XFS_IOLOCK_SHARED);
 	if (whichfork == XFS_DATA_FORK && !(iflags & BMV_IF_DELALLOC)) {
 		if (ip->i_delayed_blks || XFS_ISIZE(ip) > ip->i_d.di_size) {
-			error = xfs_flush_pages(ip, 0, -1, 0, FI_REMAPF);
+			error = -filemap_write_and_wait(VFS_I(ip)->i_mapping);
 			if (error)
 				goto out_unlock_iolock;
 		}
diff --git a/fs/xfs/xfs_fs_subr.c b/fs/xfs/xfs_fs_subr.c
index 3365823..b538089 100644
--- a/fs/xfs/xfs_fs_subr.c
+++ b/fs/xfs/xfs_fs_subr.c
@@ -44,27 +44,3 @@ xfs_flushinval_pages(
 		truncate_inode_pages_range(mapping, first, last);
 	return -ret;
 }
-
-int
-xfs_flush_pages(
-	xfs_inode_t	*ip,
-	xfs_off_t	first,
-	xfs_off_t	last,
-	uint64_t	flags,
-	int		fiopt)
-{
-	struct address_space *mapping = VFS_I(ip)->i_mapping;
-	int		ret = 0;
-	int		ret2;
-
-	xfs_iflags_clear(ip, XFS_ITRUNCATED);
-	ret = -filemap_fdatawrite_range(mapping, first,
-				last == -1 ? LLONG_MAX : last);
-	if (flags & XBF_ASYNC)
-		return ret;
-	ret2 = -filemap_fdatawait_range(mapping, first,
-				last == -1 ? XFS_ISIZE(ip) - 1 : last);
-	if (!ret)
-		ret = ret2;
-	return ret;
-}
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 81f5c49..d82efaa 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -780,8 +780,8 @@ xfs_setattr_size(
 	 * care about here.
 	 */
 	if (oldsize != ip->i_d.di_size && newsize > ip->i_d.di_size) {
-		error = xfs_flush_pages(ip, ip->i_d.di_size, newsize, 0,
-					FI_NONE);
+		error = -filemap_write_and_wait_range(VFS_I(ip)->i_mapping,
+						      ip->i_d.di_size, newsize);
 		if (error)
 			goto out_unlock;
 	}
diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index f7de578..31de73e 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -428,8 +428,11 @@ xfs_release(
 		truncated = xfs_iflags_test_and_clear(ip, XFS_ITRUNCATED);
 		if (truncated) {
 			xfs_iflags_clear(ip, XFS_IDIRTY_RELEASE);
-			if (VN_DIRTY(VFS_I(ip)) && ip->i_delayed_blks > 0)
-				xfs_flush_pages(ip, 0, -1, XBF_ASYNC, FI_NONE);
+			if (VN_DIRTY(VFS_I(ip)) && ip->i_delayed_blks > 0) {
+				error = -filemap_flush(VFS_I(ip)->i_mapping);
+				if (error)
+					return error;
+			}
 		}
 	}
 
diff --git a/fs/xfs/xfs_vnodeops.h b/fs/xfs/xfs_vnodeops.h
index c8ad48b..73cb3cb 100644
--- a/fs/xfs/xfs_vnodeops.h
+++ b/fs/xfs/xfs_vnodeops.h
@@ -50,8 +50,6 @@ int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
 		int flags, struct attrlist_cursor_kern *cursor);
 int xfs_flushinval_pages(struct xfs_inode *ip, xfs_off_t first,
 		xfs_off_t last, int fiopt);
-int xfs_flush_pages(struct xfs_inode *ip, xfs_off_t first,
-		xfs_off_t last, uint64_t flags, int fiopt);
 
 int xfs_zero_eof(struct xfs_inode *, xfs_off_t, xfs_fsize_t);
 int xfs_free_eofblocks(struct xfs_mount *, struct xfs_inode *, bool);
-- 
1.7.10

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/4] xfs: remove xfs_flushinval_pages

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

It's just a simple wrapper around VFS functionality, and is actually
bugging in that it doesn't remove mappings before invalidating the
page cache. Remove it and replace it with the correct VFS
functionality.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/Makefile       |    1 -
 fs/xfs/xfs_dfrag.c    |   10 ++++------
 fs/xfs/xfs_file.c     |   23 ++++++++++++-----------
 fs/xfs/xfs_fs_subr.c  |   46 ----------------------------------------------
 fs/xfs/xfs_vnodeops.c |   11 +++++------
 fs/xfs/xfs_vnodeops.h |    2 --
 6 files changed, 21 insertions(+), 72 deletions(-)
 delete mode 100644 fs/xfs/xfs_fs_subr.c

diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
index e65357b..d02201d 100644
--- a/fs/xfs/Makefile
+++ b/fs/xfs/Makefile
@@ -37,7 +37,6 @@ xfs-y				+= xfs_aops.o \
 				   xfs_file.o \
 				   xfs_filestream.o \
 				   xfs_fsops.o \
-				   xfs_fs_subr.o \
 				   xfs_globals.o \
 				   xfs_icache.o \
 				   xfs_ioctl.o \
diff --git a/fs/xfs/xfs_dfrag.c b/fs/xfs/xfs_dfrag.c
index b2c63a2..d0e9c74 100644
--- a/fs/xfs/xfs_dfrag.c
+++ b/fs/xfs/xfs_dfrag.c
@@ -246,12 +246,10 @@ xfs_swap_extents(
 		goto out_unlock;
 	}
 
-	if (VN_CACHED(VFS_I(tip)) != 0) {
-		error = xfs_flushinval_pages(tip, 0, -1,
-				FI_REMAPF_LOCKED);
-		if (error)
-			goto out_unlock;
-	}
+	error = -filemap_write_and_wait(VFS_I(ip)->i_mapping);
+	if (error)
+		goto out_unlock;
+	truncate_pagecache_range(VFS_I(ip), 0, -1);
 
 	/* Verify O_DIRECT for ftmp */
 	if (VN_CACHED(VFS_I(tip)) != 0) {
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index daf4066..c42f99e 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -255,15 +255,14 @@ xfs_file_aio_read(
 		xfs_buftarg_t	*target =
 			XFS_IS_REALTIME_INODE(ip) ?
 				mp->m_rtdev_targp : mp->m_ddev_targp;
-		if ((iocb->ki_pos & target->bt_smask) ||
-		    (size & target->bt_smask)) {
-			if (iocb->ki_pos == i_size_read(inode))
+		if ((pos & target->bt_smask) || (size & target->bt_smask)) {
+			if (pos == i_size_read(inode))
 				return 0;
 			return -XFS_ERROR(EINVAL);
 		}
 	}
 
-	n = mp->m_super->s_maxbytes - iocb->ki_pos;
+	n = mp->m_super->s_maxbytes - pos;
 	if (n <= 0 || size == 0)
 		return 0;
 
@@ -289,20 +288,21 @@ xfs_file_aio_read(
 		xfs_rw_ilock(ip, XFS_IOLOCK_EXCL);
 
 		if (inode->i_mapping->nrpages) {
-			ret = -xfs_flushinval_pages(ip,
-					(iocb->ki_pos & PAGE_CACHE_MASK),
-					-1, FI_REMAPF_LOCKED);
+			ret = -filemap_write_and_wait_range(
+							VFS_I(ip)->i_mapping,
+							pos, -1);
 			if (ret) {
 				xfs_rw_iunlock(ip, XFS_IOLOCK_EXCL);
 				return ret;
 			}
+			truncate_pagecache_range(VFS_I(ip), pos, -1);
 		}
 		xfs_rw_ilock_demote(ip, XFS_IOLOCK_EXCL);
 	}
 
-	trace_xfs_file_read(ip, size, iocb->ki_pos, ioflags);
+	trace_xfs_file_read(ip, size, pos, ioflags);
 
-	ret = generic_file_aio_read(iocb, iovp, nr_segs, iocb->ki_pos);
+	ret = generic_file_aio_read(iocb, iovp, nr_segs, pos);
 	if (ret > 0)
 		XFS_STATS_ADD(xs_read_bytes, ret);
 
@@ -670,10 +670,11 @@ xfs_file_dio_aio_write(
 		goto out;
 
 	if (mapping->nrpages) {
-		ret = -xfs_flushinval_pages(ip, (pos & PAGE_CACHE_MASK), -1,
-							FI_REMAPF_LOCKED);
+		ret = -filemap_write_and_wait_range(VFS_I(ip)->i_mapping,
+						    pos, -1);
 		if (ret)
 			goto out;
+		truncate_pagecache_range(VFS_I(ip), pos, -1);
 	}
 
 	/*
diff --git a/fs/xfs/xfs_fs_subr.c b/fs/xfs/xfs_fs_subr.c
deleted file mode 100644
index b538089..0000000
--- a/fs/xfs/xfs_fs_subr.c
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- * Copyright (c) 2000-2002,2005-2006 Silicon Graphics, Inc.
- * All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write the Free Software Foundation,
- * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- */
-#include "xfs.h"
-#include "xfs_vnodeops.h"
-#include "xfs_bmap_btree.h"
-#include "xfs_inode.h"
-#include "xfs_trace.h"
-
-/*
- * note: all filemap functions return negative error codes. These
- * need to be inverted before returning to the xfs core functions.
- */
-int
-xfs_flushinval_pages(
-	xfs_inode_t	*ip,
-	xfs_off_t	first,
-	xfs_off_t	last,
-	int		fiopt)
-{
-	struct address_space *mapping = VFS_I(ip)->i_mapping;
-	int		ret = 0;
-
-	trace_xfs_pagecache_inval(ip, first, last);
-
-	xfs_iflags_clear(ip, XFS_ITRUNCATED);
-	ret = filemap_write_and_wait_range(mapping, first,
-				last == -1 ? LLONG_MAX : last);
-	if (!ret)
-		truncate_inode_pages_range(mapping, first, last);
-	return -ret;
-}
diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index 31de73e..165cb92 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -1958,12 +1958,11 @@ xfs_free_file_space(
 
 	rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
 	ioffset = offset & ~(rounding - 1);
-
-	if (VN_CACHED(VFS_I(ip)) != 0) {
-		error = xfs_flushinval_pages(ip, ioffset, -1, FI_REMAPF_LOCKED);
-		if (error)
-			goto out_unlock_iolock;
-	}
+	error = -filemap_write_and_wait_range(VFS_I(ip)->i_mapping,
+					      ioffset, -1);
+	if (error)
+		goto out_unlock_iolock;
+	truncate_pagecache_range(VFS_I(ip), ioffset, -1);
 
 	/*
 	 * Need to zero the stuff we're not freeing, on disk.
diff --git a/fs/xfs/xfs_vnodeops.h b/fs/xfs/xfs_vnodeops.h
index 73cb3cb..91a03fa 100644
--- a/fs/xfs/xfs_vnodeops.h
+++ b/fs/xfs/xfs_vnodeops.h
@@ -48,8 +48,6 @@ int xfs_attr_set(struct xfs_inode *dp, const unsigned char *name,
 int xfs_attr_remove(struct xfs_inode *dp, const unsigned char *name, int flags);
 int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
 		int flags, struct attrlist_cursor_kern *cursor);
-int xfs_flushinval_pages(struct xfs_inode *ip, xfs_off_t first,
-		xfs_off_t last, int fiopt);
 
 int xfs_zero_eof(struct xfs_inode *, xfs_off_t, xfs_fsize_t);
 int xfs_free_eofblocks(struct xfs_mount *, struct xfs_inode *, bool);
-- 
1.7.10

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/4] xfs: fs/xfs/xfs_fs_subr.c die die die

[Ben Myers]

Hey Dave,

On Fri, Nov 09, 2012 at 09:10:00PM +1100, Dave Chinner wrote:
> This patch series replaces all the functions in fs/xfs/xfs_fs_subr.c
> with direct calls to their VFS equivalents. This fixes bugs and
> removes code at the same time.
> 
> I haven't bothered to change the ranges passed to the functions -
> that requires a lot more scrutiny and testing - the ranges have now
> may not be optimal but they are a known factor. The changes have
> passed smoke testing on several different configurations and about 6
> hours of continuous fsx testing, which is what I'd expect seeing as
> I have changed the ranges at all.

Thanks for the quick turnaround on this.  It looks like this will indeed be
appropriate for -stable.  I'd like to have seen Andrew go through the whole
process on this bug.  Maybe next time.  We still have the xfstest to do I
guess.  Putting this series on the 3.8 queue.

Regards,
	Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/4] xfs: remove xfs_tosspages

[Andrew Dahl]

On 11/09/2012 04:10 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> It's a buggy, unnecessary wrapper that is duplicating
> truncate_pagecache_range().
> 
> When replacing the call in xfs_change_file_space(), also ensure that
> the length being allocated/freed is always positive before making
> any changes. These checks are done in the lower extent manipulation
> functions, too, but we need to do them before any page cache
> operations.
> 
> Reported-by: Andrew Dahl <adahl@sgi.com>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/xfs_dfrag.c    |    3 +--
>  fs/xfs/xfs_fs_subr.c  |   12 ------------
>  fs/xfs/xfs_vnodeops.c |   28 +++++++++++++++++++++++-----
>  fs/xfs/xfs_vnodeops.h |    2 --
>  4 files changed, 24 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/xfs/xfs_dfrag.c b/fs/xfs/xfs_dfrag.c
> index b9b8646..b2c63a2 100644
> --- a/fs/xfs/xfs_dfrag.c
> +++ b/fs/xfs/xfs_dfrag.c
> @@ -315,8 +315,7 @@ xfs_swap_extents(
>  	 * are safe.  We don't really care if non-io related
>  	 * fields change.
>  	 */
> -
> -	xfs_tosspages(ip, 0, -1, FI_REMAPF);
> +	truncate_pagecache_range(VFS_I(ip), 0, -1);
>  
>  	tp = xfs_trans_alloc(mp, XFS_TRANS_SWAPEXT);
>  	if ((error = xfs_trans_reserve(tp, 0,
> diff --git a/fs/xfs/xfs_fs_subr.c b/fs/xfs/xfs_fs_subr.c
> index 652b875..d49de3d 100644
> --- a/fs/xfs/xfs_fs_subr.c
> +++ b/fs/xfs/xfs_fs_subr.c
> @@ -25,18 +25,6 @@
>   * note: all filemap functions return negative error codes. These
>   * need to be inverted before returning to the xfs core functions.
>   */
> -void
> -xfs_tosspages(
> -	xfs_inode_t	*ip,
> -	xfs_off_t	first,
> -	xfs_off_t	last,
> -	int		fiopt)
> -{
> -	/* can't toss partial tail pages, so mask them out */
> -	last &= ~(PAGE_SIZE - 1);
> -	truncate_inode_pages_range(VFS_I(ip)->i_mapping, first, last - 1);
> -}
> -
>  int
>  xfs_flushinval_pages(
>  	xfs_inode_t	*ip,
> diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> index c2ddd7a..f7de578 100644
> --- a/fs/xfs/xfs_vnodeops.c
> +++ b/fs/xfs/xfs_vnodeops.c
> @@ -2118,7 +2118,6 @@ xfs_change_file_space(
>  	xfs_fsize_t	fsize;
>  	int		setprealloc;
>  	xfs_off_t	startoffset;
> -	xfs_off_t	llen;
>  	xfs_trans_t	*tp;
>  	struct iattr	iattr;
>  	int		prealloc_type;
> @@ -2139,12 +2138,30 @@ xfs_change_file_space(
>  		return XFS_ERROR(EINVAL);
>  	}
>  
> -	llen = bf->l_len > 0 ? bf->l_len - 1 : bf->l_len;
> +	/*
> +	 * length of <= 0 for resv/unresv/zero is invalid.  length for
> +	 * alloc/free is ignored completely and we have no idea what userspace
> +	 * might have set it to, so set it to zero to allow range
> +	 * checks to pass.
> +	 */
> +	switch (cmd) {
> +	case XFS_IOC_ZERO_RANGE:
> +	case XFS_IOC_RESVSP:
> +	case XFS_IOC_RESVSP64:
> +	case XFS_IOC_UNRESVSP:
> +	case XFS_IOC_UNRESVSP64:
> +		if (bf->l_len <= 0)
> +			return XFS_ERROR(EINVAL);
> +		break;
> +	default:
> +		bf->l_len = 0;
> +		break;
> +	}
>  
>  	if (bf->l_start < 0 ||
>  	    bf->l_start > mp->m_super->s_maxbytes ||
> -	    bf->l_start + llen < 0 ||
> -	    bf->l_start + llen > mp->m_super->s_maxbytes)
> +	    bf->l_start + bf->l_len < 0 ||
> +	    bf->l_start + bf->l_len >= mp->m_super->s_maxbytes)
>  		return XFS_ERROR(EINVAL);
>  
>  	bf->l_whence = 0;
> @@ -2169,7 +2186,8 @@ xfs_change_file_space(
>  	switch (cmd) {
>  	case XFS_IOC_ZERO_RANGE:
>  		prealloc_type |= XFS_BMAPI_CONVERT;
> -		xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
> +		truncate_pagecache_range(VFS_I(ip), startoffset,
> +			 round_down(startoffset + bf->l_len, PAGE_SIZE) - 1);

When calling XFS_IOC_ZERO_RANGE with a range [0, 4095] or [0,1] it's
tossing pages because the call to round_down() is returning 0 and
passing -1 in for the end will toss all pages.  So, we need to make sure
round_down() isn't going to return a 0, or that (startoffset +
bf->l_len) > PAGE_SIZE.

So, something like...


xfs_off_t       end;

[...]

if((end = round_down(startoffset + bf->l_len, PAGE_SIZE)) > 0) {
	truncate_pagecache_range(VFS_I(ip), startoffset, end - 1);
}


>  		/* FALLTHRU */
>  	case XFS_IOC_RESVSP:
>  	case XFS_IOC_RESVSP64:
> diff --git a/fs/xfs/xfs_vnodeops.h b/fs/xfs/xfs_vnodeops.h
> index 52fafc4..d48141d 100644
> --- a/fs/xfs/xfs_vnodeops.h
> +++ b/fs/xfs/xfs_vnodeops.h
> @@ -48,8 +48,6 @@ int xfs_attr_set(struct xfs_inode *dp, const unsigned char *name,
>  int xfs_attr_remove(struct xfs_inode *dp, const unsigned char *name, int flags);
>  int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
>  		int flags, struct attrlist_cursor_kern *cursor);
> -void xfs_tosspages(struct xfs_inode *inode, xfs_off_t first,
> -		xfs_off_t last, int fiopt);
>  int xfs_flushinval_pages(struct xfs_inode *ip, xfs_off_t first,
>  		xfs_off_t last, int fiopt);
>  int xfs_flush_pages(struct xfs_inode *ip, xfs_off_t first,
> 

Beyond that one change, it looks good!

-Andrew

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/4] xfs: remove xfs_wait_on_pages()

[Andrew Dahl]

On 11/09/2012 04:10 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> It's just a simple wrapper around a VFS function that is only called
> by another function in xfs_fs_subr.c. Remove it and call the VFS
> function directly.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/xfs_fs_subr.c  |   18 ++----------------
>  fs/xfs/xfs_vnodeops.h |    1 -
>  2 files changed, 2 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/xfs/xfs_fs_subr.c b/fs/xfs/xfs_fs_subr.c
> index d49de3d..3365823 100644
> --- a/fs/xfs/xfs_fs_subr.c
> +++ b/fs/xfs/xfs_fs_subr.c
> @@ -62,23 +62,9 @@ xfs_flush_pages(
>  				last == -1 ? LLONG_MAX : last);
>  	if (flags & XBF_ASYNC)
>  		return ret;
> -	ret2 = xfs_wait_on_pages(ip, first, last);
> +	ret2 = -filemap_fdatawait_range(mapping, first,
> +				last == -1 ? XFS_ISIZE(ip) - 1 : last);
>  	if (!ret)
>  		ret = ret2;
>  	return ret;
>  }
> -
> -int
> -xfs_wait_on_pages(
> -	xfs_inode_t	*ip,
> -	xfs_off_t	first,
> -	xfs_off_t	last)
> -{
> -	struct address_space *mapping = VFS_I(ip)->i_mapping;
> -
> -	if (mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) {
> -		return -filemap_fdatawait_range(mapping, first,
> -					last == -1 ? XFS_ISIZE(ip) - 1 : last);
> -	}
> -	return 0;
> -}
> diff --git a/fs/xfs/xfs_vnodeops.h b/fs/xfs/xfs_vnodeops.h
> index d48141d..c8ad48b 100644
> --- a/fs/xfs/xfs_vnodeops.h
> +++ b/fs/xfs/xfs_vnodeops.h
> @@ -52,7 +52,6 @@ int xfs_flushinval_pages(struct xfs_inode *ip, xfs_off_t first,
>  		xfs_off_t last, int fiopt);
>  int xfs_flush_pages(struct xfs_inode *ip, xfs_off_t first,
>  		xfs_off_t last, uint64_t flags, int fiopt);
> -int xfs_wait_on_pages(struct xfs_inode *ip, xfs_off_t first, xfs_off_t last);
>  
>  int xfs_zero_eof(struct xfs_inode *, xfs_off_t, xfs_fsize_t);
>  int xfs_free_eofblocks(struct xfs_mount *, struct xfs_inode *, bool);
> 

Looks good!

Reviewed-by: Andrew Dahl <adahl@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/4] xfs: remove xfs_flush_pages

[Andrew Dahl]

On 11/09/2012 04:10 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> It is a complex wrapper around VFS functions, but there are VFS
> functions that provide exactly the same functionality. Call the VFS
> functions directly and remove the unnecessary indirection and
> complexity.
> 
> We don't need to care about clearing the XFS_ITRUNCATED flag, as
> that is done during .writepages. Hence is cleared by the VFS
> writeback path if there is anything to write back during the flush.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/xfs_aops.c     |    2 +-
>  fs/xfs/xfs_bmap.c     |    2 +-
>  fs/xfs/xfs_fs_subr.c  |   24 ------------------------
>  fs/xfs/xfs_iops.c     |    4 ++--
>  fs/xfs/xfs_vnodeops.c |    7 +++++--
>  fs/xfs/xfs_vnodeops.h |    2 --
>  6 files changed, 9 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index e562dd4..42ef842 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -1617,7 +1617,7 @@ xfs_vm_bmap(
>  
>  	trace_xfs_vm_bmap(XFS_I(inode));
>  	xfs_ilock(ip, XFS_IOLOCK_SHARED);
> -	xfs_flush_pages(ip, (xfs_off_t)0, -1, 0, FI_REMAPF);
> +	filemap_write_and_wait(mapping);
>  	xfs_iunlock(ip, XFS_IOLOCK_SHARED);
>  	return generic_block_bmap(mapping, block, xfs_get_blocks);
>  }
> diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
> index 83d0cf3..a60f3d1 100644
> --- a/fs/xfs/xfs_bmap.c
> +++ b/fs/xfs/xfs_bmap.c
> @@ -5599,7 +5599,7 @@ xfs_getbmap(
>  	xfs_ilock(ip, XFS_IOLOCK_SHARED);
>  	if (whichfork == XFS_DATA_FORK && !(iflags & BMV_IF_DELALLOC)) {
>  		if (ip->i_delayed_blks || XFS_ISIZE(ip) > ip->i_d.di_size) {
> -			error = xfs_flush_pages(ip, 0, -1, 0, FI_REMAPF);
> +			error = -filemap_write_and_wait(VFS_I(ip)->i_mapping);
>  			if (error)
>  				goto out_unlock_iolock;
>  		}
> diff --git a/fs/xfs/xfs_fs_subr.c b/fs/xfs/xfs_fs_subr.c
> index 3365823..b538089 100644
> --- a/fs/xfs/xfs_fs_subr.c
> +++ b/fs/xfs/xfs_fs_subr.c
> @@ -44,27 +44,3 @@ xfs_flushinval_pages(
>  		truncate_inode_pages_range(mapping, first, last);
>  	return -ret;
>  }
> -
> -int
> -xfs_flush_pages(
> -	xfs_inode_t	*ip,
> -	xfs_off_t	first,
> -	xfs_off_t	last,
> -	uint64_t	flags,
> -	int		fiopt)
> -{
> -	struct address_space *mapping = VFS_I(ip)->i_mapping;
> -	int		ret = 0;
> -	int		ret2;
> -
> -	xfs_iflags_clear(ip, XFS_ITRUNCATED);
> -	ret = -filemap_fdatawrite_range(mapping, first,
> -				last == -1 ? LLONG_MAX : last);
> -	if (flags & XBF_ASYNC)
> -		return ret;
> -	ret2 = -filemap_fdatawait_range(mapping, first,
> -				last == -1 ? XFS_ISIZE(ip) - 1 : last);
> -	if (!ret)
> -		ret = ret2;
> -	return ret;
> -}
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 81f5c49..d82efaa 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -780,8 +780,8 @@ xfs_setattr_size(
>  	 * care about here.
>  	 */
>  	if (oldsize != ip->i_d.di_size && newsize > ip->i_d.di_size) {
> -		error = xfs_flush_pages(ip, ip->i_d.di_size, newsize, 0,
> -					FI_NONE);
> +		error = -filemap_write_and_wait_range(VFS_I(ip)->i_mapping,
> +						      ip->i_d.di_size, newsize);
>  		if (error)
>  			goto out_unlock;
>  	}
> diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> index f7de578..31de73e 100644
> --- a/fs/xfs/xfs_vnodeops.c
> +++ b/fs/xfs/xfs_vnodeops.c
> @@ -428,8 +428,11 @@ xfs_release(
>  		truncated = xfs_iflags_test_and_clear(ip, XFS_ITRUNCATED);
>  		if (truncated) {
>  			xfs_iflags_clear(ip, XFS_IDIRTY_RELEASE);
> -			if (VN_DIRTY(VFS_I(ip)) && ip->i_delayed_blks > 0)
> -				xfs_flush_pages(ip, 0, -1, XBF_ASYNC, FI_NONE);
> +			if (VN_DIRTY(VFS_I(ip)) && ip->i_delayed_blks > 0) {
> +				error = -filemap_flush(VFS_I(ip)->i_mapping);
> +				if (error)
> +					return error;
> +			}
>  		}
>  	}
>  
> diff --git a/fs/xfs/xfs_vnodeops.h b/fs/xfs/xfs_vnodeops.h
> index c8ad48b..73cb3cb 100644
> --- a/fs/xfs/xfs_vnodeops.h
> +++ b/fs/xfs/xfs_vnodeops.h
> @@ -50,8 +50,6 @@ int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
>  		int flags, struct attrlist_cursor_kern *cursor);
>  int xfs_flushinval_pages(struct xfs_inode *ip, xfs_off_t first,
>  		xfs_off_t last, int fiopt);
> -int xfs_flush_pages(struct xfs_inode *ip, xfs_off_t first,
> -		xfs_off_t last, uint64_t flags, int fiopt);
>  
>  int xfs_zero_eof(struct xfs_inode *, xfs_off_t, xfs_fsize_t);
>  int xfs_free_eofblocks(struct xfs_mount *, struct xfs_inode *, bool);
> 

Looks good!

Reviewed-by: Andrew Dahl <adahl@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 4/4] xfs: remove xfs_flushinval_pages

[Andrew Dahl]

On 11/09/2012 04:10 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> It's just a simple wrapper around VFS functionality, and is actually
> bugging in that it doesn't remove mappings before invalidating the
> page cache. Remove it and replace it with the correct VFS
> functionality.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Looks Good!

Reviewed-by: Andrew Dahl <adahl@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/4] xfs: remove xfs_tosspages

[Dave Chinner]

On Mon, Nov 12, 2012 at 02:43:03PM -0600, Andrew Dahl wrote:
> On 11/09/2012 04:10 AM, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > It's a buggy, unnecessary wrapper that is duplicating
> > truncate_pagecache_range().
> > 
> > When replacing the call in xfs_change_file_space(), also ensure that
> > the length being allocated/freed is always positive before making
> > any changes. These checks are done in the lower extent manipulation
> > functions, too, but we need to do them before any page cache
> > operations.
> > 
> > Reported-by: Andrew Dahl <adahl@sgi.com>
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
....
> > @@ -2169,7 +2186,8 @@ xfs_change_file_space(
> >  	switch (cmd) {
> >  	case XFS_IOC_ZERO_RANGE:
> >  		prealloc_type |= XFS_BMAPI_CONVERT;
> > -		xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
> > +		truncate_pagecache_range(VFS_I(ip), startoffset,
> > +			 round_down(startoffset + bf->l_len, PAGE_SIZE) - 1);
> 
> When calling XFS_IOC_ZERO_RANGE with a range [0, 4095] or [0,1] it's
> tossing pages because the call to round_down() is returning 0 and
> passing -1 in for the end will toss all pages.  So, we need to make sure
> round_down() isn't going to return a 0, or that (startoffset +
> bf->l_len) > PAGE_SIZE.

Right, which was the original bug. I didn't think that through
fully....

> So, something like...
> 
> 
> xfs_off_t       end;
> 
> [...]
> 
> if((end = round_down(startoffset + bf->l_len, PAGE_SIZE)) > 0) {
> 	truncate_pagecache_range(VFS_I(ip), startoffset, end - 1);
> }

Actually, it is more complex than that. Think of the range [4096,
4097]. That would result in passing 4096, 4095 to
truncate_pagecache_range(), which is also wrong but implicitly
handled correctly in truncate_inode_pages_range(). Hence it should
be:

		end = round_down(startoffset + bf->l_len, PAGE_SIZE) - 1;
		if (startoffset > end)
			truncate_pagecache_range(VFS_I(ip), startoffset, end);

That's similar to your original fix, which had a last < first check
in it. I'll post a fix in a few minutes...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/4] xfs: remove xfs_tosspages

[Ben Myers]

Hi Dave,

On Tue, Nov 13, 2012 at 10:00:29AM +1100, Dave Chinner wrote:
> On Mon, Nov 12, 2012 at 02:43:03PM -0600, Andrew Dahl wrote:
> > On 11/09/2012 04:10 AM, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > It's a buggy, unnecessary wrapper that is duplicating
> > > truncate_pagecache_range().
> > > 
> > > When replacing the call in xfs_change_file_space(), also ensure that
> > > the length being allocated/freed is always positive before making
> > > any changes. These checks are done in the lower extent manipulation
> > > functions, too, but we need to do them before any page cache
> > > operations.
> > > 
> > > Reported-by: Andrew Dahl <adahl@sgi.com>
> > > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ....
> > > @@ -2169,7 +2186,8 @@ xfs_change_file_space(
> > >  	switch (cmd) {
> > >  	case XFS_IOC_ZERO_RANGE:
> > >  		prealloc_type |= XFS_BMAPI_CONVERT;
> > > -		xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
> > > +		truncate_pagecache_range(VFS_I(ip), startoffset,
> > > +			 round_down(startoffset + bf->l_len, PAGE_SIZE) - 1);
> > 
> > When calling XFS_IOC_ZERO_RANGE with a range [0, 4095] or [0,1] it's
> > tossing pages because the call to round_down() is returning 0 and
> > passing -1 in for the end will toss all pages.  So, we need to make sure
> > round_down() isn't going to return a 0, or that (startoffset +
> > bf->l_len) > PAGE_SIZE.
> 
> Right, which was the original bug. I didn't think that through
> fully....
> 
> > So, something like...
> > 
> > 
> > xfs_off_t       end;
> > 
> > [...]
> > 
> > if((end = round_down(startoffset + bf->l_len, PAGE_SIZE)) > 0) {
> > 	truncate_pagecache_range(VFS_I(ip), startoffset, end - 1);
> > }
> 
> Actually, it is more complex than that. Think of the range [4096,
> 4097]. That would result in passing 4096, 4095 to
> truncate_pagecache_range(), which is also wrong but implicitly
> handled correctly in truncate_inode_pages_range(). Hence it should
> be:
> 
> 		end = round_down(startoffset + bf->l_len, PAGE_SIZE) - 1;
> 		if (startoffset > end)
> 			truncate_pagecache_range(VFS_I(ip), startoffset, end);
> 
> That's similar to your original fix, which had a last < first check
> in it. I'll post a fix in a few minutes...

I'd like to pull this in tomorrow.  Could you get this one sorted out today?
If not, I think we could also enlist Andrew to fix it.

Regards,
	Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs