Ocfs2 devel - Aug 2015 - [patch 10/28] ocfs2: do not set fs read-only if rec[0] is empty while committing truncate

From: Xue jiufei <xuejiufei at huawei.com>
Subject: ocfs2: do not set fs read-only if rec[0] is empty while committing
truncate

While appending an extent to a file, it will call these functions:
ocfs2_insert_extent

  -> call ocfs2_grow_tree() if there's no free rec
     -> ocfs2_add_branch add a new branch to extent tree,
        now rec[0] in the leaf of rightmost path is empty
  -> ocfs2_do_insert_extent
     -> ocfs2_rotate_tree_right
       -> ocfs2_extend_rotate_transaction
          -> jbd2_journal_restart if jbd2_journal_extend fail
     -> ocfs2_insert_path
        -> ocfs2_extend_trans
          -> jbd2_journal_restart if jbd2_journal_extend fail
        -> ocfs2_insert_at_leaf
     -> ocfs2_et_update_clusters
Function jbd2_journal_restart() may be called and it may happened that
buffers dirtied in ocfs2_add_branch() are committed
while buffers dirtied in ocfs2_insert_at_leaf() and
ocfs2_et_update_clusters() are not.
So an empty rec[0] is left in rightmost path which will cause
read-only filesystem when call ocfs2_commit_truncate()
with the error message: "Inode %lu has an empty extent record".

This is not a serious problem, so remove the rightmost path when call
ocfs2_commit_truncate().

Signed-off-by: joyce.xue <xuejiufei at huawei.com>
Cc: Mark Fasheh <mfasheh at suse.com>
Cc: Joel Becker <jlbec at evilplan.org>
Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
---

 fs/ocfs2/alloc.c |   44 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 6 deletions(-)

diff -puN
fs/ocfs2/alloc.c~ocfs2-do-not-set-fs-read-only-if-rec-is-empty-while-committing-truncate
fs/ocfs2/alloc.c
---
a/fs/ocfs2/alloc.c~ocfs2-do-not-set-fs-read-only-if-rec-is-empty-while-committing-truncate
+++ a/fs/ocfs2/alloc.c
@@ -3104,6 +3104,30 @@ out:
 	return ret;
 }
 
+static int ocfs2_remove_rightmost_empty_extent(struct ocfs2_super *osb,
+				struct ocfs2_extent_tree *et,
+				struct ocfs2_path *path,
+				struct ocfs2_cached_dealloc_ctxt *dealloc)
+{
+	handle_t *handle;
+	int ret;
+	int credits = path->p_tree_depth * 2 + 1;
+
+	handle = ocfs2_start_trans(osb, credits);
+	if (IS_ERR(handle)) {
+		ret = PTR_ERR(handle);
+		mlog_errno(ret);
+		return ret;
+	}
+
+	ret = ocfs2_remove_rightmost_path(handle, et, path, dealloc);
+	if (ret)
+		mlog_errno(ret);
+
+	ocfs2_commit_trans(osb, handle);
+	return ret;
+}
+
 /*
  * Left rotation of btree records.
  *
@@ -7134,15 +7158,23 @@ start:
 		 * to check it up here before changing the tree.
 		*/
 		if (root_el->l_tree_depth && rec->e_int_clusters == 0) {
-			ocfs2_error(inode->i_sb, "Inode %lu has an empty "
+			mlog(ML_ERROR, "Inode %lu has an empty "
 				    "extent record, depth %u\n", inode->i_ino,
 				    le16_to_cpu(root_el->l_tree_depth));
-			status = -EROFS;
-			goto bail;
+			status = ocfs2_remove_rightmost_empty_extent(osb,
+					&et, path, &dealloc);
+			if (status) {
+				mlog_errno(status);
+				goto bail;
+			}
+
+			ocfs2_reinit_path(path, 1);
+			goto start;
+		} else {
+			trunc_cpos = le32_to_cpu(rec->e_cpos);
+			trunc_len = 0;
+			blkno = 0;
 		}
-		trunc_cpos = le32_to_cpu(rec->e_cpos);
-		trunc_len = 0;
-		blkno = 0;
 	} else if (le32_to_cpu(rec->e_cpos) >= new_highest_cpos) {
 		/*
 		 * Truncate entire record.
_

On Wed, Aug 26, 2015 at 03:11:46PM -0700, Andrew Morton
wrote:
> From: Xue jiufei <xuejiufei at huawei.com>
> Subject: ocfs2: do not set fs read-only if rec[0] is empty while committing
truncate
> 
> While appending an extent to a file, it will call these functions:
> ocfs2_insert_extent
> 
>   -> call ocfs2_grow_tree() if there's no free rec
>      -> ocfs2_add_branch add a new branch to extent tree,
>         now rec[0] in the leaf of rightmost path is empty
>   -> ocfs2_do_insert_extent
>      -> ocfs2_rotate_tree_right
>        -> ocfs2_extend_rotate_transaction
>           -> jbd2_journal_restart if jbd2_journal_extend fail
>      -> ocfs2_insert_path
>         -> ocfs2_extend_trans
>           -> jbd2_journal_restart if jbd2_journal_extend fail
>         -> ocfs2_insert_at_leaf
>      -> ocfs2_et_update_clusters
> Function jbd2_journal_restart() may be called and it may happened that
> buffers dirtied in ocfs2_add_branch() are committed
> while buffers dirtied in ocfs2_insert_at_leaf() and
> ocfs2_et_update_clusters() are not.
> So an empty rec[0] is left in rightmost path which will cause
> read-only filesystem when call ocfs2_commit_truncate()
> with the error message: "Inode %lu has an empty extent record".
> 
> This is not a serious problem, so remove the rightmost path when call
> ocfs2_commit_truncate().
> 
> Signed-off-by: joyce.xue <xuejiufei at huawei.com>
> Cc: Mark Fasheh <mfasheh at suse.com>
> Cc: Joel Becker <jlbec at evilplan.org>
> Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
Reviewed-by: Mark Fasheh <mfasheh at suse.de>



--
Mark Fasheh