| Message ID | 20180710094129.1052-1-zlang@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Jul 10, 2018 at 05:41:29PM +0800, Zorro Lang wrote: > The xfs_repair "-t" option shouldn't be used alone. An interval must > follow the -t option, or xfs_repair will report errors. And only > modify reporting interval is useless, if we don't enable ag_stride. > > Signed-off-by: Zorro Lang <zlang@redhat.com> Makes sense to me, at least given the getopt arg ("t:") in xfs_repair.c. Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > > Hi, > > I don't know why we must need the -t option for xfs_repair, I can't > find any description to explain it. But I can find an explanation > about why we use "-t" for _xfs_check. > > # xfs_check runs out of memory on large files, so even providing the test > # option (-t) to avoid indexing the free space trees doesn't make it pass on > # large filesystems. Avoid it. > > The -t option for xfs_repair is totally different with it for > xfs_check, maybe -m option is more useful if we think about the > memory size. > > And the -t option need to work with -o ag_stride together. I'd like > to remove the "-t" option directly, due to I really don't know why > we need it, or how to give it a proper number. > > If the original author knows why we need it, and can give me some > suggestions, please help. > > Thanks, > Zorro > > common/xfs | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/common/xfs b/common/xfs > index ecf54bbf..61a3c2d9 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -193,7 +193,6 @@ _scratch_xfs_repair() > SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV" > [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \ > SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -r$SCRATCH_RTDEV" > - [ "$LARGE_SCRATCH_DEV" = yes ] && SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t" > $XFS_REPAIR_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV > } > > -- > 2.14.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/10/18 4:41 AM, Zorro Lang wrote: > The xfs_repair "-t" option shouldn't be used alone. An interval must > follow the -t option, or xfs_repair will report errors. And only > modify reporting interval is useless, if we don't enable ag_stride. > > Signed-off-by: Zorro Lang <zlang@redhat.com> > --- I agree, -t looks like it should only have been used for xfs_check. This seems to go back a long way in history [1], but I think it has always been broken. Reviewed-by: Eric Sandeen <sandeen@redhat.com> [1] commit 6c6ba523c29d78a009d237e2a019d65beb5fced8 Author: fsgqa <fsgqa> Date: Fri Aug 29 06:04:54 2003 +0000 QA updates to enable simplified large filesystem testing > Hi, > > I don't know why we must need the -t option for xfs_repair, I can't > find any description to explain it. But I can find an explanation > about why we use "-t" for _xfs_check. > > # xfs_check runs out of memory on large files, so even providing the test > # option (-t) to avoid indexing the free space trees doesn't make it pass on > # large filesystems. Avoid it. > > The -t option for xfs_repair is totally different with it for > xfs_check, maybe -m option is more useful if we think about the > memory size. > > And the -t option need to work with -o ag_stride together. I'd like > to remove the "-t" option directly, due to I really don't know why > we need it, or how to give it a proper number. > > If the original author knows why we need it, and can give me some > suggestions, please help. > > Thanks, > Zorro > > common/xfs | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/common/xfs b/common/xfs > index ecf54bbf..61a3c2d9 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -193,7 +193,6 @@ _scratch_xfs_repair() > SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV" > [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \ > SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -r$SCRATCH_RTDEV" > - [ "$LARGE_SCRATCH_DEV" = yes ] && SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t" > $XFS_REPAIR_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV > } > > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/common/xfs b/common/xfs index ecf54bbf..61a3c2d9 100644 --- a/common/xfs +++ b/common/xfs @@ -193,7 +193,6 @@ _scratch_xfs_repair() SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV" [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \ SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -r$SCRATCH_RTDEV" - [ "$LARGE_SCRATCH_DEV" = yes ] && SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t" $XFS_REPAIR_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV }
The xfs_repair "-t" option shouldn't be used alone. An interval must follow the -t option, or xfs_repair will report errors. And only modify reporting interval is useless, if we don't enable ag_stride. Signed-off-by: Zorro Lang <zlang@redhat.com> --- Hi, I don't know why we must need the -t option for xfs_repair, I can't find any description to explain it. But I can find an explanation about why we use "-t" for _xfs_check. # xfs_check runs out of memory on large files, so even providing the test # option (-t) to avoid indexing the free space trees doesn't make it pass on # large filesystems. Avoid it. The -t option for xfs_repair is totally different with it for xfs_check, maybe -m option is more useful if we think about the memory size. And the -t option need to work with -o ag_stride together. I'd like to remove the "-t" option directly, due to I really don't know why we need it, or how to give it a proper number. If the original author knows why we need it, and can give me some suggestions, please help. Thanks, Zorro common/xfs | 1 - 1 file changed, 1 deletion(-)