public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Eric Sandeen <sandeen@redhat.com>
Cc: xfs <linux-xfs@vger.kernel.org>
Subject: [PATCH] xfs_repair: fix totally broken unit conversion in directory invalidation
Date: Tue, 17 Dec 2019 20:24:02 -0800	[thread overview]
Message-ID: <20191218042402.GL12765@magnolia> (raw)

From: Darrick J. Wong <darrick.wong@oracle.com>

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.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/xfs_dir2.h |   10 ++++++++++
 repair/phase6.c   |    8 ++------
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/libxfs/xfs_dir2.h b/libxfs/xfs_dir2.h
index f5424477..13221243 100644
--- a/libxfs/xfs_dir2.h
+++ b/libxfs/xfs_dir2.h
@@ -308,6 +308,16 @@ xfs_dir2_leaf_tail_p(struct xfs_da_geometry *geo, struct xfs_dir2_leaf *lp)
 		  sizeof(struct xfs_dir2_leaf_tail));
 }
 
+/*
+ * For a given dir/attr geometry and extent mapping record, walk every file
+ * offset block (xfs_dablk_t) in the mapping that corresponds to the start
+ * of a logical directory block (xfs_dir2_db_t).
+ */
+#define for_each_xfs_bmap_dabno(geo, irec, dabno) \
+	for ((dabno) = round_up((irec)->br_startoff, (geo)->fsbcount); \
+	     (dabno) < (irec)->br_startoff + (irec)->br_blockcount; \
+	     (dabno) += (geo)->fsbcount)
+
 /*
  * The Linux API doesn't pass down the total size of the buffer
  * we read into down to the filesystem.  With the filldir concept
diff --git a/repair/phase6.c b/repair/phase6.c
index 91d208a6..a4dd3188 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,7 @@ 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_each_xfs_bmap_dabno(geo, &rec, dabno) {
 			bp = NULL;
 			error = -libxfs_da_get_buf(tp, ip, dabno, -2, &bp,
 					whichfork);

             reply	other threads:[~2019-12-18  4:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-18  4:24 Darrick J. Wong [this message]
2019-12-18  4:26 ` [PATCH] xfs_repair: fix totally broken unit conversion in directory invalidation Darrick J. Wong
2019-12-24  8:38 ` Christoph Hellwig
2019-12-24 16:52   ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191218042402.GL12765@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox