Ocfs2 devel - May 2015 - [PATCH v2] ocfs2: do not BUG if jbd2_journal_dirty_metadata fails

jbd2_journal_dirty_metadata may fail. Currently it cannot take care of
non zero return value and just BUG in ocfs2_journal_dirty.
This patch is aborting the handle and journal instead of BUG.

Signed-off-by: Joseph Qi <joseph.qi at huawei.com>
Cc: joyce.xue <xuejiufei at huawei.com>
---
 fs/ocfs2/journal.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index ff53192..eefca1e 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -775,7 +775,17 @@ void ocfs2_journal_dirty(handle_t *handle, struct
buffer_head *bh)
 	trace_ocfs2_journal_dirty((unsigned long long)bh->b_blocknr);

 	status = jbd2_journal_dirty_metadata(handle, bh);
-	BUG_ON(status);
+	if (status) {
+		mlog_errno(status);
+		if (!is_handle_aborted(handle)) {
+			journal_t *journal = handle->h_transaction->t_journal;
+
+			mlog(ML_ERROR, "jbd2_journal_dirty_metadata failed. "
+					"Aborting transaction and journal.");
+			handle->h_err = status;
+			jbd2_journal_abort_handle(handle);
+			jbd2_journal_abort(journal, status);
+		}
+	}
 }

 #define OCFS2_DEFAULT_COMMIT_INTERVAL	(HZ * JBD2_DEFAULT_MAX_COMMIT_AGE)
-- 
1.8.4.3

On Tue, 12 May 2015 09:53:47 +0800 Joseph Qi <joseph.qi at huawei.com>
wrote:
> jbd2_journal_dirty_metadata may fail. Currently it cannot take care of
> non zero return value and just BUG in ocfs2_journal_dirty.
> This patch is aborting the handle and journal instead of BUG.
> 
This patch is internally inconsistent.
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -775,7 +775,17 @@ void ocfs2_journal_dirty(handle_t *handle, struct
buffer_head *bh)
The "-775,7 +775,17" means "the 7 lines at line 775 get turned
into 17
lines".  ie: 10 lines are added.
>  	trace_ocfs2_journal_dirty((unsigned long long)bh->b_blocknr);
> 
>  	status = jbd2_journal_dirty_metadata(handle, bh);
> -	BUG_ON(status);
> +	if (status) {
> +		mlog_errno(status);
> +		if (!is_handle_aborted(handle)) {
> +			journal_t *journal = handle->h_transaction->t_journal;
> +
> +			mlog(ML_ERROR, "jbd2_journal_dirty_metadata failed. "
> +					"Aborting transaction and journal.");
> +			handle->h_err = status;
> +			jbd2_journal_abort_handle(handle);
> +			jbd2_journal_abort(journal, status);
> +		}
> +	}
>  }
But the patch deletes 1 line and adds 12, for a net addition of 11 lines,
not 10.

So the patch doesn't apply.  Changing the "17" to "18"
fixes that.


Whatever tool you used for generating this patch needs slapping.

On 05/12/2015 09:53 AM, Joseph Qi wrote:
> jbd2_journal_dirty_metadata may fail. Currently it cannot take care of
> non zero return value and just BUG in ocfs2_journal_dirty.
> This patch is aborting the handle and journal instead of BUG.
> 
> Signed-off-by: Joseph Qi <joseph.qi at huawei.com>
> Cc: joyce.xue <xuejiufei at huawei.com>
> ---
>  fs/ocfs2/journal.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index ff53192..eefca1e 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -775,7 +775,17 @@ void ocfs2_journal_dirty(handle_t *handle, struct
buffer_head *bh)
>  	trace_ocfs2_journal_dirty((unsigned long long)bh->b_blocknr);
> 
>  	status = jbd2_journal_dirty_metadata(handle, bh);
> -	BUG_ON(status);
> +	if (status) {
> +		mlog_errno(status);
> +		if (!is_handle_aborted(handle)) {
> +			journal_t *journal = handle->h_transaction->t_journal;
> +
> +			mlog(ML_ERROR, "jbd2_journal_dirty_metadata failed. "
> +					"Aborting transaction and journal.");
> +			handle->h_err = status;
> +			jbd2_journal_abort_handle(handle);
> +			jbd2_journal_abort(journal, status);
Let fs go after journal lose affect seemed not safe, may we set fs
read-only here?

Thanks,
Junxiao.

> +		}
> +	}
>  }
> 
>  #define OCFS2_DEFAULT_COMMIT_INTERVAL	(HZ * JBD2_DEFAULT_MAX_COMMIT_AGE)
>