xfs: remove xfs_itruncate_data

This wrapper isn't overly useful, not to say rather confusing.

Around the call to xfs_itruncate_extents it does:

 - add tracing
 - add a few asserts in debug builds
 - conditionally update the inode size in two places
 - log the inode

Both the tracing and the inode logging can be moved to xfs_itruncate_extents
as they are useful for the attribute fork as well - in fact the attr code
already does an equivalent xfs_trans_log_inode call just after calling
xfs_itruncate_extents.  The conditional size updates are a mess, and there
was no reason to do them in two places anyway, as the first one was
conditional on the inode having extents - but without extents we
xfs_itruncate_extents would be a no-op and the placement wouldn't matter
anyway.  Instead move the size assignments and the asserts that make sense
to the callers that want it.

As a side effect of this clean up xfs_setattr_size by introducing variables
for the old and new inode size, and moving the size updates into a common
place.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ben Myers <bpm@sgi.com>

1 files changed, 30 insertions, 17 deletions

diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index f9babd179223..f02eaa298d3c 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -750,6 +750,7 @@ xfs_setattr_size(
         struct xfs_mount        *mp = ip->i_mount;
         struct inode            *inode = VFS_I(ip);
         int                     mask = iattr->ia_valid;
+        xfs_off_t               oldsize, newsize;
         struct xfs_trans        *tp;
         int                     error;
         uint                    lock_flags;
@@ -777,11 +778,13 @@ xfs_setattr_size(
                 lock_flags |= XFS_IOLOCK_EXCL;
         xfs_ilock(ip, lock_flags);
 
+        oldsize = ip->i_size;
+        newsize = iattr->ia_size;
+
         /*
          * Short circuit the truncate case for zero length files.
          */
-        if (iattr->ia_size == 0 &&
-            ip->i_size == 0 && ip->i_d.di_nextents == 0) {
+        if (newsize == 0 && oldsize == 0 && ip->i_d.di_nextents == 0) {
                 if (!(mask & (ATTR_CTIME|ATTR_MTIME)))
                         goto out_unlock;
 
@@ -807,14 +810,14 @@ xfs_setattr_size(
          * the inode to the transaction, because the inode cannot be unlocked
          * once it is a part of the transaction.
          */
-        if (iattr->ia_size > ip->i_size) {
+        if (newsize > oldsize) {
                 /*
                  * Do the first part of growing a file: zero any data in the
                  * last block that is beyond the old EOF.  We need to do this
                  * before the inode is joined to the transaction to modify
                  * i_size.
                  */
-                error = xfs_zero_eof(ip, iattr->ia_size, ip->i_size);
+                error = xfs_zero_eof(ip, newsize, oldsize);
                 if (error)
                         goto out_unlock;
         }
@@ -833,8 +836,8 @@ xfs_setattr_size(
          * here and prevents waiting for other data not within the range we
          * care about here.
          */
-        if (ip->i_size != ip->i_d.di_size && iattr->ia_size > ip->i_d.di_size) {
-                error = xfs_flush_pages(ip, ip->i_d.di_size, iattr->ia_size, 0,
+        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);
                 if (error)
                         goto out_unlock;
@@ -845,8 +848,7 @@ xfs_setattr_size(
          */
         inode_dio_wait(inode);
 
-        error = -block_truncate_page(inode->i_mapping, iattr->ia_size,
-                                     xfs_get_blocks);
+        error = -block_truncate_page(inode->i_mapping, newsize, xfs_get_blocks);
         if (error)
                 goto out_unlock;
 
@@ -857,7 +859,7 @@ xfs_setattr_size(
         if (error)
                 goto out_trans_cancel;
 
-        truncate_setsize(inode, iattr->ia_size);
+        truncate_setsize(inode, newsize);
 
         commit_flags = XFS_TRANS_RELEASE_LOG_RES;
         lock_flags |= XFS_ILOCK_EXCL;
@@ -876,19 +878,30 @@ xfs_setattr_size(
          * these flags set.  For all other operations the VFS set these flags
          * explicitly if it wants a timestamp update.
          */
-        if (iattr->ia_size != ip->i_size &&
-            (!(mask & (ATTR_CTIME | ATTR_MTIME)))) {
+        if (newsize != oldsize && (!(mask & (ATTR_CTIME | ATTR_MTIME)))) {
                 iattr->ia_ctime = iattr->ia_mtime =
                         current_fs_time(inode->i_sb);
                 mask |= ATTR_CTIME | ATTR_MTIME;
         }
 
-        if (iattr->ia_size > ip->i_size) {
-                ip->i_d.di_size = iattr->ia_size;
-                ip->i_size = iattr->ia_size;
-        } else if (iattr->ia_size <= ip->i_size ||
-                   (iattr->ia_size == 0 && ip->i_d.di_nextents)) {
-                error = xfs_itruncate_data(&tp, ip, iattr->ia_size);
+        /*
+         * The first thing we do is set the size to new_size permanently on
+         * disk.  This way we don't have to worry about anyone ever being able
+         * to look at the data being freed even in the face of a crash.
+         * What we're getting around here is the case where we free a block, it
+         * is allocated to another file, it is written to, and then we crash.
+         * If the new data gets written to the file but the log buffers
+         * containing the free and reallocation don't, then we'd end up with
+         * garbage in the blocks being freed.  As long as we make the new size
+         * permanent before actually freeing any blocks it doesn't matter if
+         * they get written to.
+         */
+        ip->i_d.di_size = newsize;
+        ip->i_size = newsize;
+        xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+
+        if (newsize <= oldsize) {
+                error = xfs_itruncate_extents(&tp, ip, XFS_DATA_FORK, newsize);
                 if (error)
                         goto out_trans_abort;