From: Darrick J. Wong <djwong@xxxxxxxxxx>
If the trylock-based dirent check fails, retain those parent pointers
and check them at the end. This may involve dropping the locks on the
file being scanned, so yay.
Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
---
fs/xfs/Makefile | 2
fs/xfs/libxfs/xfs_parent.c | 38 +++++++
fs/xfs/libxfs/xfs_parent.h | 10 ++
fs/xfs/scrub/parent.c | 246 +++++++++++++++++++++++++++++++++++++++++++-
fs/xfs/scrub/trace.h | 33 ++++++
5 files changed, 324 insertions(+), 5 deletions(-)
diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
index a32f6da27a86..0a908382d033 100644
--- a/fs/xfs/Makefile
+++ b/fs/xfs/Makefile
@@ -168,6 +168,7 @@ xfs-y += $(addprefix scrub/, \
scrub.o \
symlink.o \
xfarray.o \
+ xfblob.o \
xfile.o \
)
@@ -181,7 +182,6 @@ xfs-y += $(addprefix scrub/, \
dir_repair.o \
repair.o \
tempfile.o \
- xfblob.o \
)
endif
endif
diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c
index fe6d4d1a7d57..36e1968337d5 100644
--- a/fs/xfs/libxfs/xfs_parent.c
+++ b/fs/xfs/libxfs/xfs_parent.c
@@ -298,3 +298,41 @@ xfs_pptr_calc_space_res(
XFS_NEXTENTADD_SPACE_RES(mp, namelen, XFS_ATTR_FORK);
}
+/*
+ * Look up the @name associated with the parent pointer (@pptr) of @ip. Caller
+ * must hold at least ILOCK_SHARED. Returns the length of the dirent name, or
+ * a negative errno. The scratchpad need not be initialized.
+ */
+int
+xfs_parent_lookup(
+ struct xfs_trans *tp,
+ struct xfs_inode *ip,
+ const struct xfs_parent_name_irec *pptr,
+ unsigned char *name,
+ unsigned int namelen,
+ struct xfs_parent_scratch *scr)
+{
+ int error;
+
+ xfs_parent_irec_to_disk(&scr->rec, NULL, NULL, pptr);
+
+ memset(&scr->args, 0, sizeof(struct xfs_da_args));
+ scr->args.attr_filter = XFS_ATTR_PARENT;
+ scr->args.dp = ip;
+ scr->args.geo = ip->i_mount->m_attr_geo;
+ scr->args.name = (const unsigned char *)&scr->rec;
+ scr->args.namelen = sizeof(struct xfs_parent_name_rec);
+ scr->args.op_flags = XFS_DA_OP_OKNOENT;
+ scr->args.trans = tp;
+ scr->args.valuelen = namelen;
+ scr->args.value = name;
+ scr->args.whichfork = XFS_ATTR_FORK;
+
+ scr->args.hashval = xfs_da_hashname(scr->args.name, scr->args.namelen);
+
+ error = xfs_attr_get_ilocked(&scr->args);
+ if (error)
+ return error;
+
+ return scr->args.valuelen;
+}
diff --git a/fs/xfs/libxfs/xfs_parent.h b/fs/xfs/libxfs/xfs_parent.h
index 4eb92fb4b11b..cd1b135195a2 100644
--- a/fs/xfs/libxfs/xfs_parent.h
+++ b/fs/xfs/libxfs/xfs_parent.h
@@ -103,4 +103,14 @@ xfs_parent_finish(
unsigned int xfs_pptr_calc_space_res(struct xfs_mount *mp,
unsigned int namelen);
+/* Scratchpad memory so that raw parent operations don't burn stack space. */
+struct xfs_parent_scratch {
+ struct xfs_parent_name_rec rec;
+ struct xfs_da_args args;
+};
+
+int xfs_parent_lookup(struct xfs_trans *tp, struct xfs_inode *ip,
+ const struct xfs_parent_name_irec *pptr, unsigned char *name,
+ unsigned int namelen, struct xfs_parent_scratch *scratch);
+
#endif /* __XFS_PARENT_H__ */
diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c
index 1bb196f2c1b2..056e11337cec 100644
--- a/fs/xfs/scrub/parent.c
+++ b/fs/xfs/scrub/parent.c
@@ -20,6 +20,9 @@
#include "scrub/common.h"
#include "scrub/readdir.h"
#include "scrub/listxattr.h"
+#include "scrub/xfile.h"
+#include "scrub/xfarray.h"
+#include "scrub/xfblob.h"
#include "scrub/trace.h"
/* Set us up to scrub parents. */
@@ -302,14 +305,43 @@ xchk_parent(
* to the child file.
*/
+/* Deferred parent pointer entry that we saved for later. */
+struct xchk_pptr {
+ /* Cookie for retrieval of the pptr name. */
+ xfblob_cookie name_cookie;
+
+ /* Parent pointer attr key. */
+ xfs_ino_t p_ino;
+ uint32_t p_gen;
+ xfs_dir2_dataptr_t p_diroffset;
+
+ /* Length of the pptr name. */
+ uint8_t namelen;
+};
+
struct xchk_pptrs {
struct xfs_scrub *sc;
/* Scratch buffer for scanning pptr xattrs */
struct xfs_parent_name_irec pptr;
+ /* Fixed-size array of xchk_pptr structures. */
+ struct xfarray *pptr_entries;
+
+ /* Blobs containing parent pointer names. */
+ struct xfblob *pptr_names;
+
/* Parent of this directory. */
xfs_ino_t parent_ino;
+
+ /* If we've cycled the ILOCK, we must revalidate all deferred pptrs. */
+ bool need_revalidate;
+
+ /* xattr key and da args for parent pointer revalidation. */
+ struct xfs_parent_scratch pptr_scratch;
+
+ /* Name buffer for revalidation. */
+ uint8_t namebuf[MAXNAMELEN];
};
/* Look up the dotdot entry so that we can check it as we walk the pptrs. */
@@ -528,8 +560,26 @@ xchk_parent_scan_attr(
/* Try to lock the inode. */
lockmode = xchk_parent_lock_dir(sc, dp);
if (!lockmode) {
- xchk_set_incomplete(sc);
- error = -ECANCELED;
+ struct xchk_pptr save_pp = {
+ .p_ino = pp->pptr.p_ino,
+ .p_gen = pp->pptr.p_gen,
+ .p_diroffset = pp->pptr.p_diroffset,
+ .namelen = pp->pptr.p_namelen,
+ };
+
+ /* Couldn't lock the inode, so save the pptr for later. */
+ trace_xchk_parent_defer(sc->ip, pp->pptr.p_name,
+ pp->pptr.p_namelen, dp->i_ino);
+
+ error = xfblob_store(pp->pptr_names, &save_pp.name_cookie,
+ pp->pptr.p_name, pp->pptr.p_namelen);
+ if (xchk_fblock_process_error(sc, XFS_ATTR_FORK, 0, &error))
+ goto out_rele;
+
+ error = xfarray_append(pp->pptr_entries, &save_pp);
+ if (xchk_fblock_process_error(sc, XFS_ATTR_FORK, 0, &error))
+ goto out_rele;
+
goto out_rele;
}
@@ -544,6 +594,173 @@ xchk_parent_scan_attr(
return error;
}
+/*
+ * Revalidate a parent pointer that we collected in the past but couldn't check
+ * because of lock contention. Returns 0 if the parent pointer is still valid,
+ * -ENOENT if it has gone away on us, or a negative errno.
+ */
+STATIC int
+xchk_parent_revalidate_pptr(
+ struct xchk_pptrs *pp)
+{
+ struct xfs_scrub *sc = pp->sc;
+ int namelen;
+
+ namelen = xfs_parent_lookup(sc->tp, sc->ip, &pp->pptr, pp->namebuf,
+ MAXNAMELEN, &pp->pptr_scratch);
+ if (namelen == -ENOATTR) {
+ /* Parent pointer went away, nothing to revalidate. */
+ return -ENOENT;
+ }
+ if (namelen < 0 && namelen != -EEXIST)
+ return namelen;
+
+ /*
+ * The dirent name changed length while we were unlocked. No need
+ * to revalidate this.
+ */
+ if (namelen != pp->pptr.p_namelen)
+ return -ENOENT;
+
+ /* The dirent name itself changed; there's nothing to revalidate. */
+ if (memcmp(pp->namebuf, pp->pptr.p_name, pp->pptr.p_namelen))
+ return -ENOENT;
+ return 0;
+}
+
+/*
+ * Check a parent pointer the slow way, which means we cycle locks a bunch
+ * and put up with revalidation until we get it done.
+ */
+STATIC int
+xchk_parent_slow_pptr(
+ struct xchk_pptrs *pp,
+ struct xchk_pptr *pptr)
+{
+ struct xfs_scrub *sc = pp->sc;
+ struct xfs_inode *dp = NULL;
+ unsigned int lockmode;
+ int error;
+
+ /* Restore the saved parent pointer into the irec. */
+ pp->pptr.p_ino = pptr->p_ino;
+ pp->pptr.p_gen = pptr->p_gen;
+ pp->pptr.p_diroffset = pptr->p_diroffset;
+
+ error = xfblob_load(pp->pptr_names, pptr->name_cookie, pp->pptr.p_name,
+ pptr->namelen);
+ if (error)
+ return error;
+ pp->pptr.p_name[MAXNAMELEN - 1] = 0;
+ pp->pptr.p_namelen = pptr->namelen;
+
+ /* Check that the deferred parent pointer still exists. */
+ if (pp->need_revalidate) {
+ error = xchk_parent_revalidate_pptr(pp);
+ if (error == -ENOENT)
+ return 0;
+ if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0,
+ &error))
+ return error;
+ }
+
+ error = xchk_parent_iget(pp, &dp);
+ if (error)
+ return error;
+ if (!dp)
+ return 0;
+
+ /*
+ * If we can grab both IOLOCK and ILOCK of the alleged parent, we
+ * can proceed with the validation.
+ */
+ lockmode = xchk_parent_lock_dir(sc, dp);
+ if (lockmode)
+ goto check_dirent;
+
+ /*
+ * We couldn't lock the parent dir. Drop all the locks and try to
+ * get them again, one at a time.
+ */
+ xchk_iunlock(sc, sc->ilock_flags);
+ pp->need_revalidate = true;
+
+ trace_xchk_parent_slowpath(sc->ip, pp->namebuf, pptr->namelen,
+ dp->i_ino);
+
+ while (true) {
+ xchk_ilock(sc, XFS_IOLOCK_EXCL);
+ if (xfs_ilock_nowait(dp, XFS_IOLOCK_SHARED)) {
+ xchk_ilock(sc, XFS_ILOCK_EXCL);
+ if (xfs_ilock_nowait(dp, XFS_ILOCK_EXCL)) {
+ break;
+ }
+ xchk_iunlock(sc, XFS_ILOCK_EXCL);
+ }
+ xchk_iunlock(sc, XFS_IOLOCK_EXCL);
+
+ if (xchk_should_terminate(sc, &error))
+ goto out_rele;
+
+ delay(1);
+ }
+ lockmode = XFS_IOLOCK_SHARED | XFS_ILOCK_EXCL;
+
+ /*
+ * If we didn't already find a parent pointer matching the dotdot
+ * entry, re-query the dotdot entry so that we can validate it.
+ */
+ if (pp->parent_ino != NULLFSINO) {
+ error = xchk_parent_dotdot(pp);
+ if (error)
+ goto out_unlock;
+ }
+
+ /* Revalidate the parent pointer now that we cycled locks. */
+ error = xchk_parent_revalidate_pptr(pp);
+ if (error == -ENOENT)
+ goto out_unlock;
+ if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, &error))
+ goto out_unlock;
+
+check_dirent:
+ error = xchk_parent_dirent(pp, dp);
+out_unlock:
+ xfs_iunlock(dp, lockmode);
+out_rele:
+ xchk_irele(sc, dp);
+ return error;
+}
+
+/* Check all the parent pointers that we deferred the first time around. */
+STATIC int
+xchk_parent_finish_slow_pptrs(
+ struct xchk_pptrs *pp)
+{
+ xfarray_idx_t array_cur;
+ int error;
+
+ foreach_xfarray_idx(pp->pptr_entries, array_cur) {
+ struct xchk_pptr pptr;
+
+ if (pp->sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
+ return 0;
+
+ error = xfarray_load(pp->pptr_entries, array_cur, &pptr);
+ if (error)
+ return error;
+
+ error = xchk_parent_slow_pptr(pp, &pptr);
+ if (error)
+ return error;
+ }
+
+ /* Empty out both xfiles now that we've checked everything. */
+ xfarray_truncate(pp->pptr_entries);
+ xfblob_truncate(pp->pptr_names);
+ return 0;
+}
+
/* Check parent pointers of a file. */
STATIC int
xchk_parent_pptr(
@@ -561,14 +778,35 @@ xchk_parent_pptr(
if (error)
goto out_pp;
+ /*
+ * Set up some staging memory for parent pointers that we can't check
+ * due to locking contention.
+ */
+ error = xfarray_create(sc->mp, "pptr entries", 0,
+ sizeof(struct xchk_pptr), &pp->pptr_entries);
+ if (error)
+ goto out_pp;
+
+ error = xfblob_create(sc->mp, "pptr names", &pp->pptr_names);
+ if (error)
+ goto out_entries;
+
error = xchk_xattr_walk(sc, sc->ip, xchk_parent_scan_attr, pp);
if (error == -ECANCELED) {
error = 0;
- goto out_pp;
+ goto out_names;
}
if (error)
- goto out_pp;
+ goto out_names;
+ error = xchk_parent_finish_slow_pptrs(pp);
+ if (error)
+ goto out_names;
+
+out_names:
+ xfblob_destroy(pp->pptr_names);
+out_entries:
+ xfarray_destroy(pp->pptr_entries);
out_pp:
kvfree(pp);
return error;
diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
index ac21759fc3e1..61f18632cb6f 100644
--- a/fs/xfs/scrub/trace.h
+++ b/fs/xfs/scrub/trace.h
@@ -929,6 +929,39 @@ TRACE_EVENT(xchk_parent_bad_dapos,
__get_str(name))
);
+DECLARE_EVENT_CLASS(xchk_pptr_class,
+ TP_PROTO(struct xfs_inode *ip, const unsigned char *name,
+ unsigned int namelen, xfs_ino_t parent_ino),
+ TP_ARGS(ip, name, namelen, parent_ino),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(xfs_ino_t, ino)
+ __field(unsigned int, namelen)
+ __dynamic_array(char, name, namelen)
+ __field(xfs_ino_t, parent_ino)
+ ),
+ TP_fast_assign(
+ __entry->dev = ip->i_mount->m_super->s_dev;
+ __entry->ino = ip->i_ino;
+ __entry->namelen = namelen;
+ memcpy(__get_str(name), name, namelen);
+ __entry->parent_ino = parent_ino;
+ ),
+ TP_printk("dev %d:%d ino 0x%llx name '%.*s' parent_ino 0x%llx",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->ino,
+ __entry->namelen,
+ __get_str(name),
+ __entry->parent_ino)
+)
+#define DEFINE_XCHK_PPTR_CLASS(name) \
+DEFINE_EVENT(xchk_pptr_class, name, \
+ TP_PROTO(struct xfs_inode *ip, const unsigned char *name, \
+ unsigned int namelen, xfs_ino_t parent_ino), \
+ TP_ARGS(ip, name, namelen, parent_ino))
+DEFINE_XCHK_PPTR_CLASS(xchk_parent_defer);
+DEFINE_XCHK_PPTR_CLASS(xchk_parent_slowpath);
+
/* repair tracepoints */
#if IS_ENABLED(CONFIG_XFS_ONLINE_REPAIR)
