All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] xfs: online health tracking support
@ 2019-04-11  1:45 Darrick J. Wong
  2019-04-11  1:45 ` [PATCH 1/8] xfs: track metadata health status Darrick J. Wong
                   ` (7 more replies)
  0 siblings, 8 replies; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11  1:45 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

Hi all,

This series adds online health tracking capabilities to XFS, which
enables userspace to discover if any metadata corruptions have been
found (and not fixed) within a given class of metadata.

Reporting to userspace is handled by three ioctl modifications:
enhancements of the existing fs geometry ioctl to include sickness
information; enhancement of the existing bulkstat ioctl to report
sickness information, and a totally new ioctl to report allocation group
geometry and sickness data.

On the userspace side of things, xfs_scrub will be adapted to give a
clean bill of health to the kernel when it is warranted, and
xfs_spaceman will be able to report the health status of a filesystem.

The first three patches add the internals of the health reporting
infrastructure and convert the "bad summary" code to use it instead of
a mount state flag.

The five patches after that implement the ioctls to report health status
to userspace by enlarging the fs geometry ioctl, creating a new ag
geometry ioctl, and then adding fields to all three so that the kernel
can report what it has checked and what it knows to be good or bad.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=health-tracking

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=health-tracking

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/8] xfs: track metadata health status
  2019-04-11  1:45 [PATCH v2 0/8] xfs: online health tracking support Darrick J. Wong
@ 2019-04-11  1:45 ` Darrick J. Wong
  2019-04-11 12:29   ` Brian Foster
  2019-04-11  1:45 ` [PATCH 2/8] xfs: replace the BAD_SUMMARY mount flag with the equivalent health code Darrick J. Wong
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11  1:45 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Add the necessary in-core metadata fields to keep track of which parts
of the filesystem have been observed and which parts were observed to be
unhealthy, and print a warning at unmount time if we have unfixed
problems.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/Makefile            |    1 
 fs/xfs/libxfs/xfs_health.h |  175 ++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_health.c        |  192 ++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_icache.c        |    8 ++
 fs/xfs/xfs_inode.h         |    8 ++
 fs/xfs/xfs_mount.c         |    1 
 fs/xfs/xfs_mount.h         |   23 +++++
 fs/xfs/xfs_trace.h         |   73 +++++++++++++++++
 8 files changed, 481 insertions(+)
 create mode 100644 fs/xfs/libxfs/xfs_health.h
 create mode 100644 fs/xfs/xfs_health.c


diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
index 7f96bdadc372..786379c143f4 100644
--- a/fs/xfs/Makefile
+++ b/fs/xfs/Makefile
@@ -73,6 +73,7 @@ xfs-y				+= xfs_aops.o \
 				   xfs_fsmap.o \
 				   xfs_fsops.o \
 				   xfs_globals.o \
+				   xfs_health.o \
 				   xfs_icache.o \
 				   xfs_ioctl.o \
 				   xfs_iomap.o \
diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
new file mode 100644
index 000000000000..30762a5d4862
--- /dev/null
+++ b/fs/xfs/libxfs/xfs_health.h
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Oracle.  All Rights Reserved.
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ */
+#ifndef __XFS_HEALTH_H__
+#define __XFS_HEALTH_H__
+
+/*
+ * In-Core Filesystem Health Assessments
+ * =====================================
+ *
+ * We'd like to be able to summarize the current health status of the
+ * filesystem so that the administrator knows when it's necessary to schedule
+ * some downtime for repairs.  Until then, we would also like to avoid abrupt
+ * shutdowns due to corrupt metadata.
+ *
+ * The online scrub feature evaluates the health of all filesystem metadata.
+ * When scrub detects corruption in a piece of metadata it will set the
+ * corresponding sickness flag, and repair will clear it if successful.
+ *
+ * If problems remain at unmount time, we can also request manual intervention
+ * by logging a notice to run xfs_repair.
+ */
+
+struct xfs_mount;
+struct xfs_perag;
+struct xfs_inode;
+
+/* Observable health issues for metadata spanning the entire filesystem. */
+#define XFS_SICK_FS_COUNTERS	(1 << 0)  /* summary counters */
+#define XFS_SICK_FS_UQUOTA	(1 << 1)  /* user quota */
+#define XFS_SICK_FS_GQUOTA	(1 << 2)  /* group quota */
+#define XFS_SICK_FS_PQUOTA	(1 << 3)  /* project quota */
+
+/* Observable health issues for realtime volume metadata. */
+#define XFS_SICK_RT_BITMAP	(1 << 0)  /* realtime bitmap */
+#define XFS_SICK_RT_SUMMARY	(1 << 1)  /* realtime summary */
+
+/* Observable health issues for AG metadata. */
+#define XFS_SICK_AG_SB		(1 << 0)  /* superblock */
+#define XFS_SICK_AG_AGF		(1 << 1)  /* AGF header */
+#define XFS_SICK_AG_AGFL	(1 << 2)  /* AGFL header */
+#define XFS_SICK_AG_AGI		(1 << 3)  /* AGI header */
+#define XFS_SICK_AG_BNOBT	(1 << 4)  /* free space by block */
+#define XFS_SICK_AG_CNTBT	(1 << 5)  /* free space by length */
+#define XFS_SICK_AG_INOBT	(1 << 6)  /* inode index */
+#define XFS_SICK_AG_FINOBT	(1 << 7)  /* free inode index */
+#define XFS_SICK_AG_RMAPBT	(1 << 8)  /* reverse mappings */
+#define XFS_SICK_AG_REFCNTBT	(1 << 9)  /* reference counts */
+
+/* Observable health issues for inode metadata. */
+#define XFS_SICK_INO_CORE	(1 << 0)  /* inode core */
+#define XFS_SICK_INO_BMBTD	(1 << 1)  /* data fork */
+#define XFS_SICK_INO_BMBTA	(1 << 2)  /* attr fork */
+#define XFS_SICK_INO_BMBTC	(1 << 3)  /* cow fork */
+#define XFS_SICK_INO_DIR	(1 << 4)  /* directory */
+#define XFS_SICK_INO_XATTR	(1 << 5)  /* extended attributes */
+#define XFS_SICK_INO_SYMLINK	(1 << 6)  /* symbolic link remote target */
+#define XFS_SICK_INO_PARENT	(1 << 7)  /* parent pointers */
+
+/* Primary evidence of health problems in a given group. */
+#define XFS_SICK_FS_PRIMARY	(XFS_SICK_FS_COUNTERS | \
+				 XFS_SICK_FS_UQUOTA | \
+				 XFS_SICK_FS_GQUOTA | \
+				 XFS_SICK_FS_PQUOTA)
+
+#define XFS_SICK_RT_PRIMARY	(XFS_SICK_RT_BITMAP | \
+				 XFS_SICK_RT_SUMMARY)
+
+#define XFS_SICK_AG_PRIMARY	(XFS_SICK_AG_SB | \
+				 XFS_SICK_AG_AGF | \
+				 XFS_SICK_AG_AGFL | \
+				 XFS_SICK_AG_AGI | \
+				 XFS_SICK_AG_BNOBT | \
+				 XFS_SICK_AG_CNTBT | \
+				 XFS_SICK_AG_INOBT | \
+				 XFS_SICK_AG_FINOBT | \
+				 XFS_SICK_AG_RMAPBT | \
+				 XFS_SICK_AG_REFCNTBT)
+
+#define XFS_SICK_INO_PRIMARY	(XFS_SICK_INO_CORE | \
+				 XFS_SICK_INO_BMBTD | \
+				 XFS_SICK_INO_BMBTA | \
+				 XFS_SICK_INO_BMBTC | \
+				 XFS_SICK_INO_DIR | \
+				 XFS_SICK_INO_XATTR | \
+				 XFS_SICK_INO_SYMLINK | \
+				 XFS_SICK_INO_PARENT)
+
+/* These functions must be provided by the xfs implementation. */
+
+void xfs_fs_mark_sick(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_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_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_healthy(struct xfs_inode *ip, unsigned int mask);
+void xfs_inode_measure_sickness(struct xfs_inode *ip, unsigned int *sick,
+		unsigned int *checked);
+
+/* Now some helpers. */
+
+static inline bool
+xfs_fs_has_sickness(struct xfs_mount *mp, unsigned int mask)
+{
+	unsigned int	sick, checked;
+
+	xfs_fs_measure_sickness(mp, &sick, &checked);
+	return sick & mask;
+}
+
+static inline bool
+xfs_rt_has_sickness(struct xfs_mount *mp, unsigned int mask)
+{
+	unsigned int	sick, checked;
+
+	xfs_rt_measure_sickness(mp, &sick, &checked);
+	return sick & mask;
+}
+
+static inline bool
+xfs_ag_has_sickness(struct xfs_perag *pag, unsigned int mask)
+{
+	unsigned int	sick, checked;
+
+	xfs_ag_measure_sickness(pag, &sick, &checked);
+	return sick & mask;
+}
+
+static inline bool
+xfs_inode_has_sickness(struct xfs_inode *ip, unsigned int mask)
+{
+	unsigned int	sick, checked;
+
+	xfs_inode_measure_sickness(ip, &sick, &checked);
+	return sick & mask;
+}
+
+static inline bool
+xfs_fs_is_healthy(struct xfs_mount *mp)
+{
+	return !xfs_fs_has_sickness(mp, -1U);
+}
+
+static inline bool
+xfs_rt_is_healthy(struct xfs_mount *mp)
+{
+	return !xfs_rt_has_sickness(mp, -1U);
+}
+
+static inline bool
+xfs_ag_is_healthy(struct xfs_perag *pag)
+{
+	return !xfs_ag_has_sickness(pag, -1U);
+}
+
+static inline bool
+xfs_inode_is_healthy(struct xfs_inode *ip)
+{
+	return !xfs_inode_has_sickness(ip, -1U);
+}
+
+#endif	/* __XFS_HEALTH_H__ */
diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
new file mode 100644
index 000000000000..941f33037e2f
--- /dev/null
+++ b/fs/xfs/xfs_health.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Oracle.  All Rights Reserved.
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ */
+#include "xfs.h"
+#include "xfs_fs.h"
+#include "xfs_shared.h"
+#include "xfs_format.h"
+#include "xfs_log_format.h"
+#include "xfs_trans_resv.h"
+#include "xfs_bit.h"
+#include "xfs_sb.h"
+#include "xfs_mount.h"
+#include "xfs_defer.h"
+#include "xfs_da_format.h"
+#include "xfs_da_btree.h"
+#include "xfs_inode.h"
+#include "xfs_trace.h"
+#include "xfs_health.h"
+
+/* Mark unhealthy per-fs metadata. */
+void
+xfs_fs_mark_sick(
+	struct xfs_mount	*mp,
+	unsigned int		mask)
+{
+	ASSERT(!(mask & ~XFS_SICK_FS_PRIMARY));
+	trace_xfs_fs_mark_sick(mp, mask);
+
+	spin_lock(&mp->m_sb_lock);
+	mp->m_fs_sick |= mask;
+	mp->m_fs_checked |= mask;
+	spin_unlock(&mp->m_sb_lock);
+}
+
+/* Mark a per-fs metadata healed. */
+void
+xfs_fs_mark_healthy(
+	struct xfs_mount	*mp,
+	unsigned int		mask)
+{
+	ASSERT(!(mask & ~XFS_SICK_FS_PRIMARY));
+	trace_xfs_fs_mark_healthy(mp, mask);
+
+	spin_lock(&mp->m_sb_lock);
+	mp->m_fs_sick &= ~mask;
+	mp->m_fs_checked |= mask;
+	spin_unlock(&mp->m_sb_lock);
+}
+
+/* Sample which per-fs metadata are unhealthy. */
+void
+xfs_fs_measure_sickness(
+	struct xfs_mount	*mp,
+	unsigned int		*sick,
+	unsigned int		*checked)
+{
+	spin_lock(&mp->m_sb_lock);
+	*sick = mp->m_fs_sick;
+	*checked = mp->m_fs_checked;
+	spin_unlock(&mp->m_sb_lock);
+}
+
+/* Mark unhealthy realtime metadata. */
+void
+xfs_rt_mark_sick(
+	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_sick |= mask;
+	mp->m_rt_checked |= mask;
+	spin_unlock(&mp->m_sb_lock);
+}
+
+/* Mark a realtime metadata healed. */
+void
+xfs_rt_mark_healthy(
+	struct xfs_mount	*mp,
+	unsigned int		mask)
+{
+	ASSERT(!(mask & ~XFS_SICK_RT_PRIMARY));
+	trace_xfs_rt_mark_healthy(mp, mask);
+
+	spin_lock(&mp->m_sb_lock);
+	mp->m_rt_sick &= ~mask;
+	mp->m_rt_checked |= mask;
+	spin_unlock(&mp->m_sb_lock);
+}
+
+/* Sample which realtime metadata are unhealthy. */
+void
+xfs_rt_measure_sickness(
+	struct xfs_mount	*mp,
+	unsigned int		*sick,
+	unsigned int		*checked)
+{
+	spin_lock(&mp->m_sb_lock);
+	*sick = mp->m_rt_sick;
+	*checked = mp->m_rt_checked;
+	spin_unlock(&mp->m_sb_lock);
+}
+
+/* Mark unhealthy per-ag metadata. */
+void
+xfs_ag_mark_sick(
+	struct xfs_perag	*pag,
+	unsigned int		mask)
+{
+	ASSERT(!(mask & ~XFS_SICK_AG_PRIMARY));
+	trace_xfs_ag_mark_sick(pag->pag_mount, pag->pag_agno, mask);
+
+	spin_lock(&pag->pag_state_lock);
+	pag->pag_sick |= mask;
+	pag->pag_checked |= mask;
+	spin_unlock(&pag->pag_state_lock);
+}
+
+/* Mark per-ag metadata ok. */
+void
+xfs_ag_mark_healthy(
+	struct xfs_perag	*pag,
+	unsigned int		mask)
+{
+	ASSERT(!(mask & ~XFS_SICK_AG_PRIMARY));
+	trace_xfs_ag_mark_healthy(pag->pag_mount, pag->pag_agno, mask);
+
+	spin_lock(&pag->pag_state_lock);
+	pag->pag_sick &= ~mask;
+	pag->pag_checked |= mask;
+	spin_unlock(&pag->pag_state_lock);
+}
+
+/* Sample which per-ag metadata are unhealthy. */
+void
+xfs_ag_measure_sickness(
+	struct xfs_perag	*pag,
+	unsigned int		*sick,
+	unsigned int		*checked)
+{
+	spin_lock(&pag->pag_state_lock);
+	*sick = pag->pag_sick;
+	*checked = pag->pag_checked;
+	spin_unlock(&pag->pag_state_lock);
+}
+
+/* Mark the unhealthy parts of an inode. */
+void
+xfs_inode_mark_sick(
+	struct xfs_inode	*ip,
+	unsigned int		mask)
+{
+	ASSERT(!(mask & ~XFS_SICK_INO_PRIMARY));
+	trace_xfs_inode_mark_sick(ip, mask);
+
+	spin_lock(&ip->i_flags_lock);
+	ip->i_sick |= mask;
+	ip->i_checked |= mask;
+	spin_unlock(&ip->i_flags_lock);
+}
+
+/* Mark parts of an inode healed. */
+void
+xfs_inode_mark_healthy(
+	struct xfs_inode	*ip,
+	unsigned int		mask)
+{
+	ASSERT(!(mask & ~XFS_SICK_INO_PRIMARY));
+	trace_xfs_inode_mark_healthy(ip, mask);
+
+	spin_lock(&ip->i_flags_lock);
+	ip->i_sick &= ~mask;
+	ip->i_checked |= mask;
+	spin_unlock(&ip->i_flags_lock);
+}
+
+/* Sample which parts of an inode are unhealthy. */
+void
+xfs_inode_measure_sickness(
+	struct xfs_inode	*ip,
+	unsigned int		*sick,
+	unsigned int		*checked)
+{
+	spin_lock(&ip->i_flags_lock);
+	*sick = ip->i_sick;
+	*checked = ip->i_checked;
+	spin_unlock(&ip->i_flags_lock);
+}
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index e70e7db29026..885decab4735 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -73,6 +73,8 @@ xfs_inode_alloc(
 	INIT_WORK(&ip->i_iodone_work, xfs_end_io);
 	INIT_LIST_HEAD(&ip->i_iodone_list);
 	spin_lock_init(&ip->i_iodone_lock);
+	ip->i_sick = 0;
+	ip->i_checked = 0;
 
 	return ip;
 }
@@ -133,6 +135,8 @@ xfs_inode_free(
 	spin_lock(&ip->i_flags_lock);
 	ip->i_flags = XFS_IRECLAIM;
 	ip->i_ino = 0;
+	ip->i_sick = 0;
+	ip->i_checked = 0;
 	spin_unlock(&ip->i_flags_lock);
 
 	__xfs_inode_free(ip);
@@ -449,6 +453,8 @@ xfs_iget_cache_hit(
 		ip->i_flags |= XFS_INEW;
 		xfs_inode_clear_reclaim_tag(pag, ip->i_ino);
 		inode->i_state = I_NEW;
+		ip->i_sick = 0;
+		ip->i_checked = 0;
 
 		ASSERT(!rwsem_is_locked(&inode->i_rwsem));
 		init_rwsem(&inode->i_rwsem);
@@ -1177,6 +1183,8 @@ xfs_reclaim_inode(
 	spin_lock(&ip->i_flags_lock);
 	ip->i_flags = XFS_IRECLAIM;
 	ip->i_ino = 0;
+	ip->i_sick = 0;
+	ip->i_checked = 0;
 	spin_unlock(&ip->i_flags_lock);
 
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 88239c2dd824..494e47ef42cb 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -45,6 +45,14 @@ typedef struct xfs_inode {
 	mrlock_t		i_lock;		/* inode lock */
 	mrlock_t		i_mmaplock;	/* inode mmap IO lock */
 	atomic_t		i_pincount;	/* inode pin count */
+
+	/*
+	 * Bitsets of inode metadata that have been checked and/or are sick.
+	 * Callers must hold i_flags_lock before accessing this field.
+	 */
+	uint16_t		i_checked;
+	uint16_t		i_sick;
+
 	spinlock_t		i_flags_lock;	/* inode i_flags lock */
 	/* Miscellaneous state. */
 	unsigned long		i_flags;	/* see defined flags below */
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index fd63b0b1307c..6581381c12be 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -231,6 +231,7 @@ xfs_initialize_perag(
 		error = xfs_iunlink_init(pag);
 		if (error)
 			goto out_hash_destroy;
+		spin_lock_init(&pag->pag_state_lock);
 	}
 
 	index = xfs_set_inode_alloc(mp, agcount);
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 110f927cf943..cf7facc36a5f 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -60,6 +60,20 @@ struct xfs_error_cfg {
 typedef struct xfs_mount {
 	struct super_block	*m_super;
 	xfs_tid_t		m_tid;		/* next unused tid for fs */
+
+	/*
+	 * Bitsets of per-fs metadata that have been checked and/or are sick.
+	 * Callers must hold m_sb_lock to access these two fields.
+	 */
+	uint8_t			m_fs_checked;
+	uint8_t			m_fs_sick;
+	/*
+	 * Bitsets of rt metadata that have been checked and/or are sick.
+	 * Callers must hold m_sb_lock to access this field.
+	 */
+	uint8_t			m_rt_checked;
+	uint8_t			m_rt_sick;
+
 	struct xfs_ail		*m_ail;		/* fs active log item list */
 
 	struct xfs_sb		m_sb;		/* copy of fs superblock */
@@ -369,6 +383,15 @@ typedef struct xfs_perag {
 	xfs_agino_t	pagl_pagino;
 	xfs_agino_t	pagl_leftrec;
 	xfs_agino_t	pagl_rightrec;
+
+	/*
+	 * Bitsets of per-ag metadata that have been checked and/or are sick.
+	 * Callers should hold pag_state_lock before accessing this field.
+	 */
+	uint16_t	pag_checked;
+	uint16_t	pag_sick;
+	spinlock_t	pag_state_lock;
+
 	spinlock_t	pagb_lock;	/* lock for pagb_tree */
 	struct rb_root	pagb_tree;	/* ordered tree of busy extents */
 	unsigned int	pagb_gen;	/* generation count for pagb_tree */
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 47fb07d86efd..f079841c7af6 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -3440,6 +3440,79 @@ DEFINE_AGINODE_EVENT(xfs_iunlink);
 DEFINE_AGINODE_EVENT(xfs_iunlink_remove);
 DEFINE_AG_EVENT(xfs_iunlink_map_prev_fallback);
 
+DECLARE_EVENT_CLASS(xfs_fs_corrupt_class,
+	TP_PROTO(struct xfs_mount *mp, unsigned int flags),
+	TP_ARGS(mp, flags),
+	TP_STRUCT__entry(
+		__field(dev_t, dev)
+		__field(unsigned int, flags)
+	),
+	TP_fast_assign(
+		__entry->dev = mp->m_super->s_dev;
+		__entry->flags = flags;
+	),
+	TP_printk("dev %d:%d flags 0x%x",
+		  MAJOR(__entry->dev), MINOR(__entry->dev),
+		  __entry->flags)
+);
+#define DEFINE_FS_CORRUPT_EVENT(name)	\
+DEFINE_EVENT(xfs_fs_corrupt_class, name,	\
+	TP_PROTO(struct xfs_mount *mp, unsigned int flags), \
+	TP_ARGS(mp, flags))
+DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_sick);
+DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_healthy);
+DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_sick);
+DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_healthy);
+
+DECLARE_EVENT_CLASS(xfs_ag_corrupt_class,
+	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, unsigned int flags),
+	TP_ARGS(mp, agno, flags),
+	TP_STRUCT__entry(
+		__field(dev_t, dev)
+		__field(xfs_agnumber_t, agno)
+		__field(unsigned int, flags)
+	),
+	TP_fast_assign(
+		__entry->dev = mp->m_super->s_dev;
+		__entry->agno = agno;
+		__entry->flags = flags;
+	),
+	TP_printk("dev %d:%d agno %u flags 0x%x",
+		  MAJOR(__entry->dev), MINOR(__entry->dev),
+		  __entry->agno, __entry->flags)
+);
+#define DEFINE_AG_CORRUPT_EVENT(name)	\
+DEFINE_EVENT(xfs_ag_corrupt_class, name,	\
+	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, \
+		 unsigned int flags), \
+	TP_ARGS(mp, agno, flags))
+DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_sick);
+DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_healthy);
+
+DECLARE_EVENT_CLASS(xfs_inode_corrupt_class,
+	TP_PROTO(struct xfs_inode *ip, unsigned int flags),
+	TP_ARGS(ip, flags),
+	TP_STRUCT__entry(
+		__field(dev_t, dev)
+		__field(xfs_ino_t, ino)
+		__field(unsigned int, flags)
+	),
+	TP_fast_assign(
+		__entry->dev = ip->i_mount->m_super->s_dev;
+		__entry->ino = ip->i_ino;
+		__entry->flags = flags;
+	),
+	TP_printk("dev %d:%d ino 0x%llx flags 0x%x",
+		  MAJOR(__entry->dev), MINOR(__entry->dev),
+		  __entry->ino, __entry->flags)
+);
+#define DEFINE_INODE_CORRUPT_EVENT(name)	\
+DEFINE_EVENT(xfs_inode_corrupt_class, name,	\
+	TP_PROTO(struct xfs_inode *ip, unsigned int flags), \
+	TP_ARGS(ip, flags))
+DEFINE_INODE_CORRUPT_EVENT(xfs_inode_mark_sick);
+DEFINE_INODE_CORRUPT_EVENT(xfs_inode_mark_healthy);
+
 #endif /* _TRACE_XFS_H */
 
 #undef TRACE_INCLUDE_PATH

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/8] xfs: replace the BAD_SUMMARY mount flag with the equivalent health code
  2019-04-11  1:45 [PATCH v2 0/8] xfs: online health tracking support Darrick J. Wong
  2019-04-11  1:45 ` [PATCH 1/8] xfs: track metadata health status Darrick J. Wong
