Ocfs2 devel - May 2023 - [PATCH 2/2] ocfs2: add error handling path when jbd2 enter ABORT status

fstest generic cases 347 361 628 629 trigger a same issue:
When jbd2 enter ABORT status, ocfs2 ignores it and keep going to commit
journal.

This commit gives ocfs2 ability to handle jbd2 ABORT case.

Signed-off-by: Heming Zhao <heming.zhao at suse.com>
---
 fs/ocfs2/alloc.c      | 10 ++++++----
 fs/ocfs2/journal.c    |  5 +++++
 fs/ocfs2/localalloc.c |  3 +++
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index 51c93929a146..d90961a1c433 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -6308,11 +6308,13 @@ void ocfs2_truncate_log_shutdown(struct ocfs2_super
*osb)
 
 	if (tl_inode) {
 		cancel_delayed_work(&osb->osb_truncate_log_wq);
-		flush_workqueue(osb->ocfs2_wq);
+		if (!is_journal_aborted(osb->journal->j_journal)) {
+			flush_workqueue(osb->ocfs2_wq);
 
-		status = ocfs2_flush_truncate_log(osb);
-		if (status < 0)
-			mlog_errno(status);
+			status = ocfs2_flush_truncate_log(osb);
+			if (status < 0)
+				mlog_errno(status);
+		}
 
 		brelse(osb->osb_tl_bh);
 		iput(osb->osb_tl_inode);
diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 25d8072ccfce..2798067a2f66 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -312,11 +312,16 @@ static int ocfs2_commit_cache(struct ocfs2_super *osb)
 	status = jbd2_journal_flush(journal->j_journal, 0);
 	jbd2_journal_unlock_updates(journal->j_journal);
 	if (status < 0) {
+		if (is_journal_aborted(journal->j_journal)) {
+			ocfs2_error(osb->sb, "jbd2 status: ABORT.\n");
+			goto reset;
+		}
 		up_write(&journal->j_trans_barrier);
 		mlog_errno(status);
 		goto finally;
 	}
 
+reset:
 	ocfs2_inc_trans_id(journal);
 
 	flushed = atomic_read(&journal->j_num_trans);
diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
index c4426d12a2ad..e2e3400717b0 100644
--- a/fs/ocfs2/localalloc.c
+++ b/fs/ocfs2/localalloc.c
@@ -378,6 +378,9 @@ void ocfs2_shutdown_local_alloc(struct ocfs2_super *osb)
 	if (osb->ocfs2_wq)
 		flush_workqueue(osb->ocfs2_wq);
 
+	if (is_journal_aborted(osb->journal->j_journal))
+		goto out;
+
 	if (osb->local_alloc_state == OCFS2_LA_UNUSED)
 		goto out;
 
-- 
2.39.2

More info for this patch, maybe I should write below info in commit log.

 From the comment of __ocfs2_abort(), there is another way to handle

journal ABORT: panic.
And fstest generic test NO. 648 will trigger
  ocfs2_abort then make kernel
panic. I don't like the panic style, ocfs2
  should elegantly handle journal
abnormal case.

Thanks,
Heming

On 4/30/23 11:13 AM, Heming Zhao wrote:
> fstest generic cases 347 361 628 629 trigger a same issue:
> When jbd2 enter ABORT status, ocfs2 ignores it and keep going to commit
> journal.
> 
> This commit gives ocfs2 ability to handle jbd2 ABORT case.
> 
> Signed-off-by: Heming Zhao <heming.zhao at suse.com>
> ---
>   fs/ocfs2/alloc.c      | 10 ++++++----
>   fs/ocfs2/journal.c    |  5 +++++
>   fs/ocfs2/localalloc.c |  3 +++
>   3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c

(sorry, my last mail format is by Thunderbird, resend by neomutt.)

----
More info for this patch, maybe I should write below info in commit log.
>From the comment of __ocfs2_abort(), there is another way to handle
journal ABORT: panic.
And fstest generic test NO. 648 will trigger ocfs2_abort then make
kernel panic. I don't like the panic style, ocfs2 should elegantly
handle journal abnormal case.

Thanks,
Heming

On Sun, Apr 30, 2023 at 11:13:02AM +0800, Heming Zhao
wrote:
> fstest generic cases 347 361 628 629 trigger a same issue:
> When jbd2 enter ABORT status, ocfs2 ignores it and keep going to commit
> journal.
> 
> This commit gives ocfs2 ability to handle jbd2 ABORT case.
> 
> Signed-off-by: Heming Zhao <heming.zhao at suse.com>
> ---
>  fs/ocfs2/alloc.c      | 10 ++++++----
>  fs/ocfs2/journal.c    |  5 +++++
>  fs/ocfs2/localalloc.c |  3 +++
>  3 files changed, 14 insertions(+), 4 deletions(-)
>

On 4/30/23 11:13 AM, Heming Zhao wrote:
> fstest generic cases 347 361 628 629 trigger a same issue:
> When jbd2 enter ABORT status, ocfs2 ignores it and keep going to commit
> journal.
What's the end user impact?

JBD2_ABORT indicates a fatal error happens, either in jounral layer or
filesystem. And we should not commit any further transactions.
It seems that we may unify the behavior like:

if (is_journal_aborted(journal))
	return -EROFS;

Thanks,
Joseph
> 
> This commit gives ocfs2 ability to handle jbd2 ABORT case.
> 
> Signed-off-by: Heming Zhao <heming.zhao at suse.com>
> ---
>  fs/ocfs2/alloc.c      | 10 ++++++----
>  fs/ocfs2/journal.c    |  5 +++++
>  fs/ocfs2/localalloc.c |  3 +++
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index 51c93929a146..d90961a1c433 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -6308,11 +6308,13 @@ void ocfs2_truncate_log_shutdown(struct ocfs2_super
*osb)
>  
>  	if (tl_inode) {
>  		cancel_delayed_work(&osb->osb_truncate_log_wq);
> -		flush_workqueue(osb->ocfs2_wq);
> +		if (!is_journal_aborted(osb->journal->j_journal)) {
> +			flush_workqueue(osb->ocfs2_wq);
>  
> -		status = ocfs2_flush_truncate_log(osb);
> -		if (status < 0)
> -			mlog_errno(status);
> +			status = ocfs2_flush_truncate_log(osb);
> +			if (status < 0)
> +				mlog_errno(status);
> +		}
>  
>  		brelse(osb->osb_tl_bh);
>  		iput(osb->osb_tl_inode);
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index 25d8072ccfce..2798067a2f66 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -312,11 +312,16 @@ static int ocfs2_commit_cache(struct ocfs2_super
*osb)
>  	status = jbd2_journal_flush(journal->j_journal, 0);
>  	jbd2_journal_unlock_updates(journal->j_journal);
>  	if (status < 0) {
> +		if (is_journal_aborted(journal->j_journal)) {
> +			ocfs2_error(osb->sb, "jbd2 status: ABORT.\n");
> +			goto reset;
> +		}
>  		up_write(&journal->j_trans_barrier);
>  		mlog_errno(status);
>  		goto finally;
>  	}
>  
> +reset:
>  	ocfs2_inc_trans_id(journal);
>  
>  	flushed = atomic_read(&journal->j_num_trans);
> diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
> index c4426d12a2ad..e2e3400717b0 100644
> --- a/fs/ocfs2/localalloc.c
> +++ b/fs/ocfs2/localalloc.c
> @@ -378,6 +378,9 @@ void ocfs2_shutdown_local_alloc(struct ocfs2_super
*osb)
>  	if (osb->ocfs2_wq)
>  		flush_workqueue(osb->ocfs2_wq);
>  
> +	if (is_journal_aborted(osb->journal->j_journal))
> +		goto out;
> +
>  	if (osb->local_alloc_state == OCFS2_LA_UNUSED)
>  		goto out;
>