[PATCH v2 21/21] buffer: clear BH_Write_EIO when a write succeeds, not when one starts

[Chao Shi <coshi036@gmail.com>]

BH_Write_EIO is cleared in __bh_submit(), when a buffer that has been
written before is submitted for write again.  That is early: it says the
error is gone at the moment we start trying to fix it, rather than when we
have.

It also loses errors.  A task whose write fails sets the flag and then goes
to look at it; if another task redirties the buffer and resubmits it in
between, the submission clears the flag and the first task sees no error at
all.  Neither of them is doing anything wrong.

Clear it on successful write completion instead, in the end io handlers -
the same four the rest of this series has been converting, plus gfs2's,
which already marked errors this way.  Then the flag means what it says:
the last write of this buffer that finished, failed.  A resubmission no
longer hides an error that has not been fixed yet, and one that has been
fixed clears the flag when the data reaches the disk.

__bh_submit() keeps setting BH_Req, which is what the rest of the tree
reads it for.

Suggested-by: Jan Kara <jack@suse.cz>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
---
 fs/buffer.c           | 15 +++++++--------
 fs/ext4/fast_commit.c |  1 +
 fs/gfs2/lops.c        |  2 ++
 fs/jbd2/commit.c      |  4 +++-
 4 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 425fbfe72ad1..68ea0ef8470e 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -202,7 +202,9 @@ void bh_end_write(struct bio *bio)
 	struct buffer_head *bh;
 	bool success = bio_endio_bh(bio, &bh);
 
-	if (!success) {
+	if (success) {
+		clear_buffer_write_io_error(bh);
+	} else {
 		buffer_io_error(bh, ", lost sync page write");
 		mark_buffer_write_io_error(bh);
 	}
@@ -433,7 +435,9 @@ void bh_end_async_write(struct bio *bio)
 	BUG_ON(!buffer_async_write(bh));
 
 	folio = bh->b_folio;
-	if (!success) {
+	if (success) {
+		clear_buffer_write_io_error(bh);
+	} else {
 		buffer_io_error(bh, ", lost async page write");
 		mark_buffer_write_io_error(bh);
 	}
@@ -1114,7 +1118,6 @@ static void __bh_submit(struct buffer_head *bh, blk_opf_t opf,
 		enum rw_hint write_hint, struct writeback_control *wbc,
 		bio_end_io_t end_bio)
 {
-	const enum req_op op = opf & REQ_OP_MASK;
 	struct bio *bio;
 
 	BUG_ON(!buffer_locked(bh));
@@ -1122,11 +1125,7 @@ static void __bh_submit(struct buffer_head *bh, blk_opf_t opf,
 	BUG_ON(buffer_delay(bh));
 	BUG_ON(buffer_unwritten(bh));
 
-	/*
-	 * Only clear out a write error when rewriting
-	 */
-	if (test_set_buffer_req(bh) && (op == REQ_OP_WRITE))
-		clear_buffer_write_io_error(bh);
+	set_buffer_req(bh);
 
 	if (buffer_meta(bh))
 		opf |= REQ_META;
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index a2028fbd4540..95998827ff01 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -209,6 +209,7 @@ static void ext4_end_buffer_io_sync(struct bio *bio)
 	if (success) {
 		ext4_debug("%s: Block %lld written",
 			   __func__, bh->b_blocknr);
+		clear_buffer_write_io_error(bh);
 	} else {
 		ext4_debug("%s: Block %lld write failed",
 			   __func__, bh->b_blocknr);
diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 3df6e4b7e8b9..7440e5b72f8a 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -179,6 +179,8 @@ static void gfs2_end_log_write_bh(struct gfs2_sbd *sdp, struct folio *folio,
 	do {
 		if (error)
 			mark_buffer_write_io_error(bh);
+		else
+			clear_buffer_write_io_error(bh);
 		unlock_buffer(bh);
 		next = bh->b_this_page;
 		size -= bh->b_size;
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index cd7ef783bd36..ebf6ba58ff4d 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -36,7 +36,9 @@ static void journal_end_buffer_io_sync(struct bio *bio)
 	struct buffer_head *orig_bh = bh->b_private;
 
 	BUFFER_TRACE(bh, "");
-	if (!success)
+	if (success)
+		clear_buffer_write_io_error(bh);
+	else
 		mark_buffer_write_io_error(bh);
 	if (orig_bh) {
 		clear_and_wake_up_bit(BH_Shadow, &orig_bh->b_state);
-- 
2.43.0