On 12/31/19 6:20 PM, Darrick J. Wong wrote:
From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
Your humble author forgot that xfs_dablk_t has the same units as
xfs_fileoff_t, and totally screwed up the directory buffer invalidation
loop in dir_binval. Not only is there an off-by-one error in the loop
conditional, but the unit conversions are wrong.
Fix all this stupidity by adding a for loop macro to take care of these
details for us so that everyone can iterate all logical directory blocks
(xfs_dir2_db_t) that start within a given bmbt record.
The pre-5.5 xfs_da_get_buf implementation mostly hides the off-by-one
error because dir_binval turns on "don't complain if no mapping" mode,
but on dirblocksize > fsblocksize filesystems the incorrect units can
cause us to miss invalidating some blocks, which can lead to other
buffer cache errors later.
Fixes: f9c559f4e4fb4 ("xfs_repair: invalidate dirty dir buffers when we zap a directory")
Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
This one looks ok to me. Thanks!
Reviewed-by: Allison Collins <allison.henderson@xxxxxxxxxx>
---
repair/phase6.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/repair/phase6.c b/repair/phase6.c
index 91d208a6..a11712a2 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -1276,7 +1276,7 @@ dir_binval(
struct xfs_ifork *ifp;
struct xfs_da_geometry *geo;
struct xfs_buf *bp;
- xfs_dablk_t dabno, end_dabno;
+ xfs_dablk_t dabno;
int error = 0;
if (ip->i_d.di_format != XFS_DINODE_FMT_EXTENTS &&
@@ -1286,11 +1286,9 @@ dir_binval(
geo = tp->t_mountp->m_dir_geo;
ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
for_each_xfs_iext(ifp, &icur, &rec) {
- dabno = xfs_dir2_db_to_da(geo, rec.br_startoff +
- geo->fsbcount - 1);
- end_dabno = xfs_dir2_db_to_da(geo, rec.br_startoff +
- rec.br_blockcount);
- for (; dabno <= end_dabno; dabno += geo->fsbcount) {
+ for (dabno = roundup(rec.br_startoff, geo->fsbcount);
+ dabno < rec.br_startoff + rec.br_blockcount;
+ dabno += geo->fsbcount) {
bp = NULL;
error = -libxfs_da_get_buf(tp, ip, dabno, -2, &bp,
whichfork);