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