From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 06/11] xfs_scrub: refactor inode prefix rendering code
Date: Tue, 15 Oct 2019 21:59:57 -0700 [thread overview]
Message-ID: <20191016045957.GV13108@magnolia> (raw)
In-Reply-To: <a4cd281d-8d05-debe-1af9-1192770c6cd1@sandeen.net>
On Tue, Oct 15, 2019 at 06:25:10PM -0500, Eric Sandeen wrote:
> How about this:
>
> Refactor all the places in the code where we try to render an inode
> number as a prefix for some sort of status message. This will help make
> message prefixes more consistent, which should help users to locate
> broken metadata.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> [sandeen: swizzle stuff]
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks good to me. :)
--D
> ---
>
> this uses a single function, scrub_render_ino_descr() with a new comment,
> and automatic space-adding for any extra format.
>
>
> diff --git a/scrub/common.c b/scrub/common.c
> index 7db47044..7f971de8 100644
> --- a/scrub/common.c
> +++ b/scrub/common.c
> @@ -354,3 +354,38 @@ within_range(
>
> return true;
> }
> +
> +/*
> + * Render an inode number into a buffer in a format suitable for use as a
> + * prefix for log messages. The buffer will be filled with:
> + * "inode <inode number> (<ag number>/<ag inode number>)"
> + * If the @format argument is non-NULL, it will be rendered into the buffer
> + * after the inode representation and a single space.
> + */
> +int
> +scrub_render_ino_descr(
> + const struct scrub_ctx *ctx,
> + char *buf,
> + size_t buflen,
> + uint64_t ino,
> + uint32_t gen,
> + const char *format,
> + ...)
> +{
> + va_list args;
> + uint32_t agno;
> + uint32_t agino;
> + int ret;
> +
> + agno = cvt_ino_to_agno(&ctx->mnt, ino);
> + agino = cvt_ino_to_agino(&ctx->mnt, ino);
> + ret = snprintf(buf, buflen, _("inode %"PRIu64" (%"PRIu32"/%"PRIu32")%s"),
> + ino, agno, agino, format ? " " : "");
> + if (ret < 0 || ret >= buflen || format == NULL)
> + return ret;
> +
> + va_start(args, format);
> + ret += vsnprintf(buf + ret, buflen - ret, format, args);
> + va_end(args);
> + return ret;
> +}
> diff --git a/scrub/common.h b/scrub/common.h
> index 33555891..9a37e9ed 100644
> --- a/scrub/common.h
> +++ b/scrub/common.h
> @@ -86,4 +86,8 @@ bool within_range(struct scrub_ctx *ctx, unsigned long long value,
> unsigned long long desired, unsigned long long abs_threshold,
> unsigned int n, unsigned int d, const char *descr);
>
> +int scrub_render_ino_descr(const struct scrub_ctx *ctx, char *buf,
> + size_t buflen, uint64_t ino, uint32_t gen,
> + const char *format, ...);
> +
> #endif /* XFS_SCRUB_COMMON_H_ */
> diff --git a/scrub/inodes.c b/scrub/inodes.c
> index 91632b55..7aa61ebe 100644
> --- a/scrub/inodes.c
> +++ b/scrub/inodes.c
> @@ -159,8 +159,8 @@ xfs_iterate_inodes_ag(
> ireq->hdr.ino = inumbers->xi_startino;
> goto igrp_retry;
> }
> - snprintf(idescr, DESCR_BUFSZ, "inode %"PRIu64,
> - (uint64_t)bs->bs_ino);
> + scrub_render_ino_descr(ctx, idescr, DESCR_BUFSZ,
> + bs->bs_ino, bs->bs_gen, NULL);
> str_info(ctx, idescr,
> _("Changed too many times during scan; giving up."));
> break;
> diff --git a/scrub/phase3.c b/scrub/phase3.c
> index 1e908c2c..0d2c9019 100644
> --- a/scrub/phase3.c
> +++ b/scrub/phase3.c
> @@ -48,14 +48,10 @@ xfs_scrub_inode_vfs_error(
> struct xfs_bulkstat *bstat)
> {
> char descr[DESCR_BUFSZ];
> - xfs_agnumber_t agno;
> - xfs_agino_t agino;
> int old_errno = errno;
>
> - agno = cvt_ino_to_agno(&ctx->mnt, bstat->bs_ino);
> - agino = cvt_ino_to_agino(&ctx->mnt, bstat->bs_ino);
> - snprintf(descr, DESCR_BUFSZ, _("inode %"PRIu64" (%u/%u)"),
> - (uint64_t)bstat->bs_ino, agno, agino);
> + scrub_render_ino_descr(ctx, descr, DESCR_BUFSZ, bstat->bs_ino,
> + bstat->bs_gen, NULL);
> errno = old_errno;
> str_errno(ctx, descr);
> }
> diff --git a/scrub/phase5.c b/scrub/phase5.c
> index 99cd51b2..27941907 100644
> --- a/scrub/phase5.c
> +++ b/scrub/phase5.c
> @@ -234,15 +234,11 @@ xfs_scrub_connections(
> bool *pmoveon = arg;
> char descr[DESCR_BUFSZ];
> bool moveon = true;
> - xfs_agnumber_t agno;
> - xfs_agino_t agino;
> int fd = -1;
> int error;
>
> - agno = cvt_ino_to_agno(&ctx->mnt, bstat->bs_ino);
> - agino = cvt_ino_to_agino(&ctx->mnt, bstat->bs_ino);
> - snprintf(descr, DESCR_BUFSZ, _("inode %"PRIu64" (%u/%u)"),
> - (uint64_t)bstat->bs_ino, agno, agino);
> + scrub_render_ino_descr(ctx, descr, DESCR_BUFSZ, bstat->bs_ino,
> + bstat->bs_gen, NULL);
> background_sleep();
>
> /* Warn about naming problems in xattrs. */
> diff --git a/scrub/phase6.c b/scrub/phase6.c
> index 8063d6ce..2ce2a19e 100644
> --- a/scrub/phase6.c
> +++ b/scrub/phase6.c
> @@ -180,15 +180,15 @@ xfs_report_verify_inode(
> int fd;
> int error;
>
> - snprintf(descr, DESCR_BUFSZ, _("inode %"PRIu64" (unlinked)"),
> - (uint64_t)bstat->bs_ino);
> -
> /* Ignore linked files and things we can't open. */
> if (bstat->bs_nlink != 0)
> return 0;
> if (!S_ISREG(bstat->bs_mode) && !S_ISDIR(bstat->bs_mode))
> return 0;
>
> + scrub_render_ino_descr(ctx, descr, DESCR_BUFSZ,
> + bstat->bs_ino, bstat->bs_gen, _("(unlinked)"));
> +
> /* Try to open the inode. */
> fd = xfs_open_handle(handle);
> if (fd < 0) {
> diff --git a/scrub/scrub.c b/scrub/scrub.c
> index d7a6b49b..0293ce30 100644
> --- a/scrub/scrub.c
> +++ b/scrub/scrub.c
> @@ -26,11 +26,13 @@
> /* Format a scrub description. */
> static void
> format_scrub_descr(
> + struct scrub_ctx *ctx,
> char *buf,
> size_t buflen,
> - struct xfs_scrub_metadata *meta,
> - const struct xfrog_scrub_descr *sc)
> + struct xfs_scrub_metadata *meta)
> {
> + const struct xfrog_scrub_descr *sc = &xfrog_scrubbers[meta->sm_type];
> +
> switch (sc->type) {
> case XFROG_SCRUB_TYPE_AGHEADER:
> case XFROG_SCRUB_TYPE_PERAG:
> @@ -38,8 +40,9 @@ format_scrub_descr(
> _(sc->descr));
> break;
> case XFROG_SCRUB_TYPE_INODE:
> - snprintf(buf, buflen, _("Inode %"PRIu64" %s"),
> - (uint64_t)meta->sm_ino, _(sc->descr));
> + scrub_render_ino_descr(ctx, buf, buflen,
> + meta->sm_ino, meta->sm_gen, "%s",
> + _(sc->descr));
> break;
> case XFROG_SCRUB_TYPE_FS:
> snprintf(buf, buflen, _("%s"), _(sc->descr));
> @@ -123,8 +126,7 @@ xfs_check_metadata(
>
> assert(!debug_tweak_on("XFS_SCRUB_NO_KERNEL"));
> assert(meta->sm_type < XFS_SCRUB_TYPE_NR);
> - format_scrub_descr(buf, DESCR_BUFSZ, meta,
> - &xfrog_scrubbers[meta->sm_type]);
> + format_scrub_descr(ctx, buf, DESCR_BUFSZ, meta);
>
> dbg_printf("check %s flags %xh\n", buf, meta->sm_flags);
> retry:
> @@ -681,8 +683,7 @@ xfs_repair_metadata(
> return CHECK_RETRY;
>
> memcpy(&oldm, &meta, sizeof(oldm));
> - format_scrub_descr(buf, DESCR_BUFSZ, &meta,
> - &xfrog_scrubbers[meta.sm_type]);
> + format_scrub_descr(ctx, buf, DESCR_BUFSZ, &meta);
>
> if (needs_repair(&meta))
> str_info(ctx, buf, _("Attempting repair."));
>
next prev parent reply other threads:[~2019-10-16 5:00 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-25 21:34 [PATCH 00/11] xfs_scrub: fix IO error detection during media verify Darrick J. Wong
2019-09-25 21:34 ` [PATCH 01/11] xfs_scrub: fix handling of read-verify pool runtime errors Darrick J. Wong
2019-10-15 17:07 ` Eric Sandeen
2019-10-15 17:19 ` Darrick J. Wong
2019-10-15 17:21 ` [PATCH v2 " Darrick J. Wong
2019-09-25 21:35 ` [PATCH 02/11] xfs_scrub: abort all read verification work immediately on error Darrick J. Wong
2019-09-25 21:35 ` [PATCH 03/11] xfs_scrub: fix read-verify pool error communication problems Darrick J. Wong
2019-09-25 21:35 ` [PATCH 04/11] xfs_scrub: fix queue-and-stash of non-contiguous verify requests Darrick J. Wong
2019-09-25 21:35 ` [PATCH 05/11] xfs_scrub: only call read_verify_force_io once per pool Darrick J. Wong
2019-09-25 21:35 ` [PATCH 06/11] xfs_scrub: refactor inode prefix rendering code Darrick J. Wong
2019-10-15 22:12 ` Eric Sandeen
2019-10-15 23:25 ` Eric Sandeen
2019-10-16 4:59 ` Darrick J. Wong [this message]
2019-09-25 21:35 ` [PATCH 07/11] xfs_scrub: record disk LBA size Darrick J. Wong
2019-10-17 1:35 ` Eric Sandeen
2019-09-25 21:35 ` [PATCH 08/11] xfs_scrub: enforce read verify pool minimum io size Darrick J. Wong
2019-10-17 1:46 ` Eric Sandeen
2019-09-25 21:35 ` [PATCH 09/11] xfs_scrub: return bytes verified from a SCSI VERIFY command Darrick J. Wong
2019-10-17 1:56 ` Eric Sandeen
2019-09-25 21:35 ` [PATCH 10/11] xfs_scrub: fix read verify disk error handling strategy Darrick J. Wong
2019-09-25 21:36 ` [PATCH 11/11] xfs_scrub: simulate errors in the read-verify phase Darrick J. Wong
2019-10-17 2:25 ` Eric Sandeen
-- strict thread matches above, loose matches on Subject: below --
2019-09-06 3:37 [PATCH 00/11] xfs_scrub: fix IO error detection during media verify Darrick J. Wong
2019-09-06 3:38 ` [PATCH 06/11] xfs_scrub: refactor inode prefix rendering code Darrick J. Wong
2019-08-26 21:29 [PATCH 00/11] xfs_scrub: fix IO error detection during media verify Darrick J. Wong
2019-08-26 21:30 ` [PATCH 06/11] xfs_scrub: refactor inode prefix rendering code 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=20191016045957.GV13108@magnolia \
--to=darrick.wong@oracle.com \
--cc=linux-xfs@vger.kernel.org \
--cc=sandeen@sandeen.net \
/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