From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Brian Foster <bfoster@redhat.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 1/9] xfs: separate the marking of sick and checked metadata
Date: Wed, 20 Nov 2019 08:12:57 -0800 [thread overview]
Message-ID: <20191120161257.GI6219@magnolia> (raw)
In-Reply-To: <20191120142035.GB15542@bfoster>
On Wed, Nov 20, 2019 at 09:20:35AM -0500, Brian Foster wrote:
> On Thu, Nov 14, 2019 at 10:19:20AM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Split the setting of the sick and checked masks into separate functions
> > as part of preparing to add the ability for regular runtime fs code
> > (i.e. not scrub) to mark metadata structures sick when corruptions are
> > found. Improve the documentation of libxfs' requirements for helper
> > behavior.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > fs/xfs/libxfs/xfs_health.h | 24 ++++++++++++++++++----
> > fs/xfs/scrub/health.c | 20 +++++++++++-------
> > fs/xfs/xfs_health.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
> > fs/xfs/xfs_mount.c | 5 ++++
> > 4 files changed, 85 insertions(+), 13 deletions(-)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
> > index 272005ac8c88..3657a9cb8490 100644
> > --- a/fs/xfs/libxfs/xfs_health.h
> > +++ b/fs/xfs/libxfs/xfs_health.h
> > @@ -26,9 +26,11 @@
> > * and the "sick" field tells us if that piece was found to need repairs.
> > * Therefore we can conclude that for a given sick flag value:
> > *
> > - * - checked && sick => metadata needs repair
> > - * - checked && !sick => metadata is ok
> > - * - !checked => has not been examined since mount
> > + * - checked && sick => metadata needs repair
> > + * - checked && !sick => metadata is ok
> > + * - !checked && sick => errors have been observed during normal operation,
> > + * but the metadata has not been checked thoroughly
> > + * - !checked && !sick => has not been examined since mount
> > */
> >
>
> I don't see this change in the provided repo. Which is the right patch?
Hmm, I guess I need to update the repo again. :/
> > struct xfs_mount;
> > @@ -97,24 +99,38 @@ struct xfs_fsop_geom;
> > XFS_SICK_INO_SYMLINK | \
> > XFS_SICK_INO_PARENT)
> >
> > -/* These functions must be provided by the xfs implementation. */
> > +/*
> > + * These functions must be provided by the xfs implementation. Function
> > + * behavior with respect to the first argument should be as follows:
> > + *
> > + * xfs_*_mark_sick: set the sick flags and do not set checked flags.
>
> Nit: It's probably not necessary to say that we don't set the checked
> flags here given the comment/function below.
Ok.
--D
> Brian
>
> > + * xfs_*_mark_checked: set the checked flags.
> > + * xfs_*_mark_healthy: clear the sick flags and set the checked flags.
> > + *
> > + * xfs_*_measure_sickness: return the sick and check status in the provided
> > + * out parameters.
> > + */
> >
> > void xfs_fs_mark_sick(struct xfs_mount *mp, unsigned int mask);
> > +void xfs_fs_mark_checked(struct xfs_mount *mp, unsigned int mask);
> > void xfs_fs_mark_healthy(struct xfs_mount *mp, unsigned int mask);
> > void xfs_fs_measure_sickness(struct xfs_mount *mp, unsigned int *sick,
> > unsigned int *checked);
> >
> > void xfs_rt_mark_sick(struct xfs_mount *mp, unsigned int mask);
> > +void xfs_rt_mark_checked(struct xfs_mount *mp, unsigned int mask);
> > void xfs_rt_mark_healthy(struct xfs_mount *mp, unsigned int mask);
> > void xfs_rt_measure_sickness(struct xfs_mount *mp, unsigned int *sick,
> > unsigned int *checked);
> >
> > void xfs_ag_mark_sick(struct xfs_perag *pag, unsigned int mask);
> > +void xfs_ag_mark_checked(struct xfs_perag *pag, unsigned int mask);
> > void xfs_ag_mark_healthy(struct xfs_perag *pag, unsigned int mask);
> > void xfs_ag_measure_sickness(struct xfs_perag *pag, unsigned int *sick,
> > unsigned int *checked);
> >
> > void xfs_inode_mark_sick(struct xfs_inode *ip, unsigned int mask);
> > +void xfs_inode_mark_checked(struct xfs_inode *ip, unsigned int mask);
> > void xfs_inode_mark_healthy(struct xfs_inode *ip, unsigned int mask);
> > void xfs_inode_measure_sickness(struct xfs_inode *ip, unsigned int *sick,
> > unsigned int *checked);
> > diff --git a/fs/xfs/scrub/health.c b/fs/xfs/scrub/health.c
> > index 83d27cdf579b..a402f9026d5f 100644
> > --- a/fs/xfs/scrub/health.c
> > +++ b/fs/xfs/scrub/health.c
> > @@ -137,30 +137,34 @@ xchk_update_health(
> > switch (type_to_health_flag[sc->sm->sm_type].group) {
> > case XHG_AG:
> > pag = xfs_perag_get(sc->mp, sc->sm->sm_agno);
> > - if (bad)
> > + if (bad) {
> > xfs_ag_mark_sick(pag, sc->sick_mask);
> > - else
> > + xfs_ag_mark_checked(pag, sc->sick_mask);
> > + } else
> > xfs_ag_mark_healthy(pag, sc->sick_mask);
> > xfs_perag_put(pag);
> > break;
> > case XHG_INO:
> > if (!sc->ip)
> > return;
> > - if (bad)
> > + if (bad) {
> > xfs_inode_mark_sick(sc->ip, sc->sick_mask);
> > - else
> > + xfs_inode_mark_checked(sc->ip, sc->sick_mask);
> > + } else
> > xfs_inode_mark_healthy(sc->ip, sc->sick_mask);
> > break;
> > case XHG_FS:
> > - if (bad)
> > + if (bad) {
> > xfs_fs_mark_sick(sc->mp, sc->sick_mask);
> > - else
> > + xfs_fs_mark_checked(sc->mp, sc->sick_mask);
> > + } else
> > xfs_fs_mark_healthy(sc->mp, sc->sick_mask);
> > break;
> > case XHG_RT:
> > - if (bad)
> > + if (bad) {
> > xfs_rt_mark_sick(sc->mp, sc->sick_mask);
> > - else
> > + xfs_rt_mark_checked(sc->mp, sc->sick_mask);
> > + } else
> > xfs_rt_mark_healthy(sc->mp, sc->sick_mask);
> > break;
> > default:
> > diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
> > index 8e0cb05a7142..860dc70c99e7 100644
> > --- a/fs/xfs/xfs_health.c
> > +++ b/fs/xfs/xfs_health.c
> > @@ -100,6 +100,18 @@ xfs_fs_mark_sick(
> >
> > spin_lock(&mp->m_sb_lock);
> > mp->m_fs_sick |= mask;
> > + spin_unlock(&mp->m_sb_lock);
> > +}
> > +
> > +/* Mark per-fs metadata as having been checked. */
> > +void
> > +xfs_fs_mark_checked(
> > + struct xfs_mount *mp,
> > + unsigned int mask)
> > +{
> > + ASSERT(!(mask & ~XFS_SICK_FS_PRIMARY));
> > +
> > + spin_lock(&mp->m_sb_lock);
> > mp->m_fs_checked |= mask;
> > spin_unlock(&mp->m_sb_lock);
> > }
> > @@ -143,6 +155,19 @@ xfs_rt_mark_sick(
> >
> > spin_lock(&mp->m_sb_lock);
> > mp->m_rt_sick |= mask;
> > + spin_unlock(&mp->m_sb_lock);
> > +}
> > +
> > +/* Mark realtime metadata as having been checked. */
> > +void
> > +xfs_rt_mark_checked(
> > + struct xfs_mount *mp,
> > + unsigned int mask)
> > +{
> > + ASSERT(!(mask & ~XFS_SICK_RT_PRIMARY));
> > + trace_xfs_rt_mark_sick(mp, mask);
> > +
> > + spin_lock(&mp->m_sb_lock);
> > mp->m_rt_checked |= mask;
> > spin_unlock(&mp->m_sb_lock);
> > }
> > @@ -186,6 +211,18 @@ xfs_ag_mark_sick(
> >
> > spin_lock(&pag->pag_state_lock);
> > pag->pag_sick |= mask;
> > + spin_unlock(&pag->pag_state_lock);
> > +}
> > +
> > +/* Mark per-ag metadata as having been checked. */
> > +void
> > +xfs_ag_mark_checked(
> > + struct xfs_perag *pag,
> > + unsigned int mask)
> > +{
> > + ASSERT(!(mask & ~XFS_SICK_AG_PRIMARY));
> > +
> > + spin_lock(&pag->pag_state_lock);
> > pag->pag_checked |= mask;
> > spin_unlock(&pag->pag_state_lock);
> > }
> > @@ -229,6 +266,18 @@ xfs_inode_mark_sick(
> >
> > spin_lock(&ip->i_flags_lock);
> > ip->i_sick |= mask;
> > + spin_unlock(&ip->i_flags_lock);
> > +}
> > +
> > +/* Mark inode metadata as having been checked. */
> > +void
> > +xfs_inode_mark_checked(
> > + struct xfs_inode *ip,
> > + unsigned int mask)
> > +{
> > + ASSERT(!(mask & ~XFS_SICK_INO_PRIMARY));
> > +
> > + spin_lock(&ip->i_flags_lock);
> > ip->i_checked |= mask;
> > spin_unlock(&ip->i_flags_lock);
> > }
> > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> > index fca65109cf24..27aa143d524b 100644
> > --- a/fs/xfs/xfs_mount.c
> > +++ b/fs/xfs/xfs_mount.c
> > @@ -555,8 +555,10 @@ xfs_check_summary_counts(
> > if (XFS_LAST_UNMOUNT_WAS_CLEAN(mp) &&
> > (mp->m_sb.sb_fdblocks > mp->m_sb.sb_dblocks ||
> > !xfs_verify_icount(mp, mp->m_sb.sb_icount) ||
> > - mp->m_sb.sb_ifree > mp->m_sb.sb_icount))
> > + mp->m_sb.sb_ifree > mp->m_sb.sb_icount)) {
> > xfs_fs_mark_sick(mp, XFS_SICK_FS_COUNTERS);
> > + xfs_fs_mark_checked(mp, XFS_SICK_FS_COUNTERS);
> > + }
> >
> > /*
> > * We can safely re-initialise incore superblock counters from the
> > @@ -1322,6 +1324,7 @@ xfs_force_summary_recalc(
> > return;
> >
> > xfs_fs_mark_sick(mp, XFS_SICK_FS_COUNTERS);
> > + xfs_fs_mark_checked(mp, XFS_SICK_FS_COUNTERS);
> > }
> >
> > /*
> >
>
next prev parent reply other threads:[~2019-11-20 16:13 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-14 18:19 [PATCH v4 0/9] xfs: report corruption to the health trackers Darrick J. Wong
2019-11-14 18:19 ` [PATCH 1/9] xfs: separate the marking of sick and checked metadata Darrick J. Wong
2019-11-20 14:20 ` Brian Foster
2019-11-20 16:12 ` Darrick J. Wong [this message]
2019-11-14 18:19 ` [PATCH 2/9] xfs: report ag header corruption errors to the health tracking system Darrick J. Wong
2019-11-20 14:20 ` Brian Foster
2019-11-20 16:43 ` Darrick J. Wong
2019-11-21 13:26 ` Brian Foster
2019-11-22 0:53 ` Darrick J. Wong
2019-11-22 11:57 ` Brian Foster
2019-11-22 18:10 ` Darrick J. Wong
2019-11-14 18:19 ` [PATCH 3/9] xfs: report block map " Darrick J. Wong
2019-11-20 14:21 ` Brian Foster
2019-11-20 16:57 ` Darrick J. Wong
2019-11-14 18:19 ` [PATCH 4/9] xfs: report btree block corruption errors to the health system Darrick J. Wong
2019-11-14 18:19 ` [PATCH 5/9] xfs: report dir/attr " Darrick J. Wong
2019-11-20 16:11 ` Brian Foster
2019-11-20 16:55 ` Darrick J. Wong
2019-11-21 13:26 ` Brian Foster
2019-11-22 1:03 ` Darrick J. Wong
2019-11-22 12:28 ` Brian Foster
2019-11-22 18:35 ` Darrick J. Wong
2019-11-14 18:19 ` [PATCH 6/9] xfs: report symlink " Darrick J. Wong
2019-11-14 18:19 ` [PATCH 7/9] xfs: report inode " Darrick J. Wong
2019-11-14 18:20 ` [PATCH 8/9] xfs: report quota block " Darrick J. Wong
2019-11-14 18:20 ` [PATCH 9/9] xfs: report realtime metadata " Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2023-12-31 19:42 [PATCHSET v29.0 09/40] xfsprogs: report corruption to the health trackers Darrick J. Wong
2023-12-31 22:11 ` [PATCH 1/9] xfs: separate the marking of sick and checked metadata 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=20191120161257.GI6219@magnolia \
--to=darrick.wong@oracle.com \
--cc=bfoster@redhat.com \
--cc=linux-xfs@vger.kernel.org \
/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 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.