@ 2019-04-11  1:45 ` Darrick J. Wong
  2019-04-11  1:45 ` [PATCH 3/8] xfs: clear BAD_SUMMARY if unmounting an unhealthy filesystem Darrick J. Wong
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11  1:45 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs, Brian Foster

From: Darrick J. Wong <darrick.wong@oracle.com>

Replace the BAD_SUMMARY mount flag with calls to the equivalent health
tracking code.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_sb.c |    5 +++--
 fs/xfs/xfs_log.c       |    3 ++-
 fs/xfs/xfs_mount.c     |    9 ++++-----
 fs/xfs/xfs_mount.h     |    1 -
 4 files changed, 9 insertions(+), 9 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 77a3a4085de3..dc5be0c631c8 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -30,6 +30,7 @@
 #include "xfs_refcount_btree.h"
 #include "xfs_da_format.h"
 #include "xfs_da_btree.h"
+#include "xfs_health.h"
 
 /*
  * Physical superblock buffer manipulations. Shared with libxfs in userspace.
@@ -905,7 +906,7 @@ xfs_initialize_perag_data(
 	/*
 	 * If the new summary counts are obviously incorrect, fail the
 	 * mount operation because that implies the AGFs are also corrupt.
-	 * Clear BAD_SUMMARY so that we don't unmount with a dirty log, which
+	 * Clear FS_COUNTERS so that we don't unmount with a dirty log, which
 	 * will prevent xfs_repair from fixing anything.
 	 */
 	if (fdblocks > sbp->sb_dblocks || ifree > ialloc) {
@@ -923,7 +924,7 @@ xfs_initialize_perag_data(
 
 	xfs_reinit_percpu_counters(mp);
 out:
-	mp->m_flags &= ~XFS_MOUNT_BAD_SUMMARY;
+	xfs_fs_mark_healthy(mp, XFS_SICK_FS_COUNTERS);
 	return error;
 }
 
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index c3b610b687d1..457ced3ee3e1 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -23,6 +23,7 @@
 #include "xfs_cksum.h"
 #include "xfs_sysfs.h"
 #include "xfs_sb.h"
+#include "xfs_health.h"
 
 kmem_zone_t	*xfs_log_ticket_zone;
 
@@ -861,7 +862,7 @@ xfs_log_write_unmount_record(
 	 * recalculated during log recovery at next mount.  Refer to
 	 * xlog_check_unmount_rec for more details.
 	 */
-	if (XFS_TEST_ERROR((mp->m_flags & XFS_MOUNT_BAD_SUMMARY), mp,
+	if (XFS_TEST_ERROR(xfs_fs_has_sickness(mp, XFS_SICK_FS_COUNTERS), mp,
 			XFS_ERRTAG_FORCE_SUMMARY_RECALC)) {
 		xfs_alert(mp, "%s: will fix summary counters at next mount",
 				__func__);
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 6581381c12be..14f454e09e6e 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -34,6 +34,7 @@
 #include "xfs_refcount_btree.h"
 #include "xfs_reflink.h"
 #include "xfs_extent_busy.h"
+#include "xfs_health.h"
 
 
 static DEFINE_MUTEX(xfs_uuid_table_mutex);
@@ -645,7 +646,7 @@ xfs_check_summary_counts(
 	    (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_flags |= XFS_MOUNT_BAD_SUMMARY;
+		xfs_fs_mark_sick(mp, XFS_SICK_FS_COUNTERS);
 
 	/*
 	 * We can safely re-initialise incore superblock counters from the
@@ -660,7 +661,7 @@ xfs_check_summary_counts(
 	 */
 	if ((!xfs_sb_version_haslazysbcount(&mp->m_sb) ||
 	     XFS_LAST_UNMOUNT_WAS_CLEAN(mp)) &&
-	    !(mp->m_flags & XFS_MOUNT_BAD_SUMMARY))
+	    !xfs_fs_has_sickness(mp, XFS_SICK_FS_COUNTERS))
 		return 0;
 
 	return xfs_initialize_perag_data(mp, mp->m_sb.sb_agcount);
@@ -1446,7 +1447,5 @@ xfs_force_summary_recalc(
 	if (!xfs_sb_version_haslazysbcount(&mp->m_sb))
 		return;
 
-	spin_lock(&mp->m_sb_lock);
-	mp->m_flags |= XFS_MOUNT_BAD_SUMMARY;
-	spin_unlock(&mp->m_sb_lock);
+	xfs_fs_mark_sick(mp, XFS_SICK_FS_COUNTERS);
 }
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index cf7facc36a5f..42b980862203 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -228,7 +228,6 @@ typedef struct xfs_mount {
 						   must be synchronous except
 						   for space allocations */
 #define XFS_MOUNT_UNMOUNTING	(1ULL << 1)	/* filesystem is unmounting */
-#define XFS_MOUNT_BAD_SUMMARY	(1ULL << 2)	/* summary counters are bad */
 #define XFS_MOUNT_WAS_CLEAN	(1ULL << 3)
 #define XFS_MOUNT_FS_SHUTDOWN	(1ULL << 4)	/* atomic stop of all filesystem
 						   operations, typically for

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/8] xfs: clear BAD_SUMMARY if unmounting an unhealthy filesystem
  2019-04-11  1:45 [PATCH v2 0/8] xfs: online health tracking support Darrick J. Wong
  2019-04-11  1:45 ` [PATCH 1/8] xfs: track metadata health status Darrick J. Wong
  2019-04-11  1:45 ` [PATCH 2/8] xfs: replace the BAD_SUMMARY mount flag with the equivalent health code Darrick J. Wong
@ 2019-04-11  1:45 ` Darrick J. Wong
  2019-04-11 12:29   ` Brian Foster
  2019-04-11  1:45 ` [PATCH 4/8] xfs: bump XFS_IOC_FSGEOMETRY to v5 structures Darrick J. Wong
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11  1:45 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

If we know the filesystem metadata isn't healthy during unmount, we want
to encourage the administrator to run xfs_repair right away.  We can't
do this if BAD_SUMMARY will cause an unclean log unmount to force
summary recalculation, so turn it off if the fs is bad.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_health.h |    2 +
 fs/xfs/xfs_health.c        |   74 ++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_mount.c         |    2 +
 fs/xfs/xfs_trace.h         |    3 ++
 4 files changed, 81 insertions(+)


diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
index 30762a5d4862..a434b47f2aa0 100644
--- a/fs/xfs/libxfs/xfs_health.h
+++ b/fs/xfs/libxfs/xfs_health.h
@@ -110,6 +110,8 @@ 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);
 
+void xfs_health_unmount(struct xfs_mount *mp);
+
 /* Now some helpers. */
 
 static inline bool
diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
index 941f33037e2f..21728228e08b 100644
--- a/fs/xfs/xfs_health.c
+++ b/fs/xfs/xfs_health.c
@@ -19,6 +19,80 @@
 #include "xfs_trace.h"
 #include "xfs_health.h"
 
+/*
+ * Warn about metadata corruption that we detected but haven't fixed, and
+ * make sure we're not sitting on anything that would get in the way of
+ * recovery.
+ */
+void
+xfs_health_unmount(
+	struct xfs_mount	*mp)
+{
+	struct xfs_perag	*pag;
+	xfs_agnumber_t		agno;
+	unsigned int		sick = 0;
+	unsigned int		checked = 0;
+	bool			warn = false;
+
+	if (XFS_FORCED_SHUTDOWN(mp))
+		return;
+
+	/* Measure AG corruption levels. */
+	for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
+		pag = xfs_perag_get(mp, agno);
+		xfs_ag_measure_sickness(pag, &sick, &checked);
+		if (sick) {
+			trace_xfs_ag_unfixed_corruption(mp, agno, sick);
+			warn = true;
+		}
+		xfs_perag_put(pag);
+	}
+
+	/* Measure realtime volume corruption levels. */
+	xfs_rt_measure_sickness(mp, &sick, &checked);
+	if (sick) {
+		trace_xfs_rt_unfixed_corruption(mp, sick);
+		warn = true;
+	}
+
+	/*
+	 * Measure fs corruption and keep the sample around for the warning.
+	 * See the note below for why we exempt FS_COUNTERS.
+	 */
+	xfs_fs_measure_sickness(mp, &sick, &checked);
+	if (sick & ~XFS_SICK_FS_COUNTERS) {
+		trace_xfs_fs_unfixed_corruption(mp, sick);
+		warn = true;
+	}
+
+	if (warn) {
+		xfs_warn(mp,
+"Uncorrected metadata errors detected; please run xfs_repair.");
+
+		/*
+		 * We discovered uncorrected metadata problems at some point
+		 * during this filesystem mount and have advised the
+		 * administrator to run repair once the unmount completes.
+		 *
+		 * However, we must be careful -- when FSCOUNTERS are flagged
+		 * unhealthy, the unmount procedure omits writing the clean
+		 * unmount record to the log so that the next mount will run
+		 * recovery and recompute the summary counters.  In other
+		 * words, we leave a dirty log to get the counters fixed.
+		 *
+		 * Unfortunately, xfs_repair cannot recover dirty logs, so if
+		 * there were filesystem problems, FSCOUNTERS was flagged, and
+		 * the administrator takes our advice to run xfs_repair,
+		 * they'll have to zap the log before repairing structures.
+		 * We don't really want to encourage this, so we mark the
+		 * FSCOUNTERS healthy so that a subsequent repair run won't see
+		 * a dirty log.
+		 */
+		if (sick & XFS_SICK_FS_COUNTERS)
+			xfs_fs_mark_healthy(mp, XFS_SICK_FS_COUNTERS);
+	}
+}
+
 /* Mark unhealthy per-fs metadata. */
 void
 xfs_fs_mark_sick(
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 14f454e09e6e..eff8b4c3eb3e 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1070,6 +1070,7 @@ xfs_mountfs(
 	 */
 	cancel_delayed_work_sync(&mp->m_reclaim_work);
 	xfs_reclaim_inodes(mp, SYNC_WAIT);
+	xfs_health_unmount(mp);
  out_log_dealloc:
 	mp->m_flags |= XFS_MOUNT_UNMOUNTING;
 	xfs_log_mount_cancel(mp);
@@ -1152,6 +1153,7 @@ xfs_unmountfs(
 	 */
 	cancel_delayed_work_sync(&mp->m_reclaim_work);
 	xfs_reclaim_inodes(mp, SYNC_WAIT);
+	xfs_health_unmount(mp);
 
 	xfs_qm_unmount(mp);
 
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index f079841c7af6..2464ea351f83 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -3461,8 +3461,10 @@ DEFINE_EVENT(xfs_fs_corrupt_class, name,	\
 	TP_ARGS(mp, flags))
 DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_sick);
 DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_healthy);
+DEFINE_FS_CORRUPT_EVENT(xfs_fs_unfixed_corruption);
 DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_sick);
 DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_healthy);
+DEFINE_FS_CORRUPT_EVENT(xfs_rt_unfixed_corruption);
 
 DECLARE_EVENT_CLASS(xfs_ag_corrupt_class,
 	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, unsigned int flags),
@@ -3488,6 +3490,7 @@ DEFINE_EVENT(xfs_ag_corrupt_class, name,	\
 	TP_ARGS(mp, agno, flags))
 DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_sick);
 DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_healthy);
+DEFINE_AG_CORRUPT_EVENT(xfs_ag_unfixed_corruption);
 
 DECLARE_EVENT_CLASS(xfs_inode_corrupt_class,
 	TP_PROTO(struct xfs_inode *ip, unsigned int flags),

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 4/8] xfs: bump XFS_IOC_FSGEOMETRY to v5 structures
  2019-04-11  1:45 [PATCH v2 0/8] xfs: online health tracking support Darrick J. Wong
                   ` (2 preceding siblings ...)
  2019-04-11  1:45 ` [PATCH 3/8] xfs: clear BAD_SUMMARY if unmounting an unhealthy filesystem Darrick J. Wong
@ 2019-04-11  1:45 ` Darrick J. Wong
  2019-04-11 12:29   ` Brian Foster
  2019-04-11  1:45 ` [PATCH 5/8] xfs: add a new ioctl to describe allocation group geometry Darrick J. Wong
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11  1:45 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs, Dave Chinner

From: Dave Chinner <dchinner@redhat.com>

Unfortunately, the V4 XFS_IOC_FSGEOMETRY structure is out of space so we
can't just add a new field to it. Hence we need to bump the definition
to V5 and and treat the V4 ioctl and structure similar to v1 to v3.

While doing this, clean up all the definitions associated with the
XFS_IOC_FSGEOMETRY ioctl.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
[darrick: forward port to 5.1, expand structure size to 256 bytes]
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_fs.h |   87 +++++++++++++++++++++++++++++++++---------------
 fs/xfs/libxfs/xfs_sb.c |    5 +++
 fs/xfs/xfs_ioctl.c     |   48 ++++++++++----------------
 fs/xfs/xfs_ioctl32.c   |    3 +-
 4 files changed, 84 insertions(+), 59 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index f3aa59302fef..cb7d0b1453cd 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -124,7 +124,7 @@ typedef struct xfs_flock64 {
 /*
  * Output for XFS_IOC_FSGEOMETRY_V1
  */
-typedef struct xfs_fsop_geom_v1 {
+struct xfs_fsop_geom_v1 {
 	__u32		blocksize;	/* filesystem (data) block size */
 	__u32		rtextsize;	/* realtime extent size		*/
 	__u32		agblocks;	/* fsblocks in an AG		*/
@@ -145,12 +145,39 @@ typedef struct xfs_fsop_geom_v1 {
 	__u32		logsectsize;	/* log sector size, bytes	*/
 	__u32		rtsectsize;	/* realtime sector size, bytes	*/
 	__u32		dirblocksize;	/* directory block size, bytes	*/
-} xfs_fsop_geom_v1_t;
+};
+
+/*
+ * Output for XFS_IOC_FSGEOMETRY_V4
+ */
+struct xfs_fsop_geom_v4 {
+	__u32		blocksize;	/* filesystem (data) block size */
+	__u32		rtextsize;	/* realtime extent size		*/
+	__u32		agblocks;	/* fsblocks in an AG		*/
+	__u32		agcount;	/* number of allocation groups	*/
+	__u32		logblocks;	/* fsblocks in the log		*/
+	__u32		sectsize;	/* (data) sector size, bytes	*/
+	__u32		inodesize;	/* inode size in bytes		*/
+	__u32		imaxpct;	/* max allowed inode space(%)	*/
+	__u64		datablocks;	/* fsblocks in data subvolume	*/
+	__u64		rtblocks;	/* fsblocks in realtime subvol	*/
+	__u64		rtextents;	/* rt extents in realtime subvol*/
+	__u64		logstart;	/* starting fsblock of the log	*/
+	unsigned char	uuid[16];	/* unique id of the filesystem	*/
+	__u32		sunit;		/* stripe unit, fsblocks	*/
+	__u32		swidth;		/* stripe width, fsblocks	*/
+	__s32		version;	/* structure version		*/
+	__u32		flags;		/* superblock version flags	*/
+	__u32		logsectsize;	/* log sector size, bytes	*/
+	__u32		rtsectsize;	/* realtime sector size, bytes	*/
+	__u32		dirblocksize;	/* directory block size, bytes	*/
+	__u32		logsunit;	/* log stripe unit, bytes	*/
+};
 
 /*
  * Output for XFS_IOC_FSGEOMETRY
  */
-typedef struct xfs_fsop_geom {
+struct xfs_fsop_geom {
 	__u32		blocksize;	/* filesystem (data) block size */
 	__u32		rtextsize;	/* realtime extent size		*/
 	__u32		agblocks;	/* fsblocks in an AG		*/
@@ -171,8 +198,9 @@ typedef struct xfs_fsop_geom {
 	__u32		logsectsize;	/* log sector size, bytes	*/
 	__u32		rtsectsize;	/* realtime sector size, bytes	*/
 	__u32		dirblocksize;	/* directory block size, bytes	*/
-	__u32		logsunit;	/* log stripe unit, bytes */
-} xfs_fsop_geom_t;
+	__u32		logsunit;	/* log stripe unit, bytes	*/
+	__u64		reserved[18];	/* reserved space		*/
+};
 
 /* Output for XFS_FS_COUNTS */
 typedef struct xfs_fsop_counts {
@@ -188,28 +216,30 @@ typedef struct xfs_fsop_resblks {
 	__u64  resblks_avail;
 } xfs_fsop_resblks_t;
 
-#define XFS_FSOP_GEOM_VERSION	0
-
-#define XFS_FSOP_GEOM_FLAGS_ATTR	0x0001	/* attributes in use	*/
-#define XFS_FSOP_GEOM_FLAGS_NLINK	0x0002	/* 32-bit nlink values	*/
-#define XFS_FSOP_GEOM_FLAGS_QUOTA	0x0004	/* quotas enabled	*/
-#define XFS_FSOP_GEOM_FLAGS_IALIGN	0x0008	/* inode alignment	*/
-#define XFS_FSOP_GEOM_FLAGS_DALIGN	0x0010	/* large data alignment */
-#define XFS_FSOP_GEOM_FLAGS_SHARED	0x0020	/* read-only shared	*/
-#define XFS_FSOP_GEOM_FLAGS_EXTFLG	0x0040	/* special extent flag	*/
-#define XFS_FSOP_GEOM_FLAGS_DIRV2	0x0080	/* directory version 2	*/
-#define XFS_FSOP_GEOM_FLAGS_LOGV2	0x0100	/* log format version 2	*/
-#define XFS_FSOP_GEOM_FLAGS_SECTOR	0x0200	/* sector sizes >1BB	*/
-#define XFS_FSOP_GEOM_FLAGS_ATTR2	0x0400	/* inline attributes rework */
-#define XFS_FSOP_GEOM_FLAGS_PROJID32	0x0800	/* 32-bit project IDs	*/
-#define XFS_FSOP_GEOM_FLAGS_DIRV2CI	0x1000	/* ASCII only CI names	*/
-#define XFS_FSOP_GEOM_FLAGS_LAZYSB	0x4000	/* lazy superblock counters */
-#define XFS_FSOP_GEOM_FLAGS_V5SB	0x8000	/* version 5 superblock */
-#define XFS_FSOP_GEOM_FLAGS_FTYPE	0x10000	/* inode directory types */
-#define XFS_FSOP_GEOM_FLAGS_FINOBT	0x20000	/* free inode btree */
-#define XFS_FSOP_GEOM_FLAGS_SPINODES	0x40000	/* sparse inode chunks	*/
-#define XFS_FSOP_GEOM_FLAGS_RMAPBT	0x80000	/* reverse mapping btree */
-#define XFS_FSOP_GEOM_FLAGS_REFLINK	0x100000 /* files can share blocks */
+#define XFS_FSOP_GEOM_VERSION		0
+#define XFS_FSOP_GEOM_VERSION_V5	5
+
+#define XFS_FSOP_GEOM_FLAGS_ATTR	(1 << 0)  /* attributes in use	   */
+#define XFS_FSOP_GEOM_FLAGS_NLINK	(1 << 1)  /* 32-bit nlink values   */
+#define XFS_FSOP_GEOM_FLAGS_QUOTA	(1 << 2)  /* quotas enabled	   */
+#define XFS_FSOP_GEOM_FLAGS_IALIGN	(1 << 3)  /* inode alignment	   */
+#define XFS_FSOP_GEOM_FLAGS_DALIGN	(1 << 4)  /* large data alignment  */
+#define XFS_FSOP_GEOM_FLAGS_SHARED	(1 << 5)  /* read-only shared	   */
+#define XFS_FSOP_GEOM_FLAGS_EXTFLG	(1 << 6)  /* special extent flag   */
+#define XFS_FSOP_GEOM_FLAGS_DIRV2	(1 << 7)  /* directory version 2   */
+#define XFS_FSOP_GEOM_FLAGS_LOGV2	(1 << 8)  /* log format version 2  */
+#define XFS_FSOP_GEOM_FLAGS_SECTOR	(1 << 9)  /* sector sizes >1BB	   */
+#define XFS_FSOP_GEOM_FLAGS_ATTR2	(1 << 10) /* inline attributes rework */
+#define XFS_FSOP_GEOM_FLAGS_PROJID32	(1 << 11) /* 32-bit project IDs	   */
+#define XFS_FSOP_GEOM_FLAGS_DIRV2CI	(1 << 12) /* ASCII only CI names   */
+	/*  -- Do not use --		(1 << 13)    SGI parent pointers   */
+#define XFS_FSOP_GEOM_FLAGS_LAZYSB	(1 << 14) /* lazy superblock counters */
+#define XFS_FSOP_GEOM_FLAGS_V5SB	(1 << 15) /* version 5 superblock  */
+#define XFS_FSOP_GEOM_FLAGS_FTYPE	(1 << 16) /* inode directory types */
+#define XFS_FSOP_GEOM_FLAGS_FINOBT	(1 << 17) /* free inode btree	   */
+#define XFS_FSOP_GEOM_FLAGS_SPINODES	(1 << 18) /* sparse inode chunks   */
+#define XFS_FSOP_GEOM_FLAGS_RMAPBT	(1 << 19) /* reverse mapping btree */
+#define XFS_FSOP_GEOM_FLAGS_REFLINK	(1 << 20) /* files can share blocks */
 
 /*
  * Minimum and maximum sizes need for growth checks.
@@ -620,8 +650,9 @@ struct xfs_scrub_metadata {
 #define XFS_IOC_FSSETDM_BY_HANDLE    _IOW ('X', 121, struct xfs_fsop_setdm_handlereq)
 #define XFS_IOC_ATTRLIST_BY_HANDLE   _IOW ('X', 122, struct xfs_fsop_attrlist_handlereq)
 #define XFS_IOC_ATTRMULTI_BY_HANDLE  _IOW ('X', 123, struct xfs_fsop_attrmulti_handlereq)
-#define XFS_IOC_FSGEOMETRY	     _IOR ('X', 124, struct xfs_fsop_geom)
+#define XFS_IOC_FSGEOMETRY_V4	     _IOR ('X', 124, struct xfs_fsop_geom_v4)
 #define XFS_IOC_GOINGDOWN	     _IOR ('X', 125, uint32_t)
+#define XFS_IOC_FSGEOMETRY	     _IOR ('X', 126, struct xfs_fsop_geom)
 /*	XFS_IOC_GETFSUUID ---------- deprecated 140	 */
 
 
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index dc5be0c631c8..6fab49f6070b 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -1166,6 +1166,11 @@ xfs_fs_geometry(
 
 	geo->logsunit = sbp->sb_logsunit;
 
+	if (struct_version < 5)
+		return 0;
+
+	geo->version = XFS_FSOP_GEOM_VERSION_V5;
+
 	return 0;
 }
 
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 67d12027f563..d243660f1826 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -778,41 +778,28 @@ xfs_ioc_bulkstat(
 	return 0;
 }
 
-STATIC int
-xfs_ioc_fsgeometry_v1(
-	xfs_mount_t		*mp,
-	void			__user *arg)
-{
-	xfs_fsop_geom_t         fsgeo;
-	int			error;
-
-	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
-	if (error)
-		return error;
-
-	/*
-	 * Caller should have passed an argument of type
-	 * xfs_fsop_geom_v1_t.  This is a proper subset of the
-	 * xfs_fsop_geom_t that xfs_fs_geometry() fills in.
-	 */
-	if (copy_to_user(arg, &fsgeo, sizeof(xfs_fsop_geom_v1_t)))
-		return -EFAULT;
-	return 0;
-}
-
 STATIC int
 xfs_ioc_fsgeometry(
-	xfs_mount_t		*mp,
-	void			__user *arg)
+	struct xfs_mount	*mp,
+	void			__user *arg,
+	int			struct_version)
 {
-	xfs_fsop_geom_t		fsgeo;
+	struct xfs_fsop_geom	fsgeo;
+	size_t			len;
 	int			error;
 
-	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 4);
+	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, struct_version);
 	if (error)
 		return error;
 
-	if (copy_to_user(arg, &fsgeo, sizeof(fsgeo)))
+	if (struct_version <= 3)
+		len = sizeof(struct xfs_fsop_geom_v1);
+	else if (struct_version == 4)
+		len = sizeof(struct xfs_fsop_geom_v4);
+	else
+		len = sizeof(fsgeo);
+
+	if (copy_to_user(arg, &fsgeo, len))
 		return -EFAULT;
 	return 0;
 }
@@ -2016,10 +2003,11 @@ xfs_file_ioctl(
 		return xfs_ioc_bulkstat(mp, cmd, arg);
 
 	case XFS_IOC_FSGEOMETRY_V1:
-		return xfs_ioc_fsgeometry_v1(mp, arg);
-
+		return xfs_ioc_fsgeometry(mp, arg, 3);
+	case XFS_IOC_FSGEOMETRY_V4:
+		return xfs_ioc_fsgeometry(mp, arg, 4);
 	case XFS_IOC_FSGEOMETRY:
-		return xfs_ioc_fsgeometry(mp, arg);
+		return xfs_ioc_fsgeometry(mp, arg, 5);
 
 	case XFS_IOC_GETVERSION:
 		return put_user(inode->i_generation, (int __user *)arg);
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 5001dca361e9..55ace6308637 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -52,7 +52,7 @@ xfs_compat_ioc_fsgeometry_v1(
 	struct xfs_mount	  *mp,
 	compat_xfs_fsop_geom_v1_t __user *arg32)
 {
-	xfs_fsop_geom_t		  fsgeo;
+	struct xfs_fsop_geom	  fsgeo;
 	int			  error;
 
 	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
@@ -561,6 +561,7 @@ xfs_file_compat_ioctl(
 	switch (cmd) {
 	/* No size or alignment issues on any arch */
 	case XFS_IOC_DIOINFO:
+	case XFS_IOC_FSGEOMETRY_V4:
 	case XFS_IOC_FSGEOMETRY:
 	case XFS_IOC_FSGETXATTR:
 	case XFS_IOC_FSSETXATTR:

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 5/8] xfs: add a new ioctl to describe allocation group geometry
  2019-04-11  1:45 [PATCH v2 0/8] xfs: online health tracking support Darrick J. Wong
                   ` (3 preceding siblings ...)
  2019-04-11  1:45 ` [PATCH 4/8] xfs: bump XFS_IOC_FSGEOMETRY to v5 structures Darrick J. Wong
@ 2019-04-11  1:45 ` Darrick J. Wong
  2019-04-11 13:08   ` Brian Foster
  2019-04-11  1:46 ` [PATCH 6/8] xfs: report fs and rt health via geometry structure Darrick J. Wong
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11  1:45 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Add a new ioctl to describe an allocation group's geometry.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_ag.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_ag.h |    2 ++
 fs/xfs/libxfs/xfs_fs.h |   14 +++++++++++++
 fs/xfs/xfs_ioctl.c     |   24 ++++++++++++++++++++++
 fs/xfs/xfs_ioctl32.c   |    1 +
 5 files changed, 93 insertions(+)


diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 1ef8acf35e7d..1c0f2a6c10b4 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -19,6 +19,7 @@
 #include "xfs_ialloc.h"
 #include "xfs_rmap.h"
 #include "xfs_ag.h"
+#include "xfs_ag_resv.h"
 
 static struct xfs_buf *
 xfs_get_aghdr_buf(
@@ -461,3 +462,54 @@ xfs_ag_extend_space(
 				len, &XFS_RMAP_OINFO_SKIP_UPDATE,
 				XFS_AG_RESV_NONE);
 }
+
+/* Retrieve AG geometry. */
+int
+xfs_ag_get_geometry(
+	struct xfs_mount	*mp,
+	xfs_agnumber_t		agno,
+	struct xfs_ag_geometry	*ageo)
+{
+	struct xfs_buf		*agi_bp;
+	struct xfs_buf		*agf_bp;
+	struct xfs_agi		*agi;
+	struct xfs_agf		*agf;
+	struct xfs_perag	*pag;
+	unsigned int		freeblks;
+	int			error;
+
+	if (agno >= mp->m_sb.sb_agcount)
+		return -EINVAL;
+
+	/* Lock the AG headers. */
+	error = xfs_ialloc_read_agi(mp, NULL, agno, &agi_bp);
+	if (error)
+		return error;
+	error = xfs_alloc_read_agf(mp, NULL, agno, 0, &agf_bp);
+	if (error)
+		goto out_agi;
+	pag = xfs_perag_get(mp, agno);
+
+	/* Fill out form. */
+	memset(ageo, 0, sizeof(*ageo));
+	ageo->ag_number = agno;
+
+	agi = XFS_BUF_TO_AGI(agi_bp);
+	ageo->ag_icount = be32_to_cpu(agi->agi_count);
+	ageo->ag_ifree = be32_to_cpu(agi->agi_freecount);
+
+	agf = XFS_BUF_TO_AGF(agf_bp);
+	ageo->ag_length = be32_to_cpu(agf->agf_length);
+	freeblks = pag->pagf_freeblks +
+		   pag->pagf_flcount +
+		   pag->pagf_btreeblks -
+		   xfs_ag_resv_needed(pag, XFS_AG_RESV_NONE);
+	ageo->ag_freeblks = freeblks;
+
+	/* Release resources. */
+	xfs_perag_put(pag);
+	xfs_buf_relse(agf_bp);
+out_agi:
+	xfs_buf_relse(agi_bp);
+	return error;
+}
diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index 412702e23f61..5166322807e7 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -26,5 +26,7 @@ struct aghdr_init_data {
 int xfs_ag_init_headers(struct xfs_mount *mp, struct aghdr_init_data *id);
 int xfs_ag_extend_space(struct xfs_mount *mp, struct xfs_trans *tp,
 			struct aghdr_init_data *id, xfs_extlen_t len);
+int xfs_ag_get_geometry(struct xfs_mount *mp, xfs_agnumber_t agno,
+			struct xfs_ag_geometry *ageo);
 
 #endif /* __LIBXFS_AG_H */
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index cb7d0b1453cd..ee33d628a240 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -267,6 +267,19 @@ typedef struct xfs_fsop_resblks {
 #define XFS_MIN_DBLOCKS(s) ((xfs_rfsblock_t)((s)->sb_agcount - 1) *	\
 			 (s)->sb_agblocks + XFS_MIN_AG_BLOCKS)
 
+/*
+ * Output for XFS_IOC_AG_GEOMETRY
+ */
+struct xfs_ag_geometry {
+	uint32_t	ag_number;	/* i/o: AG number */
+	uint32_t	ag_length;	/* o: length in blocks */
+	uint32_t	ag_freeblks;	/* o: free space */
+	uint32_t	ag_icount;	/* o: inodes allocated */
+	uint32_t	ag_ifree;	/* o: inodes free */
+	uint32_t	ag_reserved32;	/* o: zero */
+	uint64_t	ag_reserved[13];/* o: zero */
+};
+
 /*
  * Structures for XFS_IOC_FSGROWFSDATA, XFS_IOC_FSGROWFSLOG & XFS_IOC_FSGROWFSRT
  */
@@ -620,6 +633,7 @@ struct xfs_scrub_metadata {
 #define XFS_IOC_FREE_EOFBLOCKS	_IOR ('X', 58, struct xfs_fs_eofblocks)
 /*	XFS_IOC_GETFSMAP ------ hoisted 59         */
 #define XFS_IOC_SCRUB_METADATA	_IOWR('X', 60, struct xfs_scrub_metadata)
+#define XFS_IOC_AG_GEOMETRY	_IOWR('X', 61, struct xfs_ag_geometry)
 
 /*
  * ioctl commands that replace IRIX syssgi()'s
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index d243660f1826..0aaf4f88524d 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -33,6 +33,7 @@
 #include "xfs_fsmap.h"
 #include "scrub/xfs_scrub.h"
 #include "xfs_sb.h"
+#include "xfs_ag.h"
 
 #include <linux/capability.h>
 #include <linux/cred.h>
@@ -804,6 +805,26 @@ xfs_ioc_fsgeometry(
 	return 0;
 }
 
+STATIC int
+xfs_ioc_ag_geometry(
+	struct xfs_mount	*mp,
+	void			__user *arg)
+{
+	struct xfs_ag_geometry	ageo;
+	int			error;
+
+	if (copy_from_user(&ageo, arg, sizeof(ageo)))
+		return -EFAULT;
+
+	error = xfs_ag_get_geometry(mp, ageo.ag_number, &ageo);
+	if (error)
+		return error;
+
+	if (copy_to_user(arg, &ageo, sizeof(ageo)))
+		return -EFAULT;
+	return 0;
+}
+
 /*
  * Linux extended inode flags interface.
  */
@@ -2009,6 +2030,9 @@ xfs_file_ioctl(
 	case XFS_IOC_FSGEOMETRY:
 		return xfs_ioc_fsgeometry(mp, arg, 5);
 
+	case XFS_IOC_AG_GEOMETRY:
+		return xfs_ioc_ag_geometry(mp, arg);
+
 	case XFS_IOC_GETVERSION:
 		return put_user(inode->i_generation, (int __user *)arg);
 
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 55ace6308637..65997a6315e9 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -563,6 +563,7 @@ xfs_file_compat_ioctl(
 	case XFS_IOC_DIOINFO:
 	case XFS_IOC_FSGEOMETRY_V4:
 	case XFS_IOC_FSGEOMETRY:
+	case XFS_IOC_AG_GEOMETRY:
 	case XFS_IOC_FSGETXATTR:
 	case XFS_IOC_FSSETXATTR:
 	case XFS_IOC_FSGETXATTRA:

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 6/8] xfs: report fs and rt health via geometry structure
  2019-04-11  1:45 [PATCH v2 0/8] xfs: online health tracking support Darrick J. Wong
                   ` (4 preceding siblings ...)
  2019-04-11  1:45 ` [PATCH 5/8] xfs: add a new ioctl to describe allocation group geometry Darrick J. Wong
@ 2019-04-11  1:46 ` Darrick J. Wong
  2019-04-11 13:09   ` Brian Foster
  2019-04-11  1:46 ` [PATCH 7/8] xfs: report AG health via AG geometry ioctl Darrick J. Wong
  2019-04-11  1:46 ` [PATCH 8/8] xfs: report inode health via bulkstat Darrick J. Wong
  7 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11  1:46 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

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/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index ee33d628a240..6b8956dbf49d 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -199,9 +199,18 @@ struct xfs_fsop_geom {
 	__u32		rtsectsize;	/* realtime sector size, bytes	*/
 	__u32		dirblocksize;	/* directory block size, bytes	*/
 	__u32		logsunit;	/* log stripe unit, bytes	*/
-	__u64		reserved[18];	/* reserved space		*/
+	uint32_t	sick;		/* o: unhealthy fs & rt metadata */
+	uint32_t	checked;	/* o: checked fs & rt metadata	*/
+	__u64		reserved[17];	/* reserved space		*/
 };
 
+#define XFS_FSOP_GEOM_SICK_COUNTERS	(1 << 0)  /* summary counters */
+#define XFS_FSOP_GEOM_SICK_UQUOTA	(1 << 1)  /* user quota */
+#define XFS_FSOP_GEOM_SICK_GQUOTA	(1 << 2)  /* group quota */
+#define XFS_FSOP_GEOM_SICK_PQUOTA	(1 << 3)  /* project quota */
+#define XFS_FSOP_GEOM_SICK_RT_BITMAP	(1 << 4)  /* realtime bitmap */
+#define XFS_FSOP_GEOM_SICK_RT_SUMMARY	(1 << 5)  /* realtime summary */
+
 /* Output for XFS_FS_COUNTS */
 typedef struct xfs_fsop_counts {
 	__u64	freedata;	/* free data section blocks */
diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
index a434b47f2aa0..c72142ec7cbb 100644
--- a/fs/xfs/libxfs/xfs_health.h
+++ b/fs/xfs/libxfs/xfs_health.h
@@ -26,6 +26,7 @@
 struct xfs_mount;
 struct xfs_perag;
 struct xfs_inode;
+struct xfs_fsop_geom;
 
 /* Observable health issues for metadata spanning the entire filesystem. */
 #define XFS_SICK_FS_COUNTERS	(1 << 0)  /* summary counters */
@@ -174,4 +175,6 @@ xfs_inode_is_healthy(struct xfs_inode *ip)
 	return !xfs_inode_has_sickness(ip, -1U);
 }
 
+void xfs_fsop_geom_health(struct xfs_mount *mp, struct xfs_fsop_geom *geo);
+
 #endif	/* __XFS_HEALTH_H__ */
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);
 }
+
+struct ioctl_sick_map {
+	unsigned int		sick_mask;
+	unsigned int		ioctl_mask;
+};
+
+static const struct ioctl_sick_map fs_map[] = {
+	{ XFS_SICK_FS_COUNTERS,	XFS_FSOP_GEOM_SICK_COUNTERS},
+	{ XFS_SICK_FS_UQUOTA,	XFS_FSOP_GEOM_SICK_UQUOTA },
+	{ XFS_SICK_FS_GQUOTA,	XFS_FSOP_GEOM_SICK_GQUOTA },
+	{ XFS_SICK_FS_PQUOTA,	XFS_FSOP_GEOM_SICK_PQUOTA },
+	{ 0, 0 },
+};
+
+static const struct ioctl_sick_map rt_map[] = {
+	{ XFS_SICK_RT_BITMAP,	XFS_FSOP_GEOM_SICK_RT_BITMAP },
+	{ XFS_SICK_RT_SUMMARY,	XFS_FSOP_GEOM_SICK_RT_SUMMARY },
+	{ 0, 0 },
+};
+
+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)
+{
+	if (checked & sick_mask)
+		geo->checked |= fsop_mask;
+	if (sick & sick_mask)
+		geo->sick |= fsop_mask;
+}
+
+/* Fill out fs geometry health info. */
+void
+xfs_fsop_geom_health(
+	struct xfs_mount	*mp,
+	struct xfs_fsop_geom	*geo)
+{
+	const struct ioctl_sick_map *m;
+	unsigned int		sick;
+	unsigned int		checked;
+
+	geo->sick = 0;
+	geo->checked = 0;
+
+	xfs_fs_measure_sickness(mp, &sick, &checked);
+	for (m = fs_map; m->sick_mask; m++)
+		xfgeo_health_tick(geo, sick, checked, m->sick_mask,
+				m->ioctl_mask);
+
+	xfs_rt_measure_sickness(mp, &sick, &checked);
+	for (m = rt_map; m->sick_mask; m++)
+		xfgeo_health_tick(geo, sick, checked, m->sick_mask,
+				m->ioctl_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
@@ -34,6 +34,7 @@
 #include "scrub/xfs_scrub.h"
 #include "xfs_sb.h"
 #include "xfs_ag.h"
+#include "xfs_health.h"
 
 #include <linux/capability.h>
 #include <linux/cred.h>
@@ -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);
 
 	if (struct_version <= 3)
 		len = sizeof(struct xfs_fsop_geom_v1);

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 7/8] xfs: report AG health via AG geometry ioctl
  2019-04-11  1:45 [PATCH v2 0/8] xfs: online health tracking support Darrick J. Wong
                   ` (5 preceding siblings ...)
  2019-04-11  1:46 ` [PATCH 6/8] xfs: report fs and rt health via geometry structure Darrick J. Wong
@ 2019-04-11  1:46 ` Darrick J. Wong
  2019-04-11 13:09   ` Brian Foster
  2019-04-11  1:46 ` [PATCH 8/8] xfs: report inode health via bulkstat Darrick J. Wong
  7 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11  1:46 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Use the AG geometry info ioctl to report health status too.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_fs.h     |   14 +++++++++++++-
 fs/xfs/libxfs/xfs_health.h |    2 ++
 fs/xfs/xfs_health.c        |   43 +++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_ioctl.c         |    2 ++
 4 files changed, 60 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 6b8956dbf49d..35d60f8017cd 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -285,9 +285,21 @@ struct xfs_ag_geometry {
 	uint32_t	ag_freeblks;	/* o: free space */
 	uint32_t	ag_icount;	/* o: inodes allocated */
 	uint32_t	ag_ifree;	/* o: inodes free */
+	uint32_t	ag_sick;	/* o: sick things in ag */
+	uint32_t	ag_checked;	/* o: checked metadata in ag */
 	uint32_t	ag_reserved32;	/* o: zero */
-	uint64_t	ag_reserved[13];/* o: zero */
+	uint64_t	ag_reserved[12];/* o: zero */
 };
+#define XFS_AG_GEOM_SICK_SB	(1 << 0)  /* superblock */
+#define XFS_AG_GEOM_SICK_AGF	(1 << 1)  /* AGF header */
+#define XFS_AG_GEOM_SICK_AGFL	(1 << 2)  /* AGFL header */
+#define XFS_AG_GEOM_SICK_AGI	(1 << 3)  /* AGI header */
+#define XFS_AG_GEOM_SICK_BNOBT	(1 << 4)  /* free space by block */
+#define XFS_AG_GEOM_SICK_CNTBT	(1 << 5)  /* free space by length */
+#define XFS_AG_GEOM_SICK_INOBT	(1 << 6)  /* inode index */
+#define XFS_AG_GEOM_SICK_FINOBT	(1 << 7)  /* free inode index */
+#define XFS_AG_GEOM_SICK_RMAPBT	(1 << 8)  /* reverse mappings */
+#define XFS_AG_GEOM_SICK_REFCNTBT (1 << 9)  /* reference counts */
 
 /*
  * Structures for XFS_IOC_FSGROWFSDATA, XFS_IOC_FSGROWFSLOG & XFS_IOC_FSGROWFSRT
diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
index c72142ec7cbb..adf9d41843a5 100644
--- a/fs/xfs/libxfs/xfs_health.h
+++ b/fs/xfs/libxfs/xfs_health.h
@@ -176,5 +176,7 @@ xfs_inode_is_healthy(struct xfs_inode *ip)
 }
 
 void xfs_fsop_geom_health(struct xfs_mount *mp, struct xfs_fsop_geom *geo);
+void xfs_ag_geom_health(struct xfs_mount *mp, xfs_agnumber_t agno,
+		struct xfs_ag_geometry *ageo);
 
 #endif	/* __XFS_HEALTH_H__ */
diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
index eb8dbc3a952a..0315b95a8d84 100644
--- a/fs/xfs/xfs_health.c
+++ b/fs/xfs/xfs_health.c
@@ -321,3 +321,46 @@ xfs_fsop_geom_health(
 		xfgeo_health_tick(geo, sick, checked, m->sick_mask,
 				m->ioctl_mask);
 }
+
+static const struct ioctl_sick_map ag_map[] = {
+	{ XFS_SICK_AG_SB,	XFS_AG_GEOM_SICK_SB },
+	{ XFS_SICK_AG_AGF,	XFS_AG_GEOM_SICK_AGF },
+	{ XFS_SICK_AG_AGFL,	XFS_AG_GEOM_SICK_AGFL },
+	{ XFS_SICK_AG_AGI,	XFS_AG_GEOM_SICK_AGI },
+	{ XFS_SICK_AG_BNOBT,	XFS_AG_GEOM_SICK_BNOBT },
+	{ XFS_SICK_AG_CNTBT,	XFS_AG_GEOM_SICK_CNTBT },
+	{ XFS_SICK_AG_INOBT,	XFS_AG_GEOM_SICK_INOBT },
+	{ XFS_SICK_AG_FINOBT,	XFS_AG_GEOM_SICK_FINOBT },
+	{ XFS_SICK_AG_RMAPBT,	XFS_AG_GEOM_SICK_RMAPBT },
+	{ XFS_SICK_AG_REFCNTBT,	XFS_AG_GEOM_SICK_REFCNTBT },
+	{ 0, 0 },
+};
+
+/* Fill out ag geometry health info. */
+void
+xfs_ag_geom_health(
+	struct xfs_mount	*mp,
+	xfs_agnumber_t		agno,
+	struct xfs_ag_geometry	*ageo)
+{
+	struct xfs_perag	*pag;
+	const struct ioctl_sick_map *m;
+	unsigned int		sick;
+	unsigned int		checked;
+
+	ASSERT(agno < mp->m_sb.sb_agcount);
+
+	ageo->ag_sick = 0;
+	ageo->ag_checked = 0;
+
+	pag = xfs_perag_get(mp, agno);
+	xfs_ag_measure_sickness(pag, &sick, &checked);
+	for (m = ag_map; m->sick_mask; m++) {
+		if (checked & m->sick_mask)
+			ageo->ag_checked |= m->ioctl_mask;
+		if (sick & m->sick_mask)
+			ageo->ag_sick |= m->ioctl_mask;
+	}
+
+	xfs_perag_put(pag);
+}
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 3e3ee197bd0f..c47f4201452c 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -822,6 +822,8 @@ xfs_ioc_ag_geometry(
 	if (error)
 		return error;
 
+	xfs_ag_geom_health(mp, ageo.ag_number, &ageo);
+
 	if (copy_to_user(arg, &ageo, sizeof(ageo)))
 		return -EFAULT;
 	return 0;

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 8/8] xfs: report inode health via bulkstat
  2019-04-11  1:45 [PATCH v2 0/8] xfs: online health tracking support Darrick J. Wong
                   ` (6 preceding siblings ...)
  2019-04-11  1:46 ` [PATCH 7/8] xfs: report AG health via AG geometry ioctl Darrick J. Wong
@ 2019-04-11  1:46 ` Darrick J. Wong
  2019-04-11 13:10   ` Brian Foster
  7 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11  1:46 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Use space in the bulkstat ioctl structure to report any problems
observed with the inode.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_fs.h     |   14 +++++++++++++-
 fs/xfs/libxfs/xfs_health.h |    1 +
 fs/xfs/xfs_health.c        |   34 ++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_itable.c        |    2 ++
 4 files changed, 50 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 35d60f8017cd..43a53b03247b 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -349,13 +349,25 @@ typedef struct xfs_bstat {
 #define	bs_projid	bs_projid_lo	/* (previously just bs_projid)	*/
 	__u16		bs_forkoff;	/* inode fork offset in bytes	*/
 	__u16		bs_projid_hi;	/* higher part of project id	*/
-	unsigned char	bs_pad[6];	/* pad space, unused		*/
+	uint16_t	bs_sick;	/* sick inode metadata		*/
+	uint16_t	bs_checked;	/* checked inode metadata	*/
+	unsigned char	bs_pad[2];	/* pad space, unused		*/
 	__u32		bs_cowextsize;	/* cow extent size		*/
 	__u32		bs_dmevmask;	/* DMIG event mask		*/
 	__u16		bs_dmstate;	/* DMIG state info		*/
 	__u16		bs_aextents;	/* attribute number of extents	*/
 } xfs_bstat_t;
 
+/* bs_sick flags */
+#define XFS_BS_SICK_INODE	(1 << 0)  /* inode core */
+#define XFS_BS_SICK_BMBTD	(1 << 1)  /* data fork */
+#define XFS_BS_SICK_BMBTA	(1 << 2)  /* attr fork */
+#define XFS_BS_SICK_BMBTC	(1 << 3)  /* cow fork */
+#define XFS_BS_SICK_DIR		(1 << 4)  /* directory */
+#define XFS_BS_SICK_XATTR	(1 << 5)  /* extended attributes */
+#define XFS_BS_SICK_SYMLINK	(1 << 6)  /* symbolic link remote target */
+#define XFS_BS_SICK_PARENT	(1 << 7)  /* parent pointers */
+
 /*
  * Project quota id helpers (previously projid was 16bit only
  * and using two 16bit values to hold new 32bit projid was choosen
diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
index adf9d41843a5..4979bf1f9bcb 100644
--- a/fs/xfs/libxfs/xfs_health.h
+++ b/fs/xfs/libxfs/xfs_health.h
@@ -178,5 +178,6 @@ xfs_inode_is_healthy(struct xfs_inode *ip)
 void xfs_fsop_geom_health(struct xfs_mount *mp, struct xfs_fsop_geom *geo);
 void xfs_ag_geom_health(struct xfs_mount *mp, xfs_agnumber_t agno,
 		struct xfs_ag_geometry *ageo);
+void xfs_bulkstat_health(struct xfs_inode *ip, struct xfs_bstat *bs);
 
 #endif	/* __XFS_HEALTH_H__ */
diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
index 0315b95a8d84..c40b8d9ef6bd 100644
--- a/fs/xfs/xfs_health.c
+++ b/fs/xfs/xfs_health.c
@@ -364,3 +364,37 @@ xfs_ag_geom_health(
 
 	xfs_perag_put(pag);
 }
+
+static const struct ioctl_sick_map ino_map[] = {
+	{ XFS_SICK_INO_CORE,	XFS_BS_SICK_INODE },
+	{ XFS_SICK_INO_BMBTD,	XFS_BS_SICK_BMBTD },
+	{ XFS_SICK_INO_BMBTA,	XFS_BS_SICK_BMBTA },
+	{ XFS_SICK_INO_BMBTC,	XFS_BS_SICK_BMBTC },
+	{ XFS_SICK_INO_DIR,	XFS_BS_SICK_DIR },
+	{ XFS_SICK_INO_XATTR,	XFS_BS_SICK_XATTR },
+	{ XFS_SICK_INO_SYMLINK,	XFS_BS_SICK_SYMLINK },
+	{ XFS_SICK_INO_PARENT,	XFS_BS_SICK_PARENT },
+	{ 0, 0 },
+};
+
+/* Fill out bulkstat health info. */
+void
+xfs_bulkstat_health(
+	struct xfs_inode	*ip,
+	struct xfs_bstat	*bs)
+{
+	const struct ioctl_sick_map *m;
+	unsigned int		sick;
+	unsigned int		checked;
+
+	bs->bs_sick = 0;
+	bs->bs_checked = 0;
+
+	xfs_inode_measure_sickness(ip, &sick, &checked);
+	for (m = ino_map; m->sick_mask; m++) {
+		if (checked & m->sick_mask)
+			bs->bs_checked |= m->ioctl_mask;
+		if (sick & m->sick_mask)
+			bs->bs_sick |= m->ioctl_mask;
+	}
+}
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 942e4aa5e729..1e1a0af1dd34 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -18,6 +18,7 @@
 #include "xfs_error.h"
 #include "xfs_trace.h"
 #include "xfs_icache.h"
+#include "xfs_health.h"
 
 /*
  * Return stat information for one inode.
@@ -84,6 +85,7 @@ xfs_bulkstat_one_int(
 	buf->bs_extsize = dic->di_extsize << mp->m_sb.sb_blocklog;
 	buf->bs_extents = dic->di_nextents;
 	memset(buf->bs_pad, 0, sizeof(buf->bs_pad));
+	xfs_bulkstat_health(ip, buf);
 	buf->bs_dmevmask = dic->di_dmevmask;
 	buf->bs_dmstate = dic->di_dmstate;
 	buf->bs_aextents = dic->di_anextents;

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/8] xfs: track metadata health status
  2019-04-11  1:45 ` [PATCH 1/8] xfs: track metadata health status Darrick J. Wong
@ 2019-04-11 12:29   ` Brian Foster
  2019-04-11 15:18     ` Darrick J. Wong
  0 siblings, 1 reply; 23+ messages in thread
From: Brian Foster @ 2019-04-11 12:29 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Wed, Apr 10, 2019 at 06:45:32PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add the necessary in-core metadata fields to keep track of which parts
> of the filesystem have been observed and which parts were observed to be
> unhealthy, and print a warning at unmount time if we have unfixed
> problems.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/Makefile            |    1 
>  fs/xfs/libxfs/xfs_health.h |  175 ++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_health.c        |  192 ++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_icache.c        |    8 ++
>  fs/xfs/xfs_inode.h         |    8 ++
>  fs/xfs/xfs_mount.c         |    1 
>  fs/xfs/xfs_mount.h         |   23 +++++
>  fs/xfs/xfs_trace.h         |   73 +++++++++++++++++
>  8 files changed, 481 insertions(+)
>  create mode 100644 fs/xfs/libxfs/xfs_health.h
>  create mode 100644 fs/xfs/xfs_health.c
> 
> 
...
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index e70e7db29026..885decab4735 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -73,6 +73,8 @@ xfs_inode_alloc(
>  	INIT_WORK(&ip->i_iodone_work, xfs_end_io);
>  	INIT_LIST_HEAD(&ip->i_iodone_list);
>  	spin_lock_init(&ip->i_iodone_lock);
> +	ip->i_sick = 0;
> +	ip->i_checked = 0;
>  
>  	return ip;
>  }
> @@ -133,6 +135,8 @@ xfs_inode_free(
>  	spin_lock(&ip->i_flags_lock);
>  	ip->i_flags = XFS_IRECLAIM;
>  	ip->i_ino = 0;
> +	ip->i_sick = 0;
> +	ip->i_checked = 0;
>  	spin_unlock(&ip->i_flags_lock);
>  

FWIW, I'm not totally clear on what the i_checked mask is for yet. That
aside, is it necessary to reset these fields in the free/reclaim paths?
I wonder if it's sufficient to zero them on alloc and the cache hit path
just below..?

Otherwise looks fine:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  	__xfs_inode_free(ip);
> @@ -449,6 +453,8 @@ xfs_iget_cache_hit(
>  		ip->i_flags |= XFS_INEW;
>  		xfs_inode_clear_reclaim_tag(pag, ip->i_ino);
>  		inode->i_state = I_NEW;
> +		ip->i_sick = 0;
> +		ip->i_checked = 0;
>  
>  		ASSERT(!rwsem_is_locked(&inode->i_rwsem));
>  		init_rwsem(&inode->i_rwsem);
> @@ -1177,6 +1183,8 @@ xfs_reclaim_inode(
>  	spin_lock(&ip->i_flags_lock);
>  	ip->i_flags = XFS_IRECLAIM;
>  	ip->i_ino = 0;
> +	ip->i_sick = 0;
> +	ip->i_checked = 0;
>  	spin_unlock(&ip->i_flags_lock);
>  
>  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 88239c2dd824..494e47ef42cb 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -45,6 +45,14 @@ typedef struct xfs_inode {
>  	mrlock_t		i_lock;		/* inode lock */
>  	mrlock_t		i_mmaplock;	/* inode mmap IO lock */
>  	atomic_t		i_pincount;	/* inode pin count */
> +
> +	/*
> +	 * Bitsets of inode metadata that have been checked and/or are sick.
> +	 * Callers must hold i_flags_lock before accessing this field.
> +	 */
> +	uint16_t		i_checked;
> +	uint16_t		i_sick;
> +
>  	spinlock_t		i_flags_lock;	/* inode i_flags lock */
>  	/* Miscellaneous state. */
>  	unsigned long		i_flags;	/* see defined flags below */
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index fd63b0b1307c..6581381c12be 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -231,6 +231,7 @@ xfs_initialize_perag(
>  		error = xfs_iunlink_init(pag);
>  		if (error)
>  			goto out_hash_destroy;
> +		spin_lock_init(&pag->pag_state_lock);
>  	}
>  
>  	index = xfs_set_inode_alloc(mp, agcount);
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 110f927cf943..cf7facc36a5f 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -60,6 +60,20 @@ struct xfs_error_cfg {
>  typedef struct xfs_mount {
>  	struct super_block	*m_super;
>  	xfs_tid_t		m_tid;		/* next unused tid for fs */
> +
> +	/*
> +	 * Bitsets of per-fs metadata that have been checked and/or are sick.
> +	 * Callers must hold m_sb_lock to access these two fields.
> +	 */
> +	uint8_t			m_fs_checked;
> +	uint8_t			m_fs_sick;
> +	/*
> +	 * Bitsets of rt metadata that have been checked and/or are sick.
> +	 * Callers must hold m_sb_lock to access this field.
> +	 */
> +	uint8_t			m_rt_checked;
> +	uint8_t			m_rt_sick;
> +
>  	struct xfs_ail		*m_ail;		/* fs active log item list */
>  
>  	struct xfs_sb		m_sb;		/* copy of fs superblock */
> @@ -369,6 +383,15 @@ typedef struct xfs_perag {
>  	xfs_agino_t	pagl_pagino;
>  	xfs_agino_t	pagl_leftrec;
>  	xfs_agino_t	pagl_rightrec;
> +
> +	/*
> +	 * Bitsets of per-ag metadata that have been checked and/or are sick.
> +	 * Callers should hold pag_state_lock before accessing this field.
> +	 */
> +	uint16_t	pag_checked;
> +	uint16_t	pag_sick;
> +	spinlock_t	pag_state_lock;
> +
>  	spinlock_t	pagb_lock;	/* lock for pagb_tree */
>  	struct rb_root	pagb_tree;	/* ordered tree of busy extents */
>  	unsigned int	pagb_gen;	/* generation count for pagb_tree */
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 47fb07d86efd..f079841c7af6 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -3440,6 +3440,79 @@ DEFINE_AGINODE_EVENT(xfs_iunlink);
>  DEFINE_AGINODE_EVENT(xfs_iunlink_remove);
>  DEFINE_AG_EVENT(xfs_iunlink_map_prev_fallback);
>  
> +DECLARE_EVENT_CLASS(xfs_fs_corrupt_class,
> +	TP_PROTO(struct xfs_mount *mp, unsigned int flags),
> +	TP_ARGS(mp, flags),
> +	TP_STRUCT__entry(
> +		__field(dev_t, dev)
> +		__field(unsigned int, flags)
> +	),
> +	TP_fast_assign(
> +		__entry->dev = mp->m_super->s_dev;
> +		__entry->flags = flags;
> +	),
> +	TP_printk("dev %d:%d flags 0x%x",
> +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> +		  __entry->flags)
> +);
> +#define DEFINE_FS_CORRUPT_EVENT(name)	\
> +DEFINE_EVENT(xfs_fs_corrupt_class, name,	\
> +	TP_PROTO(struct xfs_mount *mp, unsigned int flags), \
> +	TP_ARGS(mp, flags))
> +DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_sick);
> +DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_healthy);
> +DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_sick);
> +DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_healthy);
> +
> +DECLARE_EVENT_CLASS(xfs_ag_corrupt_class,
> +	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, unsigned int flags),
> +	TP_ARGS(mp, agno, flags),
> +	TP_STRUCT__entry(
> +		__field(dev_t, dev)
> +		__field(xfs_agnumber_t, agno)
> +		__field(unsigned int, flags)
> +	),
> +	TP_fast_assign(
> +		__entry->dev = mp->m_super->s_dev;
> +		__entry->agno = agno;
> +		__entry->flags = flags;
> +	),
> +	TP_printk("dev %d:%d agno %u flags 0x%x",
> +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> +		  __entry->agno, __entry->flags)
> +);
> +#define DEFINE_AG_CORRUPT_EVENT(name)	\
> +DEFINE_EVENT(xfs_ag_corrupt_class, name,	\
> +	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, \
> +		 unsigned int flags), \
> +	TP_ARGS(mp, agno, flags))
> +DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_sick);
> +DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_healthy);
> +
> +DECLARE_EVENT_CLASS(xfs_inode_corrupt_class,
> +	TP_PROTO(struct xfs_inode *ip, unsigned int flags),
> +	TP_ARGS(ip, flags),
> +	TP_STRUCT__entry(
> +		__field(dev_t, dev)
> +		__field(xfs_ino_t, ino)
> +		__field(unsigned int, flags)
> +	),
> +	TP_fast_assign(
> +		__entry->dev = ip->i_mount->m_super->s_dev;
> +		__entry->ino = ip->i_ino;
> +		__entry->flags = flags;
> +	),
> +	TP_printk("dev %d:%d ino 0x%llx flags 0x%x",
> +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> +		  __entry->ino, __entry->flags)
> +);
> +#define DEFINE_INODE_CORRUPT_EVENT(name)	\
> +DEFINE_EVENT(xfs_inode_corrupt_class, name,	\
> +	TP_PROTO(struct xfs_inode *ip, unsigned int flags), \
> +	TP_ARGS(ip, flags))
> +DEFINE_INODE_CORRUPT_EVENT(xfs_inode_mark_sick);
> +DEFINE_INODE_CORRUPT_EVENT(xfs_inode_mark_healthy);
> +
>  #endif /* _TRACE_XFS_H */
>  
>  #undef TRACE_INCLUDE_PATH
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/8] xfs: clear BAD_SUMMARY if unmounting an unhealthy filesystem
  2019-04-11  1:45 ` [PATCH 3/8] xfs: clear BAD_SUMMARY if unmounting an unhealthy filesystem Darrick J. Wong
@ 2019-04-11 12:29   ` Brian Foster
  0 siblings, 0 replies; 23+ messages in thread
From: Brian Foster @ 2019-04-11 12:29 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Wed, Apr 10, 2019 at 06:45:45PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> If we know the filesystem metadata isn't healthy during unmount, we want
> to encourage the administrator to run xfs_repair right away.  We can't
> do this if BAD_SUMMARY will cause an unclean log unmount to force
> summary recalculation, so turn it off if the fs is bad.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_health.h |    2 +
>  fs/xfs/xfs_health.c        |   74 ++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_mount.c         |    2 +
>  fs/xfs/xfs_trace.h         |    3 ++
>  4 files changed, 81 insertions(+)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
> index 30762a5d4862..a434b47f2aa0 100644
> --- a/fs/xfs/libxfs/xfs_health.h
> +++ b/fs/xfs/libxfs/xfs_health.h
> @@ -110,6 +110,8 @@ 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);
>  
> +void xfs_health_unmount(struct xfs_mount *mp);
> +
>  /* Now some helpers. */
>  
>  static inline bool
> diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
> index 941f33037e2f..21728228e08b 100644
> --- a/fs/xfs/xfs_health.c
> +++ b/fs/xfs/xfs_health.c
> @@ -19,6 +19,80 @@
>  #include "xfs_trace.h"
>  #include "xfs_health.h"
>  
> +/*
> + * Warn about metadata corruption that we detected but haven't fixed, and
> + * make sure we're not sitting on anything that would get in the way of
> + * recovery.
> + */
> +void
> +xfs_health_unmount(
> +	struct xfs_mount	*mp)
> +{
> +	struct xfs_perag	*pag;
> +	xfs_agnumber_t		agno;
> +	unsigned int		sick = 0;
> +	unsigned int		checked = 0;
> +	bool			warn = false;
> +
> +	if (XFS_FORCED_SHUTDOWN(mp))
> +		return;
> +
> +	/* Measure AG corruption levels. */
> +	for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
> +		pag = xfs_perag_get(mp, agno);
> +		xfs_ag_measure_sickness(pag, &sick, &checked);
> +		if (sick) {
> +			trace_xfs_ag_unfixed_corruption(mp, agno, sick);
> +			warn = true;
> +		}
> +		xfs_perag_put(pag);
> +	}
> +
> +	/* Measure realtime volume corruption levels. */
> +	xfs_rt_measure_sickness(mp, &sick, &checked);
> +	if (sick) {
> +		trace_xfs_rt_unfixed_corruption(mp, sick);
> +		warn = true;
> +	}
> +
> +	/*
> +	 * Measure fs corruption and keep the sample around for the warning.
> +	 * See the note below for why we exempt FS_COUNTERS.
> +	 */
> +	xfs_fs_measure_sickness(mp, &sick, &checked);
> +	if (sick & ~XFS_SICK_FS_COUNTERS) {
> +		trace_xfs_fs_unfixed_corruption(mp, sick);
> +		warn = true;
> +	}
> +
> +	if (warn) {
> +		xfs_warn(mp,
> +"Uncorrected metadata errors detected; please run xfs_repair.");
> +
> +		/*
> +		 * We discovered uncorrected metadata problems at some point
> +		 * during this filesystem mount and have advised the
> +		 * administrator to run repair once the unmount completes.
> +		 *
> +		 * However, we must be careful -- when FSCOUNTERS are flagged
> +		 * unhealthy, the unmount procedure omits writing the clean
> +		 * unmount record to the log so that the next mount will run
> +		 * recovery and recompute the summary counters.  In other
> +		 * words, we leave a dirty log to get the counters fixed.
> +		 *
> +		 * Unfortunately, xfs_repair cannot recover dirty logs, so if
> +		 * there were filesystem problems, FSCOUNTERS was flagged, and
> +		 * the administrator takes our advice to run xfs_repair,
> +		 * they'll have to zap the log before repairing structures.
> +		 * We don't really want to encourage this, so we mark the
> +		 * FSCOUNTERS healthy so that a subsequent repair run won't see
> +		 * a dirty log.
> +		 */
> +		if (sick & XFS_SICK_FS_COUNTERS)
> +			xfs_fs_mark_healthy(mp, XFS_SICK_FS_COUNTERS);
> +	}
> +}
> +
>  /* Mark unhealthy per-fs metadata. */
>  void
>  xfs_fs_mark_sick(
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 14f454e09e6e..eff8b4c3eb3e 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -1070,6 +1070,7 @@ xfs_mountfs(
>  	 */
>  	cancel_delayed_work_sync(&mp->m_reclaim_work);
>  	xfs_reclaim_inodes(mp, SYNC_WAIT);
> +	xfs_health_unmount(mp);
>   out_log_dealloc:
>  	mp->m_flags |= XFS_MOUNT_UNMOUNTING;
>  	xfs_log_mount_cancel(mp);
> @@ -1152,6 +1153,7 @@ xfs_unmountfs(
>  	 */
>  	cancel_delayed_work_sync(&mp->m_reclaim_work);
>  	xfs_reclaim_inodes(mp, SYNC_WAIT);
> +	xfs_health_unmount(mp);
>  
>  	xfs_qm_unmount(mp);
>  
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index f079841c7af6..2464ea351f83 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -3461,8 +3461,10 @@ DEFINE_EVENT(xfs_fs_corrupt_class, name,	\
>  	TP_ARGS(mp, flags))
>  DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_sick);
>  DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_healthy);
> +DEFINE_FS_CORRUPT_EVENT(xfs_fs_unfixed_corruption);
>  DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_sick);
>  DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_healthy);
> +DEFINE_FS_CORRUPT_EVENT(xfs_rt_unfixed_corruption);
>  
>  DECLARE_EVENT_CLASS(xfs_ag_corrupt_class,
>  	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, unsigned int flags),
> @@ -3488,6 +3490,7 @@ DEFINE_EVENT(xfs_ag_corrupt_class, name,	\
>  	TP_ARGS(mp, agno, flags))
>  DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_sick);
>  DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_healthy);
> +DEFINE_AG_CORRUPT_EVENT(xfs_ag_unfixed_corruption);
>  
>  DECLARE_EVENT_CLASS(xfs_inode_corrupt_class,
>  	TP_PROTO(struct xfs_inode *ip, unsigned int flags),
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/8] xfs: bump XFS_IOC_FSGEOMETRY to v5 structures
  2019-04-11  1:45 ` [PATCH 4/8] xfs: bump XFS_IOC_FSGEOMETRY to v5 structures Darrick J. Wong
@ 2019-04-11 12:29   ` Brian Foster
  0 siblings, 0 replies; 23+ messages in thread
