xfs: cancel dfops on xfs_defer_finish() error

Source kernel commit: 9b1f4e9831df29776031e86e112e68784f1fc079

The current semantics of xfs_defer_finish() require the caller to
call xfs_defer_cancel() on error. This is slightly inconsistent with
transaction commit error handling where a failed commit cleans up
the transaction before returning.

More significantly, the only requirement for exposure of
->dop_pending outside of xfs_defer_finish() is so that
xfs_defer_cancel() can drain it on error. Since the only recourse of
xfs_defer_finish() errors is cancellation, mirror the transaction
logic and cancel remaining dfops before returning from
xfs_defer_finish() with an error.

Beside simplifying xfs_defer_finish() semantics, this ensures that
xfs_defer_finish() always returns with an empty ->dop_pending and
thus facilitates removal of the list from xfs_defer_ops.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

diff --git a/libxfs/trans.c b/libxfs/trans.c
index 01a531de108214768c7a095a30a13544edae7bf5..4121af59814e12335708157f2aaff72a5ed4901d 100644 (file)
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -999,10 +999,8 @@ __xfs_trans_commit(
                     !(tp->t_flags & XFS_TRANS_PERM_LOG_RES));
        if (!regrant && (tp->t_flags & XFS_TRANS_PERM_LOG_RES)) {
                error = xfs_defer_finish_noroll(&tp);
-               if (error) {
-                       xfs_defer_cancel(tp);
+               if (error)
                        goto out_unreserve;
-               }
        }
 
        if (!(tp->t_flags & XFS_TRANS_DIRTY)) {

diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index 0e20d56147676d211167227f11c03eaaf2feb169..bbfe4ee0f59ddbf0916f9f1f3e69dbc8fe16f70c 100644 (file)
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -585,7 +585,7 @@ xfs_attr_leaf_addname(
                        goto out_defer_cancel;
                error = xfs_defer_finish(&args->trans);
                if (error)
-                       goto out_defer_cancel;
+                       return error;
 
                /*
                 * Commit the current trans (including the inode) and start
@@ -673,7 +673,7 @@ xfs_attr_leaf_addname(
                                goto out_defer_cancel;
                        error = xfs_defer_finish(&args->trans);
                        if (error)
-                               goto out_defer_cancel;
+                               return error;
                }
 
                /*
@@ -736,7 +736,7 @@ xfs_attr_leaf_removename(
                        goto out_defer_cancel;
                error = xfs_defer_finish(&args->trans);
                if (error)
-                       goto out_defer_cancel;
+                       return error;
        }
        return 0;
 out_defer_cancel:
@@ -862,7 +862,7 @@ restart:
                                goto out_defer_cancel;
                        error = xfs_defer_finish(&args->trans);
                        if (error)
-                               goto out_defer_cancel;
+                               goto out;
 
                        /*
                         * Commit the node conversion and start the next
@@ -886,7 +886,7 @@ restart:
                        goto out_defer_cancel;
                error = xfs_defer_finish(&args->trans);
                if (error)
-                       goto out_defer_cancel;
+                       goto out;
        } else {
                /*
                 * Addition succeeded, update Btree hashvals.
@@ -982,7 +982,7 @@ restart:
                                goto out_defer_cancel;
                        error = xfs_defer_finish(&args->trans);
                        if (error)
-                               goto out_defer_cancel;
+                               goto out;
                }
 
                /*
@@ -1105,7 +1105,7 @@ xfs_attr_node_removename(
                        goto out_defer_cancel;
                error = xfs_defer_finish(&args->trans);
                if (error)
-                       goto out_defer_cancel;
+                       goto out;
                /*
                 * Commit the Btree join operation and start a new trans.
                 */
@@ -1136,7 +1136,7 @@ xfs_attr_node_removename(
                                goto out_defer_cancel;
                        error = xfs_defer_finish(&args->trans);
                        if (error)
-                               goto out_defer_cancel;
+                               goto out;
                } else
                        xfs_trans_brelse(args->trans, bp);
        }

diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index faebeed4e5944ebcd491a956a7ce39fa1861e815..1d078efe7c689ff304f16d7f8e0f7d7229cd1e0a 100644 (file)
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -483,7 +483,7 @@ xfs_attr_rmtval_set(
                        goto out_defer_cancel;
                error = xfs_defer_finish(&args->trans);
                if (error)
-                       goto out_defer_cancel;
+                       return error;
 
                ASSERT(nmap == 1);
                ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
@@ -623,7 +623,7 @@ xfs_attr_rmtval_remove(
                        goto out_defer_cancel;
                error = xfs_defer_finish(&args->trans);
                if (error)
-                       goto out_defer_cancel;
+                       return error;
 
                /*
                 * Close out trans and start the next one in the chain.

diff --git a/libxfs/xfs_defer.c b/libxfs/xfs_defer.c
index 136f2974d027c847883da733ce329e1d4a0235e4..c8d35982247ca780f6b3855eddf3dca54d744aa8 100644 (file)
--- a/libxfs/xfs_defer.c
+++ b/libxfs/xfs_defer.c
@@ -414,14 +414,15 @@ xfs_defer_finish_noroll(
        }
 
 out:
-       if (error)
+       if (error) {
                trace_xfs_defer_finish_error((*tp)->t_mountp, (*tp)->t_dfops,
                                             error);
-        else
-               trace_xfs_defer_finish_done((*tp)->t_mountp, (*tp)->t_dfops,
-                                           _RET_IP_);
+               xfs_defer_cancel(*tp);
+               return error;
+       }
 
-       return error;
+       trace_xfs_defer_finish_done((*tp)->t_mountp, (*tp)->t_dfops, _RET_IP_);
+       return 0;
 }
 
 int