* [PATCH] xfs: fix the forward progress assertion in xfs_iwalk_run_callbacks
@ 2020-12-08 17:16 Darrick J. Wong
2020-12-08 17:45 ` Zorro Lang
2020-12-09 4:27 ` Dave Chinner
0 siblings, 2 replies; 3+ messages in thread
From: Darrick J. Wong @ 2020-12-08 17:16 UTC (permalink / raw)
To: Zorro Lang; +Cc: xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
In commit 27c14b5daa82 we started tracking the last inode seen during an
inode walk to avoid infinite loops if a corrupt inobt record happens to
have a lower ir_startino than the record preceeding it. Unfortunately,
the assertion trips over the case where there are completely empty inobt
records (which can happen quite easily on 64k page filesystems) because
we advance the tracking cursor without actually putting the empty record
into the processing buffer. Fix the assert to allow for this case.
Reported-by: zlang@redhat.com
Fixes: 27c14b5daa82 ("xfs: ensure inobt record walks always make forward progress")
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_iwalk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
index 2a45138831e3..eae3aff9bc97 100644
--- a/fs/xfs/xfs_iwalk.c
+++ b/fs/xfs/xfs_iwalk.c
@@ -363,7 +363,7 @@ xfs_iwalk_run_callbacks(
/* Delete cursor but remember the last record we cached... */
xfs_iwalk_del_inobt(tp, curpp, agi_bpp, 0);
irec = &iwag->recs[iwag->nr_recs - 1];
- ASSERT(next_agino == irec->ir_startino + XFS_INODES_PER_CHUNK);
+ ASSERT(next_agino >= irec->ir_startino + XFS_INODES_PER_CHUNK);
error = xfs_iwalk_ag_recs(iwag);
if (error)
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] xfs: fix the forward progress assertion in xfs_iwalk_run_callbacks
2020-12-08 17:16 [PATCH] xfs: fix the forward progress assertion in xfs_iwalk_run_callbacks Darrick J. Wong
@ 2020-12-08 17:45 ` Zorro Lang
2020-12-09 4:27 ` Dave Chinner
1 sibling, 0 replies; 3+ messages in thread
From: Zorro Lang @ 2020-12-08 17:45 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs
On Tue, Dec 08, 2020 at 09:16:51AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> In commit 27c14b5daa82 we started tracking the last inode seen during an
> inode walk to avoid infinite loops if a corrupt inobt record happens to
> have a lower ir_startino than the record preceeding it. Unfortunately,
> the assertion trips over the case where there are completely empty inobt
> records (which can happen quite easily on 64k page filesystems) because
> we advance the tracking cursor without actually putting the empty record
> into the processing buffer. Fix the assert to allow for this case.
>
> Reported-by: zlang@redhat.com
> Fixes: 27c14b5daa82 ("xfs: ensure inobt record walks always make forward progress")
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
Looks good to me, and I just gave it a test on the same P9 machine which triggered
this bug, test passed.
Reviewed-by: Zorro Lang <zlang@redhat.com>
> fs/xfs/xfs_iwalk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
> index 2a45138831e3..eae3aff9bc97 100644
> --- a/fs/xfs/xfs_iwalk.c
> +++ b/fs/xfs/xfs_iwalk.c
> @@ -363,7 +363,7 @@ xfs_iwalk_run_callbacks(
> /* Delete cursor but remember the last record we cached... */
> xfs_iwalk_del_inobt(tp, curpp, agi_bpp, 0);
> irec = &iwag->recs[iwag->nr_recs - 1];
> - ASSERT(next_agino == irec->ir_startino + XFS_INODES_PER_CHUNK);
> + ASSERT(next_agino >= irec->ir_startino + XFS_INODES_PER_CHUNK);
>
> error = xfs_iwalk_ag_recs(iwag);
> if (error)
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] xfs: fix the forward progress assertion in xfs_iwalk_run_callbacks
2020-12-08 17:16 [PATCH] xfs: fix the forward progress assertion in xfs_iwalk_run_callbacks Darrick J. Wong
2020-12-08 17:45 ` Zorro Lang
@ 2020-12-09 4:27 ` Dave Chinner
1 sibling, 0 replies; 3+ messages in thread
From: Dave Chinner @ 2020-12-09 4:27 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Zorro Lang, xfs
On Tue, Dec 08, 2020 at 09:16:51AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> In commit 27c14b5daa82 we started tracking the last inode seen during an
> inode walk to avoid infinite loops if a corrupt inobt record happens to
> have a lower ir_startino than the record preceeding it. Unfortunately,
> the assertion trips over the case where there are completely empty inobt
> records (which can happen quite easily on 64k page filesystems) because
> we advance the tracking cursor without actually putting the empty record
> into the processing buffer. Fix the assert to allow for this case.
>
> Reported-by: zlang@redhat.com
> Fixes: 27c14b5daa82 ("xfs: ensure inobt record walks always make forward progress")
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/xfs_iwalk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
> index 2a45138831e3..eae3aff9bc97 100644
> --- a/fs/xfs/xfs_iwalk.c
> +++ b/fs/xfs/xfs_iwalk.c
> @@ -363,7 +363,7 @@ xfs_iwalk_run_callbacks(
> /* Delete cursor but remember the last record we cached... */
> xfs_iwalk_del_inobt(tp, curpp, agi_bpp, 0);
> irec = &iwag->recs[iwag->nr_recs - 1];
> - ASSERT(next_agino == irec->ir_startino + XFS_INODES_PER_CHUNK);
> + ASSERT(next_agino >= irec->ir_startino + XFS_INODES_PER_CHUNK);
Looks sensible.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-09 4:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-08 17:16 [PATCH] xfs: fix the forward progress assertion in xfs_iwalk_run_callbacks Darrick J. Wong
2020-12-08 17:45 ` Zorro Lang
2020-12-09 4:27 ` Dave Chinner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.