* [PATCH] xfs: don't write a corrupt unmount record to force summary counter recalc
@ 2020-03-27 1:14 Darrick J. Wong
2020-03-27 9:06 ` Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Darrick J. Wong @ 2020-03-27 1:14 UTC (permalink / raw)
To: Dave Chinner, Brian Foster; +Cc: xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
In commit f467cad95f5e3, I added the ability to force a recalculation of
the filesystem summary counters if they seemed incorrect. This was done
(not entirely correctly) by tweaking the log code to write an unmount
record without the UMOUNT_TRANS flag set. At next mount, the log
recovery code will fail to find the unmount record and go into recovery,
which triggers the recalculation.
What actually gets written to the log is what ought to be an unmount
record, but without any flags set to indicate what kind of record it
actually is. This worked to trigger the recalculation, but we shouldn't
write bogus log records when we could simply write nothing.
Fixes: f467cad95f5e3 ("xfs: force summary counter recalc at next mount")
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_log.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 46108ca20d85..00fda2e8e738 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -835,19 +835,6 @@ xlog_unmount_write(
if (error)
goto out_err;
- /*
- * If we think the summary counters are bad, clear the unmount header
- * flag in the unmount record so that the summary counters will be
- * recalculated during log recovery at next mount. Refer to
- * xlog_check_unmount_rec for more details.
- */
- if (XFS_TEST_ERROR(xfs_fs_has_sickness(mp, XFS_SICK_FS_COUNTERS), mp,
- XFS_ERRTAG_FORCE_SUMMARY_RECALC)) {
- xfs_alert(mp, "%s: will fix summary counters at next mount",
- __func__);
- flags &= ~XLOG_UNMOUNT_TRANS;
- }
-
error = xlog_write_unmount_record(log, tic, &lsn, flags);
/*
* At this point, we're umounting anyway, so there's no point in
@@ -913,6 +900,20 @@ xfs_log_unmount_write(
if (XLOG_FORCED_SHUTDOWN(log))
return;
+
+ /*
+ * If we think the summary counters are bad, avoid writing the unmount
+ * record to force log recovery at next mount, after which the summary
+ * counters will be recalculated. Refer to xlog_check_unmount_rec for
+ * more details.
+ */
+ if (XFS_TEST_ERROR(xfs_fs_has_sickness(mp, XFS_SICK_FS_COUNTERS), mp,
+ XFS_ERRTAG_FORCE_SUMMARY_RECALC)) {
+ xfs_alert(mp, "%s: will fix summary counters at next mount",
+ __func__);
+ return;
+ }
+
xfs_log_unmount_verify_iclog(log);
xlog_unmount_write(log);
}
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] xfs: don't write a corrupt unmount record to force summary counter recalc
2020-03-27 1:14 [PATCH] xfs: don't write a corrupt unmount record to force summary counter recalc Darrick J. Wong
@ 2020-03-27 9:06 ` Christoph Hellwig
2020-03-27 12:31 ` Brian Foster
2020-03-29 23:04 ` Dave Chinner
2 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2020-03-27 9:06 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Dave Chinner, Brian Foster, xfs
On Thu, Mar 26, 2020 at 06:14:17PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> In commit f467cad95f5e3, I added the ability to force a recalculation of
> the filesystem summary counters if they seemed incorrect. This was done
> (not entirely correctly) by tweaking the log code to write an unmount
> record without the UMOUNT_TRANS flag set. At next mount, the log
> recovery code will fail to find the unmount record and go into recovery,
> which triggers the recalculation.
>
> What actually gets written to the log is what ought to be an unmount
> record, but without any flags set to indicate what kind of record it
> actually is. This worked to trigger the recalculation, but we shouldn't
> write bogus log records when we could simply write nothing.
>
> Fixes: f467cad95f5e3 ("xfs: force summary counter recalc at next mount")
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks good (assuming the "xfs: refactor unmount record writing" is
applied):
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] xfs: don't write a corrupt unmount record to force summary counter recalc
2020-03-27 1:14 [PATCH] xfs: don't write a corrupt unmount record to force summary counter recalc Darrick J. Wong
2020-03-27 9:06 ` Christoph Hellwig
@ 2020-03-27 12:31 ` Brian Foster
2020-03-29 23:04 ` Dave Chinner
2 siblings, 0 replies; 4+ messages in thread
From: Brian Foster @ 2020-03-27 12:31 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Dave Chinner, xfs
On Thu, Mar 26, 2020 at 06:14:17PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> In commit f467cad95f5e3, I added the ability to force a recalculation of
> the filesystem summary counters if they seemed incorrect. This was done
> (not entirely correctly) by tweaking the log code to write an unmount
> record without the UMOUNT_TRANS flag set. At next mount, the log
> recovery code will fail to find the unmount record and go into recovery,
> which triggers the recalculation.
>
> What actually gets written to the log is what ought to be an unmount
> record, but without any flags set to indicate what kind of record it
> actually is. This worked to trigger the recalculation, but we shouldn't
> write bogus log records when we could simply write nothing.
>
> Fixes: f467cad95f5e3 ("xfs: force summary counter recalc at next mount")
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/xfs_log.c | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 46108ca20d85..00fda2e8e738 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -835,19 +835,6 @@ xlog_unmount_write(
> if (error)
> goto out_err;
>
> - /*
> - * If we think the summary counters are bad, clear the unmount header
> - * flag in the unmount record so that the summary counters will be
> - * recalculated during log recovery at next mount. Refer to
> - * xlog_check_unmount_rec for more details.
> - */
> - if (XFS_TEST_ERROR(xfs_fs_has_sickness(mp, XFS_SICK_FS_COUNTERS), mp,
> - XFS_ERRTAG_FORCE_SUMMARY_RECALC)) {
> - xfs_alert(mp, "%s: will fix summary counters at next mount",
> - __func__);
> - flags &= ~XLOG_UNMOUNT_TRANS;
> - }
> -
> error = xlog_write_unmount_record(log, tic, &lsn, flags);
> /*
> * At this point, we're umounting anyway, so there's no point in
> @@ -913,6 +900,20 @@ xfs_log_unmount_write(
>
> if (XLOG_FORCED_SHUTDOWN(log))
> return;
> +
> + /*
> + * If we think the summary counters are bad, avoid writing the unmount
> + * record to force log recovery at next mount, after which the summary
> + * counters will be recalculated. Refer to xlog_check_unmount_rec for
> + * more details.
> + */
> + if (XFS_TEST_ERROR(xfs_fs_has_sickness(mp, XFS_SICK_FS_COUNTERS), mp,
> + XFS_ERRTAG_FORCE_SUMMARY_RECALC)) {
> + xfs_alert(mp, "%s: will fix summary counters at next mount",
> + __func__);
> + return;
> + }
> +
> xfs_log_unmount_verify_iclog(log);
> xlog_unmount_write(log);
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] xfs: don't write a corrupt unmount record to force summary counter recalc
2020-03-27 1:14 [PATCH] xfs: don't write a corrupt unmount record to force summary counter recalc Darrick J. Wong
2020-03-27 9:06 ` Christoph Hellwig
2020-03-27 12:31 ` Brian Foster
@ 2020-03-29 23:04 ` Dave Chinner
2 siblings, 0 replies; 4+ messages in thread
From: Dave Chinner @ 2020-03-29 23:04 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Brian Foster, xfs
On Thu, Mar 26, 2020 at 06:14:17PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> In commit f467cad95f5e3, I added the ability to force a recalculation of
> the filesystem summary counters if they seemed incorrect. This was done
> (not entirely correctly) by tweaking the log code to write an unmount
> record without the UMOUNT_TRANS flag set. At next mount, the log
> recovery code will fail to find the unmount record and go into recovery,
> which triggers the recalculation.
>
> What actually gets written to the log is what ought to be an unmount
> record, but without any flags set to indicate what kind of record it
> actually is. This worked to trigger the recalculation, but we shouldn't
> write bogus log records when we could simply write nothing.
>
> Fixes: f467cad95f5e3 ("xfs: force summary counter recalc at next mount")
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/xfs_log.c | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
Looks fine.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-29 23:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-27 1:14 [PATCH] xfs: don't write a corrupt unmount record to force summary counter recalc Darrick J. Wong
2020-03-27 9:06 ` Christoph Hellwig
2020-03-27 12:31 ` Brian Foster
2020-03-29 23:04 ` Dave Chinner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).