diff mbox series

[09/17] xfs_scrub: actually iterate all the bulkstat records

Message ID 173888086198.2738568.10758609523199339681.stgit@frogsfrogsfrogs (mailing list archive)
State Not Applicable, archived
Headers show
Series [01/17] libxfs: unmap xmbuf pages to avoid disaster | expand

Commit Message

Darrick J. Wong Feb. 6, 2025, 10:33 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

In scan_ag_bulkstat, we have a for loop that iterates all the
xfs_bulkstat records in breq->bulkstat.  The loop condition test should
test against the array length, not the number of bits set in an
unrelated data structure.  If ocount > xi_alloccount then we miss some
inodes; if ocount < xi_alloccount then we've walked off the end of the
array.

Cc: <linux-xfs@vger.kernel.org> # v5.18.0
Fixes: 245c72a6eeb720 ("xfs_scrub: balance inode chunk scan across CPUs")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 scrub/inodes.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Christoph Hellwig Feb. 7, 2025, 4:40 a.m. UTC | #1 
On Thu, Feb 06, 2025 at 02:33:01PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> In scan_ag_bulkstat, we have a for loop that iterates all the
> xfs_bulkstat records in breq->bulkstat.  The loop condition test should
> test against the array length, not the number of bits set in an
> unrelated data structure.  If ocount > xi_alloccount then we miss some
> inodes; if ocount < xi_alloccount then we've walked off the end of the
> array.

Look good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/scrub/inodes.c b/scrub/inodes.c
index 8bdfa0b35d6172..4d8b137a698004 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -216,7 +216,7 @@  scan_ag_bulkstat(
 	struct xfs_inumbers_req	*ireq = ichunk_to_inumbers(ichunk);
 	struct xfs_bulkstat_req	*breq = ichunk_to_bulkstat(ichunk);
 	struct scan_inodes	*si = ichunk->si;
-	struct xfs_bulkstat	*bs;
+	struct xfs_bulkstat	*bs = &breq->bulkstat[0];
 	struct xfs_inumbers	*inumbers = &ireq->inumbers[0];
 	uint64_t		last_ino = 0;
 	int			i;
@@ -231,8 +231,7 @@  scan_ag_bulkstat(
 	bulkstat_for_inumbers(ctx, &dsc_inumbers, inumbers, breq);
 
 	/* Iterate all the inodes. */
-	bs = &breq->bulkstat[0];
-	for (i = 0; !si->aborted && i < inumbers->xi_alloccount; i++, bs++) {
+	for (i = 0; !si->aborted && i < breq->hdr.ocount; i++, bs++) {
 		uint64_t	scan_ino = bs->bs_ino;
 
 		/* ensure forward progress if we retried */