| Message ID | 20241012085530.2147846-1-chengzhihao@huaweicloud.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | jbd2: Make b_frozen_data allocation always succeed | expand |
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 >
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);
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,
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);