| Message ID | 20210414134737.2366971-4-yi.zhang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ext4, jbd2: fix 3 issues about bdev_try_to_free_page() | expand |
On Wed 14-04-21 21:47:33, Zhang Yi wrote: > Now, we can make sure to abort the journal once the original buffer was ^^ Now that we can be sure the journal is aborted once a buffer has failed to be written back to disk, ... > failed to write back to disk, we can remove the journal abort logic in > jbd2_journal_try_to_free_buffers() which was introduced in > <c044f3d8360d> ("jbd2: abort journal if free a async write error > metadata buffer"), because it may costs and propably not safe. ^^ cost ^^ probably is > Signed-off-by: Zhang Yi <yi.zhang@huawei.com> Looks good. You can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/jbd2/transaction.c | 17 ----------------- > 1 file changed, 17 deletions(-) > > diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c > index 9396666b7314..3e0db4953fe4 100644 > --- a/fs/jbd2/transaction.c > +++ b/fs/jbd2/transaction.c > @@ -2118,7 +2118,6 @@ int jbd2_journal_try_to_free_buffers(journal_t *journal, struct page *page) > { > struct buffer_head *head; > struct buffer_head *bh; > - bool has_write_io_error = false; > int ret = 0; > > J_ASSERT(PageLocked(page)); > @@ -2143,26 +2142,10 @@ int jbd2_journal_try_to_free_buffers(journal_t *journal, struct page *page) > jbd2_journal_put_journal_head(jh); > 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.4 >
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index 9396666b7314..3e0db4953fe4 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -2118,7 +2118,6 @@ int jbd2_journal_try_to_free_buffers(journal_t *journal, struct page *page) { struct buffer_head *head; struct buffer_head *bh; - bool has_write_io_error = false; int ret = 0; J_ASSERT(PageLocked(page)); @@ -2143,26 +2142,10 @@ int jbd2_journal_try_to_free_buffers(journal_t *journal, struct page *page) jbd2_journal_put_journal_head(jh); 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; }
Now, we can make sure to abort the journal once the original buffer was failed to write back to disk, we can remove the journal abort logic in jbd2_journal_try_to_free_buffers() which was introduced in <c044f3d8360d> ("jbd2: abort journal if free a async write error metadata buffer"), because it may costs and propably not safe. Signed-off-by: Zhang Yi <yi.zhang@huawei.com> --- fs/jbd2/transaction.c | 17 ----------------- 1 file changed, 17 deletions(-)