Re: [PATCH 6/8] xfs: report fs and rt health via geometry structure

["Darrick J. Wong" <darrick.wong@oracle.com>]

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@oracle.com>
> > 
> > Use our newly expanded geometry structure to report the overall fs and
> > realtime health status.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  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);
> >