* [PATCH AUTOSEL 4.4 1/6] jbd2: make sure jh have b_transaction set in refile/unfile_buffer
@ 2020-08-24 16:39 Sasha Levin
2020-08-24 16:39 ` [PATCH AUTOSEL 4.4 2/6] jbd2: abort journal if free a async write error metadata buffer Sasha Levin
0 siblings, 1 reply; 2+ messages in thread
From: Sasha Levin @ 2020-08-24 16:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Lukas Czerner, Jan Kara, Theodore Ts'o, Sasha Levin,
linux-ext4
From: Lukas Czerner <lczerner@redhat.com>
[ Upstream commit 24dc9864914eb5813173cfa53313fcd02e4aea7d ]
Callers of __jbd2_journal_unfile_buffer() and
__jbd2_journal_refile_buffer() assume that the b_transaction is set. In
fact if it's not, we can end up with journal_head refcounting errors
leading to crash much later that might be very hard to track down. Add
asserts to make sure that is the case.
We also make sure that b_next_transaction is NULL in
__jbd2_journal_unfile_buffer() since the callers expect that as well and
we should not get into that stage in this state anyway, leading to
problems later on if we do.
Tested with fstests.
Signed-off-by: Lukas Czerner <lczerner@redhat.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://lore.kernel.org/r/20200617092549.6712-1-lczerner@redhat.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/jbd2/transaction.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 3233e5ac9774f..622610934c9ad 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -1906,6 +1906,9 @@ static void __jbd2_journal_temp_unlink_buffer(struct journal_head *jh)
*/
static void __jbd2_journal_unfile_buffer(struct journal_head *jh)
{
+ J_ASSERT_JH(jh, jh->b_transaction != NULL);
+ J_ASSERT_JH(jh, jh->b_next_transaction == NULL);
+
__jbd2_journal_temp_unlink_buffer(jh);
jh->b_transaction = NULL;
jbd2_journal_put_journal_head(jh);
@@ -2453,6 +2456,13 @@ void __jbd2_journal_refile_buffer(struct journal_head *jh)
was_dirty = test_clear_buffer_jbddirty(bh);
__jbd2_journal_temp_unlink_buffer(jh);
+
+ /*
+ * b_transaction must be set, otherwise the new b_transaction won't
+ * be holding jh reference
+ */
+ J_ASSERT_JH(jh, jh->b_transaction != NULL);
+
/*
* We set b_transaction here because b_next_transaction will inherit
* our jh reference and thus __jbd2_journal_file_buffer() must not
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH AUTOSEL 4.4 2/6] jbd2: abort journal if free a async write error metadata buffer
2020-08-24 16:39 [PATCH AUTOSEL 4.4 1/6] jbd2: make sure jh have b_transaction set in refile/unfile_buffer Sasha Levin
@ 2020-08-24 16:39 ` Sasha Levin
0 siblings, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2020-08-24 16:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: zhangyi (F), Theodore Ts'o, Sasha Levin, linux-ext4
From: "zhangyi (F)" <yi.zhang@huawei.com>
[ Upstream commit c044f3d8360d2ecf831ba2cc9f08cf9fb2c699fb ]
If we free a metadata buffer which has been failed to async write out
in the background, the jbd2 checkpoint procedure will not detect this
failure in jbd2_log_do_checkpoint(), so it may lead to filesystem
inconsistency after cleanup journal tail. This patch abort the journal
if free a buffer has write_io_error flag to prevent potential further
inconsistency.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Link: https://lore.kernel.org/r/20200620025427.1756360-5-yi.zhang@huawei.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/jbd2/transaction.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 622610934c9ad..ce2bf9d74224c 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -2000,6 +2000,7 @@ int jbd2_journal_try_to_free_buffers(journal_t *journal,
{
struct buffer_head *head;
struct buffer_head *bh;
+ bool has_write_io_error = false;
int ret = 0;
J_ASSERT(PageLocked(page));
@@ -2024,11 +2025,26 @@ int jbd2_journal_try_to_free_buffers(journal_t *journal,
jbd_unlock_bh_state(bh);
if (buffer_jbd(bh))
goto busy;
+
+ /*
+ * If we free a metadata buffer which has been failed to
+ * write out, the jbd2 checkpoint procedure will not detect
+ * this failure and may lead to filesystem inconsistency
+ * after cleanup journal tail.
+ */
+ if (buffer_write_io_error(bh)) {
+ pr_err("JBD2: Error while async write back metadata bh %llu.",
+ (unsigned long long)bh->b_blocknr);
+ has_write_io_error = true;
+ }
} while ((bh = bh->b_this_page) != head);
ret = try_to_free_buffers(page);
busy:
+ if (has_write_io_error)
+ jbd2_journal_abort(journal, -EIO);
+
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-08-24 16:42 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-24 16:39 [PATCH AUTOSEL 4.4 1/6] jbd2: make sure jh have b_transaction set in refile/unfile_buffer Sasha Levin
2020-08-24 16:39 ` [PATCH AUTOSEL 4.4 2/6] jbd2: abort journal if free a async write error metadata buffer Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).