[PATCH] jbd2: Make b_frozen_data allocation always succeed

[PATCH] jbd2: Make b_frozen_data allocation always succeed

[Zhihao Cheng]

From: Zhihao Cheng <chengzhihao1@huawei.com>

The b_frozen_data allocation should not be failed during journal
committing process, otherwise jbd2 will abort.
Since commit 490c1b444ce653d("jbd2: do not fail journal because of
frozen_buffer allocation failure") already added '__GFP_NOFAIL' flag
in do_get_write_access(), just add '__GFP_NOFAIL' flag for all allocations
in jbd2_journal_write_metadata_buffer(), like 'new_bh' allocation does.
Besides, remove all error handling branches for do_get_write_access().

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/jbd2/commit.c  | 4 ----
 fs/jbd2/journal.c | 8 +-------
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index 4305a1ac808a..9153ff3a08e7 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -662,10 +662,6 @@ void jbd2_journal_commit_transaction(journal_t *journal)
 		JBUFFER_TRACE(jh, "ph3: write metadata");
 		escape = jbd2_journal_write_metadata_buffer(commit_transaction,
 						jh, &wbuf[bufs], blocknr);
-		if (escape < 0) {
-			jbd2_journal_abort(journal, escape);
-			continue;
-		}
 		jbd2_file_log_bh(&io_bufs, wbuf[bufs]);
 
 		/* Record the new block's tag in the current descriptor
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 97f487c3d8fc..29d30eddf727 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -318,7 +318,6 @@ static inline void jbd2_data_do_escape(char *data)
  *
  *
  * Return value:
- *  <0: Error
  *  =0: Finished OK without escape
  *  =1: Finished OK with escape
  */
@@ -386,12 +385,7 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
 			goto escape_done;
 
 		spin_unlock(&jh_in->b_state_lock);
-		tmp = jbd2_alloc(bh_in->b_size, GFP_NOFS);
-		if (!tmp) {
-			brelse(new_bh);
-			free_buffer_head(new_bh);
-			return -ENOMEM;
-		}
+		tmp = jbd2_alloc(bh_in->b_size, GFP_NOFS | __GFP_NOFAIL);
 		spin_lock(&jh_in->b_state_lock);
 		if (jh_in->b_frozen_data) {
 			jbd2_free(tmp, bh_in->b_size);
-- 
2.39.2

Re: [PATCH] jbd2: Make b_frozen_data allocation always succeed

[Jan Kara]

On Sat 12-10-24 16:55:30, Zhihao Cheng wrote:
> From: Zhihao Cheng <chengzhihao1@huawei.com>
> 
> The b_frozen_data allocation should not be failed during journal
> committing process, otherwise jbd2 will abort.
> Since commit 490c1b444ce653d("jbd2: do not fail journal because of
> frozen_buffer allocation failure") already added '__GFP_NOFAIL' flag
> in do_get_write_access(), just add '__GFP_NOFAIL' flag for all allocations
> in jbd2_journal_write_metadata_buffer(), like 'new_bh' allocation does.
> Besides, remove all error handling branches for do_get_write_access().
> 
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/jbd2/commit.c  | 4 ----
>  fs/jbd2/journal.c | 8 +-------
>  2 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index 4305a1ac808a..9153ff3a08e7 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -662,10 +662,6 @@ void jbd2_journal_commit_transaction(journal_t *journal)
>  		JBUFFER_TRACE(jh, "ph3: write metadata");
>  		escape = jbd2_journal_write_metadata_buffer(commit_transaction,
>  						jh, &wbuf[bufs], blocknr);
> -		if (escape < 0) {
> -			jbd2_journal_abort(journal, escape);
> -			continue;
> -		}
>  		jbd2_file_log_bh(&io_bufs, wbuf[bufs]);
>  
>  		/* Record the new block's tag in the current descriptor
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 97f487c3d8fc..29d30eddf727 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -318,7 +318,6 @@ static inline void jbd2_data_do_escape(char *data)
>   *
>   *
>   * Return value:
> - *  <0: Error
>   *  =0: Finished OK without escape
>   *  =1: Finished OK with escape
>   */
> @@ -386,12 +385,7 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
>  			goto escape_done;
>  
>  		spin_unlock(&jh_in->b_state_lock);
> -		tmp = jbd2_alloc(bh_in->b_size, GFP_NOFS);
> -		if (!tmp) {
> -			brelse(new_bh);
> -			free_buffer_head(new_bh);
> -			return -ENOMEM;
> -		}
> +		tmp = jbd2_alloc(bh_in->b_size, GFP_NOFS | __GFP_NOFAIL);
>  		spin_lock(&jh_in->b_state_lock);
>  		if (jh_in->b_frozen_data) {
>  			jbd2_free(tmp, bh_in->b_size);
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] jbd2: Make b_frozen_data allocation always succeed

[Zhang Yi]

On 2024/10/12 16:55, Zhihao Cheng wrote:
> From: Zhihao Cheng <chengzhihao1@huawei.com>
> 
> The b_frozen_data allocation should not be failed during journal
> committing process, otherwise jbd2 will abort.
> Since commit 490c1b444ce653d("jbd2: do not fail journal because of
> frozen_buffer allocation failure") already added '__GFP_NOFAIL' flag
> in do_get_write_access(), just add '__GFP_NOFAIL' flag for all allocations
> in jbd2_journal_write_metadata_buffer(), like 'new_bh' allocation does.
> Besides, remove all error handling branches for do_get_write_access().
> 
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>

Looks good to me.

Reviewed-by: Zhang Yi <yi.zhang@huawei.com>

> ---
>  fs/jbd2/commit.c  | 4 ----
>  fs/jbd2/journal.c | 8 +-------
>  2 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index 4305a1ac808a..9153ff3a08e7 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -662,10 +662,6 @@ void jbd2_journal_commit_transaction(journal_t *journal)
>  		JBUFFER_TRACE(jh, "ph3: write metadata");
>  		escape = jbd2_journal_write_metadata_buffer(commit_transaction,
>  						jh, &wbuf[bufs], blocknr);
> -		if (escape < 0) {
> -			jbd2_journal_abort(journal, escape);
> -			continue;
> -		}
>  		jbd2_file_log_bh(&io_bufs, wbuf[bufs]);
>  
>  		/* Record the new block's tag in the current descriptor
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 97f487c3d8fc..29d30eddf727 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -318,7 +318,6 @@ static inline void jbd2_data_do_escape(char *data)
>   *
>   *
>   * Return value:
> - *  <0: Error
>   *  =0: Finished OK without escape
>   *  =1: Finished OK with escape
>   */
> @@ -386,12 +385,7 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
>  			goto escape_done;
>  
>  		spin_unlock(&jh_in->b_state_lock);
> -		tmp = jbd2_alloc(bh_in->b_size, GFP_NOFS);
> -		if (!tmp) {
> -			brelse(new_bh);
> -			free_buffer_head(new_bh);
> -			return -ENOMEM;
> -		}
> +		tmp = jbd2_alloc(bh_in->b_size, GFP_NOFS | __GFP_NOFAIL);
>  		spin_lock(&jh_in->b_state_lock);
>  		if (jh_in->b_frozen_data) {
>  			jbd2_free(tmp, bh_in->b_size);

Re: [PATCH] jbd2: Make b_frozen_data allocation always succeed

[Theodore Ts'o]

On Sat, 12 Oct 2024 16:55:30 +0800, Zhihao Cheng wrote:
> The b_frozen_data allocation should not be failed during journal
> committing process, otherwise jbd2 will abort.
> Since commit 490c1b444ce653d("jbd2: do not fail journal because of
> frozen_buffer allocation failure") already added '__GFP_NOFAIL' flag
> in do_get_write_access(), just add '__GFP_NOFAIL' flag for all allocations
> in jbd2_journal_write_metadata_buffer(), like 'new_bh' allocation does.
> Besides, remove all error handling branches for do_get_write_access().
> 
> [...]

Applied, thanks!

[1/1] jbd2: Make b_frozen_data allocation always succeed
      commit: abe1ac7ca84236513a3d8ede02cc47584437f24f

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>