[PATCH 1/2] xfs: hole-punch use truncate_pagecache_range

[PATCH 1/2] xfs: hole-punch use truncate_pagecache_range

[Hugh Dickins]

When truncating a file, we unmap pages from userspace first, as that's
usually more efficient than relying, page by page, on the fallback in
truncate_inode_page() - particularly if the file is mapped many times.

Do the same when punching a hole: 3.4 added truncate_pagecache_range()
to do the unmap and trunc, so use it in xfs_flushinval_pages(), instead
of calling truncate_inode_pages_range() directly.

Should xfs_tosspages() be using it too?  I don't know: left unchanged.

Signed-off-by: Hugh Dickins <hughd@google.com>
---

 fs/xfs/xfs_fs_subr.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- next-20120511/fs/xfs/xfs_fs_subr.c	2012-01-17 20:42:07.879627688 -0800
+++ linux/fs/xfs/xfs_fs_subr.c	2012-05-12 18:01:14.988654723 -0700
@@ -53,7 +53,7 @@ xfs_flushinval_pages(
 	ret = filemap_write_and_wait_range(mapping, first,
 				last == -1 ? LLONG_MAX : last);
 	if (!ret)
-		truncate_inode_pages_range(mapping, first, last);
+		truncate_pagecache_range(VFS_I(ip), first, last);
 	return -ret;
 }
 

[PATCH 2/2] xfs: hole-punch retaining cache beyond

[Hugh Dickins]

xfs has a very inefficient hole-punch implementation, invalidating all
the cache beyond the hole (after flushing dirty back to disk, from which
all must be read back if wanted again).  So if you punch a hole in a
file mlock()ed into userspace, pages beyond the hole are inadvertently
munlock()ed until they are touched again.

Is there a strong internal reason why that has to be so on xfs?
Or is it just a relic from xfs supporting XFS_IOC_UNRESVSP long
before Linux 2.6.16 provided truncate_inode_pages_range()?

If the latter, then this patch mostly fixes it, by passing the proper
range to xfs_flushinval_pages().  But a little more should be done to
get it just right: a partial page on either side of the hole is still
written back to disk, invalidated and munlocked.

Signed-off-by: Hugh Dickins <hughd@google.com>
---

 fs/xfs/xfs_vnodeops.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

--- next-20120511/fs/xfs/xfs_vnodeops.c	2012-05-11 00:22:26.095158149 -0700
+++ linux/fs/xfs/xfs_vnodeops.c	2012-05-12 18:01:14.988654723 -0700
@@ -2040,7 +2040,8 @@ xfs_free_file_space(
 	xfs_fsblock_t		firstfsb;
 	xfs_bmap_free_t		free_list;
 	xfs_bmbt_irec_t		imap;
-	xfs_off_t		ioffset;
+	xfs_off_t		startoffset;
+	xfs_off_t		endoffset;
 	xfs_extlen_t		mod=0;
 	xfs_mount_t		*mp;
 	int			nimap;
@@ -2074,11 +2075,18 @@ xfs_free_file_space(
 		inode_dio_wait(VFS_I(ip));
 	}
 
+	/*
+	 * Round startoffset down and endoffset up: we write out any dirty
+	 * blocks in between before truncating, so we can read partial blocks
+	 * back from disk afterwards (but that may munlock the partial pages).
+	 */
 	rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
-	ioffset = offset & ~(rounding - 1);
+	startoffset = round_down(offset, rounding);
+	endoffset = round_up(offset + len, rounding) - 1;
 
 	if (VN_CACHED(VFS_I(ip)) != 0) {
-		error = xfs_flushinval_pages(ip, ioffset, -1, FI_REMAPF_LOCKED);
+		error = xfs_flushinval_pages(ip, startoffset, endoffset,
+							FI_REMAPF_LOCKED);
 		if (error)
 			goto out_unlock_iolock;
 	}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] xfs: hole-punch use truncate_pagecache_range

[Christoph Hellwig]

On Sun, May 13, 2012 at 01:50:06PM -0700, Hugh Dickins wrote:
> When truncating a file, we unmap pages from userspace first, as that's
> usually more efficient than relying, page by page, on the fallback in
> truncate_inode_page() - particularly if the file is mapped many times.
> 
> Do the same when punching a hole: 3.4 added truncate_pagecache_range()
> to do the unmap and trunc, so use it in xfs_flushinval_pages(), instead
> of calling truncate_inode_pages_range() directly.

This change looks fine.

> Should xfs_tosspages() be using it too?  I don't know: left unchanged.

I'll look at it.  I've been planning to simplify and/or kill the
xfs_fs_subr.c wrappers which tend to confuse the code for a while now,
and deciding what exactly to do should be a fallout from that.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] xfs: hole-punch retaining cache beyond

[Christoph Hellwig]

On Sun, May 13, 2012 at 01:51:18PM -0700, Hugh Dickins wrote:
> xfs has a very inefficient hole-punch implementation, invalidating all
> the cache beyond the hole (after flushing dirty back to disk, from which
> all must be read back if wanted again).  So if you punch a hole in a
> file mlock()ed into userspace, pages beyond the hole are inadvertently
> munlock()ed until they are touched again.
> 
> Is there a strong internal reason why that has to be so on xfs?
> Or is it just a relic from xfs supporting XFS_IOC_UNRESVSP long
> before Linux 2.6.16 provided truncate_inode_pages_range()?
> 
> If the latter, then this patch mostly fixes it, by passing the proper
> range to xfs_flushinval_pages().  But a little more should be done to
> get it just right: a partial page on either side of the hole is still
> written back to disk, invalidated and munlocked.

I think the original reason is that no range version of the macros
existed.  Giving the somewhat odd calling convention I'd prefer to
simplify deprecate the old wrappers and convert the callers to direct
calls of the VM functions on a 1 by 1 basis.