xfs: set inode sick state flags when we zap either ondisk fork

In a few patches, we'll add some online repair code that tries to
massage the ondisk inode record just enough to get it to pass the inode
verifiers so that we can continue with more file repairs.  Part of that
massaging can include zapping the ondisk forks to clear errors.  After
that point, the bmap fork repair functions will rebuild the zapped
forks.

Christoph asked for stronger protections against online repair zapping a
fork to get the inode to load vs. other threads trying to access the
partially repaired file.  Do this by adding a special "[DA]FORK_ZAPPED"
inode health flag whenever repair zaps a fork, and sprinkling checks for
that flag into the various file operations for things that don't like
handling an unexpected zero-extents fork.

In practice xfs_scrub will scrub and fix the forks almost immediately
after zapping them, so the window is very small.  However, if a crash or
unmount should occur, we can still detect these zapped inode forks by
looking for a zero-extents fork when data was expected.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>

diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
index 99e796256c5d114a3cbe9029e27f18a74dab3920..6296993ff8f3df52a6e534f81ace861dd495d323 100644 (file)
--- a/fs/xfs/libxfs/xfs_health.h
+++ b/fs/xfs/libxfs/xfs_health.h
@@ -68,6 +68,11 @@ struct xfs_fsop_geom;
 #define XFS_SICK_INO_SYMLINK   (1 << 6)  /* symbolic link remote target */
 #define XFS_SICK_INO_PARENT    (1 << 7)  /* parent pointers */
 
+#define XFS_SICK_INO_BMBTD_ZAPPED      (1 << 8)  /* data fork erased */
+#define XFS_SICK_INO_BMBTA_ZAPPED      (1 << 9)  /* attr fork erased */
+#define XFS_SICK_INO_DIR_ZAPPED                (1 << 10) /* directory erased */
+#define XFS_SICK_INO_SYMLINK_ZAPPED    (1 << 11) /* symlink erased */
+
 /* Primary evidence of health problems in a given group. */
 #define XFS_SICK_FS_PRIMARY    (XFS_SICK_FS_COUNTERS | \
                                 XFS_SICK_FS_UQUOTA | \
@@ -97,6 +102,11 @@ struct xfs_fsop_geom;
                                 XFS_SICK_INO_SYMLINK | \
                                 XFS_SICK_INO_PARENT)
 
+#define XFS_SICK_INO_ZAPPED    (XFS_SICK_INO_BMBTD_ZAPPED | \
+                                XFS_SICK_INO_BMBTA_ZAPPED | \
+                                XFS_SICK_INO_DIR_ZAPPED | \
+                                XFS_SICK_INO_SYMLINK_ZAPPED)
+
 /* These functions must be provided by the xfs implementation. */
 
 void xfs_fs_mark_sick(struct xfs_mount *mp, unsigned int mask);

diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
index f74bd2a97c7f734d3208a13ab43a01f817a5635c..1487aaf3d95f3290f42a7a8549190d631ad0d9d6 100644 (file)
--- a/fs/xfs/scrub/bmap.c
+++ b/fs/xfs/scrub/bmap.c
@@ -19,9 +19,11 @@
 #include "xfs_bmap_btree.h"
 #include "xfs_rmap.h"
 #include "xfs_rmap_btree.h"
+#include "xfs_health.h"
 #include "scrub/scrub.h"
 #include "scrub/common.h"
 #include "scrub/btree.h"
+#include "scrub/health.h"
 #include "xfs_ag.h"
 
 /* Set us up with an inode's bmap. */
@@ -943,7 +945,20 @@ int
 xchk_bmap_data(
        struct xfs_scrub        *sc)
 {
-       return xchk_bmap(sc, XFS_DATA_FORK);
+       int                     error;
+
+       if (xchk_file_looks_zapped(sc, XFS_SICK_INO_BMBTD_ZAPPED)) {
+               xchk_ino_set_corrupt(sc, sc->ip->i_ino);
+               return 0;
+       }
+
+       error = xchk_bmap(sc, XFS_DATA_FORK);
+       if (error)
+               return error;
+
+       /* If the data fork is clean, it is clearly not zapped. */
+       xchk_mark_healthy_if_clean(sc, XFS_SICK_INO_BMBTD_ZAPPED);
+       return 0;
 }
 
 /* Scrub an inode's attr fork. */
