| Message ID | 20191003220613.10791-20-jack@suse.cz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ext4: Fix transaction overflow due to revoke descriptors | expand |
On Fri, Oct 04, 2019 at 12:06:06AM +0200, Jan Kara wrote: > Make checking of available credits in jbd2_journal_dirty_metadata() more > strict. There should be always enough credits in the handle to write all > potential revoke descriptors. Also we warn in case there are not enough > credits since this is a bug in the filesystem. > > Signed-off-by: Jan Kara <jack@suse.cz> This is fine, but I wonder if we should also be returning an error in jbd2_journal_revoke() --- of course, one problem is ext4_forget() is getting called from ext4_free_blocks(), which currently doesn't return an error. But we can capture the error return in __ext4_forget(), and at that point we can give a much more useful error message, since we can print the function caller and line number. Feel free to add: Reviewed-by: Theodore Ts'o <tytso@mit.edu>
On Mon 21-10-19 18:29:59, Theodore Y. Ts'o wrote: > On Fri, Oct 04, 2019 at 12:06:06AM +0200, Jan Kara wrote: > > Make checking of available credits in jbd2_journal_dirty_metadata() more > > strict. There should be always enough credits in the handle to write all > > potential revoke descriptors. Also we warn in case there are not enough > > credits since this is a bug in the filesystem. > > > > Signed-off-by: Jan Kara <jack@suse.cz> > > This is fine, but I wonder if we should also be returning an error in > jbd2_journal_revoke() --- of course, one problem is ext4_forget() is > getting called from ext4_free_blocks(), which currently doesn't return > an error. But we can capture the error return in __ext4_forget(), and > at that point we can give a much more useful error message, since we > can print the function caller and line number. Yeah, that's a good point. I'll add a sanity check to jbd2_journal_revoke() and then generate some error message in ext4. > Feel free to add: > > Reviewed-by: Theodore Ts'o <tytso@mit.edu> Thanks! Honza
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index 8851cbbe3579..66fad49d45df 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -1458,7 +1458,7 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh) * of the transaction. This needs to be done * once a transaction -bzzz */ - if (handle->h_total_credits <= 0) { + if (WARN_ON_ONCE(jbd2_handle_buffer_credits(handle) <= 0)) { ret = -ENOSPC; goto out_unlock_bh; }
Make checking of available credits in jbd2_journal_dirty_metadata() more strict. There should be always enough credits in the handle to write all potential revoke descriptors. Also we warn in case there are not enough credits since this is a bug in the filesystem. Signed-off-by: Jan Kara <jack@suse.cz> --- fs/jbd2/transaction.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)