From: Brian Foster @ 2019-04-11 12:29 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, Dave Chinner

On Wed, Apr 10, 2019 at 06:45:51PM -0700, Darrick J. Wong wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Unfortunately, the V4 XFS_IOC_FSGEOMETRY structure is out of space so we
> can't just add a new field to it. Hence we need to bump the definition
> to V5 and and treat the V4 ioctl and structure similar to v1 to v3.
> 
> While doing this, clean up all the definitions associated with the
> XFS_IOC_FSGEOMETRY ioctl.
> 
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> [darrick: forward port to 5.1, expand structure size to 256 bytes]
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_fs.h |   87 +++++++++++++++++++++++++++++++++---------------
>  fs/xfs/libxfs/xfs_sb.c |    5 +++
>  fs/xfs/xfs_ioctl.c     |   48 ++++++++++----------------
>  fs/xfs/xfs_ioctl32.c   |    3 +-
>  4 files changed, 84 insertions(+), 59 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index f3aa59302fef..cb7d0b1453cd 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -124,7 +124,7 @@ typedef struct xfs_flock64 {
>  /*
>   * Output for XFS_IOC_FSGEOMETRY_V1
>   */
> -typedef struct xfs_fsop_geom_v1 {
> +struct xfs_fsop_geom_v1 {
>  	__u32		blocksize;	/* filesystem (data) block size */
>  	__u32		rtextsize;	/* realtime extent size		*/
>  	__u32		agblocks;	/* fsblocks in an AG		*/
> @@ -145,12 +145,39 @@ typedef struct xfs_fsop_geom_v1 {
>  	__u32		logsectsize;	/* log sector size, bytes	*/
>  	__u32		rtsectsize;	/* realtime sector size, bytes	*/
>  	__u32		dirblocksize;	/* directory block size, bytes	*/
> -} xfs_fsop_geom_v1_t;
> +};
> +
> +/*
> + * Output for XFS_IOC_FSGEOMETRY_V4
> + */
> +struct xfs_fsop_geom_v4 {
> +	__u32		blocksize;	/* filesystem (data) block size */
> +	__u32		rtextsize;	/* realtime extent size		*/
> +	__u32		agblocks;	/* fsblocks in an AG		*/
> +	__u32		agcount;	/* number of allocation groups	*/
> +	__u32		logblocks;	/* fsblocks in the log		*/
> +	__u32		sectsize;	/* (data) sector size, bytes	*/
> +	__u32		inodesize;	/* inode size in bytes		*/
> +	__u32		imaxpct;	/* max allowed inode space(%)	*/
> +	__u64		datablocks;	/* fsblocks in data subvolume	*/
> +	__u64		rtblocks;	/* fsblocks in realtime subvol	*/
> +	__u64		rtextents;	/* rt extents in realtime subvol*/
> +	__u64		logstart;	/* starting fsblock of the log	*/
> +	unsigned char	uuid[16];	/* unique id of the filesystem	*/
> +	__u32		sunit;		/* stripe unit, fsblocks	*/
> +	__u32		swidth;		/* stripe width, fsblocks	*/
> +	__s32		version;	/* structure version		*/
> +	__u32		flags;		/* superblock version flags	*/
> +	__u32		logsectsize;	/* log sector size, bytes	*/
> +	__u32		rtsectsize;	/* realtime sector size, bytes	*/
> +	__u32		dirblocksize;	/* directory block size, bytes	*/
> +	__u32		logsunit;	/* log stripe unit, bytes	*/
> +};
>  
>  /*
>   * Output for XFS_IOC_FSGEOMETRY
>   */
> -typedef struct xfs_fsop_geom {
> +struct xfs_fsop_geom {
>  	__u32		blocksize;	/* filesystem (data) block size */
>  	__u32		rtextsize;	/* realtime extent size		*/
>  	__u32		agblocks;	/* fsblocks in an AG		*/
> @@ -171,8 +198,9 @@ typedef struct xfs_fsop_geom {
>  	__u32		logsectsize;	/* log sector size, bytes	*/
>  	__u32		rtsectsize;	/* realtime sector size, bytes	*/
>  	__u32		dirblocksize;	/* directory block size, bytes	*/
> -	__u32		logsunit;	/* log stripe unit, bytes */
> -} xfs_fsop_geom_t;
> +	__u32		logsunit;	/* log stripe unit, bytes	*/
> +	__u64		reserved[18];	/* reserved space		*/
> +};
>  
>  /* Output for XFS_FS_COUNTS */
>  typedef struct xfs_fsop_counts {
> @@ -188,28 +216,30 @@ typedef struct xfs_fsop_resblks {
>  	__u64  resblks_avail;
>  } xfs_fsop_resblks_t;
>  
> -#define XFS_FSOP_GEOM_VERSION	0
> -
> -#define XFS_FSOP_GEOM_FLAGS_ATTR	0x0001	/* attributes in use	*/
> -#define XFS_FSOP_GEOM_FLAGS_NLINK	0x0002	/* 32-bit nlink values	*/
> -#define XFS_FSOP_GEOM_FLAGS_QUOTA	0x0004	/* quotas enabled	*/
> -#define XFS_FSOP_GEOM_FLAGS_IALIGN	0x0008	/* inode alignment	*/
> -#define XFS_FSOP_GEOM_FLAGS_DALIGN	0x0010	/* large data alignment */
> -#define XFS_FSOP_GEOM_FLAGS_SHARED	0x0020	/* read-only shared	*/
> -#define XFS_FSOP_GEOM_FLAGS_EXTFLG	0x0040	/* special extent flag	*/
> -#define XFS_FSOP_GEOM_FLAGS_DIRV2	0x0080	/* directory version 2	*/
> -#define XFS_FSOP_GEOM_FLAGS_LOGV2	0x0100	/* log format version 2	*/
> -#define XFS_FSOP_GEOM_FLAGS_SECTOR	0x0200	/* sector sizes >1BB	*/
> -#define XFS_FSOP_GEOM_FLAGS_ATTR2	0x0400	/* inline attributes rework */
> -#define XFS_FSOP_GEOM_FLAGS_PROJID32	0x0800	/* 32-bit project IDs	*/
> -#define XFS_FSOP_GEOM_FLAGS_DIRV2CI	0x1000	/* ASCII only CI names	*/
> -#define XFS_FSOP_GEOM_FLAGS_LAZYSB	0x4000	/* lazy superblock counters */
> -#define XFS_FSOP_GEOM_FLAGS_V5SB	0x8000	/* version 5 superblock */
> -#define XFS_FSOP_GEOM_FLAGS_FTYPE	0x10000	/* inode directory types */
> -#define XFS_FSOP_GEOM_FLAGS_FINOBT	0x20000	/* free inode btree */
> -#define XFS_FSOP_GEOM_FLAGS_SPINODES	0x40000	/* sparse inode chunks	*/
> -#define XFS_FSOP_GEOM_FLAGS_RMAPBT	0x80000	/* reverse mapping btree */
> -#define XFS_FSOP_GEOM_FLAGS_REFLINK	0x100000 /* files can share blocks */
> +#define XFS_FSOP_GEOM_VERSION		0
> +#define XFS_FSOP_GEOM_VERSION_V5	5
> +
> +#define XFS_FSOP_GEOM_FLAGS_ATTR	(1 << 0)  /* attributes in use	   */
> +#define XFS_FSOP_GEOM_FLAGS_NLINK	(1 << 1)  /* 32-bit nlink values   */
> +#define XFS_FSOP_GEOM_FLAGS_QUOTA	(1 << 2)  /* quotas enabled	   */
> +#define XFS_FSOP_GEOM_FLAGS_IALIGN	(1 << 3)  /* inode alignment	   */
> +#define XFS_FSOP_GEOM_FLAGS_DALIGN	(1 << 4)  /* large data alignment  */
> +#define XFS_FSOP_GEOM_FLAGS_SHARED	(1 << 5)  /* read-only shared	   */
> +#define XFS_FSOP_GEOM_FLAGS_EXTFLG	(1 << 6)  /* special extent flag   */
> +#define XFS_FSOP_GEOM_FLAGS_DIRV2	(1 << 7)  /* directory version 2   */
> +#define XFS_FSOP_GEOM_FLAGS_LOGV2	(1 << 8)  /* log format version 2  */
> +#define XFS_FSOP_GEOM_FLAGS_SECTOR	(1 << 9)  /* sector sizes >1BB	   */
> +#define XFS_FSOP_GEOM_FLAGS_ATTR2	(1 << 10) /* inline attributes rework */
> +#define XFS_FSOP_GEOM_FLAGS_PROJID32	(1 << 11) /* 32-bit project IDs	   */
> +#define XFS_FSOP_GEOM_FLAGS_DIRV2CI	(1 << 12) /* ASCII only CI names   */
> +	/*  -- Do not use --		(1 << 13)    SGI parent pointers   */
> +#define XFS_FSOP_GEOM_FLAGS_LAZYSB	(1 << 14) /* lazy superblock counters */
> +#define XFS_FSOP_GEOM_FLAGS_V5SB	(1 << 15) /* version 5 superblock  */
> +#define XFS_FSOP_GEOM_FLAGS_FTYPE	(1 << 16) /* inode directory types */
> +#define XFS_FSOP_GEOM_FLAGS_FINOBT	(1 << 17) /* free inode btree	   */
> +#define XFS_FSOP_GEOM_FLAGS_SPINODES	(1 << 18) /* sparse inode chunks   */
> +#define XFS_FSOP_GEOM_FLAGS_RMAPBT	(1 << 19) /* reverse mapping btree */
> +#define XFS_FSOP_GEOM_FLAGS_REFLINK	(1 << 20) /* files can share blocks */
>  
>  /*
>   * Minimum and maximum sizes need for growth checks.
> @@ -620,8 +650,9 @@ struct xfs_scrub_metadata {
>  #define XFS_IOC_FSSETDM_BY_HANDLE    _IOW ('X', 121, struct xfs_fsop_setdm_handlereq)
>  #define XFS_IOC_ATTRLIST_BY_HANDLE   _IOW ('X', 122, struct xfs_fsop_attrlist_handlereq)
>  #define XFS_IOC_ATTRMULTI_BY_HANDLE  _IOW ('X', 123, struct xfs_fsop_attrmulti_handlereq)
> -#define XFS_IOC_FSGEOMETRY	     _IOR ('X', 124, struct xfs_fsop_geom)
> +#define XFS_IOC_FSGEOMETRY_V4	     _IOR ('X', 124, struct xfs_fsop_geom_v4)
>  #define XFS_IOC_GOINGDOWN	     _IOR ('X', 125, uint32_t)
> +#define XFS_IOC_FSGEOMETRY	     _IOR ('X', 126, struct xfs_fsop_geom)
>  /*	XFS_IOC_GETFSUUID ---------- deprecated 140	 */
>  
>  
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index dc5be0c631c8..6fab49f6070b 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -1166,6 +1166,11 @@ xfs_fs_geometry(
>  
>  	geo->logsunit = sbp->sb_logsunit;
>  
> +	if (struct_version < 5)
> +		return 0;
> +
> +	geo->version = XFS_FSOP_GEOM_VERSION_V5;
> +
>  	return 0;
>  }
>  
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 67d12027f563..d243660f1826 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -778,41 +778,28 @@ xfs_ioc_bulkstat(
>  	return 0;
>  }
>  
> -STATIC int
> -xfs_ioc_fsgeometry_v1(
> -	xfs_mount_t		*mp,
> -	void			__user *arg)
> -{
> -	xfs_fsop_geom_t         fsgeo;
> -	int			error;
> -
> -	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
> -	if (error)
> -		return error;
> -
> -	/*
> -	 * Caller should have passed an argument of type
> -	 * xfs_fsop_geom_v1_t.  This is a proper subset of the
> -	 * xfs_fsop_geom_t that xfs_fs_geometry() fills in.
> -	 */
> -	if (copy_to_user(arg, &fsgeo, sizeof(xfs_fsop_geom_v1_t)))
> -		return -EFAULT;
> -	return 0;
> -}
> -
>  STATIC int
>  xfs_ioc_fsgeometry(
> -	xfs_mount_t		*mp,
> -	void			__user *arg)
> +	struct xfs_mount	*mp,
> +	void			__user *arg,
> +	int			struct_version)
>  {
> -	xfs_fsop_geom_t		fsgeo;
> +	struct xfs_fsop_geom	fsgeo;
> +	size_t			len;
>  	int			error;
>  
> -	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 4);
> +	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, struct_version);
>  	if (error)
>  		return error;
>  
> -	if (copy_to_user(arg, &fsgeo, sizeof(fsgeo)))
> +	if (struct_version <= 3)
> +		len = sizeof(struct xfs_fsop_geom_v1);
> +	else if (struct_version == 4)
> +		len = sizeof(struct xfs_fsop_geom_v4);
> +	else
> +		len = sizeof(fsgeo);
> +
> +	if (copy_to_user(arg, &fsgeo, len))
>  		return -EFAULT;
>  	return 0;
>  }
> @@ -2016,10 +2003,11 @@ xfs_file_ioctl(
>  		return xfs_ioc_bulkstat(mp, cmd, arg);
>  
>  	case XFS_IOC_FSGEOMETRY_V1:
> -		return xfs_ioc_fsgeometry_v1(mp, arg);
> -
> +		return xfs_ioc_fsgeometry(mp, arg, 3);
> +	case XFS_IOC_FSGEOMETRY_V4:
> +		return xfs_ioc_fsgeometry(mp, arg, 4);
>  	case XFS_IOC_FSGEOMETRY:
> -		return xfs_ioc_fsgeometry(mp, arg);
> +		return xfs_ioc_fsgeometry(mp, arg, 5);
>  
>  	case XFS_IOC_GETVERSION:
>  		return put_user(inode->i_generation, (int __user *)arg);
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index 5001dca361e9..55ace6308637 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -52,7 +52,7 @@ xfs_compat_ioc_fsgeometry_v1(
>  	struct xfs_mount	  *mp,
>  	compat_xfs_fsop_geom_v1_t __user *arg32)
>  {
> -	xfs_fsop_geom_t		  fsgeo;
> +	struct xfs_fsop_geom	  fsgeo;
>  	int			  error;
>  
>  	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
> @@ -561,6 +561,7 @@ xfs_file_compat_ioctl(
>  	switch (cmd) {
>  	/* No size or alignment issues on any arch */
>  	case XFS_IOC_DIOINFO:
> +	case XFS_IOC_FSGEOMETRY_V4:
>  	case XFS_IOC_FSGEOMETRY:
>  	case XFS_IOC_FSGETXATTR:
>  	case XFS_IOC_FSSETXATTR:
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 5/8] xfs: add a new ioctl to describe allocation group geometry
  2019-04-11  1:45 ` [PATCH 5/8] xfs: add a new ioctl to describe allocation group geometry Darrick J. Wong
@ 2019-04-11 13:08   ` Brian Foster
  0 siblings, 0 replies; 23+ messages in thread
From: Brian Foster @ 2019-04-11 13:08 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Wed, Apr 10, 2019 at 06:45:58PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add a new ioctl to describe an allocation group's geometry.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_ag.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_ag.h |    2 ++
>  fs/xfs/libxfs/xfs_fs.h |   14 +++++++++++++
>  fs/xfs/xfs_ioctl.c     |   24 ++++++++++++++++++++++
>  fs/xfs/xfs_ioctl32.c   |    1 +
>  5 files changed, 93 insertions(+)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index 1ef8acf35e7d..1c0f2a6c10b4 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -19,6 +19,7 @@
>  #include "xfs_ialloc.h"
>  #include "xfs_rmap.h"
>  #include "xfs_ag.h"
> +#include "xfs_ag_resv.h"
>  
>  static struct xfs_buf *
>  xfs_get_aghdr_buf(
> @@ -461,3 +462,54 @@ xfs_ag_extend_space(
>  				len, &XFS_RMAP_OINFO_SKIP_UPDATE,
>  				XFS_AG_RESV_NONE);
>  }
> +
> +/* Retrieve AG geometry. */
> +int
> +xfs_ag_get_geometry(
> +	struct xfs_mount	*mp,
> +	xfs_agnumber_t		agno,
> +	struct xfs_ag_geometry	*ageo)
> +{
> +	struct xfs_buf		*agi_bp;
> +	struct xfs_buf		*agf_bp;
> +	struct xfs_agi		*agi;
> +	struct xfs_agf		*agf;
> +	struct xfs_perag	*pag;
> +	unsigned int		freeblks;
> +	int			error;
> +
> +	if (agno >= mp->m_sb.sb_agcount)
> +		return -EINVAL;
> +
> +	/* Lock the AG headers. */
> +	error = xfs_ialloc_read_agi(mp, NULL, agno, &agi_bp);
> +	if (error)
> +		return error;
> +	error = xfs_alloc_read_agf(mp, NULL, agno, 0, &agf_bp);
> +	if (error)
> +		goto out_agi;
> +	pag = xfs_perag_get(mp, agno);
> +
> +	/* Fill out form. */
> +	memset(ageo, 0, sizeof(*ageo));
> +	ageo->ag_number = agno;
> +
> +	agi = XFS_BUF_TO_AGI(agi_bp);
> +	ageo->ag_icount = be32_to_cpu(agi->agi_count);
> +	ageo->ag_ifree = be32_to_cpu(agi->agi_freecount);
> +
> +	agf = XFS_BUF_TO_AGF(agf_bp);
> +	ageo->ag_length = be32_to_cpu(agf->agf_length);
> +	freeblks = pag->pagf_freeblks +
> +		   pag->pagf_flcount +
> +		   pag->pagf_btreeblks -
> +		   xfs_ag_resv_needed(pag, XFS_AG_RESV_NONE);
> +	ageo->ag_freeblks = freeblks;
> +
> +	/* Release resources. */
> +	xfs_perag_put(pag);
> +	xfs_buf_relse(agf_bp);
> +out_agi:
> +	xfs_buf_relse(agi_bp);
> +	return error;
> +}
> diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> index 412702e23f61..5166322807e7 100644
> --- a/fs/xfs/libxfs/xfs_ag.h
> +++ b/fs/xfs/libxfs/xfs_ag.h
> @@ -26,5 +26,7 @@ struct aghdr_init_data {
>  int xfs_ag_init_headers(struct xfs_mount *mp, struct aghdr_init_data *id);
>  int xfs_ag_extend_space(struct xfs_mount *mp, struct xfs_trans *tp,
>  			struct aghdr_init_data *id, xfs_extlen_t len);
> +int xfs_ag_get_geometry(struct xfs_mount *mp, xfs_agnumber_t agno,
> +			struct xfs_ag_geometry *ageo);
>  
>  #endif /* __LIBXFS_AG_H */
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index cb7d0b1453cd..ee33d628a240 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -267,6 +267,19 @@ typedef struct xfs_fsop_resblks {
>  #define XFS_MIN_DBLOCKS(s) ((xfs_rfsblock_t)((s)->sb_agcount - 1) *	\
>  			 (s)->sb_agblocks + XFS_MIN_AG_BLOCKS)
>  
> +/*
> + * Output for XFS_IOC_AG_GEOMETRY
> + */
> +struct xfs_ag_geometry {
> +	uint32_t	ag_number;	/* i/o: AG number */
> +	uint32_t	ag_length;	/* o: length in blocks */
> +	uint32_t	ag_freeblks;	/* o: free space */
> +	uint32_t	ag_icount;	/* o: inodes allocated */
> +	uint32_t	ag_ifree;	/* o: inodes free */
> +	uint32_t	ag_reserved32;	/* o: zero */
> +	uint64_t	ag_reserved[13];/* o: zero */
> +};
> +
>  /*
>   * Structures for XFS_IOC_FSGROWFSDATA, XFS_IOC_FSGROWFSLOG & XFS_IOC_FSGROWFSRT
>   */
> @@ -620,6 +633,7 @@ struct xfs_scrub_metadata {
>  #define XFS_IOC_FREE_EOFBLOCKS	_IOR ('X', 58, struct xfs_fs_eofblocks)
>  /*	XFS_IOC_GETFSMAP ------ hoisted 59         */
>  #define XFS_IOC_SCRUB_METADATA	_IOWR('X', 60, struct xfs_scrub_metadata)
> +#define XFS_IOC_AG_GEOMETRY	_IOWR('X', 61, struct xfs_ag_geometry)
>  
>  /*
>   * ioctl commands that replace IRIX syssgi()'s
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index d243660f1826..0aaf4f88524d 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -33,6 +33,7 @@
>  #include "xfs_fsmap.h"
>  #include "scrub/xfs_scrub.h"
>  #include "xfs_sb.h"
> +#include "xfs_ag.h"
>  
>  #include <linux/capability.h>
>  #include <linux/cred.h>
> @@ -804,6 +805,26 @@ xfs_ioc_fsgeometry(
>  	return 0;
>  }
>  
> +STATIC int
> +xfs_ioc_ag_geometry(
> +	struct xfs_mount	*mp,
> +	void			__user *arg)
> +{
> +	struct xfs_ag_geometry	ageo;
> +	int			error;
> +
> +	if (copy_from_user(&ageo, arg, sizeof(ageo)))
> +		return -EFAULT;
> +
> +	error = xfs_ag_get_geometry(mp, ageo.ag_number, &ageo);
> +	if (error)
> +		return error;
> +
> +	if (copy_to_user(arg, &ageo, sizeof(ageo)))
> +		return -EFAULT;
> +	return 0;
> +}
> +
>  /*
>   * Linux extended inode flags interface.
>   */
> @@ -2009,6 +2030,9 @@ xfs_file_ioctl(
>  	case XFS_IOC_FSGEOMETRY:
>  		return xfs_ioc_fsgeometry(mp, arg, 5);
>  
> +	case XFS_IOC_AG_GEOMETRY:
> +		return xfs_ioc_ag_geometry(mp, arg);
> +
>  	case XFS_IOC_GETVERSION:
>  		return put_user(inode->i_generation, (int __user *)arg);
>  
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index 55ace6308637..65997a6315e9 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -563,6 +563,7 @@ xfs_file_compat_ioctl(
>  	case XFS_IOC_DIOINFO:
>  	case XFS_IOC_FSGEOMETRY_V4:
>  	case XFS_IOC_FSGEOMETRY:
> +	case XFS_IOC_AG_GEOMETRY:
>  	case XFS_IOC_FSGETXATTR:
>  	case XFS_IOC_FSSETXATTR:
>  	case XFS_IOC_FSGETXATTRA:
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/8] xfs: report fs and rt health via geometry structure
  2019-04-11  1:46 ` [PATCH 6/8] xfs: report fs and rt health via geometry structure Darrick J. Wong
@ 2019-04-11 13:09   ` Brian Foster
  2019-04-11 15:30     ` Darrick J. Wong
  0 siblings, 1 reply; 23+ messages in thread
From: Brian Foster @ 2019-04-11 13:09 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/8] xfs: report AG health via AG geometry ioctl
  2019-04-11  1:46 ` [PATCH 7/8] xfs: report AG health via AG geometry ioctl Darrick J. Wong
@ 2019-04-11 13:09   ` Brian Foster
  2019-04-11 15:33     ` Darrick J. Wong
  0 siblings, 1 reply; 23+ messages in thread
From: Brian Foster @ 2019-04-11 13:09 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Wed, Apr 10, 2019 at 06:46:10PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use the AG geometry info ioctl to report health status too.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_fs.h     |   14 +++++++++++++-
>  fs/xfs/libxfs/xfs_health.h |    2 ++
>  fs/xfs/xfs_health.c        |   43 +++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_ioctl.c         |    2 ++
>  4 files changed, 60 insertions(+), 1 deletion(-)
> 
> 
...
> diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
> index eb8dbc3a952a..0315b95a8d84 100644
> --- a/fs/xfs/xfs_health.c
> +++ b/fs/xfs/xfs_health.c
> @@ -321,3 +321,46 @@ xfs_fsop_geom_health(
...
> +/* Fill out ag geometry health info. */
> +void
> +xfs_ag_geom_health(
> +	struct xfs_mount	*mp,
> +	xfs_agnumber_t		agno,
> +	struct xfs_ag_geometry	*ageo)
> +{
> +	struct xfs_perag	*pag;
> +	const struct ioctl_sick_map *m;
> +	unsigned int		sick;
> +	unsigned int		checked;
> +
> +	ASSERT(agno < mp->m_sb.sb_agcount);
> +
> +	ageo->ag_sick = 0;
> +	ageo->ag_checked = 0;
> +
> +	pag = xfs_perag_get(mp, agno);
> +	xfs_ag_measure_sickness(pag, &sick, &checked);
> +	for (m = ag_map; m->sick_mask; m++) {
> +		if (checked & m->sick_mask)
> +			ageo->ag_checked |= m->ioctl_mask;
> +		if (sick & m->sick_mask)
> +			ageo->ag_sick |= m->ioctl_mask;
> +	}

FWIW this is pretty much what I was asking for in the mapping helper in
the previous patch. I didn't realize this was going to be reused for
another ioctl/struct, however, so disregard my previous comment about
renaming ->ioctl_mask. ;)

> +
> +	xfs_perag_put(pag);
> +}
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 3e3ee197bd0f..c47f4201452c 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -822,6 +822,8 @@ xfs_ioc_ag_geometry(
>  	if (error)
>  		return error;
>  
> +	xfs_ag_geom_health(mp, ageo.ag_number, &ageo);
> +

Any reason not to push this down into the get_geom() helper since it's
part of the initial version of the ag geometry structure and we already
have the xfs_perag there?

Brian

>  	if (copy_to_user(arg, &ageo, sizeof(ageo)))
>  		return -EFAULT;
>  	return 0;
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 8/8] xfs: report inode health via bulkstat
  2019-04-11  1:46 ` [PATCH 8/8] xfs: report inode health via bulkstat Darrick J. Wong
@ 2019-04-11 13:10   ` Brian Foster
  0 siblings, 0 replies; 23+ messages in thread
From: Brian Foster @ 2019-04-11 13:10 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Wed, Apr 10, 2019 at 06:46:17PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use space in the bulkstat ioctl structure to report any problems
> observed with the inode.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_fs.h     |   14 +++++++++++++-
>  fs/xfs/libxfs/xfs_health.h |    1 +
>  fs/xfs/xfs_health.c        |   34 ++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_itable.c        |    2 ++
>  4 files changed, 50 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index 35d60f8017cd..43a53b03247b 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -349,13 +349,25 @@ typedef struct xfs_bstat {
>  #define	bs_projid	bs_projid_lo	/* (previously just bs_projid)	*/
>  	__u16		bs_forkoff;	/* inode fork offset in bytes	*/
>  	__u16		bs_projid_hi;	/* higher part of project id	*/
> -	unsigned char	bs_pad[6];	/* pad space, unused		*/
> +	uint16_t	bs_sick;	/* sick inode metadata		*/
> +	uint16_t	bs_checked;	/* checked inode metadata	*/
> +	unsigned char	bs_pad[2];	/* pad space, unused		*/
>  	__u32		bs_cowextsize;	/* cow extent size		*/
>  	__u32		bs_dmevmask;	/* DMIG event mask		*/
>  	__u16		bs_dmstate;	/* DMIG state info		*/
>  	__u16		bs_aextents;	/* attribute number of extents	*/
>  } xfs_bstat_t;
>  
> +/* bs_sick flags */
> +#define XFS_BS_SICK_INODE	(1 << 0)  /* inode core */
> +#define XFS_BS_SICK_BMBTD	(1 << 1)  /* data fork */
> +#define XFS_BS_SICK_BMBTA	(1 << 2)  /* attr fork */
> +#define XFS_BS_SICK_BMBTC	(1 << 3)  /* cow fork */
> +#define XFS_BS_SICK_DIR		(1 << 4)  /* directory */
> +#define XFS_BS_SICK_XATTR	(1 << 5)  /* extended attributes */
> +#define XFS_BS_SICK_SYMLINK	(1 << 6)  /* symbolic link remote target */
> +#define XFS_BS_SICK_PARENT	(1 << 7)  /* parent pointers */
> +
>  /*
>   * Project quota id helpers (previously projid was 16bit only
>   * and using two 16bit values to hold new 32bit projid was choosen
> diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
> index adf9d41843a5..4979bf1f9bcb 100644
> --- a/fs/xfs/libxfs/xfs_health.h
> +++ b/fs/xfs/libxfs/xfs_health.h
> @@ -178,5 +178,6 @@ xfs_inode_is_healthy(struct xfs_inode *ip)
>  void xfs_fsop_geom_health(struct xfs_mount *mp, struct xfs_fsop_geom *geo);
>  void xfs_ag_geom_health(struct xfs_mount *mp, xfs_agnumber_t agno,
>  		struct xfs_ag_geometry *ageo);
> +void xfs_bulkstat_health(struct xfs_inode *ip, struct xfs_bstat *bs);
>  
>  #endif	/* __XFS_HEALTH_H__ */
> diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
> index 0315b95a8d84..c40b8d9ef6bd 100644
> --- a/fs/xfs/xfs_health.c
> +++ b/fs/xfs/xfs_health.c
> @@ -364,3 +364,37 @@ xfs_ag_geom_health(
>  
>  	xfs_perag_put(pag);
>  }
> +
> +static const struct ioctl_sick_map ino_map[] = {
> +	{ XFS_SICK_INO_CORE,	XFS_BS_SICK_INODE },
> +	{ XFS_SICK_INO_BMBTD,	XFS_BS_SICK_BMBTD },
> +	{ XFS_SICK_INO_BMBTA,	XFS_BS_SICK_BMBTA },
> +	{ XFS_SICK_INO_BMBTC,	XFS_BS_SICK_BMBTC },
> +	{ XFS_SICK_INO_DIR,	XFS_BS_SICK_DIR },
> +	{ XFS_SICK_INO_XATTR,	XFS_BS_SICK_XATTR },
> +	{ XFS_SICK_INO_SYMLINK,	XFS_BS_SICK_SYMLINK },
> +	{ XFS_SICK_INO_PARENT,	XFS_BS_SICK_PARENT },
> +	{ 0, 0 },
> +};
> +
> +/* Fill out bulkstat health info. */
> +void
> +xfs_bulkstat_health(
> +	struct xfs_inode	*ip,
> +	struct xfs_bstat	*bs)
> +{
> +	const struct ioctl_sick_map *m;
> +	unsigned int		sick;
> +	unsigned int		checked;
> +
> +	bs->bs_sick = 0;
> +	bs->bs_checked = 0;
> +
> +	xfs_inode_measure_sickness(ip, &sick, &checked);
> +	for (m = ino_map; m->sick_mask; m++) {
> +		if (checked & m->sick_mask)
> +			bs->bs_checked |= m->ioctl_mask;
> +		if (sick & m->sick_mask)
> +			bs->bs_sick |= m->ioctl_mask;
> +	}
> +}
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index 942e4aa5e729..1e1a0af1dd34 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -18,6 +18,7 @@
>  #include "xfs_error.h"
>  #include "xfs_trace.h"
>  #include "xfs_icache.h"
> +#include "xfs_health.h"
>  
>  /*
>   * Return stat information for one inode.
> @@ -84,6 +85,7 @@ xfs_bulkstat_one_int(
>  	buf->bs_extsize = dic->di_extsize << mp->m_sb.sb_blocklog;
>  	buf->bs_extents = dic->di_nextents;
>  	memset(buf->bs_pad, 0, sizeof(buf->bs_pad));
> +	xfs_bulkstat_health(ip, buf);
>  	buf->bs_dmevmask = dic->di_dmevmask;
>  	buf->bs_dmstate = dic->di_dmstate;
>  	buf->bs_aextents = dic->di_anextents;
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/8] xfs: track metadata health status
  2019-04-11 12:29   ` Brian Foster
@ 2019-04-11 15:18     ` Darrick J. Wong
  2019-04-11 16:05       ` Brian Foster
  0 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11 15:18 UTC (permalink / raw)
  To: Brian Foster; +Cc: linux-xfs

On Thu, Apr 11, 2019 at 08:29:04AM -0400, Brian Foster wrote:
> On Wed, Apr 10, 2019 at 06:45:32PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Add the necessary in-core metadata fields to keep track of which parts
> > of the filesystem have been observed and which parts were observed to be
> > unhealthy, and print a warning at unmount time if we have unfixed
> > problems.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/Makefile            |    1 
> >  fs/xfs/libxfs/xfs_health.h |  175 ++++++++++++++++++++++++++++++++++++++++
> >  fs/xfs/xfs_health.c        |  192 ++++++++++++++++++++++++++++++++++++++++++++
> >  fs/xfs/xfs_icache.c        |    8 ++
> >  fs/xfs/xfs_inode.h         |    8 ++
> >  fs/xfs/xfs_mount.c         |    1 
> >  fs/xfs/xfs_mount.h         |   23 +++++
> >  fs/xfs/xfs_trace.h         |   73 +++++++++++++++++
> >  8 files changed, 481 insertions(+)
> >  create mode 100644 fs/xfs/libxfs/xfs_health.h
> >  create mode 100644 fs/xfs/xfs_health.c
> > 
> > 
> ...
> > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> > index e70e7db29026..885decab4735 100644
> > --- a/fs/xfs/xfs_icache.c
> > +++ b/fs/xfs/xfs_icache.c
> > @@ -73,6 +73,8 @@ xfs_inode_alloc(
> >  	INIT_WORK(&ip->i_iodone_work, xfs_end_io);
> >  	INIT_LIST_HEAD(&ip->i_iodone_list);
> >  	spin_lock_init(&ip->i_iodone_lock);
> > +	ip->i_sick = 0;
> > +	ip->i_checked = 0;
> >  
> >  	return ip;
> >  }
> > @@ -133,6 +135,8 @@ xfs_inode_free(
> >  	spin_lock(&ip->i_flags_lock);
> >  	ip->i_flags = XFS_IRECLAIM;
> >  	ip->i_ino = 0;
> > +	ip->i_sick = 0;
> > +	ip->i_checked = 0;
> >  	spin_unlock(&ip->i_flags_lock);
> >  
> 
> FWIW, I'm not totally clear on what the i_checked mask is for yet.

Bleh, I forgot to update the introductory comment. :(

/*
 * <introductory stuff that's in xfs_health.h now>
 *
 * Each health tracking group uses a pair of fields for reporting.  The
 * "checked" field tell us if a given piece of metadata has ever been examined,
 * and the "sick" field tells us if that piece was found to need repairs.
 * Therefore we can conclude that for a given mask:
 *
 *  - checked && sick  => metadata needs repair
 *  - checked && !sick => metadata is ok
 *  - !checked         => has not been examined since mount
 */

In any case, I worked out the need for this new checked field when I was
writing the manual pages describing how all this worked:

https://djwong.org/docs/man/ioctl_xfs_fsop_geometry.2.html
https://djwong.org/docs/man/ioctl_xfs_ag_geometry.2.html
https://djwong.org/docs/man/ioctl_xfs_fsbulkstat.2.html

(See the part "The fields sick and checked indicate...")

@checked is a mask of all the metadata types that scrub has looked at,
whether or not the metadata was any good.  @sick is the mask of all the
metadata that scrub thought was bad, so we now can report to userspace
if something's good, bad, or unchecked.

> That aside, is it necessary to reset these fields in the free/reclaim
> paths?  I wonder if it's sufficient to zero them on alloc and the
> cache hit path just below..?

I think it's not strictly needed, but once we've broken the association
between a (struct xfs_inode *) buffer and a particular inode number, we
ought to zero out the health data just in case that buffer resurfaces
during the rcu grace period.

--D

> Otherwise looks fine:
> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> 
> >  	__xfs_inode_free(ip);
> > @@ -449,6 +453,8 @@ xfs_iget_cache_hit(
> >  		ip->i_flags |= XFS_INEW;
> >  		xfs_inode_clear_reclaim_tag(pag, ip->i_ino);
> >  		inode->i_state = I_NEW;
> > +		ip->i_sick = 0;
> > +		ip->i_checked = 0;
> >  
> >  		ASSERT(!rwsem_is_locked(&inode->i_rwsem));
> >  		init_rwsem(&inode->i_rwsem);
> > @@ -1177,6 +1183,8 @@ xfs_reclaim_inode(
> >  	spin_lock(&ip->i_flags_lock);
> >  	ip->i_flags = XFS_IRECLAIM;
> >  	ip->i_ino = 0;
> > +	ip->i_sick = 0;
> > +	ip->i_checked = 0;
> >  	spin_unlock(&ip->i_flags_lock);
> >  
> >  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> > index 88239c2dd824..494e47ef42cb 100644
> > --- a/fs/xfs/xfs_inode.h
> > +++ b/fs/xfs/xfs_inode.h
> > @@ -45,6 +45,14 @@ typedef struct xfs_inode {
> >  	mrlock_t		i_lock;		/* inode lock */
> >  	mrlock_t		i_mmaplock;	/* inode mmap IO lock */
> >  	atomic_t		i_pincount;	/* inode pin count */
> > +
> > +	/*
> > +	 * Bitsets of inode metadata that have been checked and/or are sick.
> > +	 * Callers must hold i_flags_lock before accessing this field.
> > +	 */
> > +	uint16_t		i_checked;
> > +	uint16_t		i_sick;
> > +
> >  	spinlock_t		i_flags_lock;	/* inode i_flags lock */
> >  	/* Miscellaneous state. */
> >  	unsigned long		i_flags;	/* see defined flags below */
> > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> > index fd63b0b1307c..6581381c12be 100644
> > --- a/fs/xfs/xfs_mount.c
> > +++ b/fs/xfs/xfs_mount.c
> > @@ -231,6 +231,7 @@ xfs_initialize_perag(
> >  		error = xfs_iunlink_init(pag);
> >  		if (error)
> >  			goto out_hash_destroy;
> > +		spin_lock_init(&pag->pag_state_lock);
> >  	}
> >  
> >  	index = xfs_set_inode_alloc(mp, agcount);
> > diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> > index 110f927cf943..cf7facc36a5f 100644
> > --- a/fs/xfs/xfs_mount.h
> > +++ b/fs/xfs/xfs_mount.h
> > @@ -60,6 +60,20 @@ struct xfs_error_cfg {
> >  typedef struct xfs_mount {
> >  	struct super_block	*m_super;
> >  	xfs_tid_t		m_tid;		/* next unused tid for fs */
> > +
> > +	/*
> > +	 * Bitsets of per-fs metadata that have been checked and/or are sick.
> > +	 * Callers must hold m_sb_lock to access these two fields.
> > +	 */
> > +	uint8_t			m_fs_checked;
> > +	uint8_t			m_fs_sick;
> > +	/*
> > +	 * Bitsets of rt metadata that have been checked and/or are sick.
> > +	 * Callers must hold m_sb_lock to access this field.
> > +	 */
> > +	uint8_t			m_rt_checked;
> > +	uint8_t			m_rt_sick;
> > +
> >  	struct xfs_ail		*m_ail;		/* fs active log item list */
> >  
> >  	struct xfs_sb		m_sb;		/* copy of fs superblock */
> > @@ -369,6 +383,15 @@ typedef struct xfs_perag {
> >  	xfs_agino_t	pagl_pagino;
> >  	xfs_agino_t	pagl_leftrec;
> >  	xfs_agino_t	pagl_rightrec;
> > +
> > +	/*
> > +	 * Bitsets of per-ag metadata that have been checked and/or are sick.
> > +	 * Callers should hold pag_state_lock before accessing this field.
> > +	 */
> > +	uint16_t	pag_checked;
> > +	uint16_t	pag_sick;
> > +	spinlock_t	pag_state_lock;
> > +
> >  	spinlock_t	pagb_lock;	/* lock for pagb_tree */
> >  	struct rb_root	pagb_tree;	/* ordered tree of busy extents */
> >  	unsigned int	pagb_gen;	/* generation count for pagb_tree */
> > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> > index 47fb07d86efd..f079841c7af6 100644
> > --- a/fs/xfs/xfs_trace.h
> > +++ b/fs/xfs/xfs_trace.h
> > @@ -3440,6 +3440,79 @@ DEFINE_AGINODE_EVENT(xfs_iunlink);
> >  DEFINE_AGINODE_EVENT(xfs_iunlink_remove);
> >  DEFINE_AG_EVENT(xfs_iunlink_map_prev_fallback);
> >  
> > +DECLARE_EVENT_CLASS(xfs_fs_corrupt_class,
> > +	TP_PROTO(struct xfs_mount *mp, unsigned int flags),
> > +	TP_ARGS(mp, flags),
> > +	TP_STRUCT__entry(
> > +		__field(dev_t, dev)
> > +		__field(unsigned int, flags)
> > +	),
> > +	TP_fast_assign(
> > +		__entry->dev = mp->m_super->s_dev;
> > +		__entry->flags = flags;
> > +	),
> > +	TP_printk("dev %d:%d flags 0x%x",
> > +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> > +		  __entry->flags)
> > +);
> > +#define DEFINE_FS_CORRUPT_EVENT(name)	\
> > +DEFINE_EVENT(xfs_fs_corrupt_class, name,	\
> > +	TP_PROTO(struct xfs_mount *mp, unsigned int flags), \
> > +	TP_ARGS(mp, flags))
> > +DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_sick);
> > +DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_healthy);
> > +DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_sick);
> > +DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_healthy);
> > +
> > +DECLARE_EVENT_CLASS(xfs_ag_corrupt_class,
> > +	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, unsigned int flags),
> > +	TP_ARGS(mp, agno, flags),
> > +	TP_STRUCT__entry(
> > +		__field(dev_t, dev)
> > +		__field(xfs_agnumber_t, agno)
> > +		__field(unsigned int, flags)
> > +	),
> > +	TP_fast_assign(
> > +		__entry->dev = mp->m_super->s_dev;
> > +		__entry->agno = agno;
> > +		__entry->flags = flags;
> > +	),
> > +	TP_printk("dev %d:%d agno %u flags 0x%x",
> > +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> > +		  __entry->agno, __entry->flags)
> > +);
> > +#define DEFINE_AG_CORRUPT_EVENT(name)	\
> > +DEFINE_EVENT(xfs_ag_corrupt_class, name,	\
> > +	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, \
> > +		 unsigned int flags), \
> > +	TP_ARGS(mp, agno, flags))
> > +DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_sick);
> > +DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_healthy);
> > +
> > +DECLARE_EVENT_CLASS(xfs_inode_corrupt_class,
> > +	TP_PROTO(struct xfs_inode *ip, unsigned int flags),
> > +	TP_ARGS(ip, flags),
> > +	TP_STRUCT__entry(
> > +		__field(dev_t, dev)
> > +		__field(xfs_ino_t, ino)
> > +		__field(unsigned int, flags)
> > +	),
> > +	TP_fast_assign(
> > +		__entry->dev = ip->i_mount->m_super->s_dev;
> > +		__entry->ino = ip->i_ino;
> > +		__entry->flags = flags;
> > +	),
> > +	TP_printk("dev %d:%d ino 0x%llx flags 0x%x",
> > +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> > +		  __entry->ino, __entry->flags)
> > +);
> > +#define DEFINE_INODE_CORRUPT_EVENT(name)	\
> > +DEFINE_EVENT(xfs_inode_corrupt_class, name,	\
> > +	TP_PROTO(struct xfs_inode *ip, unsigned int flags), \
> > +	TP_ARGS(ip, flags))
> > +DEFINE_INODE_CORRUPT_EVENT(xfs_inode_mark_sick);
> > +DEFINE_INODE_CORRUPT_EVENT(xfs_inode_mark_healthy);
> > +
> >  #endif /* _TRACE_XFS_H */
> >  
> >  #undef TRACE_INCLUDE_PATH
> > 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/8] xfs: report fs and rt health via geometry structure
  2019-04-11 13:09   ` Brian Foster
@ 2019-04-11 15:30     ` Darrick J. Wong
  0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11 15:30 UTC (permalink / raw)
  To: Brian Foster; +Cc: linux-xfs

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/8] xfs: report AG health via AG geometry ioctl
  2019-04-11 13:09   ` Brian Foster
@ 2019-04-11 15:33     ` Darrick J. Wong
  0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11 15:33 UTC (permalink / raw)
  To: Brian Foster; +Cc: linux-xfs

On Thu, Apr 11, 2019 at 09:09:57AM -0400, Brian Foster wrote:
> On Wed, Apr 10, 2019 at 06:46:10PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Use the AG geometry info ioctl to report health status too.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/libxfs/xfs_fs.h     |   14 +++++++++++++-
> >  fs/xfs/libxfs/xfs_health.h |    2 ++
> >  fs/xfs/xfs_health.c        |   43 +++++++++++++++++++++++++++++++++++++++++++
> >  fs/xfs/xfs_ioctl.c         |    2 ++
> >  4 files changed, 60 insertions(+), 1 deletion(-)
> > 
> > 
> ...
> > diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
> > index eb8dbc3a952a..0315b95a8d84 100644
> > --- a/fs/xfs/xfs_health.c
> > +++ b/fs/xfs/xfs_health.c
> > @@ -321,3 +321,46 @@ xfs_fsop_geom_health(
> ...
> > +/* Fill out ag geometry health info. */
> > +void
> > +xfs_ag_geom_health(
> > +	struct xfs_mount	*mp,
> > +	xfs_agnumber_t		agno,
> > +	struct xfs_ag_geometry	*ageo)
> > +{
> > +	struct xfs_perag	*pag;
> > +	const struct ioctl_sick_map *m;
> > +	unsigned int		sick;
> > +	unsigned int		checked;
> > +
> > +	ASSERT(agno < mp->m_sb.sb_agcount);
> > +
> > +	ageo->ag_sick = 0;
> > +	ageo->ag_checked = 0;
> > +
> > +	pag = xfs_perag_get(mp, agno);
> > +	xfs_ag_measure_sickness(pag, &sick, &checked);
> > +	for (m = ag_map; m->sick_mask; m++) {
> > +		if (checked & m->sick_mask)
> > +			ageo->ag_checked |= m->ioctl_mask;
> > +		if (sick & m->sick_mask)
> > +			ageo->ag_sick |= m->ioctl_mask;
> > +	}
> 
> FWIW this is pretty much what I was asking for in the mapping helper in
> the previous patch. I didn't realize this was going to be reused for
> another ioctl/struct, however, so disregard my previous comment about
> renaming ->ioctl_mask. ;)
> 
> > +
> > +	xfs_perag_put(pag);
> > +}
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index 3e3ee197bd0f..c47f4201452c 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -822,6 +822,8 @@ xfs_ioc_ag_geometry(
> >  	if (error)
> >  		return error;
> >  
> > +	xfs_ag_geom_health(mp, ageo.ag_number, &ageo);
> > +
> 
> Any reason not to push this down into the get_geom() helper since it's
> part of the initial version of the ag geometry structure and we already
> have the xfs_perag there?

No particularly strong reason not to.  Will fix.

Thanks for reviewing all this stuff!

--D

> Brian
> 
> >  	if (copy_to_user(arg, &ageo, sizeof(ageo)))
> >  		return -EFAULT;
> >  	return 0;
> > 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/8] xfs: track metadata health status
  2019-04-11 15:18     ` Darrick J. Wong
@ 2019-04-11 16:05       ` Brian Foster
  2019-04-11 18:31         ` Darrick J. Wong
  0 siblings, 1 reply; 23+ messages in thread
From: Brian Foster @ 2019-04-11 16:05 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Thu, Apr 11, 2019 at 08:18:45AM -0700, Darrick J. Wong wrote:
> On Thu, Apr 11, 2019 at 08:29:04AM -0400, Brian Foster wrote:
> > On Wed, Apr 10, 2019 at 06:45:32PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > Add the necessary in-core metadata fields to keep track of which parts
> > > of the filesystem have been observed and which parts were observed to be
> > > unhealthy, and print a warning at unmount time if we have unfixed
> > > problems.
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > >  fs/xfs/Makefile            |    1 
> > >  fs/xfs/libxfs/xfs_health.h |  175 ++++++++++++++++++++++++++++++++++++++++
> > >  fs/xfs/xfs_health.c        |  192 ++++++++++++++++++++++++++++++++++++++++++++
> > >  fs/xfs/xfs_icache.c        |    8 ++
> > >  fs/xfs/xfs_inode.h         |    8 ++
> > >  fs/xfs/xfs_mount.c         |    1 
> > >  fs/xfs/xfs_mount.h         |   23 +++++
> > >  fs/xfs/xfs_trace.h         |   73 +++++++++++++++++
> > >  8 files changed, 481 insertions(+)
> > >  create mode 100644 fs/xfs/libxfs/xfs_health.h
> > >  create mode 100644 fs/xfs/xfs_health.c
> > > 
> > > 
> > ...
> > > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> > > index e70e7db29026..885decab4735 100644
> > > --- a/fs/xfs/xfs_icache.c
> > > +++ b/fs/xfs/xfs_icache.c
> > > @@ -73,6 +73,8 @@ xfs_inode_alloc(
> > >  	INIT_WORK(&ip->i_iodone_work, xfs_end_io);
> > >  	INIT_LIST_HEAD(&ip->i_iodone_list);
> > >  	spin_lock_init(&ip->i_iodone_lock);
> > > +	ip->i_sick = 0;
> > > +	ip->i_checked = 0;
> > >  
> > >  	return ip;
> > >  }
> > > @@ -133,6 +135,8 @@ xfs_inode_free(
> > >  	spin_lock(&ip->i_flags_lock);
> > >  	ip->i_flags = XFS_IRECLAIM;
> > >  	ip->i_ino = 0;
> > > +	ip->i_sick = 0;
> > > +	ip->i_checked = 0;
> > >  	spin_unlock(&ip->i_flags_lock);
> > >  
> > 
> > FWIW, I'm not totally clear on what the i_checked mask is for yet.
> 
> Bleh, I forgot to update the introductory comment. :(
> 
> /*
>  * <introductory stuff that's in xfs_health.h now>
>  *
>  * Each health tracking group uses a pair of fields for reporting.  The
>  * "checked" field tell us if a given piece of metadata has ever been examined,
>  * and the "sick" field tells us if that piece was found to need repairs.
>  * Therefore we can conclude that for a given mask:
>  *
>  *  - checked && sick  => metadata needs repair
>  *  - checked && !sick => metadata is ok
>  *  - !checked         => has not been examined since mount
>  */
> 
> In any case, I worked out the need for this new checked field when I was
> writing the manual pages describing how all this worked:
> 
> https://djwong.org/docs/man/ioctl_xfs_fsop_geometry.2.html
> https://djwong.org/docs/man/ioctl_xfs_ag_geometry.2.html
> https://djwong.org/docs/man/ioctl_xfs_fsbulkstat.2.html
> 
> (See the part "The fields sick and checked indicate...")
> 
> @checked is a mask of all the metadata types that scrub has looked at,
> whether or not the metadata was any good.  @sick is the mask of all the
> metadata that scrub thought was bad, so we now can report to userspace
> if something's good, bad, or unchecked.
> 

Ok, thanks.

> > That aside, is it necessary to reset these fields in the free/reclaim
> > paths?  I wonder if it's sufficient to zero them on alloc and the
> > cache hit path just below..?
> 
> I think it's not strictly needed, but once we've broken the association
> between a (struct xfs_inode *) buffer and a particular inode number, we
> ought to zero out the health data just in case that buffer resurfaces
> during the rcu grace period.
> 

I thought freeing the inode was imminent at that point. We set
XFS_IRECLAIM then call into the RCU mechanism to free the memory. If
lookup finds the inode, we retry on XFS_IRECLAIM or attempt to reuse on
XFS_IRECLAIMABLE (which is covered by the fields being reset in
iget_cache_hit()).

Brian

> --D
> 
> > Otherwise looks fine:
> > 
> > Reviewed-by: Brian Foster <bfoster@redhat.com>
> > 
> > >  	__xfs_inode_free(ip);
> > > @@ -449,6 +453,8 @@ xfs_iget_cache_hit(
> > >  		ip->i_flags |= XFS_INEW;
> > >  		xfs_inode_clear_reclaim_tag(pag, ip->i_ino);
> > >  		inode->i_state = I_NEW;
> > > +		ip->i_sick = 0;
> > > +		ip->i_checked = 0;
> > >  
> > >  		ASSERT(!rwsem_is_locked(&inode->i_rwsem));
> > >  		init_rwsem(&inode->i_rwsem);
> > > @@ -1177,6 +1183,8 @@ xfs_reclaim_inode(
> > >  	spin_lock(&ip->i_flags_lock);
> > >  	ip->i_flags = XFS_IRECLAIM;
> > >  	ip->i_ino = 0;
> > > +	ip->i_sick = 0;
> > > +	ip->i_checked = 0;
> > >  	spin_unlock(&ip->i_flags_lock);
> > >  
> > >  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> > > index 88239c2dd824..494e47ef42cb 100644
> > > --- a/fs/xfs/xfs_inode.h
> > > +++ b/fs/xfs/xfs_inode.h
> > > @@ -45,6 +45,14 @@ typedef struct xfs_inode {
> > >  	mrlock_t		i_lock;		/* inode lock */
> > >  	mrlock_t		i_mmaplock;	/* inode mmap IO lock */
> > >  	atomic_t		i_pincount;	/* inode pin count */
> > > +
> > > +	/*
> > > +	 * Bitsets of inode metadata that have been checked and/or are sick.
> > > +	 * Callers must hold i_flags_lock before accessing this field.
> > > +	 */
> > > +	uint16_t		i_checked;
> > > +	uint16_t		i_sick;
> > > +
> > >  	spinlock_t		i_flags_lock;	/* inode i_flags lock */
> > >  	/* Miscellaneous state. */
> > >  	unsigned long		i_flags;	/* see defined flags below */
> > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> > > index fd63b0b1307c..6581381c12be 100644
> > > --- a/fs/xfs/xfs_mount.c
> > > +++ b/fs/xfs/xfs_mount.c
> > > @@ -231,6 +231,7 @@ xfs_initialize_perag(
> > >  		error = xfs_iunlink_init(pag);
> > >  		if (error)
> > >  			goto out_hash_destroy;
> > > +		spin_lock_init(&pag->pag_state_lock);
> > >  	}
> > >  
> > >  	index = xfs_set_inode_alloc(mp, agcount);
> > > diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> > > index 110f927cf943..cf7facc36a5f 100644
> > > --- a/fs/xfs/xfs_mount.h
> > > +++ b/fs/xfs/xfs_mount.h
> > > @@ -60,6 +60,20 @@ struct xfs_error_cfg {
> > >  typedef struct xfs_mount {
> > >  	struct super_block	*m_super;
> > >  	xfs_tid_t		m_tid;		/* next unused tid for fs */
> > > +
> > > +	/*
> > > +	 * Bitsets of per-fs metadata that have been checked and/or are sick.
> > > +	 * Callers must hold m_sb_lock to access these two fields.
> > > +	 */
> > > +	uint8_t			m_fs_checked;
> > > +	uint8_t			m_fs_sick;
> > > +	/*
> > > +	 * Bitsets of rt metadata that have been checked and/or are sick.
> > > +	 * Callers must hold m_sb_lock to access this field.
> > > +	 */
> > > +	uint8_t			m_rt_checked;
> > > +	uint8_t			m_rt_sick;
> > > +
> > >  	struct xfs_ail		*m_ail;		/* fs active log item list */
> > >  
> > >  	struct xfs_sb		m_sb;		/* copy of fs superblock */
> > > @@ -369,6 +383,15 @@ typedef struct xfs_perag {
> > >  	xfs_agino_t	pagl_pagino;
> > >  	xfs_agino_t	pagl_leftrec;
> > >  	xfs_agino_t	pagl_rightrec;
> > > +
> > > +	/*
> > > +	 * Bitsets of per-ag metadata that have been checked and/or are sick.
> > > +	 * Callers should hold pag_state_lock before accessing this field.
> > > +	 */
> > > +	uint16_t	pag_checked;
> > > +	uint16_t	pag_sick;
> > > +	spinlock_t	pag_state_lock;
> > > +
> > >  	spinlock_t	pagb_lock;	/* lock for pagb_tree */
> > >  	struct rb_root	pagb_tree;	/* ordered tree of busy extents */
> > >  	unsigned int	pagb_gen;	/* generation count for pagb_tree */
> > > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> > > index 47fb07d86efd..f079841c7af6 100644
> > > --- a/fs/xfs/xfs_trace.h
> > > +++ b/fs/xfs/xfs_trace.h
> > > @@ -3440,6 +3440,79 @@ DEFINE_AGINODE_EVENT(xfs_iunlink);
> > >  DEFINE_AGINODE_EVENT(xfs_iunlink_remove);
> > >  DEFINE_AG_EVENT(xfs_iunlink_map_prev_fallback);
> > >  
> > > +DECLARE_EVENT_CLASS(xfs_fs_corrupt_class,
> > > +	TP_PROTO(struct xfs_mount *mp, unsigned int flags),
> > > +	TP_ARGS(mp, flags),
> > > +	TP_STRUCT__entry(
> > > +		__field(dev_t, dev)
> > > +		__field(unsigned int, flags)
> > > +	),
> > > +	TP_fast_assign(
> > > +		__entry->dev = mp->m_super->s_dev;
> > > +		__entry->flags = flags;
> > > +	),
> > > +	TP_printk("dev %d:%d flags 0x%x",
> > > +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> > > +		  __entry->flags)
> > > +);
> > > +#define DEFINE_FS_CORRUPT_EVENT(name)	\
> > > +DEFINE_EVENT(xfs_fs_corrupt_class, name,	\
> > > +	TP_PROTO(struct xfs_mount *mp, unsigned int flags), \
> > > +	TP_ARGS(mp, flags))
> > > +DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_sick);
> > > +DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_healthy);
> > > +DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_sick);
> > > +DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_healthy);
> > > +
> > > +DECLARE_EVENT_CLASS(xfs_ag_corrupt_class,
> > > +	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, unsigned int flags),
> > > +	TP_ARGS(mp, agno, flags),
> > > +	TP_STRUCT__entry(
> > > +		__field(dev_t, dev)
> > > +		__field(xfs_agnumber_t, agno)
> > > +		__field(unsigned int, flags)
> > > +	),
> > > +	TP_fast_assign(
> > > +		__entry->dev = mp->m_super->s_dev;
> > > +		__entry->agno = agno;
> > > +		__entry->flags = flags;
> > > +	),
> > > +	TP_printk("dev %d:%d agno %u flags 0x%x",
> > > +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> > > +		  __entry->agno, __entry->flags)
> > > +);
> > > +#define DEFINE_AG_CORRUPT_EVENT(name)	\
> > > +DEFINE_EVENT(xfs_ag_corrupt_class, name,	\
> > > +	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, \
> > > +		 unsigned int flags), \
> > > +	TP_ARGS(mp, agno, flags))
> > > +DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_sick);
> > > +DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_healthy);
> > > +
> > > +DECLARE_EVENT_CLASS(xfs_inode_corrupt_class,
> > > +	TP_PROTO(struct xfs_inode *ip, unsigned int flags),
> > > +	TP_ARGS(ip, flags),
> > > +	TP_STRUCT__entry(
> > > +		__field(dev_t, dev)
> > > +		__field(xfs_ino_t, ino)
> > > +		__field(unsigned int, flags)
> > > +	),
> > > +	TP_fast_assign(
> > > +		__entry->dev = ip->i_mount->m_super->s_dev;
> > > +		__entry->ino = ip->i_ino;
> > > +		__entry->flags = flags;
> > > +	),
> > > +	TP_printk("dev %d:%d ino 0x%llx flags 0x%x",
> > > +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> > > +		  __entry->ino, __entry->flags)
> > > +);
> > > +#define DEFINE_INODE_CORRUPT_EVENT(name)	\
> > > +DEFINE_EVENT(xfs_inode_corrupt_class, name,	\
> > > +	TP_PROTO(struct xfs_inode *ip, unsigned int flags), \
> > > +	TP_ARGS(ip, flags))
> > > +DEFINE_INODE_CORRUPT_EVENT(xfs_inode_mark_sick);
> > > +DEFINE_INODE_CORRUPT_EVENT(xfs_inode_mark_healthy);
> > > +
> > >  #endif /* _TRACE_XFS_H */
> > >  
> > >  #undef TRACE_INCLUDE_PATH
> > > 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/8] xfs: track metadata health status
  2019-04-11 16:05       ` Brian Foster
@ 2019-04-11 18:31         ` Darrick J. Wong
  0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-11 18:31 UTC (permalink / raw)
  To: Brian Foster; +Cc: linux-xfs

On Thu, Apr 11, 2019 at 12:05:30PM -0400, Brian Foster wrote:
> On Thu, Apr 11, 2019 at 08:18:45AM -0700, Darrick J. Wong wrote:
> > On Thu, Apr 11, 2019 at 08:29:04AM -0400, Brian Foster wrote:
> > > On Wed, Apr 10, 2019 at 06:45:32PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > > 
> > > > Add the necessary in-core metadata fields to keep track of which parts
> > > > of the filesystem have been observed and which parts were observed to be
> > > > unhealthy, and print a warning at unmount time if we have unfixed
> > > > problems.
> > > > 
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > ---
> > > >  fs/xfs/Makefile            |    1 
> > > >  fs/xfs/libxfs/xfs_health.h |  175 ++++++++++++++++++++++++++++++++++++++++
> > > >  fs/xfs/xfs_health.c        |  192 ++++++++++++++++++++++++++++++++++++++++++++
> > > >  fs/xfs/xfs_icache.c        |    8 ++
> > > >  fs/xfs/xfs_inode.h         |    8 ++
> > > >  fs/xfs/xfs_mount.c         |    1 
> > > >  fs/xfs/xfs_mount.h         |   23 +++++
> > > >  fs/xfs/xfs_trace.h         |   73 +++++++++++++++++
> > > >  8 files changed, 481 insertions(+)
> > > >  create mode 100644 fs/xfs/libxfs/xfs_health.h
> > > >  create mode 100644 fs/xfs/xfs_health.c
> > > > 
> > > > 
> > > ...
> > > > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> > > > index e70e7db29026..885decab4735 100644
> > > > --- a/fs/xfs/xfs_icache.c
> > > > +++ b/fs/xfs/xfs_icache.c
> > > > @@ -73,6 +73,8 @@ xfs_inode_alloc(
> > > >  	INIT_WORK(&ip->i_iodone_work, xfs_end_io);
> > > >  	INIT_LIST_HEAD(&ip->i_iodone_list);
> > > >  	spin_lock_init(&ip->i_iodone_lock);
> > > > +	ip->i_sick = 0;
> > > > +	ip->i_checked = 0;
> > > >  
> > > >  	return ip;
> > > >  }
> > > > @@ -133,6 +135,8 @@ xfs_inode_free(
> > > >  	spin_lock(&ip->i_flags_lock);
> > > >  	ip->i_flags = XFS_IRECLAIM;
> > > >  	ip->i_ino = 0;
> > > > +	ip->i_sick = 0;
> > > > +	ip->i_checked = 0;
> > > >  	spin_unlock(&ip->i_flags_lock);
> > > >  
> > > 
> > > FWIW, I'm not totally clear on what the i_checked mask is for yet.
> > 
> > Bleh, I forgot to update the introductory comment. :(
> > 
> > /*
> >  * <introductory stuff that's in xfs_health.h now>
> >  *
> >  * Each health tracking group uses a pair of fields for reporting.  The
> >  * "checked" field tell us if a given piece of metadata has ever been examined,
> >  * and the "sick" field tells us if that piece was found to need repairs.
> >  * Therefore we can conclude that for a given mask:
> >  *
> >  *  - checked && sick  => metadata needs repair
> >  *  - checked && !sick => metadata is ok
> >  *  - !checked         => has not been examined since mount
> >  */
> > 
> > In any case, I worked out the need for this new checked field when I was
> > writing the manual pages describing how all this worked:
> > 
> > https://djwong.org/docs/man/ioctl_xfs_fsop_geometry.2.html
> > https://djwong.org/docs/man/ioctl_xfs_ag_geometry.2.html
> > https://djwong.org/docs/man/ioctl_xfs_fsbulkstat.2.html
> > 
> > (See the part "The fields sick and checked indicate...")
> > 
> > @checked is a mask of all the metadata types that scrub has looked at,
> > whether or not the metadata was any good.  @sick is the mask of all the
> > metadata that scrub thought was bad, so we now can report to userspace
> > if something's good, bad, or unchecked.
> > 
> 
> Ok, thanks.
> 
> > > That aside, is it necessary to reset these fields in the free/reclaim
> > > paths?  I wonder if it's sufficient to zero them on alloc and the
> > > cache hit path just below..?
> > 
> > I think it's not strictly needed, but once we've broken the association
> > between a (struct xfs_inode *) buffer and a particular inode number, we
> > ought to zero out the health data just in case that buffer resurfaces
> > during the rcu grace period.
> > 
> 
> I thought freeing the inode was imminent at that point. We set
> XFS_IRECLAIM then call into the RCU mechanism to free the memory. If
> lookup finds the inode, we retry on XFS_IRECLAIM or attempt to reuse on
> XFS_IRECLAIMABLE (which is covered by the fields being reset in
> iget_cache_hit()).

<nod> The i_ino change effectively prevents anyone else from seeing
stale sick/checked contents, so I might as well drop this for v3.

--D

> Brian
> 
> > --D
> > 
> > > Otherwise looks fine:
> > > 
> > > Reviewed-by: Brian Foster <bfoster@redhat.com>
> > > 
> > > >  	__xfs_inode_free(ip);
> > > > @@ -449,6 +453,8 @@ xfs_iget_cache_hit(
> > > >  		ip->i_flags |= XFS_INEW;
> > > >  		xfs_inode_clear_reclaim_tag(pag, ip->i_ino);
> > > >  		inode->i_state = I_NEW;
> > > > +		ip->i_sick = 0;
> > > > +		ip->i_checked = 0;
> > > >  
> > > >  		ASSERT(!rwsem_is_locked(&inode->i_rwsem));
> > > >  		init_rwsem(&inode->i_rwsem);
> > > > @@ -1177,6 +1183,8 @@ xfs_reclaim_inode(
> > > >  	spin_lock(&ip->i_flags_lock);
> > > >  	ip->i_flags = XFS_IRECLAIM;
> > > >  	ip->i_ino = 0;
> > > > +	ip->i_sick = 0;
> > > > +	ip->i_checked = 0;
> > > >  	spin_unlock(&ip->i_flags_lock);
> > > >  
> > > >  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > > > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> > > > index 88239c2dd824..494e47ef42cb 100644
> > > > --- a/fs/xfs/xfs_inode.h
> > > > +++ b/fs/xfs/xfs_inode.h
> > > > @@ -45,6 +45,14 @@ typedef struct xfs_inode {
> > > >  	mrlock_t		i_lock;		/* inode lock */
> > > >  	mrlock_t		i_mmaplock;	/* inode mmap IO lock */
> > > >  	atomic_t		i_pincount;	/* inode pin count */
> > > > +
> > > > +	/*
> > > > +	 * Bitsets of inode metadata that have been checked and/or are sick.
> > > > +	 * Callers must hold i_flags_lock before accessing this field.
> > > > +	 */
> > > > +	uint16_t		i_checked;
> > > > +	uint16_t		i_sick;
> > > > +
> > > >  	spinlock_t		i_flags_lock;	/* inode i_flags lock */
> > > >  	/* Miscellaneous state. */
> > > >  	unsigned long		i_flags;	/* see defined flags below */
> > > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> > > > index fd63b0b1307c..6581381c12be 100644
> > > > --- a/fs/xfs/xfs_mount.c
> > > > +++ b/fs/xfs/xfs_mount.c
> > > > @@ -231,6 +231,7 @@ xfs_initialize_perag(
> > > >  		error = xfs_iunlink_init(pag);
> > > >  		if (error)
> > > >  			goto out_hash_destroy;
> > > > +		spin_lock_init(&pag->pag_state_lock);
> > > >  	}
> > > >  
> > > >  	index = xfs_set_inode_alloc(mp, agcount);
> > > > diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> > > > index 110f927cf943..cf7facc36a5f 100644
> > > > --- a/fs/xfs/xfs_mount.h
> > > > +++ b/fs/xfs/xfs_mount.h
> > > > @@ -60,6 +60,20 @@ struct xfs_error_cfg {
> > > >  typedef struct xfs_mount {
> > > >  	struct super_block	*m_super;
> > > >  	xfs_tid_t		m_tid;		/* next unused tid for fs */
> > > > +
> > > > +	/*
> > > > +	 * Bitsets of per-fs metadata that have been checked and/or are sick.
> > > > +	 * Callers must hold m_sb_lock to access these two fields.
> > > > +	 */
> > > > +	uint8_t			m_fs_checked;
> > > > +	uint8_t			m_fs_sick;
> > > > +	/*
> > > > +	 * Bitsets of rt metadata that have been checked and/or are sick.
> > > > +	 * Callers must hold m_sb_lock to access this field.
> > > > +	 */
> > > > +	uint8_t			m_rt_checked;
> > > > +	uint8_t			m_rt_sick;
> > > > +
> > > >  	struct xfs_ail		*m_ail;		/* fs active log item list */
> > > >  
> > > >  	struct xfs_sb		m_sb;		/* copy of fs superblock */
> > > > @@ -369,6 +383,15 @@ typedef struct xfs_perag {
> > > >  	xfs_agino_t	pagl_pagino;
> > > >  	xfs_agino_t	pagl_leftrec;
> > > >  	xfs_agino_t	pagl_rightrec;
> > > > +
> > > > +	/*
> > > > +	 * Bitsets of per-ag metadata that have been checked and/or are sick.
> > > > +	 * Callers should hold pag_state_lock before accessing this field.
> > > > +	 */
> > > > +	uint16_t	pag_checked;
> > > > +	uint16_t	pag_sick;
> > > > +	spinlock_t	pag_state_lock;
> > > > +
> > > >  	spinlock_t	pagb_lock;	/* lock for pagb_tree */
> > > >  	struct rb_root	pagb_tree;	/* ordered tree of busy extents */
> > > >  	unsigned int	pagb_gen;	/* generation count for pagb_tree */
> > > > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> > > > index 47fb07d86efd..f079841c7af6 100644
> > > > --- a/fs/xfs/xfs_trace.h
> > > > +++ b/fs/xfs/xfs_trace.h
> > > > @@ -3440,6 +3440,79 @@ DEFINE_AGINODE_EVENT(xfs_iunlink);
> > > >  DEFINE_AGINODE_EVENT(xfs_iunlink_remove);
> > > >  DEFINE_AG_EVENT(xfs_iunlink_map_prev_fallback);
> > > >  
> > > > +DECLARE_EVENT_CLASS(xfs_fs_corrupt_class,
> > > > +	TP_PROTO(struct xfs_mount *mp, unsigned int flags),
> > > > +	TP_ARGS(mp, flags),
> > > > +	TP_STRUCT__entry(
> > > > +		__field(dev_t, dev)
> > > > +		__field(unsigned int, flags)
> > > > +	),
> > > > +	TP_fast_assign(
> > > > +		__entry->dev = mp->m_super->s_dev;
> > > > +		__entry->flags = flags;
> > > > +	),
> > > > +	TP_printk("dev %d:%d flags 0x%x",
> > > > +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> > > > +		  __entry->flags)
> > > > +);
> > > > +#define DEFINE_FS_CORRUPT_EVENT(name)	\
> > > > +DEFINE_EVENT(xfs_fs_corrupt_class, name,	\
> > > > +	TP_PROTO(struct xfs_mount *mp, unsigned int flags), \
> > > > +	TP_ARGS(mp, flags))
> > > > +DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_sick);
> > > > +DEFINE_FS_CORRUPT_EVENT(xfs_fs_mark_healthy);
> > > > +DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_sick);
> > > > +DEFINE_FS_CORRUPT_EVENT(xfs_rt_mark_healthy);
> > > > +
> > > > +DECLARE_EVENT_CLASS(xfs_ag_corrupt_class,
> > > > +	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, unsigned int flags),
> > > > +	TP_ARGS(mp, agno, flags),
> > > > +	TP_STRUCT__entry(
> > > > +		__field(dev_t, dev)
> > > > +		__field(xfs_agnumber_t, agno)
> > > > +		__field(unsigned int, flags)
> > > > +	),
> > > > +	TP_fast_assign(
> > > > +		__entry->dev = mp->m_super->s_dev;
> > > > +		__entry->agno = agno;
> > > > +		__entry->flags = flags;
> > > > +	),
> > > > +	TP_printk("dev %d:%d agno %u flags 0x%x",
> > > > +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> > > > +		  __entry->agno, __entry->flags)
> > > > +);
> > > > +#define DEFINE_AG_CORRUPT_EVENT(name)	\
> > > > +DEFINE_EVENT(xfs_ag_corrupt_class, name,	\
> > > > +	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, \
> > > > +		 unsigned int flags), \
> > > > +	TP_ARGS(mp, agno, flags))
> > > > +DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_sick);
> > > > +DEFINE_AG_CORRUPT_EVENT(xfs_ag_mark_healthy);
> > > > +
> > > > +DECLARE_EVENT_CLASS(xfs_inode_corrupt_class,
> > > > +	TP_PROTO(struct xfs_inode *ip, unsigned int flags),
> > > > +	TP_ARGS(ip, flags),
> > > > +	TP_STRUCT__entry(
> > > > +		__field(dev_t, dev)
> > > > +		__field(xfs_ino_t, ino)
> > > > +		__field(unsigned int, flags)
> > > > +	),
> > > > +	TP_fast_assign(
> > > > +		__entry->dev = ip->i_mount->m_super->s_dev;
> > > > +		__entry->ino = ip->i_ino;
> > > > +		__entry->flags = flags;
> > > > +	),
> > > > +	TP_printk("dev %d:%d ino 0x%llx flags 0x%x",
> > > > +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> > > > +		  __entry->ino, __entry->flags)
> > > > +);
> > > > +#define DEFINE_INODE_CORRUPT_EVENT(name)	\
> > > > +DEFINE_EVENT(xfs_inode_corrupt_class, name,	\
> > > > +	TP_PROTO(struct xfs_inode *ip, unsigned int flags), \
> > > > +	TP_ARGS(ip, flags))
> > > > +DEFINE_INODE_CORRUPT_EVENT(xfs_inode_mark_sick);
> > > > +DEFINE_INODE_CORRUPT_EVENT(xfs_inode_mark_healthy);
> > > > +
> > > >  #endif /* _TRACE_XFS_H */
> > > >  
> > > >  #undef TRACE_INCLUDE_PATH
> > > > 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/8] xfs: report AG health via AG geometry ioctl
  2019-04-12  6:28 ` [PATCH 7/8] xfs: report AG health via AG geometry ioctl Darrick J. Wong
@ 2019-04-12 11:14   ` Brian Foster
  0 siblings, 0 replies; 23+ messages in thread
From: Brian Foster @ 2019-04-12 11:14 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Thu, Apr 11, 2019 at 11:28:47PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use the AG geometry info ioctl to report health status too.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_ag.c     |    2 ++
>  fs/xfs/libxfs/xfs_fs.h     |   14 +++++++++++++-
>  fs/xfs/libxfs/xfs_health.h |    1 +
>  fs/xfs/xfs_health.c        |   36 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 52 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index 1c0f2a6c10b4..b0c89f54d1bb 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -20,6 +20,7 @@
>  #include "xfs_rmap.h"
>  #include "xfs_ag.h"
>  #include "xfs_ag_resv.h"
> +#include "xfs_health.h"
>  
>  static struct xfs_buf *
>  xfs_get_aghdr_buf(
> @@ -505,6 +506,7 @@ xfs_ag_get_geometry(
>  		   pag->pagf_btreeblks -
>  		   xfs_ag_resv_needed(pag, XFS_AG_RESV_NONE);
>  	ageo->ag_freeblks = freeblks;
> +	xfs_ag_geom_health(pag, ageo);
>  
>  	/* Release resources. */
>  	xfs_perag_put(pag);
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index 6b8956dbf49d..35d60f8017cd 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -285,9 +285,21 @@ struct xfs_ag_geometry {
>  	uint32_t	ag_freeblks;	/* o: free space */
>  	uint32_t	ag_icount;	/* o: inodes allocated */
>  	uint32_t	ag_ifree;	/* o: inodes free */
> +	uint32_t	ag_sick;	/* o: sick things in ag */
> +	uint32_t	ag_checked;	/* o: checked metadata in ag */
>  	uint32_t	ag_reserved32;	/* o: zero */
> -	uint64_t	ag_reserved[13];/* o: zero */
> +	uint64_t	ag_reserved[12];/* o: zero */
>  };
> +#define XFS_AG_GEOM_SICK_SB	(1 << 0)  /* superblock */
> +#define XFS_AG_GEOM_SICK_AGF	(1 << 1)  /* AGF header */
> +#define XFS_AG_GEOM_SICK_AGFL	(1 << 2)  /* AGFL header */
> +#define XFS_AG_GEOM_SICK_AGI	(1 << 3)  /* AGI header */
> +#define XFS_AG_GEOM_SICK_BNOBT	(1 << 4)  /* free space by block */
> +#define XFS_AG_GEOM_SICK_CNTBT	(1 << 5)  /* free space by length */
> +#define XFS_AG_GEOM_SICK_INOBT	(1 << 6)  /* inode index */
> +#define XFS_AG_GEOM_SICK_FINOBT	(1 << 7)  /* free inode index */
> +#define XFS_AG_GEOM_SICK_RMAPBT	(1 << 8)  /* reverse mappings */
> +#define XFS_AG_GEOM_SICK_REFCNTBT (1 << 9)  /* reference counts */
>  
>  /*
>   * Structures for XFS_IOC_FSGROWFSDATA, XFS_IOC_FSGROWFSLOG & XFS_IOC_FSGROWFSRT
> diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
> index 3fffdcc80970..e392457023a4 100644
> --- a/fs/xfs/libxfs/xfs_health.h
> +++ b/fs/xfs/libxfs/xfs_health.h
> @@ -184,5 +184,6 @@ xfs_inode_is_healthy(struct xfs_inode *ip)
>  }
>  
>  void xfs_fsop_geom_health(struct xfs_mount *mp, struct xfs_fsop_geom *geo);
> +void xfs_ag_geom_health(struct xfs_perag *pag, struct xfs_ag_geometry *ageo);
>  
>  #endif	/* __XFS_HEALTH_H__ */
> diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
> index d137b8f13869..5431c4070f27 100644
> --- a/fs/xfs/xfs_health.c
> +++ b/fs/xfs/xfs_health.c
> @@ -320,3 +320,39 @@ xfs_fsop_geom_health(
>  	for (m = rt_map; m->sick_mask; m++)
>  		xfgeo_health_tick(geo, sick, checked, m);
>  }
> +
> +static const struct ioctl_sick_map ag_map[] = {
> +	{ XFS_SICK_AG_SB,	XFS_AG_GEOM_SICK_SB },
> +	{ XFS_SICK_AG_AGF,	XFS_AG_GEOM_SICK_AGF },
> +	{ XFS_SICK_AG_AGFL,	XFS_AG_GEOM_SICK_AGFL },
> +	{ XFS_SICK_AG_AGI,	XFS_AG_GEOM_SICK_AGI },
> +	{ XFS_SICK_AG_BNOBT,	XFS_AG_GEOM_SICK_BNOBT },
> +	{ XFS_SICK_AG_CNTBT,	XFS_AG_GEOM_SICK_CNTBT },
> +	{ XFS_SICK_AG_INOBT,	XFS_AG_GEOM_SICK_INOBT },
> +	{ XFS_SICK_AG_FINOBT,	XFS_AG_GEOM_SICK_FINOBT },
> +	{ XFS_SICK_AG_RMAPBT,	XFS_AG_GEOM_SICK_RMAPBT },
> +	{ XFS_SICK_AG_REFCNTBT,	XFS_AG_GEOM_SICK_REFCNTBT },
> +	{ 0, 0 },
> +};
> +
> +/* Fill out ag geometry health info. */
> +void
> +xfs_ag_geom_health(
> +	struct xfs_perag		*pag,
> +	struct xfs_ag_geometry		*ageo)
> +{
> +	const struct ioctl_sick_map	*m;
> +	unsigned int			sick;
> +	unsigned int			checked;
> +
> +	ageo->ag_sick = 0;
> +	ageo->ag_checked = 0;
> +
> +	xfs_ag_measure_sickness(pag, &sick, &checked);
> +	for (m = ag_map; m->sick_mask; m++) {
> +		if (checked & m->sick_mask)
> +			ageo->ag_checked |= m->ioctl_mask;
> +		if (sick & m->sick_mask)
> +			ageo->ag_sick |= m->ioctl_mask;
> +	}
> +}
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 7/8] xfs: report AG health via AG geometry ioctl
  2019-04-12  6:28 [PATCH v3 0/8] xfs: online health tracking support Darrick J. Wong
@ 2019-04-12  6:28 ` Darrick J. Wong
  2019-04-12 11:14   ` Brian Foster
  0 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2019-04-12  6:28 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Use the AG geometry info ioctl to report health status too.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_ag.c     |    2 ++
 fs/xfs/libxfs/xfs_fs.h     |   14 +++++++++++++-
 fs/xfs/libxfs/xfs_health.h |    1 +
 fs/xfs/xfs_health.c        |   36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 1c0f2a6c10b4..b0c89f54d1bb 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -20,6 +20,7 @@
 #include "xfs_rmap.h"
 #include "xfs_ag.h"
 #include "xfs_ag_resv.h"
+#include "xfs_health.h"
 
 static struct xfs_buf *
 xfs_get_aghdr_buf(
@@ -505,6 +506,7 @@ xfs_ag_get_geometry(
 		   pag->pagf_btreeblks -
 		   xfs_ag_resv_needed(pag, XFS_AG_RESV_NONE);
 	ageo->ag_freeblks = freeblks;
+	xfs_ag_geom_health(pag, ageo);
 
 	/* Release resources. */
 	xfs_perag_put(pag);
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 6b8956dbf49d..35d60f8017cd 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -285,9 +285,21 @@ struct xfs_ag_geometry {
 	uint32_t	ag_freeblks;	/* o: free space */
 	uint32_t	ag_icount;	/* o: inodes allocated */
 	uint32_t	ag_ifree;	/* o: inodes free */
+	uint32_t	ag_sick;	/* o: sick things in ag */
+	uint32_t	ag_checked;	/* o: checked metadata in ag */
 	uint32_t	ag_reserved32;	/* o: zero */
-	uint64_t	ag_reserved[13];/* o: zero */
+	uint64_t	ag_reserved[12];/* o: zero */
 };
+#define XFS_AG_GEOM_SICK_SB	(1 << 0)  /* superblock */
+#define XFS_AG_GEOM_SICK_AGF	(1 << 1)  /* AGF header */
+#define XFS_AG_GEOM_SICK_AGFL	(1 << 2)  /* AGFL header */
+#define XFS_AG_GEOM_SICK_AGI	(1 << 3)  /* AGI header */
+#define XFS_AG_GEOM_SICK_BNOBT	(1 << 4)  /* free space by block */
+#define XFS_AG_GEOM_SICK_CNTBT	(1 << 5)  /* free space by length */
+#define XFS_AG_GEOM_SICK_INOBT	(1 << 6)  /* inode index */
+#define XFS_AG_GEOM_SICK_FINOBT	(1 << 7)  /* free inode index */
+#define XFS_AG_GEOM_SICK_RMAPBT	(1 << 8)  /* reverse mappings */
+#define XFS_AG_GEOM_SICK_REFCNTBT (1 << 9)  /* reference counts */
 
 /*
  * Structures for XFS_IOC_FSGROWFSDATA, XFS_IOC_FSGROWFSLOG & XFS_IOC_FSGROWFSRT
diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
index 3fffdcc80970..e392457023a4 100644
--- a/fs/xfs/libxfs/xfs_health.h
+++ b/fs/xfs/libxfs/xfs_health.h
@@ -184,5 +184,6 @@ xfs_inode_is_healthy(struct xfs_inode *ip)
 }
 
 void xfs_fsop_geom_health(struct xfs_mount *mp, struct xfs_fsop_geom *geo);
+void xfs_ag_geom_health(struct xfs_perag *pag, struct xfs_ag_geometry *ageo);
 
 #endif	/* __XFS_HEALTH_H__ */
diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
index d137b8f13869..5431c4070f27 100644
--- a/fs/xfs/xfs_health.c
+++ b/fs/xfs/xfs_health.c
@@ -320,3 +320,39 @@ xfs_fsop_geom_health(
 	for (m = rt_map; m->sick_mask; m++)
 		xfgeo_health_tick(geo, sick, checked, m);
 }
+
+static const struct ioctl_sick_map ag_map[] = {
+	{ XFS_SICK_AG_SB,	XFS_AG_GEOM_SICK_SB },
+	{ XFS_SICK_AG_AGF,	XFS_AG_GEOM_SICK_AGF },
+	{ XFS_SICK_AG_AGFL,	XFS_AG_GEOM_SICK_AGFL },
+	{ XFS_SICK_AG_AGI,	XFS_AG_GEOM_SICK_AGI },
+	{ XFS_SICK_AG_BNOBT,	XFS_AG_GEOM_SICK_BNOBT },
+	{ XFS_SICK_AG_CNTBT,	XFS_AG_GEOM_SICK_CNTBT },
+	{ XFS_SICK_AG_INOBT,	XFS_AG_GEOM_SICK_INOBT },
+	{ XFS_SICK_AG_FINOBT,	XFS_AG_GEOM_SICK_FINOBT },
+	{ XFS_SICK_AG_RMAPBT,	XFS_AG_GEOM_SICK_RMAPBT },
+	{ XFS_SICK_AG_REFCNTBT,	XFS_AG_GEOM_SICK_REFCNTBT },
+	{ 0, 0 },
+};
+
+/* Fill out ag geometry health info. */
+void
+xfs_ag_geom_health(
+	struct xfs_perag		*pag,
+	struct xfs_ag_geometry		*ageo)
+{
+	const struct ioctl_sick_map	*m;
+	unsigned int			sick;
+	unsigned int			checked;
+
+	ageo->ag_sick = 0;
+	ageo->ag_checked = 0;
+
+	xfs_ag_measure_sickness(pag, &sick, &checked);
+	for (m = ag_map; m->sick_mask; m++) {
+		if (checked & m->sick_mask)
+			ageo->ag_checked |= m->ioctl_mask;
+		if (sick & m->sick_mask)
+			ageo->ag_sick |= m->ioctl_mask;
+	}
+}

^ permalink raw reply related	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2019-04-12 11:14 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-11  1:45 [PATCH v2 0/8] xfs: online health tracking support Darrick J. Wong
2019-04-11  1:45 ` [PATCH 1/8] xfs: track metadata health status Darrick J. Wong
2019-04-11 12:29   ` Brian Foster
2019-04-11 15:18     ` Darrick J. Wong
2019-04-11 16:05       ` Brian Foster
2019-04-11 18:31         ` Darrick J. Wong
2019-04-11  1:45 ` [PATCH 2/8] xfs: replace the BAD_SUMMARY mount flag with the equivalent health code Darrick J. Wong
2019-04-11  1:45 ` [PATCH 3/8] xfs: clear BAD_SUMMARY if unmounting an unhealthy filesystem Darrick J. Wong
2019-04-11 12:29   ` Brian Foster
2019-04-11  1:45 ` [PATCH 4/8] xfs: bump XFS_IOC_FSGEOMETRY to v5 structures Darrick J. Wong
2019-04-11 12:29   ` Brian Foster
2019-04-11  1:45 ` [PATCH 5/8] xfs: add a new ioctl to describe allocation group geometry Darrick J. Wong
2019-04-11 13:08   ` Brian Foster
2019-04-11  1:46 ` [PATCH 6/8] xfs: report fs and rt health via geometry structure Darrick J. Wong
2019-04-11 13:09   ` Brian Foster
2019-04-11 15:30     ` Darrick J. Wong
2019-04-11  1:46 ` [PATCH 7/8] xfs: report AG health via AG geometry ioctl Darrick J. Wong
2019-04-11 13:09   ` Brian Foster
2019-04-11 15:33     ` Darrick J. Wong
2019-04-11  1:46 ` [PATCH 8/8] xfs: report inode health via bulkstat Darrick J. Wong
2019-04-11 13:10   ` Brian Foster
2019-04-12  6:28 [PATCH v3 0/8] xfs: online health tracking support Darrick J. Wong
2019-04-12  6:28 ` [PATCH 7/8] xfs: report AG health via AG geometry ioctl Darrick J. Wong
2019-04-12 11:14   ` Brian Foster

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.