[PATCH v2] ocfs2: free unused clusters on defrag move errors

[Guangshuo Li]

ocfs2_defrag_extent() claims new clusters before calling
__ocfs2_move_extent(). If the move fails before the extent tree update
is attempted, the claimed clusters are not referenced by the inode and
must be released.

The current error path only logs the error and continues to
ocfs2_cow_sync_writeback(), which can overwrite the original error with
zero. The claimed clusters are also left allocated.

Not every __ocfs2_move_extent() error can free the new clusters,
however. Once ocfs2_split_extent() has been called, the extent tree may
already reference them even if a later operation fails. Freeing them in
that case would leave the extent tree pointing to clusters marked free.

Track whether the extent split has been started. On errors before that
point, report that the new clusters remain unused so the defrag caller
can release them. Once the split has started, leave the clusters
allocated. Return all move errors through the transaction cleanup path
without allowing writeback to overwrite them.

Fixes: 202ee5facb2c ("Ocfs2/move_extents: defrag a range of extent.")
Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
---
v2:
  - Do not free the new clusters after the extent-tree update has
    started, as pointed out by Joseph Qi.
  - Track whether the new clusters remain unused on error.
  - Preserve __ocfs2_move_extent() errors instead of allowing the
    subsequent writeback call to overwrite them.

 fs/ocfs2/move_extents.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c
index ad1678ee7cc4..138c65d21668 100644
--- a/fs/ocfs2/move_extents.c
+++ b/fs/ocfs2/move_extents.c
@@ -49,9 +49,10 @@ struct ocfs2_move_extents_context {
 static int __ocfs2_move_extent(handle_t *handle,
 			       struct ocfs2_move_extents_context *context,
 			       u32 cpos, u32 len, u32 p_cpos, u32 new_p_cpos,
-			       int ext_flags)
+			       int ext_flags, bool *new_clusters_unused)
 {
 	int ret = 0, index;
+	bool split_started = false;
 	struct inode *inode = context->inode;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	struct ocfs2_extent_rec *rec, replace_rec;
@@ -59,6 +60,10 @@ static int __ocfs2_move_extent(handle_t *handle,
 	struct ocfs2_extent_list *el;
 	u64 ino = ocfs2_metadata_cache_owner(context->et.et_ci);
 	u64 old_blkno = ocfs2_clusters_to_blocks(inode->i_sb, p_cpos);
+	bool split_started = false;
+
+	if (new_clusters_unused)
+		*new_clusters_unused = false;
 
 	ret = ocfs2_duplicate_clusters_by_page(handle, inode, cpos,
 					       p_cpos, new_p_cpos, len);
@@ -111,6 +116,7 @@ static int __ocfs2_move_extent(handle_t *handle,
 	 */
 	replace_rec.e_flags = ext_flags & ~OCFS2_EXT_REFCOUNTED;
 
+	split_started = true;
 	ret = ocfs2_split_extent(handle, &context->et, path, index,
 				 &replace_rec, context->meta_ac,
 				 &context->dealloc);
@@ -138,6 +144,9 @@ static int __ocfs2_move_extent(handle_t *handle,
 
 	ocfs2_update_inode_fsync_trans(handle, inode, 0);
 out:
+	if (ret && !split_started && new_clusters_unused)
+		*new_clusters_unused = true;
+
 	ocfs2_free_path(path);
 	return ret;
 }
@@ -209,6 +218,7 @@ static int ocfs2_defrag_extent(struct ocfs2_move_extents_context *context,
 	struct ocfs2_refcount_tree *ref_tree = NULL;
 	u32 new_phys_cpos, new_len;
 	u64 phys_blkno = ocfs2_clusters_to_blocks(inode->i_sb, phys_cpos);
+	bool new_clusters_unused;
 	int need_free = 0;
 
 	if ((ext_flags & OCFS2_EXT_REFCOUNTED) && *len) {
@@ -309,9 +319,14 @@ static int ocfs2_defrag_extent(struct ocfs2_move_extents_context *context,
 	     phys_cpos, new_phys_cpos);
 
 	ret = __ocfs2_move_extent(handle, context, cpos, new_len, phys_cpos,
-				  new_phys_cpos, ext_flags);
-	if (ret)
+				  new_phys_cpos, ext_flags,
+				  &new_clusters_unused);
+	if (ret) {
 		mlog_errno(ret);
+		if (new_clusters_unused)
+			need_free = 1;
+		goto out_commit;
+	}
 
 	if (partial && (new_len != *len))
 		*len = new_len;
@@ -684,7 +699,7 @@ static int ocfs2_move_extent(struct ocfs2_move_extents_context *context,
 	}
 
 	ret = __ocfs2_move_extent(handle, context, cpos, len, phys_cpos,
-				  *new_phys_cpos, ext_flags);
+				  *new_phys_cpos, ext_flags, NULL);
 	if (ret) {
 		mlog_errno(ret);
 		goto out_commit;
-- 
2.43.0