According to discussions in [1], this patchset adds errseq in journal to
enable JDB2 detecting meatadata writeback error of fs dev. Then, orginal
checking mechanism could be removed.
[1] https://lore.kernel.org/all/[email protected]/T/
Zhihao Cheng (5):
jbd2: Add errseq to detect client fs's bdev writeback error
jbd2: Replace journal state flag by checking errseq
jbd2: Remove unused 'JBD2_CHECKPOINT_IO_ERROR' and 'j_atomic_flags'
jbd2: Abort journal when detecting metadata writeback error of fs dev
ext4: Move ext4_check_bdev_write_error() into nojournal mode
fs/ext4/ext4_jbd2.c | 5 ++---
fs/jbd2/checkpoint.c | 11 -----------
fs/jbd2/journal.c | 11 ++++++-----
fs/jbd2/recovery.c | 7 +------
fs/jbd2/transaction.c | 14 ++++++++++++++
include/linux/jbd2.h | 37 ++++++++++++++++++++++++++-----------
6 files changed, 49 insertions(+), 36 deletions(-)
--
2.39.2
Add errseq in journal, so that JBD2 can detect whether metadata is
successfully fallen on fs bdev. This patch adds detection in recovery
process to replace original solution(using local variable wb_err).
Signed-off-by: Zhihao Cheng <[email protected]>
Suggested-by: Jan Kara <[email protected]>
---
fs/jbd2/journal.c | 1 +
fs/jbd2/recovery.c | 7 +------
include/linux/jbd2.h | 26 ++++++++++++++++++++++++++
3 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 30dec2bd2ecc..a655d9a88f79 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1535,6 +1535,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
journal->j_fs_dev = fs_dev;
journal->j_blk_offset = start;
journal->j_total_len = len;
+ jbd2_init_fs_dev_write_error(journal);
err = journal_load_superblock(journal);
if (err)
diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
index 01f744cb97a4..1f7664984d6e 100644
--- a/fs/jbd2/recovery.c
+++ b/fs/jbd2/recovery.c
@@ -289,8 +289,6 @@ int jbd2_journal_recover(journal_t *journal)
journal_superblock_t * sb;
struct recovery_info info;
- errseq_t wb_err;
- struct address_space *mapping;
memset(&info, 0, sizeof(info));
sb = journal->j_superblock;
@@ -308,9 +306,6 @@ int jbd2_journal_recover(journal_t *journal)
return 0;
}
- wb_err = 0;
- mapping = journal->j_fs_dev->bd_inode->i_mapping;
- errseq_check_and_advance(&mapping->wb_err, &wb_err);
err = do_one_pass(journal, &info, PASS_SCAN);
if (!err)
err = do_one_pass(journal, &info, PASS_REVOKE);
@@ -334,7 +329,7 @@ int jbd2_journal_recover(journal_t *journal)
err2 = sync_blockdev(journal->j_fs_dev);
if (!err)
err = err2;
- err2 = errseq_check_and_advance(&mapping->wb_err, &wb_err);
+ err2 = jbd2_check_fs_dev_write_error(journal);
if (!err)
err = err2;
/* Make sure all replayed data is on permanent storage */
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 52772c826c86..15798f88ade4 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -998,6 +998,13 @@ struct journal_s
*/
struct block_device *j_fs_dev;
+ /**
+ * @j_fs_dev_wb_err:
+ *
+ * Records the errseq of the client fs's backing block device.
+ */
+ errseq_t j_fs_dev_wb_err;
+
/**
* @j_total_len: Total maximum capacity of the journal region on disk.
*/
@@ -1695,6 +1702,25 @@ static inline void jbd2_journal_abort_handle(handle_t *handle)
handle->h_aborted = 1;
}
+static inline void jbd2_init_fs_dev_write_error(journal_t *journal)
+{
+ struct address_space *mapping = journal->j_fs_dev->bd_inode->i_mapping;
+
+ /*
+ * Save the original wb_err value of client fs's bdev mapping which
+ * could be used to detect the client fs's metadata async write error.
+ */
+ errseq_check_and_advance(&mapping->wb_err, &journal->j_fs_dev_wb_err);
+}
+
+static inline int jbd2_check_fs_dev_write_error(journal_t *journal)
+{
+ struct address_space *mapping = journal->j_fs_dev->bd_inode->i_mapping;
+
+ return errseq_check(&mapping->wb_err,
+ READ_ONCE(journal->j_fs_dev_wb_err));
+}
+
#endif /* __KERNEL__ */
/* Comparison functions for transaction IDs: perform comparisons using
--
2.39.2
Now JBD2 detects metadata writeback error of fs dev according to errseq.
Replace journal state flag by checking errseq.
Signed-off-by: Zhihao Cheng <[email protected]>
Suggested-by: Jan Kara <[email protected]>
---
fs/jbd2/journal.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index a655d9a88f79..b60d19505f8a 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1850,7 +1850,7 @@ int jbd2_journal_update_sb_log_tail(journal_t *journal, tid_t tail_tid,
if (is_journal_aborted(journal))
return -EIO;
- if (test_bit(JBD2_CHECKPOINT_IO_ERROR, &journal->j_atomic_flags)) {
+ if (jbd2_check_fs_dev_write_error(journal)) {
jbd2_journal_abort(journal, -EIO);
return -EIO;
}
@@ -2148,12 +2148,12 @@ int jbd2_journal_destroy(journal_t *journal)
/*
* OK, all checkpoint transactions have been checked, now check the
- * write out io error flag and abort the journal if some buffer failed
- * to write back to the original location, otherwise the filesystem
- * may become inconsistent.
+ * writeback errseq of fs dev and abort the journal if some buffer
+ * failed to write back to the original location, otherwise the
+ * filesystem may become inconsistent.
*/
if (!is_journal_aborted(journal) &&
- test_bit(JBD2_CHECKPOINT_IO_ERROR, &journal->j_atomic_flags))
+ jbd2_check_fs_dev_write_error(journal))
jbd2_journal_abort(journal, -EIO);
if (journal->j_sb_buffer) {
--
2.39.2
This is a replacement solution of commit bc71726c725767 ("ext4: abort
the filesystem if failed to async write metadata buffer"), JBD2 can
detects metadata writeback error of fs dev by 'j_fs_dev_wb_err'.
Signed-off-by: Zhihao Cheng <[email protected]>
---
fs/jbd2/transaction.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 5f08b5fd105a..cb0b8d6fc0c6 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -1231,11 +1231,25 @@ static bool jbd2_write_access_granted(handle_t *handle, struct buffer_head *bh,
int jbd2_journal_get_write_access(handle_t *handle, struct buffer_head *bh)
{
struct journal_head *jh;
+ journal_t *journal;
int rc;
if (is_handle_aborted(handle))
return -EROFS;
+ journal = handle->h_transaction->t_journal;
+ if (jbd2_check_fs_dev_write_error(journal)) {
+ /*
+ * If the fs dev has writeback errors, it may have failed
+ * to async write out metadata buffers in the background.
+ * In this case, we could read old data from disk and write
+ * it out again, which may lead to on-disk filesystem
+ * inconsistency. Aborting journal can avoid it happen.
+ */
+ jbd2_journal_abort(journal, -EIO);
+ return -EIO;
+ }
+
if (jbd2_write_access_granted(handle, bh, false))
return 0;
--
2.39.2
Since 'JBD2_CHECKPOINT_IO_ERROR' and j_atomic_flags' are not useful
anymore after fs dev's errseq is imported into jbd2, just remove them.
Signed-off-by: Zhihao Cheng <[email protected]>
---
fs/jbd2/checkpoint.c | 11 -----------
include/linux/jbd2.h | 11 -----------
2 files changed, 22 deletions(-)
diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 118699fff2f9..1c97e64c4784 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -556,7 +556,6 @@ int __jbd2_journal_remove_checkpoint(struct journal_head *jh)
struct transaction_chp_stats_s *stats;
transaction_t *transaction;
journal_t *journal;
- struct buffer_head *bh = jh2bh(jh);
JBUFFER_TRACE(jh, "entry");
@@ -569,16 +568,6 @@ int __jbd2_journal_remove_checkpoint(struct journal_head *jh)
JBUFFER_TRACE(jh, "removing from transaction");
- /*
- * If we have failed to write the buffer out to disk, the filesystem
- * may become inconsistent. We cannot abort the journal here since
- * we hold j_list_lock and we have to be careful about races with
- * jbd2_journal_destroy(). So mark the writeback IO error in the
- * journal here and we abort the journal later from a better context.
- */
- if (buffer_write_io_error(bh))
- set_bit(JBD2_CHECKPOINT_IO_ERROR, &journal->j_atomic_flags);
-
__buffer_unlink(jh);
jh->b_cp_transaction = NULL;
percpu_counter_dec(&journal->j_checkpoint_jh_count);
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 15798f88ade4..bdde776b90d9 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -755,11 +755,6 @@ struct journal_s
*/
unsigned long j_flags;
- /**
- * @j_atomic_flags: Atomic journaling state flags.
- */
- unsigned long j_atomic_flags;
-
/**
* @j_errno:
*
@@ -1403,12 +1398,6 @@ JBD2_FEATURE_INCOMPAT_FUNCS(fast_commit, FAST_COMMIT)
#define JBD2_JOURNAL_FLUSH_VALID (JBD2_JOURNAL_FLUSH_DISCARD | \
JBD2_JOURNAL_FLUSH_ZEROOUT)
-/*
- * Journal atomic flag definitions
- */
-#define JBD2_CHECKPOINT_IO_ERROR 0x001 /* Detect io error while writing
- * buffer back to disk */
-
/*
* Function declarations for the journaling transaction and buffer
* management
--
2.39.2
Since JBD2 takes care of all metadata writeback errors of fs dev,
ext4_check_bdev_write_error() is useful only in nojournal mode.
Move it into '!ext4_handle_valid(handle)' branch.
Signed-off-by: Zhihao Cheng <[email protected]>
Suggested-by: Jan Kara <[email protected]>
---
fs/ext4/ext4_jbd2.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
index d1a2e6624401..5d8055161acd 100644
--- a/fs/ext4/ext4_jbd2.c
+++ b/fs/ext4/ext4_jbd2.c
@@ -235,8 +235,6 @@ int __ext4_journal_get_write_access(const char *where, unsigned int line,
might_sleep();
- ext4_check_bdev_write_error(sb);
-
if (ext4_handle_valid(handle)) {
err = jbd2_journal_get_write_access(handle, bh);
if (err) {
@@ -244,7 +242,8 @@ int __ext4_journal_get_write_access(const char *where, unsigned int line,
handle, err);
return err;
}
- }
+ } else
+ ext4_check_bdev_write_error(sb);
if (trigger_type == EXT4_JTR_NONE || !ext4_has_metadata_csum(sb))
return 0;
BUG_ON(trigger_type >= EXT4_JOURNAL_TRIGGER_COUNT);
--
2.39.2
For series.
Reviewed-by: Zhang Yi <[email protected]>
On 2023/11/3 22:52, Zhihao Cheng wrote:
> According to discussions in [1], this patchset adds errseq in journal to
> enable JDB2 detecting meatadata writeback error of fs dev. Then, orginal
> checking mechanism could be removed.
>
> [1] https://lore.kernel.org/all/[email protected]/T/
>
> Zhihao Cheng (5):
> jbd2: Add errseq to detect client fs's bdev writeback error
> jbd2: Replace journal state flag by checking errseq
> jbd2: Remove unused 'JBD2_CHECKPOINT_IO_ERROR' and 'j_atomic_flags'
> jbd2: Abort journal when detecting metadata writeback error of fs dev
> ext4: Move ext4_check_bdev_write_error() into nojournal mode
>
> fs/ext4/ext4_jbd2.c | 5 ++---
> fs/jbd2/checkpoint.c | 11 -----------
> fs/jbd2/journal.c | 11 ++++++-----
> fs/jbd2/recovery.c | 7 +------
> fs/jbd2/transaction.c | 14 ++++++++++++++
> include/linux/jbd2.h | 37 ++++++++++++++++++++++++++-----------
> 6 files changed, 49 insertions(+), 36 deletions(-)
>
On Fri 03-11-23 22:52:46, Zhihao Cheng wrote:
> Add errseq in journal, so that JBD2 can detect whether metadata is
> successfully fallen on fs bdev. This patch adds detection in recovery
^^^^^^^^^ written to
> process to replace original solution(using local variable wb_err).
>
> Signed-off-by: Zhihao Cheng <[email protected]>
> Suggested-by: Jan Kara <[email protected]>
Otherwise the patch looks good. Feel free to add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 30dec2bd2ecc..a655d9a88f79 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1535,6 +1535,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
> journal->j_fs_dev = fs_dev;
> journal->j_blk_offset = start;
> journal->j_total_len = len;
> + jbd2_init_fs_dev_write_error(journal);
>
> err = journal_load_superblock(journal);
> if (err)
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 01f744cb97a4..1f7664984d6e 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -289,8 +289,6 @@ int jbd2_journal_recover(journal_t *journal)
> journal_superblock_t * sb;
>
> struct recovery_info info;
> - errseq_t wb_err;
> - struct address_space *mapping;
>
> memset(&info, 0, sizeof(info));
> sb = journal->j_superblock;
> @@ -308,9 +306,6 @@ int jbd2_journal_recover(journal_t *journal)
> return 0;
> }
>
> - wb_err = 0;
> - mapping = journal->j_fs_dev->bd_inode->i_mapping;
> - errseq_check_and_advance(&mapping->wb_err, &wb_err);
> err = do_one_pass(journal, &info, PASS_SCAN);
> if (!err)
> err = do_one_pass(journal, &info, PASS_REVOKE);
> @@ -334,7 +329,7 @@ int jbd2_journal_recover(journal_t *journal)
> err2 = sync_blockdev(journal->j_fs_dev);
> if (!err)
> err = err2;
> - err2 = errseq_check_and_advance(&mapping->wb_err, &wb_err);
> + err2 = jbd2_check_fs_dev_write_error(journal);
> if (!err)
> err = err2;
> /* Make sure all replayed data is on permanent storage */
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 52772c826c86..15798f88ade4 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -998,6 +998,13 @@ struct journal_s
> */
> struct block_device *j_fs_dev;
>
> + /**
> + * @j_fs_dev_wb_err:
> + *
> + * Records the errseq of the client fs's backing block device.
> + */
> + errseq_t j_fs_dev_wb_err;
> +
> /**
> * @j_total_len: Total maximum capacity of the journal region on disk.
> */
> @@ -1695,6 +1702,25 @@ static inline void jbd2_journal_abort_handle(handle_t *handle)
> handle->h_aborted = 1;
> }
>
> +static inline void jbd2_init_fs_dev_write_error(journal_t *journal)
> +{
> + struct address_space *mapping = journal->j_fs_dev->bd_inode->i_mapping;
> +
> + /*
> + * Save the original wb_err value of client fs's bdev mapping which
> + * could be used to detect the client fs's metadata async write error.
> + */
> + errseq_check_and_advance(&mapping->wb_err, &journal->j_fs_dev_wb_err);
> +}
> +
> +static inline int jbd2_check_fs_dev_write_error(journal_t *journal)
> +{
> + struct address_space *mapping = journal->j_fs_dev->bd_inode->i_mapping;
> +
> + return errseq_check(&mapping->wb_err,
> + READ_ONCE(journal->j_fs_dev_wb_err));
> +}
> +
> #endif /* __KERNEL__ */
>
> /* Comparison functions for transaction IDs: perform comparisons using
> --
> 2.39.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Fri 03-11-23 22:52:47, Zhihao Cheng wrote:
> Now JBD2 detects metadata writeback error of fs dev according to errseq.
> Replace journal state flag by checking errseq.
>
> Signed-off-by: Zhihao Cheng <[email protected]>
> Suggested-by: Jan Kara <[email protected]>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> ---
> fs/jbd2/journal.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index a655d9a88f79..b60d19505f8a 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1850,7 +1850,7 @@ int jbd2_journal_update_sb_log_tail(journal_t *journal, tid_t tail_tid,
>
> if (is_journal_aborted(journal))
> return -EIO;
> - if (test_bit(JBD2_CHECKPOINT_IO_ERROR, &journal->j_atomic_flags)) {
> + if (jbd2_check_fs_dev_write_error(journal)) {
> jbd2_journal_abort(journal, -EIO);
> return -EIO;
> }
> @@ -2148,12 +2148,12 @@ int jbd2_journal_destroy(journal_t *journal)
>
> /*
> * OK, all checkpoint transactions have been checked, now check the
> - * write out io error flag and abort the journal if some buffer failed
> - * to write back to the original location, otherwise the filesystem
> - * may become inconsistent.
> + * writeback errseq of fs dev and abort the journal if some buffer
> + * failed to write back to the original location, otherwise the
> + * filesystem may become inconsistent.
> */
> if (!is_journal_aborted(journal) &&
> - test_bit(JBD2_CHECKPOINT_IO_ERROR, &journal->j_atomic_flags))
> + jbd2_check_fs_dev_write_error(journal))
> jbd2_journal_abort(journal, -EIO);
>
> if (journal->j_sb_buffer) {
> --
> 2.39.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Fri 03-11-23 22:52:48, Zhihao Cheng wrote:
> Since 'JBD2_CHECKPOINT_IO_ERROR' and j_atomic_flags' are not useful
> anymore after fs dev's errseq is imported into jbd2, just remove them.
>
> Signed-off-by: Zhihao Cheng <[email protected]>
Nice! Feel free to add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> ---
> fs/jbd2/checkpoint.c | 11 -----------
> include/linux/jbd2.h | 11 -----------
> 2 files changed, 22 deletions(-)
>
> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> index 118699fff2f9..1c97e64c4784 100644
> --- a/fs/jbd2/checkpoint.c
> +++ b/fs/jbd2/checkpoint.c
> @@ -556,7 +556,6 @@ int __jbd2_journal_remove_checkpoint(struct journal_head *jh)
> struct transaction_chp_stats_s *stats;
> transaction_t *transaction;
> journal_t *journal;
> - struct buffer_head *bh = jh2bh(jh);
>
> JBUFFER_TRACE(jh, "entry");
>
> @@ -569,16 +568,6 @@ int __jbd2_journal_remove_checkpoint(struct journal_head *jh)
>
> JBUFFER_TRACE(jh, "removing from transaction");
>
> - /*
> - * If we have failed to write the buffer out to disk, the filesystem
> - * may become inconsistent. We cannot abort the journal here since
> - * we hold j_list_lock and we have to be careful about races with
> - * jbd2_journal_destroy(). So mark the writeback IO error in the
> - * journal here and we abort the journal later from a better context.
> - */
> - if (buffer_write_io_error(bh))
> - set_bit(JBD2_CHECKPOINT_IO_ERROR, &journal->j_atomic_flags);
> -
> __buffer_unlink(jh);
> jh->b_cp_transaction = NULL;
> percpu_counter_dec(&journal->j_checkpoint_jh_count);
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 15798f88ade4..bdde776b90d9 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -755,11 +755,6 @@ struct journal_s
> */
> unsigned long j_flags;
>
> - /**
> - * @j_atomic_flags: Atomic journaling state flags.
> - */
> - unsigned long j_atomic_flags;
> -
> /**
> * @j_errno:
> *
> @@ -1403,12 +1398,6 @@ JBD2_FEATURE_INCOMPAT_FUNCS(fast_commit, FAST_COMMIT)
> #define JBD2_JOURNAL_FLUSH_VALID (JBD2_JOURNAL_FLUSH_DISCARD | \
> JBD2_JOURNAL_FLUSH_ZEROOUT)
>
> -/*
> - * Journal atomic flag definitions
> - */
> -#define JBD2_CHECKPOINT_IO_ERROR 0x001 /* Detect io error while writing
> - * buffer back to disk */
> -
> /*
> * Function declarations for the journaling transaction and buffer
> * management
> --
> 2.39.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Fri 03-11-23 22:52:49, Zhihao Cheng wrote:
> This is a replacement solution of commit bc71726c725767 ("ext4: abort
> the filesystem if failed to async write metadata buffer"), JBD2 can
> detects metadata writeback error of fs dev by 'j_fs_dev_wb_err'.
^^^ detect
> Signed-off-by: Zhihao Cheng <[email protected]>
Otherwise looks good. Feel free to add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> ---
> fs/jbd2/transaction.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 5f08b5fd105a..cb0b8d6fc0c6 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -1231,11 +1231,25 @@ static bool jbd2_write_access_granted(handle_t *handle, struct buffer_head *bh,
> int jbd2_journal_get_write_access(handle_t *handle, struct buffer_head *bh)
> {
> struct journal_head *jh;
> + journal_t *journal;
> int rc;
>
> if (is_handle_aborted(handle))
> return -EROFS;
>
> + journal = handle->h_transaction->t_journal;
> + if (jbd2_check_fs_dev_write_error(journal)) {
> + /*
> + * If the fs dev has writeback errors, it may have failed
> + * to async write out metadata buffers in the background.
> + * In this case, we could read old data from disk and write
> + * it out again, which may lead to on-disk filesystem
> + * inconsistency. Aborting journal can avoid it happen.
> + */
> + jbd2_journal_abort(journal, -EIO);
> + return -EIO;
> + }
> +
> if (jbd2_write_access_granted(handle, bh, false))
> return 0;
>
> --
> 2.39.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Fri 03-11-23 22:52:50, Zhihao Cheng wrote:
> Since JBD2 takes care of all metadata writeback errors of fs dev,
> ext4_check_bdev_write_error() is useful only in nojournal mode.
> Move it into '!ext4_handle_valid(handle)' branch.
>
> Signed-off-by: Zhihao Cheng <[email protected]>
> Suggested-by: Jan Kara <[email protected]>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> ---
> fs/ext4/ext4_jbd2.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
> index d1a2e6624401..5d8055161acd 100644
> --- a/fs/ext4/ext4_jbd2.c
> +++ b/fs/ext4/ext4_jbd2.c
> @@ -235,8 +235,6 @@ int __ext4_journal_get_write_access(const char *where, unsigned int line,
>
> might_sleep();
>
> - ext4_check_bdev_write_error(sb);
> -
> if (ext4_handle_valid(handle)) {
> err = jbd2_journal_get_write_access(handle, bh);
> if (err) {
> @@ -244,7 +242,8 @@ int __ext4_journal_get_write_access(const char *where, unsigned int line,
> handle, err);
> return err;
> }
> - }
> + } else
> + ext4_check_bdev_write_error(sb);
> if (trigger_type == EXT4_JTR_NONE || !ext4_has_metadata_csum(sb))
> return 0;
> BUG_ON(trigger_type >= EXT4_JOURNAL_TRIGGER_COUNT);
> --
> 2.39.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Fri 03-11-23 22:52:45, Zhihao Cheng wrote:
> According to discussions in [1], this patchset adds errseq in journal to
> enable JDB2 detecting meatadata writeback error of fs dev. Then, orginal
> checking mechanism could be removed.
>
> [1] https://lore.kernel.org/all/[email protected]/T/
Thanks for the series! I'm sorry for the very delayed review. There has
been a lot of work, conference and other stuff happening lately... Anyway
the series looks good, I had just some language corrections.
Honza
>
> Zhihao Cheng (5):
> jbd2: Add errseq to detect client fs's bdev writeback error
> jbd2: Replace journal state flag by checking errseq
> jbd2: Remove unused 'JBD2_CHECKPOINT_IO_ERROR' and 'j_atomic_flags'
> jbd2: Abort journal when detecting metadata writeback error of fs dev
> ext4: Move ext4_check_bdev_write_error() into nojournal mode
>
> fs/ext4/ext4_jbd2.c | 5 ++---
> fs/jbd2/checkpoint.c | 11 -----------
> fs/jbd2/journal.c | 11 ++++++-----
> fs/jbd2/recovery.c | 7 +------
> fs/jbd2/transaction.c | 14 ++++++++++++++
> include/linux/jbd2.h | 37 ++++++++++++++++++++++++++-----------
> 6 files changed, 49 insertions(+), 36 deletions(-)
>
> --
> 2.39.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR