From: Zhang Yi <yi.zhang@xxxxxxxxxx> Before removing checkpoint buffer from the t_checkpoint_list, we have to check both BH_Dirty and BH_Lock bits together to distinguish buffers have not been or were being written back. But __cp_buffer_busy() checks them separately, it first check lock state and then check dirty, the window between these two checks could be raced by writing back procedure, which locks buffer and clears buffer dirty before I/O completes. So it cannot guarantee checkpointing buffers been written back to disk if some error happens later. Finally, it may clean checkpoint transactions and lead to inconsistent filesystem. jbd2_journal_forget() and __journal_try_to_free_buffer() also have the same problem, so fix them by introduce a new helper to check the busy state atomically. Link: https://bugzilla.kernel.org/show_bug.cgi?id=217490 Cc: stable@xxxxxxxxxxxxxxx Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx> --- fs/jbd2/checkpoint.c | 8 ++++---- fs/jbd2/transaction.c | 4 ++-- include/linux/jbd2.h | 3 +++ 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c index 620f3d345f3d..2dde5fd1f0dd 100644 --- a/fs/jbd2/checkpoint.c +++ b/fs/jbd2/checkpoint.c @@ -45,11 +45,11 @@ static inline void __buffer_unlink(struct journal_head *jh) * * Requires j_list_lock */ -static inline bool __cp_buffer_busy(struct journal_head *jh) +static inline bool cp_buffer_busy(struct journal_head *jh) { struct buffer_head *bh = jh2bh(jh); - return (jh->b_transaction || buffer_locked(bh) || buffer_dirty(bh)); + return (jh->b_transaction || __cp_buffer_busy(bh)); } /* @@ -369,7 +369,7 @@ static int journal_clean_one_cp_list(struct journal_head *jh, bool destroy) jh = next_jh; next_jh = jh->b_cpnext; - if (!destroy && __cp_buffer_busy(jh)) + if (!destroy && cp_buffer_busy(jh)) return 0; if (__jbd2_journal_remove_checkpoint(jh)) @@ -413,7 +413,7 @@ static unsigned long journal_shrink_one_cp_list(struct journal_head *jh, next_jh = jh->b_cpnext; (*nr_to_scan)--; - if (__cp_buffer_busy(jh)) + if (cp_buffer_busy(jh)) continue; nr_freed++; diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index 18611241f451..04863787c93e 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -1784,7 +1784,7 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh) * Otherwise, if the buffer has been written to disk, * it is safe to remove the checkpoint and drop it. */ - if (!buffer_dirty(bh)) { + if (!__cp_buffer_busy(bh)) { __jbd2_journal_remove_checkpoint(jh); spin_unlock(&journal->j_list_lock); goto drop; @@ -2112,7 +2112,7 @@ __journal_try_to_free_buffer(journal_t *journal, struct buffer_head *bh) jh = bh2jh(bh); - if (buffer_locked(bh) || buffer_dirty(bh)) + if (__cp_buffer_busy(bh)) goto out; if (jh->b_next_transaction != NULL || jh->b_transaction != NULL) diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h index 91a2cf4bc575..b17d1efab787 100644 --- a/include/linux/jbd2.h +++ b/include/linux/jbd2.h @@ -1440,6 +1440,9 @@ void jbd2_update_log_tail(journal_t *journal, tid_t tid, unsigned long block); extern void jbd2_journal_commit_transaction(journal_t *); /* Checkpoint list management */ +#define __cp_buffer_busy(bh) \ + ((bh)->b_state & ((1ul << BH_Dirty) | (1ul << BH_Lock))) + void __jbd2_journal_clean_checkpoint_list(journal_t *journal, bool destroy); unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal, unsigned long *nr_to_scan); int __jbd2_journal_remove_checkpoint(struct journal_head *); -- 2.31.1