@@ -951,7 +966,27 @@ int
 xchk_bmap_attr(
        struct xfs_scrub        *sc)
 {
-       return xchk_bmap(sc, XFS_ATTR_FORK);
+       int                     error;
+
+       /*
+        * If the attr fork has been zapped, it's possible that forkoff was
+        * reset to zero and hence sc->ip->i_afp is NULL.  We don't want the
+        * NULL ifp check in xchk_bmap to conclude that the attr fork is ok,
+        * so short circuit that logic by setting the corruption flag and
+        * returning immediately.
+        */
+       if (xchk_file_looks_zapped(sc, XFS_SICK_INO_BMBTA_ZAPPED)) {
+               xchk_ino_set_corrupt(sc, sc->ip->i_ino);
+               return 0;
+       }
+
+       error = xchk_bmap(sc, XFS_ATTR_FORK);
+       if (error)
+               return error;
+
+       /* If the attr fork is clean, it is clearly not zapped. */
+       xchk_mark_healthy_if_clean(sc, XFS_SICK_INO_BMBTA_ZAPPED);
+       return 0;
 }
 
 /* Scrub an inode's CoW fork. */

diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index bff0a374fb1b40fb0dc7a87548a7f65900718d13..f0207e71e5dc6f3044425a9d184a1025c4d2c97d 100644 (file)
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -1160,6 +1160,7 @@ xchk_metadata_inode_subtype(
        unsigned int            scrub_type)
 {
        __u32                   smtype = sc->sm->sm_type;
+       unsigned int            sick_mask = sc->sick_mask;
        int                     error;
 
        sc->sm->sm_type = scrub_type;
@@ -1177,6 +1178,7 @@ xchk_metadata_inode_subtype(
                break;
        }
 
+       sc->sick_mask = sick_mask;
        sc->sm->sm_type = smtype;
        return error;
 }

diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index 0b491784b7594cf84b6f7f767a9d4da2786464b4..b366fab699ac505e8ceea96b62816020b56587b2 100644 (file)
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -15,10 +15,12 @@
 #include "xfs_icache.h"
 #include "xfs_dir2.h"
 #include "xfs_dir2_priv.h"
+#include "xfs_health.h"
 #include "scrub/scrub.h"
 #include "scrub/common.h"
 #include "scrub/dabtree.h"
 #include "scrub/readdir.h"
+#include "scrub/health.h"
 
 /* Set us up to scrub directories. */
 int
