Re: On https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=775c17386a6fd

On Wed, 2026-05-27 at 15:52 +0800, Joseph Qi wrote:

> ocfs2_validate_dx_root() returns -EROFS
> ocfs2_read_blocks() goto read_failure → put_bh, bhs[0]=NULL, return -EROFS
> ocfs2_read_block() returns -EROFS
> ocfs2_read_dx_root() returns -EROFS
> ocfs2_find_entry_dx() goto out, returns -EROFS
> 
> On a second read of the same block, ocfs2_buffer_uptodate(ci, bh)
> returns false (block was never added to the ocfs2 metadata cache because
> the first read failed), so the block is re-read from disk and
> re-validated — failing again.
> 
> Am I missing something?

It seems that I've got the point. In ocfs2_read_blocks(), it's wrong
to assume that non-NULL BH returned by sb_getblk() is exclusively
owned by the caller and so put_bh() always drops b_count from 1 to 0.
If it is not so, BH remains on hold and likely to be returned by the
next call to sb_getblk() unchanged - that is, with BH_Uptodate bit set
even if it has failed validation previously. So BH_Uptodate should be
cleared immediately after validate() callback has detected some data
inconsistency, regardless of what may happens next with that BH:

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index 701d27d908d4..6114299b121e 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -350,8 +350,6 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 						wait_on_buffer(bh);
 					put_bh(bh);
 					bhs[i] = NULL;
-				} else if (bh && buffer_uptodate(bh)) {
-					clear_buffer_uptodate(bh);
 				}
 				continue;
 			}
@@ -380,8 +378,11 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 				BUG_ON(buffer_jbd(bh));
 				clear_buffer_needs_validate(bh);
 				status = validate(sb, bh);
-				if (status)
+				if (status) {
+					if (buffer_uptodate(bh))
+						clear_buffer_uptodate(bh);
 					goto read_failure;
+				}
 			}
 		}
 
Dmitry