ocfs2: do not BUG if jbd2_journal_dirty_metadata fails

PATCH New Joseph Qi 2 participants 4 messages Apr 28, 2015 – May 11, 2015 OCFS2

Share & tools

Joseph Qi

author

2015-04-28 12:23 UTC #0 lore raw

PATCH 1/1 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 instead of BUG.

Signed-off-by Joseph Qi <[email protected]>

---
 fs/ocfs2/journal.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

fs/ocfs2/journal.c +9 -1

index ff53192..4482420 100644
@@ -775,7 +775,15 @@ 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)) {
+			handle->h_err = status;
+			mlog(ML_ERROR, "jbd2_journal_dirty_metadata failed. "
+					"Aborting transaction.");
+			jbd2_journal_abort_handle(handle);
+		}
+	}
 }
 
 #define OCFS2_DEFAULT_COMMIT_INTERVAL	(HZ * JBD2_DEFAULT_MAX_COMMIT_AGE)

Signature · 1 line

1.8.4.3

Xue jiufei

2015-04-29 02:28 UTC #1 lore raw

PATCH 1/1 ocfs2: do not BUG if jbd2_journal_dirty_metadata fails

Hi Joseph,

Quoted 15 lines · Joseph Qi

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 instead of BUG.
Signed-off-by Joseph Qi <[email protected]>
---
 fs/ocfs2/journal.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
fs/ocfs2/journal.c +0 -0
index ff53192..4482420 100644
@@ -775,7 +775,15 @@ void ocfs2_journal_dirty(handle_t *handle, struct buffer_head *bh)
 	trace_ocfs2_journal_dirty((unsigned long long)bh->b_blocknr);

fs/ocfs2/journal.c void ocfs2_journal_dirty(handle_t *handle, struct buffer_head *bh) · line 784

Quoted 10 lines

fs/ocfs2/journal.c +7 -1

 
 	status = jbd2_journal_dirty_metadata(handle, bh);
-	BUG_ON(status);
+	if (status) {
+		mlog_errno(status);
+		if (!is_handle_aborted(handle)) {
+			handle->h_err = status;
+			mlog(ML_ERROR, "jbd2_journal_dirty_metadata failed. "
+					"Aborting transaction.");
+			jbd2_journal_abort_handle(handle);

The buffers dirtied before are still committed to disk while handle is aborted and may cause some inconsistency.
Maybe the journal should also be set aborted if jbd2_journal_dirty_metadata fails like ext4?

Quoted 6 lines

fs/ocfs2/journal.c +2 -0

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

Joseph Qi

author

2015-04-29 06:18 UTC #2 lore raw

PATCH 1/1 ocfs2: do not BUG if jbd2_journal_dirty_metadata fails

Quoted 18 lines · Xue jiufei

Hi Joseph,
Quoted 16 lines · Joseph Qi
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 instead of BUG.
Signed-off-by Joseph Qi <[email protected]>
---
 fs/ocfs2/journal.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
fs/ocfs2/journal.c +0 -0
index ff53192..4482420 100644
@@ -775,7 +775,15 @@ void ocfs2_journal_dirty(handle_t *handle, struct buffer_head *bh)
 	trace_ocfs2_journal_dirty((unsigned long long)bh->b_blocknr);
 

Quoted 13 lines

fs/ocfs2/journal.c void ocfs2_journal_dirty(handle_t *handle, struct buffer_head *bh) · line 784
Quoted 9 lines
fs/ocfs2/journal.c +7 -1
 	status = jbd2_journal_dirty_metadata(handle, bh);
-	BUG_ON(status);
+	if (status) {
+		mlog_errno(status);
+		if (!is_handle_aborted(handle)) {
+			handle->h_err = status;
+			mlog(ML_ERROR, "jbd2_journal_dirty_metadata failed. "
+					"Aborting transaction.");
+			jbd2_journal_abort_handle(handle);
The buffers dirtied before are still committed to disk while handle is aborted and may cause some inconsistency.
Maybe the journal should also be set aborted if jbd2_journal_dirty_metadata fails like ext4?

You are right. I'll take this and send a new version. Thanks.
--Joseph

Quoted 11 lines

fs/ocfs2/journal.c void ocfs2_journal_dirty(handle_t *handle, struct buffer_head *bh) · line 789
Quoted 6 lines
fs/ocfs2/journal.c +2 -0
+		}
+	}
 }
 
 #define OCFS2_DEFAULT_COMMIT_INTERVAL	(HZ * JBD2_DEFAULT_MAX_COMMIT_AGE)
.

Joseph Qi

author

2015-05-11 07:48 UTC #3 lore raw

PATCH 1/1 ocfs2: do not BUG if jbd2_journal_dirty_metadata fails

In reply to Xue jiufei

Quoted 18 lines · Xue jiufei

Hi Joseph,
Quoted 16 lines · Joseph Qi
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 instead of BUG.
Signed-off-by Joseph Qi <[email protected]>
---
 fs/ocfs2/journal.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
fs/ocfs2/journal.c +0 -0
index ff53192..4482420 100644
@@ -775,7 +775,15 @@ void ocfs2_journal_dirty(handle_t *handle, struct buffer_head *bh)
 	trace_ocfs2_journal_dirty((unsigned long long)bh->b_blocknr);
 

Quoted 13 lines

fs/ocfs2/journal.c void ocfs2_journal_dirty(handle_t *handle, struct buffer_head *bh) · line 784
Quoted 9 lines
fs/ocfs2/journal.c +7 -1
 	status = jbd2_journal_dirty_metadata(handle, bh);
-	BUG_ON(status);
+	if (status) {
+		mlog_errno(status);
+		if (!is_handle_aborted(handle)) {
+			handle->h_err = status;
+			mlog(ML_ERROR, "jbd2_journal_dirty_metadata failed. "
+					"Aborting transaction.");
+			jbd2_journal_abort_handle(handle);
The buffers dirtied before are still committed to disk while handle is aborted and may cause some inconsistency.
Maybe the journal should also be set aborted if jbd2_journal_dirty_metadata fails like ext4?

Maybe aborting the handle and journal is also not enough. Though the journal is aborted, the caller still returns success. Then the user space thinks the operation is successful, but the metadata cannot be committed to disk due to aborted journal.
Do we also have to return the error in ocfs2_journal_dirty and then handle it in caller? If so, it must be huge work:)

Quoted 11 lines

fs/ocfs2/journal.c void ocfs2_journal_dirty(handle_t *handle, struct buffer_head *bh) · line 789
Quoted 6 lines
fs/ocfs2/journal.c +2 -0
+		}
+	}
 }
 
 #define OCFS2_DEFAULT_COMMIT_INTERVAL	(HZ * JBD2_DEFAULT_MAX_COMMIT_AGE)
.