4 files changed, 58 insertions, 47 deletions

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 04e79d57bca6..e9d401ce93bb 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -574,8 +574,8 @@ xfs_attr_shortform_add(xfs_da_args_t *args, int forkoff)
  * After the last attribute is removed revert to original inode format,
  * making all literal area available to the data fork once more.
  */
-STATIC void
-xfs_attr_fork_reset(
+void
+xfs_attr_fork_remove(
         struct xfs_inode        *ip,
         struct xfs_trans        *tp)
 {
@@ -641,7 +641,7 @@ xfs_attr_shortform_remove(xfs_da_args_t *args)
             (mp->m_flags & XFS_MOUNT_ATTR2) &&
             (dp->i_d.di_format != XFS_DINODE_FMT_BTREE) &&
             !(args->op_flags & XFS_DA_OP_ADDNAME)) {
-                xfs_attr_fork_reset(dp, args->trans);
+                xfs_attr_fork_remove(dp, args->trans);
         } else {
                 xfs_idata_realloc(dp, -size, XFS_ATTR_FORK);
                 dp->i_d.di_forkoff = xfs_attr_shortform_bytesfit(dp, totsize);
@@ -905,7 +905,7 @@ xfs_attr3_leaf_to_shortform(
         if (forkoff == -1) {
                 ASSERT(dp->i_mount->m_flags & XFS_MOUNT_ATTR2);
                 ASSERT(dp->i_d.di_format != XFS_DINODE_FMT_BTREE);
-                xfs_attr_fork_reset(dp, args->trans);
+                xfs_attr_fork_remove(dp, args->trans);
                 goto out;
         }
 
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
index 025c4b820c03..882c8d338891 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.h
+++ b/fs/xfs/libxfs/xfs_attr_leaf.h
@@ -53,7 +53,7 @@ int	xfs_attr_shortform_remove(struct xfs_da_args *args);
 int     xfs_attr_shortform_list(struct xfs_attr_list_context *context);
 int     xfs_attr_shortform_allfit(struct xfs_buf *bp, struct xfs_inode *dp);
 int     xfs_attr_shortform_bytesfit(xfs_inode_t *dp, int bytes);
-
+void    xfs_attr_fork_remove(struct xfs_inode *ip, struct xfs_trans *tp);
 
 /*
  * Internal routines when attribute fork size == XFS_LBSIZE(mp).
diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index f9c1c64782d3..3fbf167cfb4c 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -380,23 +380,31 @@ xfs_attr3_root_inactive(
         return error;
 }
 
+/*
+ * xfs_attr_inactive kills all traces of an attribute fork on an inode. It
+ * removes both the on-disk and in-memory inode fork. Note that this also has to
+ * handle the condition of inodes without attributes but with an attribute fork
+ * configured, so we can't use xfs_inode_hasattr() here.
+ *
+ * The in-memory attribute fork is removed even on error.
+ */
 int
-xfs_attr_inactive(xfs_inode_t *dp)
+xfs_attr_inactive(
+        struct xfs_inode        *dp)
 {
-        xfs_trans_t *trans;
-        xfs_mount_t *mp;
-        int error;
+        struct xfs_trans        *trans;
+        struct xfs_mount        *mp;
+        int                     cancel_flags = 0;
+        int                     lock_mode = XFS_ILOCK_SHARED;
+        int                     error = 0;
 
         mp = dp->i_mount;
         ASSERT(! XFS_NOT_DQATTACHED(mp, dp));
 
-        xfs_ilock(dp, XFS_ILOCK_SHARED);
-        if (!xfs_inode_hasattr(dp) ||
-            dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
-                xfs_iunlock(dp, XFS_ILOCK_SHARED);
-                return 0;
-        }
-        xfs_iunlock(dp, XFS_ILOCK_SHARED);
+        xfs_ilock(dp, lock_mode);
+        if (!XFS_IFORK_Q(dp))
+                goto out_destroy_fork;
+        xfs_iunlock(dp, lock_mode);
 
         /*
          * Start our first transaction of the day.
@@ -408,13 +416,18 @@ xfs_attr_inactive(xfs_inode_t *dp)
          * the inode in every transaction to let it float upward through
          * the log.
          */
+        lock_mode = 0;
         trans = xfs_trans_alloc(mp, XFS_TRANS_ATTRINVAL);
         error = xfs_trans_reserve(trans, &M_RES(mp)->tr_attrinval, 0, 0);
-        if (error) {
-                xfs_trans_cancel(trans, 0);
-                return error;
-        }
-        xfs_ilock(dp, XFS_ILOCK_EXCL);
+        if (error)
+                goto out_cancel;
+
+        lock_mode = XFS_ILOCK_EXCL;
+        cancel_flags = XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT;
+        xfs_ilock(dp, lock_mode);
+
+        if (!XFS_IFORK_Q(dp))
+                goto out_cancel;
 
         /*
          * No need to make quota reservations here. We expect to release some
@@ -422,29 +435,31 @@ xfs_attr_inactive(xfs_inode_t *dp)
          */
         xfs_trans_ijoin(trans, dp, 0);
 
-        /*
-         * Decide on what work routines to call based on the inode size.
-         */
-        if (!xfs_inode_hasattr(dp) ||
-            dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
-                error = 0;
-                goto out;
+        /* invalidate and truncate the attribute fork extents */
+        if (dp->i_d.di_aformat != XFS_DINODE_FMT_LOCAL) {
+                error = xfs_attr3_root_inactive(&trans, dp);
+                if (error)
+                        goto out_cancel;
+
+                error = xfs_itruncate_extents(&trans, dp, XFS_ATTR_FORK, 0);
+                if (error)
+                        goto out_cancel;
         }
-        error = xfs_attr3_root_inactive(&trans, dp);
-        if (error)
-                goto out;
 
-        error = xfs_itruncate_extents(&trans, dp, XFS_ATTR_FORK, 0);
-        if (error)
-                goto out;
+        /* Reset the attribute fork - this also destroys the in-core fork */
+        xfs_attr_fork_remove(dp, trans);
 
         error = xfs_trans_commit(trans, XFS_TRANS_RELEASE_LOG_RES);
-        xfs_iunlock(dp, XFS_ILOCK_EXCL);
-
+        xfs_iunlock(dp, lock_mode);
         return error;
 
-out:
-        xfs_trans_cancel(trans, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_ABORT);
-        xfs_iunlock(dp, XFS_ILOCK_EXCL);
+out_cancel:
+        xfs_trans_cancel(trans, cancel_flags);
+out_destroy_fork:
+        /* kill the in-core attr fork before we drop the inode lock */
+        if (dp->i_afp)
+                xfs_idestroy_fork(dp, XFS_ATTR_FORK);
+        if (lock_mode)
+                xfs_iunlock(dp, lock_mode);
         return error;
 }
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index d6ebc85192b7..1117dd3ba123 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1946,21 +1946,17 @@ xfs_inactive(
         /*
          * If there are attributes associated with the file then blow them away
          * now.  The code calls a routine that recursively deconstructs the
-         * attribute fork.  We need to just commit the current transaction
-         * because we can't use it for xfs_attr_inactive().
+         * attribute fork. If also blows away the in-core attribute fork.
          */
-        if (ip->i_d.di_anextents > 0) {
-                ASSERT(ip->i_d.di_forkoff != 0);
-
+        if (XFS_IFORK_Q(ip)) {
                 error = xfs_attr_inactive(ip);
                 if (error)
                         return;
         }
 
-        if (ip->i_afp)
-                xfs_idestroy_fork(ip, XFS_ATTR_FORK);
-
+        ASSERT(!ip->i_afp);
         ASSERT(ip->i_d.di_anextents == 0);
+        ASSERT(ip->i_d.di_forkoff == 0);
 
         /*
          * Free the inode.