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

[Brian Foster <bfoster@redhat.com>]

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.
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).

Brian

>  	if (struct_version <= 3)
>  		len = sizeof(struct xfs_fsop_geom_v1);
>