| Message ID | 20180618032232.25481-2-tytso@mit.edu |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [1/5] ext4: never move the system.data xattr out of the inode body | expand |
> On Jun 17, 2018, at 9:22 PM, Theodore Ts'o <tytso@mit.edu> wrote: > > Do not set the b_modified flag in block's journal head should not > until after we're sure that jbd2_journal_dirty_metadat() will not > abort with an error due to there not being enough space reserved in > the jbd2 handle. Looks like you partially edited this sentence after it was written, but didn't get it back to coherency? > Otherwise, future attempts to modify the buffer may lead a large > number of spurious errors and warnings. ... lead *to* a large number ... > > https://bugzilla.kernel.org/show_bug.cgi?id=200071 > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> > --- > fs/jbd2/transaction.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c > index 51dd68e67b0f..c0b66a7a795b 100644 > --- a/fs/jbd2/transaction.c > +++ b/fs/jbd2/transaction.c > @@ -1361,6 +1361,13 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh) > if (jh->b_transaction == transaction && > jh->b_jlist != BJ_Metadata) { > jbd_lock_bh_state(bh); > + if (jh->b_transaction == transaction && > + jh->b_jlist != BJ_Metadata) > + pr_err("JBD2: assertion failure: h_type=%u " > + "h_line_no=%u block_no=%llu jlist=%u\n", > + handle->h_type, handle->h_line_no, > + (unsigned long long) bh->b_blocknr, > + jh->b_jlist); > J_ASSERT_JH(jh, jh->b_transaction != transaction || > jh->b_jlist == BJ_Metadata); Should the J_ASSERT_JH() be inside the second "if" block that is rechecked under lock? Otherwise, I don't see the point in getting the bh_state lock since either jh->b_jlist can't change and there is no need to lock it, or it *can* change and we shouldn't assert if the condition is no longer wrong enough to print the error message. > jbd_unlock_bh_state(bh); > @@ -1380,11 +1387,11 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh) > * of the transaction. This needs to be done > * once a transaction -bzzz > */ > - jh->b_modified = 1; > if (handle->h_buffer_credits <= 0) { > ret = -ENOSPC; > goto out_unlock_bh; > } > + jh->b_modified = 1; > handle->h_buffer_credits--; > } > > -- > 2.18.0.rc0 > Cheers, Andreas
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index 51dd68e67b0f..c0b66a7a795b 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -1361,6 +1361,13 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh) if (jh->b_transaction == transaction && jh->b_jlist != BJ_Metadata) { jbd_lock_bh_state(bh); + if (jh->b_transaction == transaction && + jh->b_jlist != BJ_Metadata) + pr_err("JBD2: assertion failure: h_type=%u " + "h_line_no=%u block_no=%llu jlist=%u\n", + handle->h_type, handle->h_line_no, + (unsigned long long) bh->b_blocknr, + jh->b_jlist); J_ASSERT_JH(jh, jh->b_transaction != transaction || jh->b_jlist == BJ_Metadata); jbd_unlock_bh_state(bh); @@ -1380,11 +1387,11 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh) * of the transaction. This needs to be done * once a transaction -bzzz */ - jh->b_modified = 1; if (handle->h_buffer_credits <= 0) { ret = -ENOSPC; goto out_unlock_bh; } + jh->b_modified = 1; handle->h_buffer_credits--; }
Do not set the b_modified flag in block's journal head should not until after we're sure that jbd2_journal_dirty_metadat() will not abort with an error due to there not being enough space reserved in the jbd2 handle. Otherwise, future attempts to modify the buffer may lead a large number of spurious errors and warnings. https://bugzilla.kernel.org/show_bug.cgi?id=200071 Signed-off-by: Theodore Ts'o <tytso@mit.edu> --- fs/jbd2/transaction.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)