@@ -760,6 +762,11 @@ xchk_directory(
        if (!S_ISDIR(VFS_I(sc->ip)->i_mode))
                return -ENOENT;
 
+       if (xchk_file_looks_zapped(sc, XFS_SICK_INO_DIR_ZAPPED)) {
+               xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
+               return 0;
+       }
+
        /* Plausible size? */
        if (sc->ip->i_disk_size < xfs_dir2_sf_hdr_size(0)) {
                xchk_ino_set_corrupt(sc, sc->ip->i_ino);
@@ -784,7 +791,10 @@ xchk_directory(
 
        /* Look up every name in this directory by hash. */
        error = xchk_dir_walk(sc, sc->ip, xchk_dir_actor, NULL);
-       if (error == -ECANCELED)
-               error = 0;
-       return error;
+       if (error && error != -ECANCELED)
+               return error;
+
+       /* If the dir is clean, it is clearly not zapped. */
+       xchk_mark_healthy_if_clean(sc, XFS_SICK_INO_DIR_ZAPPED);
+       return 0;
 }

diff --git a/fs/xfs/scrub/health.c b/fs/xfs/scrub/health.c
index 5e2b09ed6e29a31011d6ea7ad3df9aa582d2be0e..df716da11226b5be3eac5a2c3a2d6cdf9bf32380 100644 (file)
--- a/fs/xfs/scrub/health.c
+++ b/fs/xfs/scrub/health.c
@@ -117,6 +117,38 @@ xchk_health_mask_for_scrub_type(
        return type_to_health_flag[scrub_type].sick_mask;
 }
 
+/*
+ * If the scrub state is clean, add @mask to the scrub sick mask to clear
+ * additional sick flags from the metadata object's sick state.
+ */
+void
+xchk_mark_healthy_if_clean(
+       struct xfs_scrub        *sc,
+       unsigned int            mask)
+{
+       if (!(sc->sm->sm_flags & (XFS_SCRUB_OFLAG_CORRUPT |
+                                 XFS_SCRUB_OFLAG_XCORRUPT)))
+               sc->sick_mask |= mask;
+}
+
+/*
+ * If we're scrubbing a piece of file metadata for the first time, does it look
+ * like it has been zapped?  Skip the check if we just repaired the metadata
+ * and are revalidating it.
+ */
+bool
+xchk_file_looks_zapped(
+       struct xfs_scrub        *sc,
+       unsigned int            mask)
+{
+       ASSERT((mask & ~XFS_SICK_INO_ZAPPED) == 0);
+
+       if (sc->flags & XREP_ALREADY_FIXED)
+               return false;
+
+       return xfs_inode_has_sickness(sc->ip, mask);
+}
+
 /*
  * Update filesystem health assessments based on what we found and did.
  *

diff --git a/fs/xfs/scrub/health.h b/fs/xfs/scrub/health.h
index 66a273f8585bcc1ea4baae787a0ef48bf775905e..a731b2467399f630e424af0316c1ddc7e2cea0fc 100644 (file)
--- a/fs/xfs/scrub/health.h
+++ b/fs/xfs/scrub/health.h
@@ -10,5 +10,7 @@ unsigned int xchk_health_mask_for_scrub_type(__u32 scrub_type);
 void xchk_update_health(struct xfs_scrub *sc);
 bool xchk_ag_btree_healthy_enough(struct xfs_scrub *sc, struct xfs_perag *pag,
                xfs_btnum_t btnum);
+void xchk_mark_healthy_if_clean(struct xfs_scrub *sc, unsigned int mask);
+bool xchk_file_looks_zapped(struct xfs_scrub *sc, unsigned int mask);
 
 #endif /* __XFS_SCRUB_HEALTH_H__ */

diff --git a/fs/xfs/scrub/symlink.c b/fs/xfs/scrub/symlink.c
index 38708fb9a5d71901af38f958d81a5813b43e9d1b..60643d791d4a2220d0cc5d6bb208b148a1982b5d 100644 (file)
--- a/fs/xfs/scrub/symlink.c
+++ b/fs/xfs/scrub/symlink.c
@@ -12,8 +12,10 @@
 #include "xfs_log_format.h"
 #include "xfs_inode.h"
 #include "xfs_symlink.h"
+#include "xfs_health.h"
 #include "scrub/scrub.h"
 #include "scrub/common.h"
+#include "scrub/health.h"
 
 /* Set us up to scrub a symbolic link. */
 int
@@ -41,13 +43,19 @@ xchk_symlink(
 
        if (!S_ISLNK(VFS_I(ip)->i_mode))
                return -ENOENT;
+
+       if (xchk_file_looks_zapped(sc, XFS_SICK_INO_SYMLINK_ZAPPED)) {
+               xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
+               return 0;
+       }
+
        ifp = xfs_ifork_ptr(ip, XFS_DATA_FORK);
        len = ip->i_disk_size;
 
        /* Plausible size? */
        if (len > XFS_SYMLINK_MAXLEN || len <= 0) {
                xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
-               goto out;
+               return 0;
        }
 
        /* Inline symlink? */
@@ -55,15 +63,17 @@ xchk_symlink(
                if (len > xfs_inode_data_fork_size(ip) ||
                    len > strnlen(ifp->if_u1.if_data, xfs_inode_data_fork_size(ip)))
                        xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
-               goto out;
+               return 0;
        }
 
        /* Remote symlink; must read the contents. */
        error = xfs_readlink_bmap_ilocked(sc->ip, sc->buf);
        if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, 0, &error))
-               goto out;
+               return error;
        if (strnlen(sc->buf, XFS_SYMLINK_MAXLEN) < len)
                xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
-out:
-       return error;
+
+       /* If a remote symlink is clean, it is clearly not zapped. */
+       xchk_mark_healthy_if_clean(sc, XFS_SICK_INO_SYMLINK_ZAPPED);
+       return 0;
 }

diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index 9f3ceb46151566834d63840162b51f4caf6cc8dd..57f42c2af0a31662e0d61a5bdb093d64d2d0e0df 100644 (file)
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -18,6 +18,7 @@
 #include "xfs_bmap.h"
 #include "xfs_trans.h"
 #include "xfs_error.h"
+#include "xfs_health.h"
 
 /*
  * Directory file type support functions
@@ -519,6 +520,8 @@ xfs_readdir(
 
        if (xfs_is_shutdown(dp->i_mount))
                return -EIO;
+       if (xfs_ifork_zapped(dp, XFS_DATA_FORK))
+               return -EIO;
 
        ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
        ASSERT(xfs_isilocked(dp, XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL));

diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
index 72a075bb2c10dd603e7d759793a2651376c7f414..9a57afee933834125f2820712fce11783a6eda45 100644 (file)
--- a/fs/xfs/xfs_health.c
+++ b/fs/xfs/xfs_health.c
@@ -222,7 +222,7 @@ xfs_inode_mark_sick(
        struct xfs_inode        *ip,
        unsigned int            mask)
 {
-       ASSERT(!(mask & ~XFS_SICK_INO_PRIMARY));
+       ASSERT(!(mask & ~(XFS_SICK_INO_PRIMARY | XFS_SICK_INO_ZAPPED)));
        trace_xfs_inode_mark_sick(ip, mask);
 
        spin_lock(&ip->i_flags_lock);
@@ -246,7 +246,7 @@ xfs_inode_mark_healthy(
        struct xfs_inode        *ip,
        unsigned int            mask)
 {
-       ASSERT(!(mask & ~XFS_SICK_INO_PRIMARY));
+       ASSERT(!(mask & ~(XFS_SICK_INO_PRIMARY | XFS_SICK_INO_ZAPPED)));
        trace_xfs_inode_mark_healthy(ip, mask);
 
        spin_lock(&ip->i_flags_lock);
@@ -369,6 +369,10 @@ static const struct ioctl_sick_map ino_map[] = {
        { XFS_SICK_INO_XATTR,   XFS_BS_SICK_XATTR },
        { XFS_SICK_INO_SYMLINK, XFS_BS_SICK_SYMLINK },
        { XFS_SICK_INO_PARENT,  XFS_BS_SICK_PARENT },
+       { XFS_SICK_INO_BMBTD_ZAPPED,    XFS_BS_SICK_BMBTD },
+       { XFS_SICK_INO_BMBTA_ZAPPED,    XFS_BS_SICK_BMBTA },
+       { XFS_SICK_INO_DIR_ZAPPED,      XFS_BS_SICK_DIR },
+       { XFS_SICK_INO_SYMLINK_ZAPPED,  XFS_BS_SICK_SYMLINK },
        { 0, 0 },
 };
 

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c0f1c89786c2ac083482ece0bb312692ab8255a3..ea6b277485a43346c0f07e2e98997c7cf20705f7 100644 (file)
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -37,6 +37,7 @@
 #include "xfs_reflink.h"
 #include "xfs_ag.h"
 #include "xfs_log_priv.h"
+#include "xfs_health.h"
 
 struct kmem_cache *xfs_inode_cache;
 
@@ -661,6 +662,8 @@ xfs_lookup(
 
        if (xfs_is_shutdown(dp->i_mount))
                return -EIO;
+       if (xfs_ifork_zapped(dp, XFS_DATA_FORK))
+               return -EIO;
 
        error = xfs_dir_lookup(NULL, dp, name, &inum, ci_name);
        if (error)
@@ -978,6 +981,8 @@ xfs_create(
 
        if (xfs_is_shutdown(mp))
                return -EIO;
+       if (xfs_ifork_zapped(dp, XFS_DATA_FORK))
+               return -EIO;
 
        prid = xfs_get_initial_prid(dp);
 
@@ -1217,6 +1222,8 @@ xfs_link(
 
        if (xfs_is_shutdown(mp))
                return -EIO;
+       if (xfs_ifork_zapped(tdp, XFS_DATA_FORK))
+               return -EIO;
 
        error = xfs_qm_dqattach(sip);
        if (error)
@@ -2506,6 +2513,8 @@ xfs_remove(
 
        if (xfs_is_shutdown(mp))
                return -EIO;
+       if (xfs_ifork_zapped(dp, XFS_DATA_FORK))
+               return -EIO;
 
        error = xfs_qm_dqattach(dp);
        if (error)
@@ -3758,3 +3767,29 @@ xfs_inode_reload_unlinked(
 
        return error;
 }
+
+/* Has this inode fork been zapped by repair? */
+bool
+xfs_ifork_zapped(
+       const struct xfs_inode  *ip,
+       int                     whichfork)
+{
+       unsigned int            datamask = 0;
+
+       switch (whichfork) {
+       case XFS_DATA_FORK:
+               switch (ip->i_vnode.i_mode & S_IFMT) {
+               case S_IFDIR:
+                       datamask = XFS_SICK_INO_DIR_ZAPPED;
+                       break;
+               case S_IFLNK:
+                       datamask = XFS_SICK_INO_SYMLINK_ZAPPED;
+                       break;
+               }
+               return ip->i_sick & (XFS_SICK_INO_BMBTD_ZAPPED | datamask);
+       case XFS_ATTR_FORK:
+               return ip->i_sick & XFS_SICK_INO_BMBTA_ZAPPED;
+       default:
+               return false;
+       }
+}

diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 3beb470f18920d6730b32e5d1edcf5530b93b327..97f63bacd4c2be08c1b5c98535edc914fe2e2fbe 100644 (file)
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -622,4 +622,6 @@ xfs_inode_unlinked_incomplete(
 int xfs_inode_reload_unlinked_bucket(struct xfs_trans *tp, struct xfs_inode *ip);
 int xfs_inode_reload_unlinked(struct xfs_inode *ip);
 
+bool xfs_ifork_zapped(const struct xfs_inode *ip, int whichfork);
+
 #endif /* __XFS_INODE_H__ */

diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 85e433df6a3f91e82a56aa2026a460ad92574b6b..7c713727f7fd371aa465875522084ac2215f6929 100644 (file)
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -23,6 +23,7 @@
 #include "xfs_trans.h"
 #include "xfs_ialloc.h"
 #include "xfs_error.h"
+#include "xfs_health.h"
 
 /* ----- Kernel only functions below ----- */
 int
@@ -108,6 +109,8 @@ xfs_readlink(
 
        if (xfs_is_shutdown(mp))
                return -EIO;
+       if (xfs_ifork_zapped(ip, XFS_DATA_FORK))
+               return -EIO;
 
        xfs_ilock(ip, XFS_ILOCK_SHARED);
 

diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
index 987843f84d03f97da5646b621b40d3c5dba1b529..364104e1b38aed1fa95bfdd7aa033c67a8588a58 100644 (file)
--- a/fs/xfs/xfs_xattr.c
+++ b/fs/xfs/xfs_xattr.c
@@ -136,6 +136,9 @@ xfs_xattr_get(const struct xattr_handler *handler, struct dentry *unused,
        };
        int                     error;
 
+       if (xfs_ifork_zapped(XFS_I(inode), XFS_ATTR_FORK))
+               return -EIO;
+
        error = xfs_attr_get(&args);
        if (error)
                return error;
@@ -294,6 +297,9 @@ xfs_vn_listxattr(
        struct inode    *inode = d_inode(dentry);
        int             error;
 
+       if (xfs_ifork_zapped(XFS_I(inode), XFS_ATTR_FORK))
+               return -EIO;
+
        /*
         * First read the regular on-disk attributes.
         */