On Thu, Apr 11, 2019 at 09:09:10AM -0400, Brian Foster wrote:
> On Wed, Apr 10, 2019 at 06:46:04PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Use our newly expanded geometry structure to report the overall fs and
> > realtime health status.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_fs.h | 11 ++++++++
> > fs/xfs/libxfs/xfs_health.h | 3 ++
> > fs/xfs/xfs_health.c | 57 ++++++++++++++++++++++++++++++++++++++++++++
> > fs/xfs/xfs_ioctl.c | 2 ++
> > 4 files changed, 72 insertions(+), 1 deletion(-)
> >
> >
> ...
> > diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
> > index 21728228e08b..eb8dbc3a952a 100644
> > --- a/fs/xfs/xfs_health.c
> > +++ b/fs/xfs/xfs_health.c
> > @@ -264,3 +264,60 @@ xfs_inode_measure_sickness(
> > *checked = ip->i_checked;
> > spin_unlock(&ip->i_flags_lock);
> > }
> ...
> > +
> > +static inline void
> > +xfgeo_health_tick(
> > + struct xfs_fsop_geom *geo,
> > + unsigned int sick,
> > + unsigned int checked,
> > + unsigned int sick_mask,
> > + unsigned int fsop_mask)
> > +{
>
> Could we just pass the struct ioctl_sick_map to this helper? IMO, that
> makes the mapping logic a bit more explicit and also doesn't introduce
> an unnecessary field name change between ->ioctl_mask and fsop_mask.
Ok.
> FWIW, I might also rename ->ioctl_mask to ->geom_mask since it's
> specific to that structure.
> > + if (checked & sick_mask)
> > + geo->checked |= fsop_mask;
> > + if (sick & sick_mask)
> > + geo->sick |= fsop_mask;
> > +}
> > +
> ...
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index 0aaf4f88524d..3e3ee197bd0f 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> ...
> > @@ -792,6 +793,7 @@ xfs_ioc_fsgeometry(
> > error = xfs_fs_geometry(&mp->m_sb, &fsgeo, struct_version);
> > if (error)
> > return error;
> > + xfs_fsop_geom_health(mp, &fsgeo);
> >
>
> Not really a problem, but it might make sense to not bother with this
> unless struct_version >= 5 (via the already existing check below).
Ok.
--D
> Brian
>
> > if (struct_version <= 3)
> > len = sizeof(struct xfs_fsop_geom_v1);
> >
