From patchwork Sun Dec 31 20:53:27 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507511 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C5704BE4D for ; Sun, 31 Dec 2023 20:53:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="tBQyfN30" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 92F77C433C7; Sun, 31 Dec 2023 20:53:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056007; bh=+6MQe/jFn5+pTTSC6NrQB5RObTG42do+qPxr5bF47ts=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=tBQyfN30RWIm/rKVtEludQo26M3nNvVIuRcuiLZpKQR3xi7RQenl6sXzHuXRGdyNW 49KBDH/5r1RX3r7sbsxbGui5vTVvuR/G7ov8MeU9AXrlkMm+HV7idBTxpgrB94PSt1 xi/dGwuz3XokRseJdMahqXq/aw9SNUxmubGBXsduEORhHGCAT+Z/zVFRLMSfw07xgG efVa9lEZroyNfgvxrh3/53CWSwT3dyHvIsOoFrXa9KNiDwtIgKSK4n/HsiLviGrfCC BAGCw2vXAO8sKhw9ELn9A0irI2BTaKBace6EFbH6CvdXNvnQ+BC/8OCQUPOFE9hRW0 LYX/ofw1AKqcQ== Date: Sun, 31 Dec 2023 12:53:27 -0800 Subject: [PATCH 01/22] xfs: check dirents have parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841764.1757392.11351513012455132613.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong If the fs has parent pointers, we need to check that each child dirent points to a file that has a parent pointer pointing back at us. Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_parent.c | 54 +++++++++++++++++++ fs/xfs/libxfs/xfs_parent.h | 10 ++++ fs/xfs/scrub/dir.c | 122 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 185 insertions(+), 1 deletion(-) diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c index 48a2dfcc465fa..09495eb368e2b 100644 --- a/fs/xfs/libxfs/xfs_parent.c +++ b/fs/xfs/libxfs/xfs_parent.c @@ -366,3 +366,57 @@ xfs_parent_irec_hashname( irec->p_namehash = xfs_dir2_hashname(mp, &dname); } + +static inline void +xfs_parent_scratch_init( + struct xfs_trans *tp, + struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, + struct xfs_parent_scratch *scr) +{ + 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_NVLOOKUP; + scr->args.trans = tp; + scr->args.value = (void *)pptr->p_name; + scr->args.valuelen = pptr->p_namelen; + scr->args.whichfork = XFS_ATTR_FORK; + scr->args.hashval = xfs_da_hashname((const void *)&scr->rec, + sizeof(struct xfs_parent_name_rec)); +} + +/* + * Look up the @name associated with the parent pointer (@pptr) of @ip. + * Caller must hold at least ILOCK_SHARED. Returns 0 if the pointer is found, + * -ENOATTR if there is no match, 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, + struct xfs_parent_scratch *scr) +{ + int error; + + /* + * Make sure the attr fork iext tree is loaded in transaction context + * before we start down the rest of the call path. + */ + if (xfs_inode_hasattr(ip)) { + error = xfs_iread_extents(tp, ip, XFS_ATTR_FORK); + if (error) + return error; + } + + xfs_parent_irec_to_disk(&scr->rec, pptr); + xfs_parent_scratch_init(tp, ip, pptr, scr); + scr->args.op_flags |= XFS_DA_OP_OKNOENT; + + return xfs_attr_get_ilocked(&scr->args); +} diff --git a/fs/xfs/libxfs/xfs_parent.h b/fs/xfs/libxfs/xfs_parent.h index e43ae5a7df826..e4443da1d86f2 100644 --- a/fs/xfs/libxfs/xfs_parent.h +++ b/fs/xfs/libxfs/xfs_parent.h @@ -152,4 +152,14 @@ void xfs_parent_irec_hashname(struct xfs_mount *mp, bool xfs_parent_verify_irec(struct xfs_mount *mp, const struct xfs_parent_name_irec *irec); +/* 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, + struct xfs_parent_scratch *scratch); + #endif /* __XFS_PARENT_H__ */ diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c index 3fe6ffcf9c062..88370804025c4 100644 --- a/fs/xfs/scrub/dir.c +++ b/fs/xfs/scrub/dir.c @@ -16,6 +16,8 @@ #include "xfs_dir2.h" #include "xfs_dir2_priv.h" #include "xfs_health.h" +#include "xfs_attr.h" +#include "xfs_parent.h" #include "scrub/scrub.h" #include "scrub/common.h" #include "scrub/dabtree.h" @@ -41,6 +43,20 @@ xchk_setup_directory( /* Directories */ +struct xchk_dir { + struct xfs_scrub *sc; + + /* Scratch buffer for scanning pptr xattrs */ + struct xfs_parent_name_irec pptr; + + /* xattr key and da args for parent pointer validation. */ + struct xfs_parent_scratch pptr_scratch; + + /* Name buffer for dirent revalidation. */ + uint8_t namebuf[MAXNAMELEN]; + +}; + /* Scrub a directory entry. */ /* Check that an inode's mode matches a given XFS_DIR3_FT_* type. */ @@ -63,6 +79,94 @@ xchk_dir_check_ftype( xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, offset); } +/* + * Try to lock a child file for checking parent pointers. Returns the inode + * flags for the locks we now hold, or zero if we failed. + */ +STATIC unsigned int +xchk_dir_lock_child( + struct xfs_scrub *sc, + struct xfs_inode *ip) +{ + if (!xfs_ilock_nowait(ip, XFS_IOLOCK_SHARED)) + return 0; + + if (!xfs_ilock_nowait(ip, XFS_ILOCK_SHARED)) { + xfs_iunlock(ip, XFS_IOLOCK_SHARED); + return 0; + } + + if (!xfs_inode_has_attr_fork(ip) || !xfs_need_iread_extents(&ip->i_af)) + return XFS_IOLOCK_SHARED | XFS_ILOCK_SHARED; + + xfs_iunlock(ip, XFS_ILOCK_SHARED); + + if (!xfs_ilock_nowait(ip, XFS_ILOCK_EXCL)) { + xfs_iunlock(ip, XFS_IOLOCK_SHARED); + return 0; + } + + return XFS_IOLOCK_SHARED | XFS_ILOCK_EXCL; +} + +/* Check the backwards link (parent pointer) associated with this dirent. */ +STATIC int +xchk_dir_parent_pointer( + struct xchk_dir *sd, + const struct xfs_name *name, + struct xfs_inode *ip) +{ + struct xfs_scrub *sc = sd->sc; + int error; + + sd->pptr.p_ino = sc->ip->i_ino; + sd->pptr.p_gen = VFS_I(sc->ip)->i_generation; + sd->pptr.p_namelen = name->len; + memcpy(sd->pptr.p_name, name->name, name->len); + xfs_parent_irec_hashname(sc->mp, &sd->pptr); + + error = xfs_parent_lookup(sc->tp, ip, &sd->pptr, &sd->pptr_scratch); + if (error == -ENOATTR) + xchk_fblock_xref_set_corrupt(sc, XFS_DATA_FORK, 0); + + return 0; +} + +/* Look for a parent pointer matching this dirent, if the child isn't busy. */ +STATIC int +xchk_dir_check_pptr_fast( + struct xchk_dir *sd, + xfs_dir2_dataptr_t dapos, + const struct xfs_name *name, + struct xfs_inode *ip) +{ + struct xfs_scrub *sc = sd->sc; + unsigned int lockmode; + int error; + + /* dot and dotdot entries do not have parent pointers */ + if (xfs_dir2_samename(name, &xfs_name_dot) || + xfs_dir2_samename(name, &xfs_name_dotdot)) + return 0; + + /* No self-referential non-dot or dotdot dirents. */ + if (ip == sc->ip) { + xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0); + return -ECANCELED; + } + + /* Try to lock the inode. */ + lockmode = xchk_dir_lock_child(sc, ip); + if (!lockmode) { + xchk_set_incomplete(sc); + return -ECANCELED; + } + + error = xchk_dir_parent_pointer(sd, name, ip); + xfs_iunlock(ip, lockmode); + return error; +} + /* * Scrub a single directory entry. * @@ -80,6 +184,7 @@ xchk_dir_actor( { struct xfs_mount *mp = dp->i_mount; struct xfs_inode *ip; + struct xchk_dir *sd = priv; xfs_ino_t lookup_ino; xfs_dablk_t offset; int error = 0; @@ -146,6 +251,14 @@ xchk_dir_actor( goto out; xchk_dir_check_ftype(sc, offset, ip, name->type); + + if (xfs_has_parent(mp)) { + error = xchk_dir_check_pptr_fast(sd, dapos, name, ip); + if (error) + goto out_rele; + } + +out_rele: xchk_irele(sc, ip); out: if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT) @@ -767,6 +880,7 @@ int xchk_directory( struct xfs_scrub *sc) { + struct xchk_dir *sd; int error; if (!S_ISDIR(VFS_I(sc->ip)->i_mode)) @@ -799,8 +913,14 @@ xchk_directory( if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT) return 0; + sd = kvzalloc(sizeof(struct xchk_dir), XCHK_GFP_FLAGS); + if (!sd) + return -ENOMEM; + sd->sc = sc; + /* Look up every name in this directory by hash. */ - error = xchk_dir_walk(sc, sc->ip, xchk_dir_actor, NULL); + error = xchk_dir_walk(sc, sc->ip, xchk_dir_actor, sd); + kvfree(sd); if (error && error != -ECANCELED) return error; From patchwork Sun Dec 31 20:53:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507512 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BC366BE47 for ; Sun, 31 Dec 2023 20:53:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="QUKRFzNN" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 43E52C433C7; Sun, 31 Dec 2023 20:53:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056023; bh=LmqYdiW5GS1uryDwBFwIrrevWVJLVeLquXfKIMnhZuI=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=QUKRFzNNSIKDiVYYlBtQAZOhfQ6h9nMv+bDcrOr3NMv+zWFFgi9ZVrIT/Q6bbJ5DA ujwliBpXLofZ7+SysnRUaQ/5usw+8ei6yBgvd/p6ETQ3XfNXVHFbR7Bt9QjWdUC4j5 vtDw6yP+JbVOiAncvXn6z/+DBxXLnFfWbCyVA54E4ZXwgvvn7fJF6dgJ7a6anOJyAN sEhfMbLCBc1n5FOnRFBf6Z9RpDI3kUiaEeau6FzhRDW8H6Fq59jxZC4hnyNiZc1SH0 h/rRF0xRvOss5AbjNZ11wjkcz1due6+SWnciiqu8B9WXHHRKdg7GUzCH/Y+nDyakEv z4069Nx5S5f6A== Date: Sun, 31 Dec 2023 12:53:42 -0800 Subject: [PATCH 02/22] xfs: deferred scrub of dirents From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841780.1757392.5129202864477314729.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong If the trylock-based parent pointer check fails, retain those dirents 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 --- fs/xfs/scrub/dir.c | 234 +++++++++++++++++++++++++++++++++++++++++++++++- fs/xfs/scrub/readdir.c | 57 ++++++++++++ fs/xfs/scrub/readdir.h | 3 + fs/xfs/scrub/trace.h | 34 +++++++ 4 files changed, 325 insertions(+), 3 deletions(-) diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c index 88370804025c4..cfaddde6a34d6 100644 --- a/fs/xfs/scrub/dir.c +++ b/fs/xfs/scrub/dir.c @@ -24,6 +24,10 @@ #include "scrub/readdir.h" #include "scrub/health.h" #include "scrub/repair.h" +#include "scrub/trace.h" +#include "scrub/xfile.h" +#include "scrub/xfarray.h" +#include "scrub/xfblob.h" /* Set us up to scrub directories. */ int @@ -43,6 +47,18 @@ xchk_setup_directory( /* Directories */ +/* Deferred directory entry that we saved for later. */ +struct xchk_dirent { + /* Cookie for retrieval of the dirent name. */ + xfblob_cookie name_cookie; + + /* Child inode number. */ + xfs_ino_t ino; + + /* Length of the pptr name. */ + uint8_t namelen; +}; + struct xchk_dir { struct xfs_scrub *sc; @@ -52,6 +68,15 @@ struct xchk_dir { /* xattr key and da args for parent pointer validation. */ struct xfs_parent_scratch pptr_scratch; + /* Fixed-size array of xchk_dirent structures. */ + struct xfarray *dir_entries; + + /* Blobs containing dirent names. */ + struct xfblob *dir_names; + + /* If we've cycled the ILOCK, we must revalidate deferred dirents. */ + bool need_revalidate; + /* Name buffer for dirent revalidation. */ uint8_t namebuf[MAXNAMELEN]; @@ -158,8 +183,26 @@ xchk_dir_check_pptr_fast( /* Try to lock the inode. */ lockmode = xchk_dir_lock_child(sc, ip); if (!lockmode) { - xchk_set_incomplete(sc); - return -ECANCELED; + struct xchk_dirent save_de = { + .namelen = name->len, + .ino = ip->i_ino, + }; + + /* Couldn't lock the inode, so save the dirent for later. */ + trace_xchk_dir_defer(sc->ip, name->name, name->len, ip->i_ino); + + error = xfblob_store(sd->dir_names, &save_de.name_cookie, + name->name, name->len); + if (!xchk_fblock_xref_process_error(sc, XFS_DATA_FORK, 0, + &error)) + return error; + + error = xfarray_append(sd->dir_entries, &save_de); + if (!xchk_fblock_xref_process_error(sc, XFS_DATA_FORK, 0, + &error)) + return error; + + return 0; } error = xchk_dir_parent_pointer(sd, name, ip); @@ -875,6 +918,147 @@ xchk_directory_blocks( return error; } +/* + * Revalidate a dirent that we collected in the past but couldn't check because + * of lock contention. Returns 0 if the dirent is still valid, -ENOENT if it + * has gone away on us, or a negative errno. + */ +STATIC int +xchk_dir_revalidate_dirent( + struct xchk_dir *sd, + const struct xfs_name *xname, + xfs_ino_t ino) +{ + struct xfs_scrub *sc = sd->sc; + xfs_ino_t child_ino; + int error; + + /* + * Look up the directory entry. If we get -ENOENT, the directory entry + * went away and there's nothing to revalidate. Return any other + * error. + */ + error = xchk_dir_lookup(sc, sc->ip, xname, &child_ino); + if (error) + return error; + + /* The inode number changed, nothing to revalidate. */ + if (ino != child_ino) + return -ENOENT; + + return 0; +} + +/* + * Check a directory entry's parent pointers the slow way, which means we cycle + * locks a bunch and put up with revalidation until we get it done. + */ +STATIC int +xchk_dir_slow_dirent( + struct xchk_dir *sd, + struct xchk_dirent *dirent) +{ + struct xfs_name xname = { + .name = sd->namebuf, + .len = dirent->namelen, + }; + struct xfs_scrub *sc = sd->sc; + struct xfs_inode *ip; + unsigned int lockmode; + int error; + + /* Check that the deferred dirent still exists. */ + if (sd->need_revalidate) { + error = xchk_dir_revalidate_dirent(sd, &xname, dirent->ino); + if (error == -ENOENT) + return 0; + if (!xchk_fblock_xref_process_error(sc, XFS_DATA_FORK, 0, + &error)) + return error; + } + + error = xchk_iget(sc, dirent->ino, &ip); + if (error == -EINVAL || error == -ENOENT) { + xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0); + return 0; + } + if (!xchk_fblock_xref_process_error(sc, XFS_DATA_FORK, 0, &error)) + return error; + + /* + * If we can grab both IOLOCK and ILOCK of the alleged child, we can + * proceed with the validation. + */ + lockmode = xchk_dir_lock_child(sc, ip); + if (lockmode) { + trace_xchk_dir_slowpath(sc->ip, xname.name, xname.len, + ip->i_ino); + goto check_pptr; + } + + /* + * We couldn't lock the child file. Drop all the locks and try to + * get them again, one at a time. + */ + xchk_iunlock(sc, sc->ilock_flags); + sd->need_revalidate = true; + + trace_xchk_dir_ultraslowpath(sc->ip, xname.name, xname.len, ip->i_ino); + + error = xchk_dir_trylock_for_pptrs(sc, ip, &lockmode); + if (error) + goto out_rele; + + /* Revalidate, since we just cycled the locks. */ + error = xchk_dir_revalidate_dirent(sd, &xname, dirent->ino); + if (error == -ENOENT) { + error = 0; + goto out_unlock; + } + if (!xchk_fblock_xref_process_error(sc, XFS_DATA_FORK, 0, &error)) + goto out_unlock; + +check_pptr: + error = xchk_dir_parent_pointer(sd, &xname, ip); +out_unlock: + xfs_iunlock(ip, lockmode); +out_rele: + xchk_irele(sc, ip); + return error; +} + +/* Check all the dirents that we deferred the first time around. */ +STATIC int +xchk_dir_finish_slow_dirents( + struct xchk_dir *sd) +{ + xfarray_idx_t array_cur; + int error; + + foreach_xfarray_idx(sd->dir_entries, array_cur) { + struct xchk_dirent dirent; + + if (sd->sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT) + return 0; + + error = xfarray_load(sd->dir_entries, array_cur, &dirent); + if (error) + return error; + + error = xfblob_load(sd->dir_names, dirent.name_cookie, + sd->namebuf, dirent.namelen); + if (error) + return error; + sd->namebuf[MAXNAMELEN - 1] = 0; + + error = xchk_dir_slow_dirent(sd, &dirent); + if (error) + return error; + } + + return 0; +} + /* Scrub a whole directory. */ int xchk_directory( @@ -918,10 +1102,54 @@ xchk_directory( return -ENOMEM; sd->sc = sc; + if (xfs_has_parent(sc->mp)) { + char *descr; + + /* + * Set up some staging memory for dirents that we can't check + * due to locking contention. + */ + descr = xchk_xfile_ino_descr(sc, "slow directory entries"); + error = xfarray_create(descr, 0, sizeof(struct xchk_dirent), + &sd->dir_entries); + kfree(descr); + if (error) + goto out_sd; + + descr = xchk_xfile_ino_descr(sc, "slow directory entry names"); + error = xfblob_create(descr, &sd->dir_names); + kfree(descr); + if (error) + goto out_entries; + } + /* Look up every name in this directory by hash. */ error = xchk_dir_walk(sc, sc->ip, xchk_dir_actor, sd); + if (error == -ECANCELED) + error = 0; + if (error) + goto out_names; + + if (xfs_has_parent(sc->mp)) { + error = xchk_dir_finish_slow_dirents(sd); + if (error == -ETIMEDOUT) { + /* Couldn't grab a lock, scrub was marked incomplete */ + error = 0; + goto out_names; + } + if (error) + goto out_names; + } + +out_names: + if (sd->dir_names) + xfblob_destroy(sd->dir_names); +out_entries: + if (sd->dir_entries) + xfarray_destroy(sd->dir_entries); +out_sd: kvfree(sd); - if (error && error != -ECANCELED) + if (error) return error; /* If the dir is clean, it is clearly not zapped. */ diff --git a/fs/xfs/scrub/readdir.c b/fs/xfs/scrub/readdir.c index d70dbbd4c9040..c9c875485b870 100644 --- a/fs/xfs/scrub/readdir.c +++ b/fs/xfs/scrub/readdir.c @@ -18,6 +18,7 @@ #include "xfs_trans.h" #include "xfs_error.h" #include "scrub/scrub.h" +#include "scrub/common.h" #include "scrub/readdir.h" /* Call a function for every entry in a shortform directory. */ @@ -382,3 +383,59 @@ xchk_dir_lookup( *ino = args.inumber; return error; } + +/* + * Try for a limited time to grab the IOLOCK and ILOCK of both the scrub target + * (@sc->ip) and the inode at the other end (@ip) of a directory or parent + * pointer link so that we can check that link. + * + * We do not know ahead of time that the directory tree is /not/ corrupt, so we + * cannot use the "lock two inode" functions because we do not know that there + * is not a racing thread trying to take the locks in opposite order. First + * take IOLOCK_EXCL of the scrub target, and then try to take IOLOCK_SHARED + * of @ip to synchronize with the VFS. Next, take ILOCK_EXCL of the scrub + * target and @ip to synchronize with XFS. + * + * If the trylocks succeed, *lockmode will be set to the locks held for @ip; + * @sc->ilock_flags will be set for the locks held for @sc->ip; and zero will + * be returned. If not, returns -EDEADLOCK to try again; or -ETIMEDOUT if + * XCHK_TRY_HARDER was set. Returns -EINTR if the process has been killed. + */ +int +xchk_dir_trylock_for_pptrs( + struct xfs_scrub *sc, + struct xfs_inode *ip, + unsigned int *lockmode) +{ + unsigned int nr; + int error = 0; + + ASSERT(sc->ilock_flags == 0); + + *lockmode = 0; + for (nr = 0; nr < HZ; nr++) { + xchk_ilock(sc, XFS_IOLOCK_EXCL); + if (xfs_ilock_nowait(ip, XFS_IOLOCK_SHARED)) { + xchk_ilock(sc, XFS_ILOCK_EXCL); + if (xfs_ilock_nowait(ip, XFS_ILOCK_EXCL)) { + *lockmode = XFS_IOLOCK_SHARED | XFS_ILOCK_EXCL; + return 0; + } + xchk_iunlock(sc, XFS_ILOCK_EXCL); + xfs_iunlock(ip, XFS_IOLOCK_SHARED); + } + xchk_iunlock(sc, XFS_IOLOCK_EXCL); + + if (xchk_should_terminate(sc, &error)) + return error; + + delay(1); + } + + if (sc->flags & XCHK_TRY_HARDER) { + xchk_set_incomplete(sc); + return -ETIMEDOUT; + } + + return -EDEADLOCK; +} diff --git a/fs/xfs/scrub/readdir.h b/fs/xfs/scrub/readdir.h index 55787f4df123f..da501877a64dd 100644 --- a/fs/xfs/scrub/readdir.h +++ b/fs/xfs/scrub/readdir.h @@ -16,4 +16,7 @@ int xchk_dir_walk(struct xfs_scrub *sc, struct xfs_inode *dp, int xchk_dir_lookup(struct xfs_scrub *sc, struct xfs_inode *dp, const struct xfs_name *name, xfs_ino_t *ino); +int xchk_dir_trylock_for_pptrs(struct xfs_scrub *sc, struct xfs_inode *ip, + unsigned int *lockmode); + #endif /* __XFS_SCRUB_READDIR_H__ */ diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h index 8d863f4737e90..651b73d33f2c4 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -1523,6 +1523,40 @@ DEFINE_EVENT(xchk_nlinks_diff_class, name, \ TP_ARGS(mp, ip, live)) DEFINE_SCRUB_NLINKS_DIFF_EVENT(xchk_nlinks_compare_inode); +DECLARE_EVENT_CLASS(xchk_pptr_class, + TP_PROTO(struct xfs_inode *ip, const unsigned char *name, + unsigned int namelen, xfs_ino_t far_ino), + TP_ARGS(ip, name, namelen, far_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, far_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->far_ino = far_ino; + ), + TP_printk("dev %d:%d ino 0x%llx name '%.*s' far_ino 0x%llx", + MAJOR(__entry->dev), MINOR(__entry->dev), + __entry->ino, + __entry->namelen, + __get_str(name), + __entry->far_ino) +) +#define DEFINE_XCHK_PPTR_EVENT(name) \ +DEFINE_EVENT(xchk_pptr_class, name, \ + TP_PROTO(struct xfs_inode *ip, const unsigned char *name, \ + unsigned int namelen, xfs_ino_t far_ino), \ + TP_ARGS(ip, name, namelen, far_ino)) +DEFINE_XCHK_PPTR_EVENT(xchk_dir_defer); +DEFINE_XCHK_PPTR_EVENT(xchk_dir_slowpath); +DEFINE_XCHK_PPTR_EVENT(xchk_dir_ultraslowpath); + /* repair tracepoints */ #if IS_ENABLED(CONFIG_XFS_ONLINE_REPAIR) From patchwork Sun Dec 31 20:53:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507513 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 281CBBE47 for ; Sun, 31 Dec 2023 20:53:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="ifqNxaQD" Received: by smtp.kernel.org (Postfix) with ESMTPSA id EFBB0C433C7; Sun, 31 Dec 2023 20:53:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056039; bh=4BtqwoN7TfcXn/JRXkfHWnYOf9sFmHW4mTBuGRYR288=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=ifqNxaQDAQQo5HzCTwRklwTFvXpiU0/IUW5xQedtK3q/i+eyEo2sTV0ToEWAYI1fi NN8nfKW/dNi3IOk+KuIXd/oxc+d2xXWzFp4N4zQAu9iY1PZDNT8fg9ekBLbmHwxfA8 CQY3LFFYYBStyq6dx+3znJZb/JzWbs5U+x8NenslqjQv0brM754jvwF8Pste1rky8H 2vaFS0tcBn+KclIl1Qltl6jCX7wd9rQ2k92a+pP/Mzlm8Ga3K6Xlfjlgb344hbsSXC I9bUmc9nNu77c6lLEY6RN1IMAIs7ZR4Iq8ognhGEt5VAEEViZ8rgBRFSJrXcvDxjmH B3t7dAkoZ8/fQ== Date: Sun, 31 Dec 2023 12:53:58 -0800 Subject: [PATCH 03/22] xfs: create a parent pointer walk function for scrubbers From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841796.1757392.12691168335602590612.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong Build a parent pointer iteration function off of the existing xattr walking code. This will be used by subsequent patches. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/listxattr.c | 84 +++++++++++++++++++++++++++++++++++++++++++--- fs/xfs/scrub/listxattr.h | 9 +++++ 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/fs/xfs/scrub/listxattr.c b/fs/xfs/scrub/listxattr.c index c8d7d7d723177..dc893f2cdc1c3 100644 --- a/fs/xfs/scrub/listxattr.c +++ b/fs/xfs/scrub/listxattr.c @@ -17,11 +17,46 @@ #include "xfs_attr_leaf.h" #include "xfs_attr_sf.h" #include "xfs_trans.h" +#include "xfs_parent.h" #include "scrub/scrub.h" #include "scrub/bitmap.h" #include "scrub/dab_bitmap.h" #include "scrub/listxattr.h" +struct xchk_pptr_walk { + struct xfs_parent_name_irec *pptr_buf; + xchk_pptr_fn fn; + void *priv; +}; + +/* Call the parent pointer callback if this xattr is a valid parent pointer. */ +STATIC int +xchk_pptr_walk_attr( + struct xfs_scrub *sc, + struct xfs_inode *ip, + unsigned int attr_flags, + const unsigned char *name, + unsigned int namelen, + const void *value, + unsigned int valuelen, + void *priv) +{ + struct xchk_pptr_walk *pw = priv; + const struct xfs_parent_name_rec *rec = (const void *)name; + + /* Ignore anything that isn't a parent pointer. */ + if (!(attr_flags & XFS_ATTR_PARENT)) + return 0; + + if (!xfs_parent_namecheck(sc->mp, rec, namelen, attr_flags)) + return -EFSCORRUPTED; + if (!xfs_parent_valuecheck(sc->mp, value, valuelen)) + return -EFSCORRUPTED; + + xfs_parent_irec_from_disk(pw->pptr_buf, rec, value, valuelen); + return pw->fn(sc, ip, pw->pptr_buf, pw->priv); +} + /* Call a function for every entry in a shortform xattr structure. */ STATIC int xchk_xattr_walk_sf( @@ -37,9 +72,16 @@ xchk_xattr_walk_sf( sf = (struct xfs_attr_shortform *)ip->i_af.if_u1.if_data; for (i = 0, sfe = &sf->list[0]; i < sf->hdr.count; i++) { - error = attr_fn(sc, ip, sfe->flags, sfe->nameval, sfe->namelen, - &sfe->nameval[sfe->namelen], sfe->valuelen, - priv); + if (attr_fn == xchk_pptr_walk_attr) + error = xchk_pptr_walk_attr(sc, ip, sfe->flags, + sfe->nameval, sfe->namelen, + &sfe->nameval[sfe->namelen], + sfe->valuelen, priv); + else + error = attr_fn(sc, ip, sfe->flags, + sfe->nameval, sfe->namelen, + &sfe->nameval[sfe->namelen], + sfe->valuelen, priv); if (error) return error; @@ -91,8 +133,12 @@ xchk_xattr_walk_leaf_entries( valuelen = be32_to_cpu(name_rmt->valuelen); } - error = attr_fn(sc, ip, entry->flags, name, namelen, value, - valuelen, priv); + if (attr_fn == xchk_pptr_walk_attr) + error = xchk_pptr_walk_attr(sc, ip, entry->flags, name, + namelen, value, valuelen, priv); + else + error = attr_fn(sc, ip, entry->flags, name, namelen, + value, valuelen, priv); if (error) return error; @@ -308,3 +354,31 @@ xchk_xattr_walk( return xchk_xattr_walk_node(sc, ip, attr_fn, priv); } + +/* + * Walk every parent pointer of this file. The parent pointer will be + * formatted into the provided @pptr_buf, which is then passed to the callback + * function. + * + * The callback function must decide if an invalid parent_ino or invalid name + * should halt the parent pointer walk; the only validation done here is the + * structure of the xattrs themselves. + */ +int +xchk_pptr_walk( + struct xfs_scrub *sc, + struct xfs_inode *ip, + xchk_pptr_fn pptr_fn, + struct xfs_parent_name_irec *pptr_buf, + void *priv) +{ + struct xchk_pptr_walk pw = { + .fn = pptr_fn, + .pptr_buf = pptr_buf, + .priv = priv, + }; + + ASSERT(xfs_has_parent(sc->mp)); + + return xchk_xattr_walk(sc, ip, xchk_pptr_walk_attr, &pw); +} diff --git a/fs/xfs/scrub/listxattr.h b/fs/xfs/scrub/listxattr.h index 48fe89d05946b..7e4bd3ae75e15 100644 --- a/fs/xfs/scrub/listxattr.h +++ b/fs/xfs/scrub/listxattr.h @@ -14,4 +14,13 @@ typedef int (*xchk_xattr_fn)(struct xfs_scrub *sc, struct xfs_inode *ip, int xchk_xattr_walk(struct xfs_scrub *sc, struct xfs_inode *ip, xchk_xattr_fn attr_fn, void *priv); +struct xfs_parent_name_irec; + +typedef int (*xchk_pptr_fn)(struct xfs_scrub *sc, struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, void *priv); + +int xchk_pptr_walk(struct xfs_scrub *sc, struct xfs_inode *ip, + xchk_pptr_fn pptr_fn, struct xfs_parent_name_irec *pptr_buf, + void *priv); + #endif /* __XFS_SCRUB_LISTXATTR_H__ */ From patchwork Sun Dec 31 20:54:14 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507514 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DBB29BE47 for ; Sun, 31 Dec 2023 20:54:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="eygVCcyL" Received: by smtp.kernel.org (Postfix) with ESMTPSA id AA825C433C8; Sun, 31 Dec 2023 20:54:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056054; bh=DFsEN+kMWF9xa9lAejan4dDw+nqwfuMkYCgtMmc6p2k=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=eygVCcyLZ6hrQyOjWdQycF+9OS1u7DaWAAoN66yEAUDmsN3MGsFd1uihXm5pkhVkd bsRejOMe/VAZllN9crFttAJIrJgdd6jYxXMggtrQ7epyZnAcUzbCUfAuUxCCete5j9 myqLhrrgeonX4+aL/1v5PosZ2sNWT7XdX1Vyz+vqua4ZlcKPdU+g0tziHCHRmbnTpj ng+J+gjkFGCVR76ElifozJl+UqOMvVx8pSdL3F6ykBZKgm3ckYoev4AFawkd4NEWx5 bjOeXznOgYokrnQL+f0fQBp8FRaS92UJQJBBGCT72mXhno6O2C7NUPIHHZ+vkxpLrb MtHM8IkYC0sXA== Date: Sun, 31 Dec 2023 12:54:14 -0800 Subject: [PATCH 04/22] xfs: scrub parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841812.1757392.16847937374256749426.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong Actually check parent pointers now. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/parent.c | 385 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 385 insertions(+) diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c index acb6282c3d148..7a5c57cdf93e4 100644 --- a/fs/xfs/scrub/parent.c +++ b/fs/xfs/scrub/parent.c @@ -15,11 +15,15 @@ #include "xfs_icache.h" #include "xfs_dir2.h" #include "xfs_dir2_priv.h" +#include "xfs_attr.h" +#include "xfs_parent.h" #include "scrub/scrub.h" #include "scrub/common.h" #include "scrub/readdir.h" #include "scrub/tempfile.h" #include "scrub/repair.h" +#include "scrub/listxattr.h" +#include "scrub/trace.h" /* Set us up to scrub parents. */ int @@ -197,6 +201,384 @@ xchk_parent_validate( return error; } +/* + * Checking of Parent Pointers + * =========================== + * + * On filesystems with directory parent pointers, we check the referential + * integrity by visiting each parent pointer of a child file and checking that + * the directory referenced by the pointer actually has a dirent pointing + * forward to the child file. + */ + +struct xchk_pptrs { + struct xfs_scrub *sc; + + /* Scratch buffer for scanning pptr xattrs */ + struct xfs_parent_name_irec pptr; + + /* How many parent pointers did we find at the end? */ + unsigned long long pptrs_found; + + /* Parent of this directory. */ + xfs_ino_t parent_ino; +}; + +/* Does this parent pointer match the dotdot entry? */ +STATIC int +xchk_parent_scan_dotdot( + struct xfs_scrub *sc, + struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, + void *priv) +{ + struct xchk_pptrs *pp = priv; + + if (pp->parent_ino == pptr->p_ino && + xfs_parent_verify_irec(sc->mp, pptr)) + return -ECANCELED; + + return 0; +} + +/* Look up the dotdot entry so that we can check it as we walk the pptrs. */ +STATIC int +xchk_parent_pptr_and_dotdot( + struct xchk_pptrs *pp) +{ + struct xfs_scrub *sc = pp->sc; + int error; + + /* Look up '..' */ + error = xchk_dir_lookup(sc, sc->ip, &xfs_name_dotdot, &pp->parent_ino); + if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, 0, &error)) + return error; + if (!xfs_verify_dir_ino(sc->mp, pp->parent_ino)) { + xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0); + return 0; + } + + /* Is this the root dir? Then '..' must point to itself. */ + if (sc->ip == sc->mp->m_rootip) { + if (sc->ip->i_ino != pp->parent_ino) + xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0); + return 0; + } + + /* + * If this is now an unlinked directory, the dotdot value is + * meaningless as long as it points to a valid inode. + */ + if (VFS_I(sc->ip)->i_nlink == 0) + return 0; + + if (pp->sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT) + return 0; + + /* Otherwise, walk the pptrs again, and check. */ + error = xchk_pptr_walk(sc, sc->ip, xchk_parent_scan_dotdot, &pp->pptr, + pp); + if (error == -ECANCELED) { + /* Found a parent pointer that matches dotdot. */ + return 0; + } + if (!error || error == -EFSCORRUPTED) { + /* Found a broken parent pointer or no match. */ + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return 0; + } + return error; +} + +/* + * Try to lock a parent directory for checking dirents. Returns the inode + * flags for the locks we now hold, or zero if we failed. + */ +STATIC unsigned int +xchk_parent_lock_dir( + struct xfs_scrub *sc, + struct xfs_inode *dp) +{ + if (!xfs_ilock_nowait(dp, XFS_IOLOCK_SHARED)) + return 0; + + if (!xfs_ilock_nowait(dp, XFS_ILOCK_SHARED)) { + xfs_iunlock(dp, XFS_IOLOCK_SHARED); + return 0; + } + + if (!xfs_need_iread_extents(&dp->i_df)) + return XFS_IOLOCK_SHARED | XFS_ILOCK_SHARED; + + xfs_iunlock(dp, XFS_ILOCK_SHARED); + + if (!xfs_ilock_nowait(dp, XFS_ILOCK_EXCL)) { + xfs_iunlock(dp, XFS_IOLOCK_SHARED); + return 0; + } + + return XFS_IOLOCK_SHARED | XFS_ILOCK_EXCL; +} + +/* Check the forward link (dirent) associated with this parent pointer. */ +STATIC int +xchk_parent_dirent( + struct xchk_pptrs *pp, + struct xfs_inode *dp) +{ + struct xfs_name xname = { + .name = pp->pptr.p_name, + .len = pp->pptr.p_namelen, + }; + struct xfs_scrub *sc = pp->sc; + xfs_ino_t child_ino; + int error; + + /* + * Use the name attached to this parent pointer to look up the + * directory entry in the alleged parent. + */ + error = xchk_dir_lookup(sc, dp, &xname, &child_ino); + if (error == -ENOENT) { + xchk_fblock_xref_set_corrupt(sc, XFS_ATTR_FORK, 0); + return 0; + } + if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, &error)) + return error; + + /* Does the inode number match? */ + if (child_ino != sc->ip->i_ino) { + xchk_fblock_xref_set_corrupt(sc, XFS_ATTR_FORK, 0); + return 0; + } + + return 0; +} + +/* Try to grab a parent directory. */ +STATIC int +xchk_parent_iget( + struct xchk_pptrs *pp, + struct xfs_inode **dpp) +{ + struct xfs_scrub *sc = pp->sc; + struct xfs_inode *ip; + int error; + + /* Validate inode number. */ + error = xfs_dir_ino_validate(sc->mp, pp->pptr.p_ino); + if (error) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + + error = xchk_iget(sc, pp->pptr.p_ino, &ip); + if (error == -EINVAL || error == -ENOENT) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, &error)) + return error; + + /* The parent must be a directory. */ + if (!S_ISDIR(VFS_I(ip)->i_mode)) { + xchk_fblock_xref_set_corrupt(sc, XFS_ATTR_FORK, 0); + goto out_rele; + } + + /* Validate generation number. */ + if (VFS_I(ip)->i_generation != pp->pptr.p_gen) { + xchk_fblock_xref_set_corrupt(sc, XFS_ATTR_FORK, 0); + goto out_rele; + } + + *dpp = ip; + return 0; +out_rele: + xchk_irele(sc, ip); + return 0; +} + +/* + * Walk an xattr of a file. If this xattr is a parent pointer, follow it up + * to a parent directory and check that the parent has a dirent pointing back + * to us. + */ +STATIC int +xchk_parent_scan_attr( + struct xfs_scrub *sc, + struct xfs_inode *ip, + unsigned int attr_flags, + const unsigned char *name, + unsigned int namelen, + const void *value, + unsigned int valuelen, + void *priv) +{ + struct xfs_name dname = { + .name = value, + .len = valuelen, + }; + struct xchk_pptrs *pp = priv; + struct xfs_inode *dp = NULL; + const struct xfs_parent_name_rec *rec = (const void *)name; + unsigned int lockmode; + xfs_dahash_t computed_hash; + int error; + + /* Ignore anything that isn't a parent pointer. */ + if (!(attr_flags & XFS_ATTR_PARENT)) + return 0; + + /* Does the ondisk parent pointer structure make sense? */ + if (!xfs_parent_namecheck(sc->mp, rec, namelen, attr_flags)) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + + if (!xfs_parent_valuecheck(sc->mp, value, valuelen)) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + + xfs_parent_irec_from_disk(&pp->pptr, rec, value, valuelen); + + if (!xfs_parent_verify_irec(sc->mp, &pp->pptr)) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + + /* No self-referential parent pointers. */ + if (pp->pptr.p_ino == sc->ip->i_ino) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + + /* + * If the namehash of the dirent name encoded in the parent pointer + * attr value doesn't match the namehash in the parent pointer key, + * the parent pointer is corrupt. + */ + computed_hash = xfs_dir2_hashname(ip->i_mount, &dname); + if (pp->pptr.p_namehash != computed_hash) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + pp->pptrs_found++; + + error = xchk_parent_iget(pp, &dp); + if (error) + return error; + if (!dp) + return 0; + + /* Try to lock the inode. */ + lockmode = xchk_parent_lock_dir(sc, dp); + if (!lockmode) { + xchk_set_incomplete(sc); + error = -ECANCELED; + goto out_rele; + } + + error = xchk_parent_dirent(pp, dp); + if (error) + goto out_unlock; + +out_unlock: + xfs_iunlock(dp, lockmode); +out_rele: + xchk_irele(sc, dp); + return error; +} + +/* + * Compare the number of parent pointers to the link count. For + * non-directories these should be the same. For unlinked directories the + * count should be zero; for linked directories, it should be nonzero. + */ +STATIC int +xchk_parent_count_pptrs( + struct xchk_pptrs *pp) +{ + struct xfs_scrub *sc = pp->sc; + + if (S_ISDIR(VFS_I(sc->ip)->i_mode)) { + if (sc->ip == sc->mp->m_rootip) + pp->pptrs_found++; + + if (VFS_I(sc->ip)->i_nlink == 0 && pp->pptrs_found > 0) + xchk_ino_set_corrupt(sc, sc->ip->i_ino); + else if (VFS_I(sc->ip)->i_nlink > 0 && + pp->pptrs_found == 0) + xchk_ino_set_corrupt(sc, sc->ip->i_ino); + } else { + if (VFS_I(sc->ip)->i_nlink != pp->pptrs_found) + xchk_ino_set_corrupt(sc, sc->ip->i_ino); + } + + return 0; +} + +/* Check parent pointers of a file. */ +STATIC int +xchk_parent_pptr( + struct xfs_scrub *sc) +{ + struct xchk_pptrs *pp; + int error; + + pp = kvzalloc(sizeof(struct xchk_pptrs), XCHK_GFP_FLAGS); + if (!pp) + return -ENOMEM; + pp->sc = sc; + + error = xchk_xattr_walk(sc, sc->ip, xchk_parent_scan_attr, pp); + if (error == -ECANCELED) { + error = 0; + goto out_pp; + } + if (error) + goto out_pp; + + if (pp->sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT) + goto out_pp; + + /* + * For subdirectories, make sure the dotdot entry references the same + * inode as the parent pointers. + * + * If we're scanning a /consistent/ directory, there should only be + * one parent pointer, and it should point to the same directory as + * the dotdot entry. + * + * However, a corrupt directory tree might feature a subdirectory with + * multiple parents. The directory loop scanner is responsible for + * correcting that kind of problem, so for now we only validate that + * the dotdot entry matches /one/ of the parents. + */ + if (S_ISDIR(VFS_I(sc->ip)->i_mode)) { + error = xchk_parent_pptr_and_dotdot(pp); + if (error) + goto out_pp; + } + + if (pp->sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT) + goto out_pp; + + /* + * Complain if the number of parent pointers doesn't match the link + * count. This could be a sign of missing parent pointers (or an + * incorrect link count). + */ + error = xchk_parent_count_pptrs(pp); + if (error) + goto out_pp; + +out_pp: + kvfree(pp); + return error; +} + /* Scrub a parent pointer. */ int xchk_parent( @@ -206,6 +588,9 @@ xchk_parent( xfs_ino_t parent_ino; int error = 0; + if (xfs_has_parent(mp)) + return xchk_parent_pptr(sc); + /* * If we're a directory, check that the '..' link points up to * a directory that has one entry pointing to us. From patchwork Sun Dec 31 20:54:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507515 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7DC73BE48 for ; Sun, 31 Dec 2023 20:54:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="pUcd2anO" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 52384C433C7; Sun, 31 Dec 2023 20:54:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056070; bh=rVRKBRG1jB29+0/pBrDLc/xjr3sLo368h+BH/aoeaHE=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=pUcd2anO1LkMdnKXeKH5CRUNtV9kN+7AyPx+VnHY6HwIFWj/Ry4B71KU7g860cbYv VwgtSXXHKhuAGEIsG/3Qw7wdnpEYMttLKLJhAOb+tWubyo1+QUoqK/RM8iUm8TSwQo dDmakzWPvWNZS99XZv5OGNTxuvLFsR6x6S4rE9/XVUExVDms3f92NwGMcXL7w/y6kp yFQjUeO6SJPkqqG1yXT9n6ILvRR3fdsrDSba4zB+PBipxgUC5+F+Vu6guFtXCM+K2K 3pDluNQ4TfFNFx6lkGAwMv68PjNLy5BFccaid0/hAl5/NhRCuFzB6Qcn4fNrdRx67d RVsiMkuAICgMg== Date: Sun, 31 Dec 2023 12:54:29 -0800 Subject: [PATCH 05/22] xfs: deferred scrub of parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841827.1757392.2308960241267202377.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong 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 --- fs/xfs/Makefile | 2 fs/xfs/scrub/parent.c | 263 ++++++++++++++++++++++++++++++++++++++++++++++++- fs/xfs/scrub/trace.h | 3 + 3 files changed, 260 insertions(+), 8 deletions(-) diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile index 49480c81eaeab..52ef808359966 100644 --- a/fs/xfs/Makefile +++ b/fs/xfs/Makefile @@ -177,6 +177,7 @@ xfs-y += $(addprefix scrub/, \ scrub.o \ symlink.o \ xfarray.o \ + xfblob.o \ xfile.o \ ) @@ -218,7 +219,6 @@ xfs-y += $(addprefix scrub/, \ rmap_repair.o \ symlink_repair.o \ tempfile.o \ - xfblob.o \ xfbtree.o \ ) diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c index 7a5c57cdf93e4..3bacd3e14f5d3 100644 --- a/fs/xfs/scrub/parent.c +++ b/fs/xfs/scrub/parent.c @@ -23,6 +23,9 @@ #include "scrub/tempfile.h" #include "scrub/repair.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. */ @@ -211,17 +214,42 @@ xchk_parent_validate( * forward 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; + + /* 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; + /* How many parent pointers did we find at the end? */ unsigned long long pptrs_found; /* 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; }; /* Does this parent pointer match the dotdot entry? */ @@ -475,8 +503,27 @@ 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, + .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_xref_process_error(sc, XFS_ATTR_FORK, 0, + &error)) + goto out_rele; + + error = xfarray_append(pp->pptr_entries, &save_pp); + if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, + &error)) + goto out_rele; + goto out_rele; } @@ -491,6 +538,159 @@ 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 error; + + error = xfs_parent_lookup(sc->tp, sc->ip, &pp->pptr, + &pp->pptr_scratch); + if (error == -ENOATTR) { + /* Parent pointer went away, nothing to revalidate. */ + return -ENOENT; + } + + return error; +} + +/* + * 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; + + 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; + xfs_parent_irec_hashname(sc->mp, &pp->pptr); + + /* 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) { + trace_xchk_parent_slowpath(sc->ip, pp->pptr.p_name, + pptr->namelen, dp->i_ino); + 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_ultraslowpath(sc->ip, pp->pptr.p_name, pptr->namelen, + dp->i_ino); + + error = xchk_dir_trylock_for_pptrs(sc, dp, &lockmode); + if (error) + goto out_rele; + + /* Revalidate the parent pointer now that we cycled locks. */ + error = xchk_parent_revalidate_pptr(pp); + if (error == -ENOENT) { + error = 0; + 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; +} + +/* Count the number of parent pointers. */ +STATIC int +xchk_parent_count_pptr( + struct xfs_scrub *sc, + struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, + void *priv) +{ + struct xchk_pptrs *pp = priv; + + if (!xfs_parent_verify_irec(sc->mp, pptr)) + return -EFSCORRUPTED; + + pp->pptrs_found++; + return 0; +} + /* * Compare the number of parent pointers to the link count. For * non-directories these should be the same. For unlinked directories the @@ -501,6 +701,24 @@ xchk_parent_count_pptrs( struct xchk_pptrs *pp) { struct xfs_scrub *sc = pp->sc; + int error; + + /* + * If we cycled the ILOCK while cross-checking parent pointers with + * dirents, then we need to recalculate the number of parent pointers. + */ + if (pp->need_revalidate) { + pp->pptrs_found = 0; + error = xchk_pptr_walk(sc, sc->ip, xchk_parent_count_pptr, + &pp->pptr, pp); + if (error == -EFSCORRUPTED) { + /* Found a bad parent pointer */ + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return 0; + } + if (error) + return error; + } if (S_ISDIR(VFS_I(sc->ip)->i_mode)) { if (sc->ip == sc->mp->m_rootip) @@ -525,6 +743,7 @@ xchk_parent_pptr( struct xfs_scrub *sc) { struct xchk_pptrs *pp; + char *descr; int error; pp = kvzalloc(sizeof(struct xchk_pptrs), XCHK_GFP_FLAGS); @@ -532,16 +751,42 @@ xchk_parent_pptr( return -ENOMEM; pp->sc = sc; + /* + * Set up some staging memory for parent pointers that we can't check + * due to locking contention. + */ + descr = xchk_xfile_ino_descr(sc, "slow parent pointer entries"); + error = xfarray_create(descr, 0, sizeof(struct xchk_pptr), + &pp->pptr_entries); + kfree(descr); + if (error) + goto out_pp; + + descr = xchk_xfile_ino_descr(sc, "slow parent pointer names"); + error = xfblob_create(descr, &pp->pptr_names); + kfree(descr); + 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 == -ETIMEDOUT) { + /* Couldn't grab a lock, scrub was marked incomplete */ + error = 0; + goto out_names; + } + if (error) + goto out_names; if (pp->sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT) - goto out_pp; + goto out_names; /* * For subdirectories, make sure the dotdot entry references the same @@ -559,7 +804,7 @@ xchk_parent_pptr( if (S_ISDIR(VFS_I(sc->ip)->i_mode)) { error = xchk_parent_pptr_and_dotdot(pp); if (error) - goto out_pp; + goto out_names; } if (pp->sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT) @@ -572,8 +817,12 @@ xchk_parent_pptr( */ error = xchk_parent_count_pptrs(pp); if (error) - goto out_pp; + 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 651b73d33f2c4..f90743453cd22 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -1556,6 +1556,9 @@ DEFINE_EVENT(xchk_pptr_class, name, \ DEFINE_XCHK_PPTR_EVENT(xchk_dir_defer); DEFINE_XCHK_PPTR_EVENT(xchk_dir_slowpath); DEFINE_XCHK_PPTR_EVENT(xchk_dir_ultraslowpath); +DEFINE_XCHK_PPTR_EVENT(xchk_parent_defer); +DEFINE_XCHK_PPTR_EVENT(xchk_parent_slowpath); +DEFINE_XCHK_PPTR_EVENT(xchk_parent_ultraslowpath); /* repair tracepoints */ #if IS_ENABLED(CONFIG_XFS_ONLINE_REPAIR) From patchwork Sun Dec 31 20:54:45 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507516 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6D1E6BE47 for ; Sun, 31 Dec 2023 20:54:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="ThcUv7LY" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E987AC433C8; Sun, 31 Dec 2023 20:54:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056086; bh=cfNXhmZjp5OESzVf6tcSygolCmlhAQzhDjiA4pQu1oE=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=ThcUv7LYYqZaSmCXxivf52CjTSZx7TP6Y8rnYAGDGUS6kdh1ulR9UbtuzRmL39Dbg kenik8qYS/duUApiGX0dT33Z/LriVv3erb/YdsoSzvPdbryXlfwU6+CUuY/7hx232y vI3n7IYKmuj+otVCnyOT+3QOIlmLPyCFnlpqm5cFXrEWvLb3tof0kL1ih8NNBdbwwd TazbRArPycgm+/b3dx6y3Qm7hvpqcp09rwOHlFrue21IbasfWk8xS0Ll8HbvSOh7Ko JcOFY+RyyglexN0P+tYrDCsgVLdHKW+tshuDOiZa91Iw9+yt6/usJhuZEHhKMeP0Ql 3gWnMi7TdFmVw== Date: Sun, 31 Dec 2023 12:54:45 -0800 Subject: [PATCH 06/22] xfs: walk directory parent pointers to determine backref count From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841843.1757392.17064146414014228788.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong If the filesystem has parent pointers enabled, walk the parent pointers of subdirectories to determine the true backref count. In theory each subdir should have a single parent reachable via dotdot, but in the case of (corrupt) subdirs with multiple parents, we need to keep the link counts high enough that the directory loop detector will be able to correct the multiple parents problems. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/common.h | 1 + fs/xfs/scrub/nlinks.c | 71 +++++++++++++++++++++++++++++++++++++++++- fs/xfs/scrub/nlinks.h | 3 ++ fs/xfs/scrub/nlinks_repair.c | 2 + fs/xfs/scrub/parent.c | 61 ++++++++++++++++++++++++++++++++++++ fs/xfs/scrub/trace.c | 1 + fs/xfs/scrub/trace.h | 27 ++++++++++++++++ 7 files changed, 165 insertions(+), 1 deletion(-) diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h index 2e6af46519b58..298669ca2eb92 100644 --- a/fs/xfs/scrub/common.h +++ b/fs/xfs/scrub/common.h @@ -212,6 +212,7 @@ static inline bool xchk_skip_xref(struct xfs_scrub_metadata *sm) } bool xchk_dir_looks_zapped(struct xfs_inode *dp); +bool xchk_pptr_looks_zapped(struct xfs_inode *ip); #ifdef CONFIG_XFS_ONLINE_REPAIR /* Decide if a repair is required. */ diff --git a/fs/xfs/scrub/nlinks.c b/fs/xfs/scrub/nlinks.c index 6f0b77da14dbb..4e62e287e1590 100644 --- a/fs/xfs/scrub/nlinks.c +++ b/fs/xfs/scrub/nlinks.c @@ -18,6 +18,7 @@ #include "xfs_dir2.h" #include "xfs_dir2_priv.h" #include "xfs_ag.h" +#include "xfs_parent.h" #include "scrub/scrub.h" #include "scrub/common.h" #include "scrub/repair.h" @@ -29,6 +30,7 @@ #include "scrub/trace.h" #include "scrub/readdir.h" #include "scrub/tempfile.h" +#include "scrub/listxattr.h" /* * Live Inode Link Count Checking @@ -268,12 +270,17 @@ xchk_nlinks_collect_dirent( * number of parents of the root directory. * * Otherwise, increment the number of backrefs pointing back to ino. + * + * If the filesystem has parent pointers, we walk the pptrs to + * determine the backref count. */ if (dotdot) { if (dp == sc->mp->m_rootip) error = xchk_nlinks_update_incore(xnc, ino, 1, 0, 0); - else + else if (!xfs_has_parent(sc->mp)) error = xchk_nlinks_update_incore(xnc, ino, 0, 1, 0); + else + error = 0; if (error) goto out_unlock; } @@ -310,6 +317,46 @@ xchk_nlinks_collect_dirent( return error; } +/* Bump the backref count for the inode referenced by this parent pointer. */ +STATIC int +xchk_nlinks_collect_pptr( + struct xfs_scrub *sc, + struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, + void *priv) +{ + struct xchk_nlink_ctrs *xnc = priv; + int error; + + if (!xfs_parent_verify_irec(sc->mp, pptr)) + return -EFSCORRUPTED; + + /* Update the shadow link counts if we haven't already failed. */ + + if (xchk_iscan_aborted(&xnc->collect_iscan)) { + error = -ECANCELED; + goto out_incomplete; + } + + trace_xchk_nlinks_collect_pptr(sc->mp, ip, pptr); + + mutex_lock(&xnc->lock); + + error = xchk_nlinks_update_incore(xnc, pptr->p_ino, 0, 1, 0); + if (error) + goto out_unlock; + + mutex_unlock(&xnc->lock); + return 0; + +out_unlock: + mutex_unlock(&xnc->lock); + xchk_iscan_abort(&xnc->collect_iscan); +out_incomplete: + xchk_set_incomplete(sc); + return error; +} + /* Walk a directory to bump the observed link counts of the children. */ STATIC int xchk_nlinks_collect_dir( @@ -356,6 +403,28 @@ xchk_nlinks_collect_dir( if (error) goto out_abort; + /* Walk the parent pointers to get real backref counts. */ + if (xfs_has_parent(sc->mp)) { + /* + * If the extended attributes look as though they has been + * zapped by the inode record repair code, we cannot scan for + * parent pointers. + */ + if (xchk_pptr_looks_zapped(dp)) { + error = -EBUSY; + goto out_unlock; + } + + error = xchk_pptr_walk(sc, dp, xchk_nlinks_collect_pptr, + &xnc->pptr, xnc); + if (error == -ECANCELED) { + error = 0; + goto out_unlock; + } + if (error) + goto out_abort; + } + xchk_iscan_mark_visited(&xnc->collect_iscan, dp); goto out_unlock; diff --git a/fs/xfs/scrub/nlinks.h b/fs/xfs/scrub/nlinks.h index f4766e01b6469..2d63cb56b6a3c 100644 --- a/fs/xfs/scrub/nlinks.h +++ b/fs/xfs/scrub/nlinks.h @@ -23,6 +23,9 @@ struct xchk_nlink_ctrs { struct xchk_iscan collect_iscan; struct xchk_iscan compare_iscan; + /* Parent pointer for finding backrefs. */ + struct xfs_parent_name_irec pptr; + /* * Hook into directory updates so that we can receive live updates * from other writer threads. diff --git a/fs/xfs/scrub/nlinks_repair.c b/fs/xfs/scrub/nlinks_repair.c index 87cb3400ff948..fb299b23d5f1d 100644 --- a/fs/xfs/scrub/nlinks_repair.c +++ b/fs/xfs/scrub/nlinks_repair.c @@ -18,6 +18,8 @@ #include "xfs_ialloc.h" #include "xfs_sb.h" #include "xfs_ag.h" +#include "xfs_dir2.h" +#include "xfs_parent.h" #include "scrub/scrub.h" #include "scrub/common.h" #include "scrub/repair.h" diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c index 3bacd3e14f5d3..555aee4b73b37 100644 --- a/fs/xfs/scrub/parent.c +++ b/fs/xfs/scrub/parent.c @@ -883,3 +883,64 @@ xchk_parent( return error; } + +/* + * Decide if this file's extended attributes (and therefore its parent + * pointers) have been zapped to satisfy the inode and ifork verifiers. + * Checking and repairing should be postponed until the extended attribute + * structure is fixed. + */ +bool +xchk_pptr_looks_zapped( + struct xfs_inode *ip) +{ + struct xfs_mount *mp = ip->i_mount; + struct inode *inode = VFS_I(ip); + + ASSERT(xfs_has_parent(mp)); + + /* + * Temporary files that cannot be linked into the directory tree do not + * have attr forks because they cannot ever have parents. + */ + if (inode->i_nlink == 0 && !(inode->i_state & I_LINKABLE)) + return false; + + /* + * Directory tree roots do not have parents, so the expected outcome + * of a parent pointer scan is always the empty set. It's safe to scan + * them even if the attr fork was zapped. + */ + if (ip == mp->m_rootip) + return false; + + /* + * Metadata inodes are all rooted in the superblock and do not have + * any parents. Hence the attr fork will not be initialized, but + * there are no parent pointers that might have been zapped. + */ + if (xfs_is_metadata_inode(ip)) + return false; + + /* + * Linked and linkable non-rootdir files should always have an + * attribute fork because that is where parent pointers are + * stored. If the fork is absent, something is amiss. + */ + if (!xfs_inode_has_attr_fork(ip)) + return true; + + /* Repair zapped this file's attr fork a short time ago */ + if (xfs_ifork_zapped(ip, XFS_ATTR_FORK)) + return true; + + /* + * If the dinode repair found a bad attr fork, it will reset the fork + * to extents format with zero records and wait for the bmapbta + * scrubber to reconstruct the block mappings. The extended attribute + * structure always contain some content when parent pointers are + * enabled, so this is a clear sign of a zapped attr fork. + */ + return ip->i_af.if_format == XFS_DINODE_FMT_EXTENTS && + ip->i_af.if_nextents == 0; +} diff --git a/fs/xfs/scrub/trace.c b/fs/xfs/scrub/trace.c index e127f6d492c35..9fe1491adbb51 100644 --- a/fs/xfs/scrub/trace.c +++ b/fs/xfs/scrub/trace.c @@ -20,6 +20,7 @@ #include "xfs_da_format.h" #include "xfs_dir2.h" #include "xfs_rmap.h" +#include "xfs_parent.h" #include "scrub/scrub.h" #include "scrub/xfile.h" #include "scrub/xfarray.h" diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h index f90743453cd22..d3a0cefea3684 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -28,6 +28,7 @@ struct xchk_fscounters; struct xfbtree; struct xfbtree_config; struct xfs_rmap_update_params; +struct xfs_parent_name_irec; /* * ftrace's __print_symbolic requires that all enum values be wrapped in the @@ -1375,6 +1376,32 @@ TRACE_EVENT(xchk_nlinks_collect_dirent, __get_str(name)) ); +TRACE_EVENT(xchk_nlinks_collect_pptr, + TP_PROTO(struct xfs_mount *mp, struct xfs_inode *dp, + const struct xfs_parent_name_irec *pptr), + TP_ARGS(mp, dp, pptr), + TP_STRUCT__entry( + __field(dev_t, dev) + __field(xfs_ino_t, dir) + __field(xfs_ino_t, ino) + __field(unsigned int, namelen) + __dynamic_array(char, name, pptr->p_namelen) + ), + TP_fast_assign( + __entry->dev = mp->m_super->s_dev; + __entry->dir = dp->i_ino; + __entry->ino = pptr->p_ino; + __entry->namelen = pptr->p_namelen; + memcpy(__get_str(name), pptr->p_name, pptr->p_namelen); + ), + TP_printk("dev %d:%d dir 0x%llx -> ino 0x%llx name '%.*s'", + MAJOR(__entry->dev), MINOR(__entry->dev), + __entry->dir, + __entry->ino, + __entry->namelen, + __get_str(name)) +); + TRACE_EVENT(xchk_nlinks_collect_metafile, TP_PROTO(struct xfs_mount *mp, xfs_ino_t ino), TP_ARGS(mp, ino), From patchwork Sun Dec 31 20:55:01 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507517 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 20C24BE48 for ; Sun, 31 Dec 2023 20:55:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Ylog1p4u" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9C993C433C7; Sun, 31 Dec 2023 20:55:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056101; bh=PVOD6VsNJh2HLiLwvJjKBI4W8e7g8wP3BcFNHRYVrYI=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=Ylog1p4urLUTm++RgYiPgJDppSmXXsc60eaea47e3me2FTwrJQCabNHNB7JalWC7G N8bx7LbF9IeAD+Cu2cKbCRY277b8ME3uHLWeVAQxnrVWEukJzyRBU985JuOtCYiqOF Duoc0JmqeuwdHkvyTzillLOMg3aG0hZPp2uDk7D8rK9I17uDe0xb6HOoh4Vbi+sz7b K+maSUIQI3BgQv/ms+xvyaSV5GtHtOVWm3xttI3+QjNqC5yRBt0w/8yvTBB4MvCnYJ SGOzjZ1DjxocmwIRE/ACDC3yFkbKx5qol6S9ULn/kF/j6P+EEV115mTAt0FRSdjGSI 1/bBJwZTCno0A== Date: Sun, 31 Dec 2023 12:55:01 -0800 Subject: [PATCH 07/22] xfs: add raw parent pointer apis to support repair From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841860.1757392.15083095858894465536.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong Add a couple of utility functions to set or remove parent pointers from a file. These functions will be used by repair code, hence they skip the xattr logging that regular parent pointer updates use. Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_dir2.c | 2 +- fs/xfs/libxfs/xfs_dir2.h | 2 +- fs/xfs/libxfs/xfs_parent.c | 46 ++++++++++++++++++++++++++++++++++++++++++++ fs/xfs/libxfs/xfs_parent.h | 8 ++++++++ 4 files changed, 56 insertions(+), 2 deletions(-) diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c index 51eed639f2dfe..525b23a3800b6 100644 --- a/fs/xfs/libxfs/xfs_dir2.c +++ b/fs/xfs/libxfs/xfs_dir2.c @@ -440,7 +440,7 @@ int xfs_dir_removename( struct xfs_trans *tp, struct xfs_inode *dp, - struct xfs_name *name, + const struct xfs_name *name, xfs_ino_t ino, xfs_extlen_t total) /* bmap's total block count */ { diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h index f99788a1f3e63..ca1949ed4f5e8 100644 --- a/fs/xfs/libxfs/xfs_dir2.h +++ b/fs/xfs/libxfs/xfs_dir2.h @@ -55,7 +55,7 @@ extern int xfs_dir_lookup(struct xfs_trans *tp, struct xfs_inode *dp, const struct xfs_name *name, xfs_ino_t *inum, struct xfs_name *ci_name); extern int xfs_dir_removename(struct xfs_trans *tp, struct xfs_inode *dp, - struct xfs_name *name, xfs_ino_t ino, + const struct xfs_name *name, xfs_ino_t ino, xfs_extlen_t tot); extern int xfs_dir_replace(struct xfs_trans *tp, struct xfs_inode *dp, const struct xfs_name *name, xfs_ino_t inum, diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c index 09495eb368e2b..3c31c04dd9a20 100644 --- a/fs/xfs/libxfs/xfs_parent.c +++ b/fs/xfs/libxfs/xfs_parent.c @@ -420,3 +420,49 @@ xfs_parent_lookup( return xfs_attr_get_ilocked(&scr->args); } + +/* + * Attach the parent pointer (@pptr -> @name) to @ip immediately. Caller must + * not have a transaction or hold the ILOCK. This is for specialized repair + * functions only. The scratchpad need not be initialized. + */ +int +xfs_parent_set( + struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, + struct xfs_parent_scratch *scr) +{ + if (XFS_IS_CORRUPT(ip->i_mount, + !xfs_parent_verify_irec(ip->i_mount, pptr))) { + return -EFSCORRUPTED; + } + + xfs_parent_irec_to_disk(&scr->rec, pptr); + xfs_parent_scratch_init(NULL, ip, pptr, scr); + scr->args.op_flags |= XFS_DA_OP_LOGGED; + + return xfs_attr_set(&scr->args); +} + +/* + * Remove the parent pointer (@rec -> @name) from @ip immediately. Caller must + * not have a transaction or hold the ILOCK. This is for specialized repair + * functions only. The scratchpad need not be initialized. + */ +int +xfs_parent_unset( + struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, + struct xfs_parent_scratch *scr) +{ + if (XFS_IS_CORRUPT(ip->i_mount, + !xfs_parent_verify_irec(ip->i_mount, pptr))) { + return -EFSCORRUPTED; + } + + xfs_parent_irec_to_disk(&scr->rec, pptr); + xfs_parent_scratch_init(NULL, ip, pptr, scr); + scr->args.op_flags |= XFS_DA_OP_LOGGED | XFS_DA_OP_REMOVE; + + return xfs_attr_set(&scr->args); +} diff --git a/fs/xfs/libxfs/xfs_parent.h b/fs/xfs/libxfs/xfs_parent.h index e4443da1d86f2..58e59af818bd2 100644 --- a/fs/xfs/libxfs/xfs_parent.h +++ b/fs/xfs/libxfs/xfs_parent.h @@ -162,4 +162,12 @@ int xfs_parent_lookup(struct xfs_trans *tp, struct xfs_inode *ip, const struct xfs_parent_name_irec *pptr, struct xfs_parent_scratch *scratch); +int xfs_parent_set(struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, + struct xfs_parent_scratch *scratch); + +int xfs_parent_unset(struct xfs_inode *ip, + const struct xfs_parent_name_irec *rec, + struct xfs_parent_scratch *scratch); + #endif /* __XFS_PARENT_H__ */ From patchwork Sun Dec 31 20:55:16 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507518 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 716F0BE48 for ; Sun, 31 Dec 2023 20:55:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="FlYvoKWN" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3ED69C433C8; Sun, 31 Dec 2023 20:55:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056117; bh=AAh2zwCRiKXP26tuh6koTV2RapJBUqzMqxLP7Y6muVQ=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=FlYvoKWNLdkwz5H/vywfebGpt0tZ384zowyj/SLEqRozQXpRyB3tHVYnoyWoOSKSZ uzBsosN8EEo4FWMDt7c9Loc9Pvc+P+IjaEZGTVunbRhywbuxAqSpjFg2M3C+2Cdref ozHRNQIuASK0kQ3qxiJmuxqVuXHBg2U6c9W5G090KZah1dj6YobhdQByldGpZ/OcgC wu3Wdwz9m+fPo+wIxCNgvFOIgiJucJifiIFUiow+axQfcibaLuPWlTM5wnl84WgRzJ LoBW0I3kiNtpLQ+uuGqZFRTKl8Muy5lpTTLXeapEWL/awsxfG9opEmDpDCOghFMg50 KOiHnxI1awydw== Date: Sun, 31 Dec 2023 12:55:16 -0800 Subject: [PATCH 08/22] xfs: set child file owner in xfs_da_args when changing parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841876.1757392.10798507434953639051.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong Now that struct xfs_da_args has an explicit file owner field, we must set it when modifying parent pointers. Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_parent.c | 13 ++++++++++--- fs/xfs/libxfs/xfs_parent.h | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c index 3c31c04dd9a20..3c3fcdf8b975b 100644 --- a/fs/xfs/libxfs/xfs_parent.c +++ b/fs/xfs/libxfs/xfs_parent.c @@ -201,6 +201,7 @@ xfs_parent_addname( args->trans = tp; args->dp = child; + args->owner = child->i_ino; xfs_init_parent_davalue(&ppargs->args, parent_name); @@ -239,6 +240,7 @@ xfs_parent_removename( args->trans = tp; args->dp = child; + args->owner = child->i_ino; xfs_init_parent_davalue(&ppargs->args, parent_name); @@ -288,6 +290,7 @@ xfs_parent_replacename( args->trans = tp; args->dp = child; + args->owner = child->i_ino; xfs_init_parent_davalue(&ppargs->args, old_name); xfs_init_parent_danewvalue(&ppargs->args, new_name); @@ -371,6 +374,7 @@ static inline void xfs_parent_scratch_init( struct xfs_trans *tp, struct xfs_inode *ip, + xfs_ino_t owner, const struct xfs_parent_name_irec *pptr, struct xfs_parent_scratch *scr) { @@ -387,6 +391,7 @@ xfs_parent_scratch_init( scr->args.whichfork = XFS_ATTR_FORK; scr->args.hashval = xfs_da_hashname((const void *)&scr->rec, sizeof(struct xfs_parent_name_rec)); + scr->args.owner = owner; } /* @@ -415,7 +420,7 @@ xfs_parent_lookup( } xfs_parent_irec_to_disk(&scr->rec, pptr); - xfs_parent_scratch_init(tp, ip, pptr, scr); + xfs_parent_scratch_init(tp, ip, ip->i_ino, pptr, scr); scr->args.op_flags |= XFS_DA_OP_OKNOENT; return xfs_attr_get_ilocked(&scr->args); @@ -429,6 +434,7 @@ xfs_parent_lookup( int xfs_parent_set( struct xfs_inode *ip, + xfs_ino_t owner, const struct xfs_parent_name_irec *pptr, struct xfs_parent_scratch *scr) { @@ -438,7 +444,7 @@ xfs_parent_set( } xfs_parent_irec_to_disk(&scr->rec, pptr); - xfs_parent_scratch_init(NULL, ip, pptr, scr); + xfs_parent_scratch_init(NULL, ip, owner, pptr, scr); scr->args.op_flags |= XFS_DA_OP_LOGGED; return xfs_attr_set(&scr->args); @@ -452,6 +458,7 @@ xfs_parent_set( int xfs_parent_unset( struct xfs_inode *ip, + xfs_ino_t owner, const struct xfs_parent_name_irec *pptr, struct xfs_parent_scratch *scr) { @@ -461,7 +468,7 @@ xfs_parent_unset( } xfs_parent_irec_to_disk(&scr->rec, pptr); - xfs_parent_scratch_init(NULL, ip, pptr, scr); + xfs_parent_scratch_init(NULL, ip, owner, pptr, scr); scr->args.op_flags |= XFS_DA_OP_LOGGED | XFS_DA_OP_REMOVE; return xfs_attr_set(&scr->args); diff --git a/fs/xfs/libxfs/xfs_parent.h b/fs/xfs/libxfs/xfs_parent.h index 58e59af818bd2..46bf96c7e3c92 100644 --- a/fs/xfs/libxfs/xfs_parent.h +++ b/fs/xfs/libxfs/xfs_parent.h @@ -162,11 +162,11 @@ int xfs_parent_lookup(struct xfs_trans *tp, struct xfs_inode *ip, const struct xfs_parent_name_irec *pptr, struct xfs_parent_scratch *scratch); -int xfs_parent_set(struct xfs_inode *ip, +int xfs_parent_set(struct xfs_inode *ip, xfs_ino_t owner, const struct xfs_parent_name_irec *pptr, struct xfs_parent_scratch *scratch); -int xfs_parent_unset(struct xfs_inode *ip, +int xfs_parent_unset(struct xfs_inode *ip, xfs_ino_t owner, const struct xfs_parent_name_irec *rec, struct xfs_parent_scratch *scratch); From patchwork Sun Dec 31 20:55:32 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507519 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1C72EBA22 for ; Sun, 31 Dec 2023 20:55:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="T5miBxkN" Received: by smtp.kernel.org (Postfix) with ESMTPSA id DE803C433C7; Sun, 31 Dec 2023 20:55:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056132; bh=Hr+qeTsHrsqVdYcuuf93Dh3Ap0WQdNuMwalY6xZLdZs=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=T5miBxkN7RTNfLECEqy9blRgWObYmJXHKWbp+NvjW/BfjX/WxyjYMQlBD/yJ7u37l QZ+OBcrmJGKFgWg6iPq9vdu/RyO74o16um9QkvcGL53MESkn/vKk5V7rzKJhyaYnNG t5MZ5u1fqsAbKHvRJED0uFX0pDW5NhySN+3oD6ww78/vyXR65zrGlvg7/sOF/r+BhX 1bWnRwNJHAXJIuom3z/+O4nF/6KS1HoeghDGw+i+7N3jmuy7H+VtbXCpHxhGDUIMCi GKEWvGZd3dRuX3s9SRwKNBauU8bb+xRhUd3OfFUFH0dCi1okW62aohIuXCnGmWa6Wj MBDBSqzDxJsCg== Date: Sun, 31 Dec 2023 12:55:32 -0800 Subject: [PATCH 09/22] xfs: salvage parent pointers when rebuilding xattr structures From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841892.1757392.12387417515945378983.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong When we're salvaging extended attributes, make sure we validate the ones that claim to be parent pointers before adding them to the salvage pile. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/attr_repair.c | 41 ++++++++++++++++++++++++++++++++--------- fs/xfs/scrub/trace.h | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 9 deletions(-) diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c index 9a88d46392626..39e64d7559451 100644 --- a/fs/xfs/scrub/attr_repair.c +++ b/fs/xfs/scrub/attr_repair.c @@ -28,6 +28,7 @@ #include "xfs_swapext.h" #include "xfs_xchgrange.h" #include "xfs_acl.h" +#include "xfs_parent.h" #include "scrub/xfs_scrub.h" #include "scrub/scrub.h" #include "scrub/common.h" @@ -124,6 +125,13 @@ xrep_xattr_want_salvage( return false; if (valuelen > XATTR_SIZE_MAX || valuelen < 0) return false; + if (attr_flags & XFS_ATTR_PARENT) { + if (!xfs_parent_namecheck(rx->sc->mp, name, namelen, + attr_flags)) + return false; + if (!xfs_parent_valuecheck(rx->sc->mp, value, valuelen)) + return false; + } return true; } @@ -151,14 +159,21 @@ xrep_xattr_salvage_key( * Truncate the name to the first character that would trip namecheck. * If we no longer have a name after that, ignore this attribute. */ - while (i < namelen && name[i] != 0) - i++; - if (i == 0) - return 0; - key.namelen = i; + if (flags & XFS_ATTR_PARENT) { + key.namelen = namelen; - trace_xrep_xattr_salvage_rec(rx->sc->ip, flags, name, key.namelen, - valuelen); + trace_xrep_xattr_salvage_pptr(rx->sc->ip, flags, name, + key.namelen, value, valuelen); + } else { + while (i < namelen && name[i] != 0) + i++; + if (i == 0) + return 0; + key.namelen = i; + + trace_xrep_xattr_salvage_rec(rx->sc->ip, flags, name, + key.namelen, valuelen); + } error = xfblob_store(rx->xattr_blobs, &key.name_cookie, name, key.namelen); @@ -562,6 +577,9 @@ xrep_xattr_insert_rec( struct xchk_xattr_buf *ab = rx->sc->buf; int error; + if (key->flags & XFS_ATTR_PARENT) + args.op_flags |= XFS_DA_OP_NVLOOKUP; + /* * Grab pointers to the scrub buffer so that we can use them to insert * attrs into the temp file. @@ -595,8 +613,13 @@ xrep_xattr_insert_rec( ab->name[key->namelen] = 0; - trace_xrep_xattr_insert_rec(rx->sc->tempip, key->flags, ab->name, - key->namelen, key->valuelen); + if (key->flags & XFS_ATTR_PARENT) + trace_xrep_xattr_insert_pptr(rx->sc->tempip, key->flags, + ab->name, key->namelen, ab->value, + key->valuelen); + else + trace_xrep_xattr_insert_rec(rx->sc->tempip, key->flags, + ab->name, key->namelen, key->valuelen); /* * xfs_attr_set creates and commits its own transaction. If the attr diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h index d3a0cefea3684..8fa26a7811118 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -2662,6 +2662,46 @@ DEFINE_EVENT(xrep_xattr_salvage_class, name, \ DEFINE_XREP_XATTR_SALVAGE_EVENT(xrep_xattr_salvage_rec); DEFINE_XREP_XATTR_SALVAGE_EVENT(xrep_xattr_insert_rec); +DECLARE_EVENT_CLASS(xrep_pptr_salvage_class, + TP_PROTO(struct xfs_inode *ip, unsigned int flags, const void *name, + unsigned int namelen, const void *value, unsigned int valuelen), + TP_ARGS(ip, flags, name, namelen, value, valuelen), + TP_STRUCT__entry( + __field(dev_t, dev) + __field(xfs_ino_t, ino) + __field(xfs_ino_t, parent_ino) + __field(unsigned int, parent_gen) + __field(unsigned int, namelen) + __dynamic_array(char, name, valuelen) + ), + TP_fast_assign( + struct xfs_parent_name_irec pptr; + + xfs_parent_irec_from_disk(&pptr, name, value, valuelen); + + __entry->dev = ip->i_mount->m_super->s_dev; + __entry->ino = ip->i_ino; + __entry->parent_ino = pptr.p_ino; + __entry->parent_gen = pptr.p_gen; + __entry->namelen = pptr.p_namelen; + memcpy(__get_str(name), pptr.p_name, pptr.p_namelen); + ), + TP_printk("dev %d:%d ino 0x%llx parent_ino 0x%llx parent_gen 0x%x name '%.*s'", + MAJOR(__entry->dev), MINOR(__entry->dev), + __entry->ino, + __entry->parent_ino, + __entry->parent_gen, + __entry->namelen, + __get_str(name)) +) +#define DEFINE_XREP_PPTR_SALVAGE_EVENT(name) \ +DEFINE_EVENT(xrep_pptr_salvage_class, name, \ + TP_PROTO(struct xfs_inode *ip, unsigned int flags, const void *name, \ + unsigned int namelen, const void *value, unsigned int valuelen), \ + TP_ARGS(ip, flags, name, namelen, value, valuelen)) +DEFINE_XREP_PPTR_SALVAGE_EVENT(xrep_xattr_salvage_pptr); +DEFINE_XREP_PPTR_SALVAGE_EVENT(xrep_xattr_insert_pptr); + TRACE_EVENT(xrep_xattr_class, TP_PROTO(struct xfs_inode *ip, struct xfs_inode *arg_ip), TP_ARGS(ip, arg_ip), From patchwork Sun Dec 31 20:55:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507520 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0E37DBA22 for ; Sun, 31 Dec 2023 20:55:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="qeugsk3u" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8BEE7C433C7; Sun, 31 Dec 2023 20:55:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056148; bh=gdkb+v7R1wApk5ryGYe7Bg3XezkfrVlA+1qZIKhaDoU=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=qeugsk3uERrvinH8fuKow+9CFDYcQNK3HRLu0fVCEcsj0+HYyxOqDimk8fX+ztiN9 3QEFLO4hd2iIAfTPqkd6DdOSyiJtzOHFTVZFypwlbZj0UJA6rMRtkU1fjVybC/pi+O PLPumUvqz8XxnLNKOMSDJ+D600IRS19bbbmsFzkZzlxDAthqAWKC6jg9zO0/UgtKmg JULnetEWx8blv6IPhvVBjP1KKF5UjpHbDWSnjb7kKZx4SWeK6QwabfpvkwLgfQ6wP/ 3FrURmeBg8zw56jIXYSNKSnaMlCkUC6N3W9cfrPKs5CezWYe8l+qqhbN30nHgIuTEA 70sflTPddW4ow== Date: Sun, 31 Dec 2023 12:55:48 -0800 Subject: [PATCH 10/22] xfs: replace namebuf with parent pointer in directory repair From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841908.1757392.15682934584944618427.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong Replace the dirent name buffer at the end of struct xrep_dir with a xfs_parent_name_irec object. The namebuf and p_name usage do not overlap, so we can save 256 bytes of memory by allowing them to overlap. Doing so makes the code a bit more complex, so this is called out separately. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/dir_repair.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c index e74f456c7b444..13a1a3ef5e714 100644 --- a/fs/xfs/scrub/dir_repair.c +++ b/fs/xfs/scrub/dir_repair.c @@ -28,6 +28,7 @@ #include "xfs_swapext.h" #include "xfs_xchgrange.h" #include "xfs_ag.h" +#include "xfs_parent.h" #include "scrub/xfs_scrub.h" #include "scrub/scrub.h" #include "scrub/common.h" @@ -131,8 +132,14 @@ struct xrep_dir { /* Should we move this directory to the orphanage? */ bool needs_adoption; - /* Directory entry name, plus the trailing null. */ - unsigned char namebuf[MAXNAMELEN]; + /* + * Scratch buffer for reading parent pointers from child files. The + * p_name field is used to flush stashed dirents into the temporary + * directory in between parent pointers. At the very end of the + * repair, it can also be used to compute the lost+found filename + * if we need to reparent the directory. + */ + struct xfs_parent_name_irec pptr; }; /* Tear down all the incore stuff we created. */ @@ -696,7 +703,7 @@ xrep_dir_replay_update( struct xfs_name name = { .len = dirent->namelen, .type = dirent->ftype, - .name = rd->namebuf, + .name = rd->pptr.p_name, }; struct xfs_mount *mp = rd->sc->mp; #ifdef DEBUG @@ -773,10 +780,10 @@ xrep_dir_replay_updates( /* The dirent name is stored in the in-core buffer. */ error = xfblob_load(rd->dir_names, dirent.name_cookie, - rd->namebuf, dirent.namelen); + rd->pptr.p_name, dirent.namelen); if (error) return error; - rd->namebuf[MAXNAMELEN - 1] = 0; + rd->pptr.p_name[MAXNAMELEN - 1] = 0; error = xrep_dir_replay_update(rd, &dirent); if (error) @@ -1416,7 +1423,7 @@ xrep_dir_move_to_orphanage( if (error) return error; - error = xrep_adoption_compute_name(&rd->adoption, rd->namebuf); + error = xrep_adoption_compute_name(&rd->adoption, rd->pptr.p_name); if (error) return error; From patchwork Sun Dec 31 20:56:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507521 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CB67FBA2B for ; Sun, 31 Dec 2023 20:56:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="J+QOAbcR" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 410F6C433C7; Sun, 31 Dec 2023 20:56:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056164; bh=NpcDrm8AHsyt+wGTzzxf3TveO6DvfZSjuPyVO6fz8NA=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=J+QOAbcRmJ8khZMxn06uN0UMJRZSn+dXyIdstwUTtyur6w3c8DgCGMErS/+7Mbq05 bN4xha8rTKrsorm+U+Bo6Xc/QUV6S42C6XO/auEva70wth9yJKTs2Un+5S4xrxC8kd JIkNGVbtpW5v905qWkDQLtygirLq5tmu4L63fFqCFaA1ergd6pLXbIo6IMGmH0xgER JkrCLmw+O3vmp5ibk9t0EwxRJxnTUH6dgGru2qAk2TK2hO47hNtdGNLCZKQLdb1a/+ U8TJohGpM0o2ux4ptOx5w9e6a3XT+Z99uGdIzH3H0/YFI0+Ha+dUUUnluz/ibPqy6a LmmtgxIbAKvfg== Date: Sun, 31 Dec 2023 12:56:03 -0800 Subject: [PATCH 11/22] xfs: repair directories by scanning directory parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841924.1757392.7346522989217789457.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong For filesystems with parent pointers, scan the entire filesystem looking for parent pointers that target the directory we're rebuilding instead of trying to salvage whatever we can from the directory data blocks. This will be more robust than salvaging, but there's more code to come. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/dir_repair.c | 327 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 323 insertions(+), 4 deletions(-) diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c index 13a1a3ef5e714..cae22ad33bca3 100644 --- a/fs/xfs/scrub/dir_repair.c +++ b/fs/xfs/scrub/dir_repair.c @@ -44,6 +44,7 @@ #include "scrub/reap.h" #include "scrub/findparent.h" #include "scrub/orphanage.h" +#include "scrub/listxattr.h" /* * Directory Repair @@ -58,6 +59,15 @@ * being repaired and the temporary directory, and will later become important * for parent pointer scanning. * + * If parent pointers are enabled on this filesystem, we instead reconstruct + * the directory by visiting each parent pointer of each file in the filesystem + * and translating the relevant parent pointer records into dirents. In this + * case, it is advantageous to stash all directory entries created from parent + * pointers for a single child file before replaying them into the temporary + * directory. To save memory, the live filesystem scan reuses the findparent + * fields. Directory repair chooses either parent pointer scanning or + * directory entry salvaging, but not both. + * * Directory entries added to the temporary directory do not elevate the link * counts of the inodes found. When salvaging completes, the remaining stashed * entries are replayed to the temporary directory. An atomic extent swap is @@ -113,7 +123,15 @@ struct xrep_dir { /* * Information used to scan the filesystem to find the inumber of the - * dotdot entry for this directory. + * dotdot entry for this directory. For directory salvaging when + * parent pointers are not enabled, we use the findparent_* functions + * on this object and access only the parent_ino field directly. + * + * When parent pointers are enabled, however, the pptr scanner uses the + * iscan, hooks, lock, and parent_ino fields of this object directly. + * @pscan.lock coordinates access to dir_entries, dir_names, + * parent_ino, subdirs, dirents, and args. This reduces the memory + * requirements of this structure. */ struct xrep_parent_scan_info pscan; @@ -1003,6 +1021,261 @@ xrep_dir_salvage_entries( } +/* + * Examine a parent pointer of a file. If it leads us back to the directory + * that we're rebuilding, create an incore dirent from the parent pointer and + * stash it. + */ +STATIC int +xrep_dir_scan_pptr( + struct xfs_scrub *sc, + struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, + void *priv) +{ + struct xfs_name xname; + struct xrep_dir *rd = priv; + int error; + + /* + * Ignore parent pointers that point back to a different dir, list the + * wrong generation number, or are invalid. + */ + if (pptr->p_ino != sc->ip->i_ino || + pptr->p_gen != VFS_I(sc->ip)->i_generation || + !xfs_parent_verify_irec(sc->mp, pptr)) + return 0; + + /* + * Transform this parent pointer into a dirent and queue it for later + * addition to the temporary directory. + */ + xname.name = pptr->p_name; + xname.len = pptr->p_namelen; + xname.type = xfs_mode_to_ftype(VFS_I(ip)->i_mode); + + mutex_lock(&rd->pscan.lock); + error = xrep_dir_stash_createname(rd, &xname, ip->i_ino); + mutex_unlock(&rd->pscan.lock); + return error; +} + +/* + * If this child dirent points to the directory being repaired, remember that + * fact so that we can reset the dotdot entry if necessary. + */ +STATIC int +xrep_dir_scan_dirent( + struct xfs_scrub *sc, + struct xfs_inode *dp, + xfs_dir2_dataptr_t dapos, + const struct xfs_name *name, + xfs_ino_t ino, + void *priv) +{ + struct xrep_dir *rd = priv; + + /* Dirent doesn't point to this directory. */ + if (ino != rd->sc->ip->i_ino) + return 0; + + /* Ignore garbage inum. */ + if (!xfs_verify_dir_ino(rd->sc->mp, ino)) + return 0; + + /* No weird looking names. */ + if (name->len >= MAXNAMELEN || name->len <= 0) + return 0; + + /* Don't pick up dot or dotdot entries; we only want child dirents. */ + if (xfs_dir2_samename(name, &xfs_name_dotdot) || + xfs_dir2_samename(name, &xfs_name_dot)) + return 0; + + trace_xrep_dir_stash_createname(sc->tempip, &xfs_name_dotdot, + dp->i_ino); + + xrep_findparent_scan_found(&rd->pscan, dp->i_ino); + return 0; +} + +/* + * Decide if we want to look for child dirents or parent pointers in this file. + * Skip the dir being repaired and any files being used to stage repairs. + */ +static inline bool +xrep_dir_want_scan( + struct xrep_dir *rd, + const struct xfs_inode *ip) +{ + return ip != rd->sc->ip && !xrep_is_tempfile(ip); +} + +/* + * Take ILOCK on a file that we want to scan. + * + * Select ILOCK_EXCL if the file is a directory with an unloaded data bmbt or + * has an unloaded attr bmbt. Otherwise, take ILOCK_SHARED. + */ +static inline unsigned int +xrep_dir_scan_ilock( + struct xrep_dir *rd, + struct xfs_inode *ip) +{ + uint lock_mode = XFS_ILOCK_SHARED; + + /* Need to take the shared ILOCK to advance the iscan cursor. */ + if (!xrep_dir_want_scan(rd, ip)) + goto lock; + + if (S_ISDIR(VFS_I(ip)->i_mode) && xfs_need_iread_extents(&ip->i_df)) { + lock_mode = XFS_ILOCK_EXCL; + goto lock; + } + + if (xfs_inode_has_attr_fork(ip) && xfs_need_iread_extents(&ip->i_af)) + lock_mode = XFS_ILOCK_EXCL; + +lock: + xfs_ilock(ip, lock_mode); + return lock_mode; +} + +/* + * Scan this file for relevant child dirents or parent pointers that point to + * the directory we're rebuilding. + */ +STATIC int +xrep_dir_scan_file( + struct xrep_dir *rd, + struct xfs_inode *ip) +{ + unsigned int lock_mode; + int error = 0; + + lock_mode = xrep_dir_scan_ilock(rd, ip); + + if (!xrep_dir_want_scan(rd, ip)) + goto scan_done; + + /* + * If the extended attributes look as though they has been zapped by + * the inode record repair code, we cannot scan for parent pointers. + */ + if (xchk_pptr_looks_zapped(ip)) { + error = -EBUSY; + goto scan_done; + } + + error = xchk_pptr_walk(rd->sc, ip, xrep_dir_scan_pptr, &rd->pptr, rd); + if (error) + goto scan_done; + + if (S_ISDIR(VFS_I(ip)->i_mode)) { + /* + * If the directory looks as though it has been zapped by the + * inode record repair code, we cannot scan for child dirents. + */ + if (xchk_dir_looks_zapped(ip)) { + error = -EBUSY; + goto scan_done; + } + + error = xchk_dir_walk(rd->sc, ip, xrep_dir_scan_dirent, rd); + if (error) + goto scan_done; + } + +scan_done: + xchk_iscan_mark_visited(&rd->pscan.iscan, ip); + xfs_iunlock(ip, lock_mode); + return error; +} + +/* + * Scan all files in the filesystem for parent pointers that we can turn into + * replacement dirents, and a dirent that we can use to set the dotdot pointer. + */ +STATIC int +xrep_dir_scan_dirtree( + struct xrep_dir *rd) +{ + struct xfs_scrub *sc = rd->sc; + struct xfs_inode *ip; + int error; + + /* Roots of directory trees are their own parents. */ + if (sc->ip == sc->mp->m_rootip) + xrep_findparent_scan_found(&rd->pscan, sc->ip->i_ino); + + /* + * Filesystem scans are time consuming. Drop the directory ILOCK and + * all other resources for the duration of the scan and hope for the + * best. The live update hooks will keep our scan information up to + * date even though we've dropped the locks. + */ + xchk_trans_cancel(sc); + if (sc->ilock_flags & (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL)) + xchk_iunlock(sc, sc->ilock_flags & (XFS_ILOCK_SHARED | + XFS_ILOCK_EXCL)); + error = xchk_trans_alloc_empty(sc); + if (error) + return error; + + while ((error = xchk_iscan_iter(&rd->pscan.iscan, &ip)) == 1) { + bool flush; + + error = xrep_dir_scan_file(rd, ip); + xchk_irele(sc, ip); + if (error) + break; + + /* Flush stashed dirent updates to constrain memory usage. */ + mutex_lock(&rd->pscan.lock); + flush = xrep_dir_want_flush_stashed(rd); + mutex_unlock(&rd->pscan.lock); + if (flush) { + xchk_trans_cancel(sc); + + error = xrep_tempfile_iolock_polled(sc); + if (error) + break; + + mutex_lock(&rd->pscan.lock); + error = xrep_dir_replay_updates(rd); + mutex_unlock(&rd->pscan.lock); + xrep_tempfile_iounlock(sc); + if (error) + break; + + error = xchk_trans_alloc_empty(sc); + if (error) + break; + } + + if (xchk_should_terminate(sc, &error)) + break; + } + xchk_iscan_iter_finish(&rd->pscan.iscan); + if (error) { + /* + * If we couldn't grab an inode that was busy with a state + * change, change the error code so that we exit to userspace + * as quickly as possible. + */ + if (error == -EBUSY) + return -ECANCELED; + return error; + } + + /* + * Cancel the empty transaction so that we can (later) use the atomic + * extent swap helpers to lock files and commit the new directory. + */ + xchk_trans_cancel(rd->sc); + return 0; +} + /* * Free all the directory blocks and reset the data fork. The caller must * join the inode to the transaction. This function returns with the inode @@ -1201,6 +1474,45 @@ xrep_dir_set_nlink( return 0; } +/* + * Finish replaying stashed dirent updates, allocate a transaction for swapping + * extents, and take the ILOCKs of both directories before we commit the new + * directory structure. + */ +STATIC int +xrep_dir_finalize_tempdir( + struct xrep_dir *rd) +{ + struct xfs_scrub *sc = rd->sc; + int error; + + if (!xfs_has_parent(sc->mp)) + return xrep_tempswap_trans_alloc(sc, XFS_DATA_FORK, &rd->tx); + + /* + * Repair relies on the ILOCK to quiesce all possible dirent updates. + * Replay all queued dirent updates into the tempdir before swapping + * the contents, even if that means dropping the ILOCKs and the + * transaction. + */ + do { + error = xrep_dir_replay_updates(rd); + if (error) + return error; + + error = xrep_tempswap_trans_alloc(sc, XFS_DATA_FORK, &rd->tx); + if (error) + return error; + + if (xfarray_length(rd->dir_entries) == 0) + break; + + xchk_trans_cancel(sc); + xrep_tempfile_iunlock_both(sc); + } while (!xchk_should_terminate(sc, &error)); + return error; +} + /* Swap the temporary directory's data fork with the one being repaired. */ STATIC int xrep_dir_swap( @@ -1300,8 +1612,12 @@ xrep_dir_rebuild_tree( if (error) return error; - /* Allocate transaction and ILOCK the scrub file and the temp file. */ - error = xrep_tempswap_trans_alloc(sc, XFS_DATA_FORK, &rd->tx); + /* + * Allocate transaction, lock inodes, and make sure that we've replayed + * all the stashed dirent updates to the tempdir. After this point, + * we're ready to swapext. + */ + error = xrep_dir_finalize_tempdir(rd); if (error) return error; @@ -1486,7 +1802,10 @@ xrep_directory( if (error) return error; - error = xrep_dir_salvage_entries(rd); + if (xfs_has_parent(sc->mp)) + error = xrep_dir_scan_dirtree(rd); + else + error = xrep_dir_salvage_entries(rd); if (error) goto out_teardown; From patchwork Sun Dec 31 20:56:19 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507522 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1EF46BA22 for ; Sun, 31 Dec 2023 20:56:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="on7H/uP+" Received: by smtp.kernel.org (Postfix) with ESMTPSA id DFE6EC433C8; Sun, 31 Dec 2023 20:56:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056180; bh=JkoZCeKAcNaexfkgzJehyfs2knKoUB63CUkGGZIayrw=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=on7H/uP+TDqiowAGSjGkib+thfBVWnsr3LLiJRMV0sriosqavt7J6ehJ3RSwJ4aMW JzlCF+sVkU246Mzx3KK3ACxWAs1IG4f6pWhAU4oKxVAymXSVVv6EglTo9o9ynj/87U 9CvE4UnCe09/1Tb8f+K1yHVpIdw6ka3YMEydorQiO4Yq2WHax69XTXvZMFCSdmXa3E 2r9xTmOECDGVn40Ou8W5G1gZ6N8VxsCeLPE+ukH8wCf9PpGPTqq3vlGypjekobD8sW j92KE1GL4imgRYnZsQOm/zPzjNSBvrpSwagHijgHvEpfxUtEHKkzjW6WigVTFpi9gw Msk4ZKjIsVqfA== Date: Sun, 31 Dec 2023 12:56:19 -0800 Subject: [PATCH 12/22] xfs: implement live updates for directory repairs From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841940.1757392.5198713958528542546.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong While we're scanning the filesystem for parent pointers that we can turn into dirents, we cannot hold the IOLOCK or ILOCK of the directory being repaired. Therefore, we need to set up a dirent hook so that we can keep the temporary directory up to date with the rest of the filesystem. Hence we add the ability to *remove* entries from the temporary dir. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/dir_repair.c | 221 +++++++++++++++++++++++++++++++++++++++++---- fs/xfs/scrub/findparent.c | 8 +- fs/xfs/scrub/findparent.h | 10 ++ fs/xfs/scrub/trace.h | 2 4 files changed, 218 insertions(+), 23 deletions(-) diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c index cae22ad33bca3..c66838fc144d5 100644 --- a/fs/xfs/scrub/dir_repair.c +++ b/fs/xfs/scrub/dir_repair.c @@ -85,6 +85,12 @@ * or to use dirent hooks to capture updates from other threads. */ +/* Create a dirent in the tempdir. */ +#define XREP_DIRENT_ADD (1) + +/* Remove a dirent from the tempdir. */ +#define XREP_DIRENT_REMOVE (2) + /* Directory entry to be restored in the new directory. */ struct xrep_dirent { /* Cookie for retrieval of the dirent name. */ @@ -98,6 +104,9 @@ struct xrep_dirent { /* File type of the dirent. */ uint8_t ftype; + + /* XREP_DIRENT_{ADD,REMOVE} */ + uint8_t action; }; /* @@ -341,6 +350,7 @@ xrep_dir_stash_createname( xfs_ino_t ino) { struct xrep_dirent dirent = { + .action = XREP_DIRENT_ADD, .ino = ino, .namelen = name->len, .ftype = name->type, @@ -357,6 +367,34 @@ xrep_dir_stash_createname( return xfarray_append(rd->dir_entries, &dirent); } +/* + * Remember that we want to remove a dirent from the tempdir. These stashed + * actions will be replayed later. + */ +STATIC int +xrep_dir_stash_removename( + struct xrep_dir *rd, + const struct xfs_name *name, + xfs_ino_t ino) +{ + struct xrep_dirent dirent = { + .action = XREP_DIRENT_REMOVE, + .ino = ino, + .namelen = name->len, + .ftype = name->type, + }; + int error; + + trace_xrep_dir_stash_removename(rd->sc->tempip, name, ino); + + error = xfblob_store(rd->dir_names, &dirent.name_cookie, name->name, + name->len); + if (error) + return error; + + return xfarray_append(rd->dir_entries, &dirent); +} + /* Allocate an in-core record to hold entries while we rebuild the dir data. */ STATIC int xrep_dir_salvage_entry( @@ -708,6 +746,43 @@ xrep_dir_replay_createname( return xfs_dir2_node_addname(&rd->args); } +/* Replay a stashed removename onto the temporary directory. */ +STATIC int +xrep_dir_replay_removename( + struct xrep_dir *rd, + const struct xfs_name *name, + xfs_extlen_t total) +{ + struct xfs_inode *dp = rd->args.dp; + bool is_block, is_leaf; + int error; + + ASSERT(S_ISDIR(VFS_I(dp)->i_mode)); + + xrep_dir_init_args(rd, dp, name); + rd->args.op_flags = 0; + rd->args.total = total; + + trace_xrep_dir_replay_removename(dp, name, 0); + + if (dp->i_df.if_format == XFS_DINODE_FMT_LOCAL) + return xfs_dir2_sf_removename(&rd->args); + + error = xfs_dir2_isblock(&rd->args, &is_block); + if (error) + return error; + if (is_block) + return xfs_dir2_block_removename(&rd->args); + + error = xfs_dir2_isleaf(&rd->args, &is_leaf); + if (error) + return error; + if (is_leaf) + return xfs_dir2_leaf_removename(&rd->args); + + return xfs_dir2_node_removename(&rd->args); +} + /* * Add this stashed incore directory entry to the temporary directory. * The caller must hold the tempdir's IOLOCK, must not hold any ILOCKs, and @@ -739,26 +814,64 @@ xrep_dir_replay_update( xrep_tempfile_ilock(rd->sc); xfs_trans_ijoin(rd->sc->tp, rd->sc->tempip, 0); - /* - * Create a replacement dirent in the temporary directory. Note that - * _createname doesn't check for existing entries. There shouldn't be - * any in the temporary dir, but we'll verify this in debug mode. - */ + switch (dirent->action) { + case XREP_DIRENT_ADD: + /* + * Create a replacement dirent in the temporary directory. + * Note that _createname doesn't check for existing entries. + * There shouldn't be any in the temporary dir, but we'll + * verify this in debug mode. + */ #ifdef DEBUG - error = xchk_dir_lookup(rd->sc, rd->sc->tempip, &name, &ino); - if (error != -ENOENT) { - ASSERT(error != -ENOENT); + error = xchk_dir_lookup(rd->sc, rd->sc->tempip, &name, &ino); + if (error != -ENOENT) { + ASSERT(error != -ENOENT); + goto out_cancel; + } +#endif + + error = xrep_dir_replay_createname(rd, &name, dirent->ino, + resblks); + if (error) + goto out_cancel; + + if (name.type == XFS_DIR3_FT_DIR) + rd->subdirs++; + rd->dirents++; + break; + case XREP_DIRENT_REMOVE: + /* + * Remove a dirent from the temporary directory. Note that + * _removename doesn't check the inode target of the exist + * entry. There should be a perfect match in the temporary + * dir, but we'll verify this in debug mode. + */ +#ifdef DEBUG + error = xchk_dir_lookup(rd->sc, rd->sc->tempip, &name, &ino); + if (error) { + ASSERT(error != 0); + goto out_cancel; + } + if (ino != dirent->ino) { + ASSERT(ino == dirent->ino); + error = -EIO; + goto out_cancel; + } +#endif + + error = xrep_dir_replay_removename(rd, &name, resblks); + if (error) + goto out_cancel; + + if (name.type == XFS_DIR3_FT_DIR) + rd->subdirs--; + rd->dirents--; + break; + default: + ASSERT(0); + error = -EIO; goto out_cancel; } -#endif - - error = xrep_dir_replay_createname(rd, &name, dirent->ino, resblks); - if (error) - goto out_cancel; - - if (name.type == XFS_DIR3_FT_DIR) - rd->subdirs++; - rd->dirents++; /* Commit and unlock. */ error = xrep_trans_commit(rd->sc); @@ -1276,6 +1389,71 @@ xrep_dir_scan_dirtree( return 0; } +/* + * Capture dirent updates being made by other threads which are relevant to the + * directory being repaired. + */ +STATIC int +xrep_dir_live_update( + struct notifier_block *nb, + unsigned long action, + void *data) +{ + struct xfs_dir_update_params *p = data; + struct xrep_dir *rd; + struct xfs_scrub *sc; + int error = 0; + + rd = container_of(nb, struct xrep_dir, pscan.hooks.dirent_hook.nb); + sc = rd->sc; + + /* + * This thread updated a child dirent in the directory that we're + * rebuilding. Stash the update for replay against the temporary + * directory. + */ + if (p->dp->i_ino == sc->ip->i_ino && + xchk_iscan_want_live_update(&rd->pscan.iscan, p->ip->i_ino)) { + mutex_lock(&rd->pscan.lock); + if (p->delta > 0) + error = xrep_dir_stash_createname(rd, p->name, + p->ip->i_ino); + else + error = xrep_dir_stash_removename(rd, p->name, + p->ip->i_ino); + mutex_unlock(&rd->pscan.lock); + if (error) + goto out_abort; + } + + /* + * This thread updated another directory's child dirent that points to + * the directory that we're rebuilding, so remember the new dotdot + * target. + */ + if (p->ip->i_ino == sc->ip->i_ino && + xchk_iscan_want_live_update(&rd->pscan.iscan, p->dp->i_ino)) { + if (p->delta > 0) { + trace_xrep_dir_stash_createname(sc->tempip, + &xfs_name_dotdot, + p->dp->i_ino); + + xrep_findparent_scan_found(&rd->pscan, p->dp->i_ino); + } else { + trace_xrep_dir_stash_removename(sc->tempip, + &xfs_name_dotdot, + rd->pscan.parent_ino); + + xrep_findparent_scan_found(&rd->pscan, NULLFSINO); + } + } + + return NOTIFY_DONE; +out_abort: + xchk_iscan_abort(&rd->pscan.iscan); + return NOTIFY_DONE; +} + /* * Free all the directory blocks and reset the data fork. The caller must * join the inode to the transaction. This function returns with the inode @@ -1621,6 +1799,9 @@ xrep_dir_rebuild_tree( if (error) return error; + if (xchk_iscan_aborted(&rd->pscan.iscan)) + return -ECANCELED; + /* * Swap the tempdir's data fork with the file being repaired. This * recreates the transaction and re-takes the ILOCK in the scrub @@ -1676,7 +1857,11 @@ xrep_dir_setup_scan( if (error) goto out_xfarray; - error = xrep_findparent_scan_start(sc, &rd->pscan); + if (xfs_has_parent(sc->mp)) + error = __xrep_findparent_scan_start(sc, &rd->pscan, + xrep_dir_live_update); + else + error = xrep_findparent_scan_start(sc, &rd->pscan); if (error) goto out_xfblob; diff --git a/fs/xfs/scrub/findparent.c b/fs/xfs/scrub/findparent.c index 87047e9d49e47..9468029f73933 100644 --- a/fs/xfs/scrub/findparent.c +++ b/fs/xfs/scrub/findparent.c @@ -238,9 +238,10 @@ xrep_findparent_live_update( * will be called when there is a dotdot update for the inode being repaired. */ int -xrep_findparent_scan_start( +__xrep_findparent_scan_start( struct xfs_scrub *sc, - struct xrep_parent_scan_info *pscan) + struct xrep_parent_scan_info *pscan, + notifier_fn_t custom_fn) { int error; @@ -262,7 +263,8 @@ xrep_findparent_scan_start( * ILOCK, which means that any in-progress inode updates will finish * before we can scan the inode. */ - xfs_hook_setup(&pscan->hooks.dirent_hook, xrep_findparent_live_update); + xfs_hook_setup(&pscan->hooks.dirent_hook, + custom_fn ? custom_fn : xrep_findparent_live_update); error = xfs_dir_hook_add(sc->mp, &pscan->hooks); if (error) goto out_iscan; diff --git a/fs/xfs/scrub/findparent.h b/fs/xfs/scrub/findparent.h index cb3a97f3fed48..29c6077df11e5 100644 --- a/fs/xfs/scrub/findparent.h +++ b/fs/xfs/scrub/findparent.h @@ -24,8 +24,14 @@ struct xrep_parent_scan_info { bool lookup_parent; }; -int xrep_findparent_scan_start(struct xfs_scrub *sc, - struct xrep_parent_scan_info *pscan); +int __xrep_findparent_scan_start(struct xfs_scrub *sc, + struct xrep_parent_scan_info *pscan, + notifier_fn_t custom_fn); +static inline int xrep_findparent_scan_start(struct xfs_scrub *sc, + struct xrep_parent_scan_info *pscan) +{ + return __xrep_findparent_scan_start(sc, pscan, NULL); +} int xrep_findparent_scan(struct xrep_parent_scan_info *pscan); void xrep_findparent_scan_teardown(struct xrep_parent_scan_info *pscan); diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h index 8fa26a7811118..34e54ebf0daba 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -2816,6 +2816,8 @@ DEFINE_XREP_DIRENT_EVENT(xrep_dir_salvage_entry); DEFINE_XREP_DIRENT_EVENT(xrep_dir_stash_createname); DEFINE_XREP_DIRENT_EVENT(xrep_dir_replay_createname); DEFINE_XREP_DIRENT_EVENT(xrep_adoption_reparent); +DEFINE_XREP_DIRENT_EVENT(xrep_dir_stash_removename); +DEFINE_XREP_DIRENT_EVENT(xrep_dir_replay_removename); DECLARE_EVENT_CLASS(xrep_adoption_class, TP_PROTO(struct xfs_inode *dp, struct xfs_inode *ip, bool moved), From patchwork Sun Dec 31 20:56:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507523 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 08FB3BA2B for ; Sun, 31 Dec 2023 20:56:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="OgcvMryd" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 94FCCC433C7; Sun, 31 Dec 2023 20:56:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056195; bh=pmrDeUM+Hv2xaz4zSYCeQ4Q/vGbWuNRAkmHNHzJUu/8=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=OgcvMryddk1YE+o42l14E7iwX/KwophkpKz3WPIWt4IfU68UqGyN1U1qLPFopQeVI XZe21DPsRmJGmvnKxiAsBW6tOymj9k+LPcwQZGavTlJlSVpSxjLCz0B3RAzya7Iljp hGxXdRaW9pvoclW3b55wfVRzZlklcfw9Dja3WatEK0GlAETajRlk/hgEkcaoZUUYsl bytXkvUwz0OG3Qgr9ng0GKqZssUR8DrI7dZhjUWNNM2BwvxcMfIlzVACYibymfzlVY UYUivrizJ+Ww2471ghYTLh4UmzzB8JuuQXiO2yaew5k4qHwI2daNQDgHrxPNHjKUnH xqaWL03JeE6+w== Date: Sun, 31 Dec 2023 12:56:35 -0800 Subject: [PATCH 13/22] xfs: replay unlocked parent pointer updates that accrue during xattr repair From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841956.1757392.213024466857359493.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong There are a few places where the extended attribute repair code drops the ILOCK to apply stashed xattrs to the temporary file. Although setxattr and removexattr are still locked out because we retain our hold on the IOLOCK, this doesn't prevent renames from updating parent pointers, because the VFS doesn't take i_rwsem on children that are being moved. Therefore, set up a dirent hook to capture parent pointer updates for this file, and replay(?) the updates. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/attr_repair.c | 457 ++++++++++++++++++++++++++++++++++++++++++++ fs/xfs/scrub/trace.h | 71 +++++++ 2 files changed, 526 insertions(+), 2 deletions(-) diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c index 39e64d7559451..cc964dc427e23 100644 --- a/fs/xfs/scrub/attr_repair.c +++ b/fs/xfs/scrub/attr_repair.c @@ -95,6 +95,56 @@ struct xrep_xattr { /* Number of attributes that we are salvaging. */ unsigned long long attrs_found; + + /* Can we flush stashed attrs to the tempfile? */ + bool can_flush; + + /* Did the live update fail, and hence the repair is now out of date? */ + bool live_update_aborted; + + /* Lock protecting parent pointer updates */ + struct mutex lock; + + /* Fixed-size array of xrep_xattr_pptr structures. */ + struct xfarray *pptr_recs; + + /* Blobs containing parent pointer names. */ + struct xfblob *pptr_names; + + /* Hook to capture parent pointer updates. */ + struct xfs_dir_hook hooks; + + /* xattr key and da args for parent pointer replay. */ + struct xfs_parent_scratch pptr_scratch; + + /* + * Scratch buffer for scanning dirents to create pptr xattrs. At the + * very end of the repair, it can also be used to compute the + * lost+found filename if we need to reparent the file. + */ + struct xfs_parent_name_irec pptr; +}; + +/* Create a parent pointer in the tempfile. */ +#define XREP_XATTR_PPTR_ADD (1) + +/* Remove a parent pointer from the tempfile. */ +#define XREP_XATTR_PPTR_REMOVE (2) + +/* A stashed parent pointer update. */ +struct xrep_xattr_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; + + /* Length of the pptr name. */ + uint8_t namelen; + + /* XREP_XATTR_PPTR_{ADD,REMOVE} */ + uint8_t action; }; /* Set up to recreate the extended attributes. */ @@ -102,6 +152,9 @@ int xrep_setup_xattr( struct xfs_scrub *sc) { + if (xfs_has_parent(sc->mp)) + xchk_fsgates_enable(sc, XCHK_FSGATES_DIRENTS); + return xrep_tempfile_create(sc, S_IFREG); } @@ -712,11 +765,122 @@ xrep_xattr_want_flush_stashed( { unsigned long long bytes; + if (!rx->can_flush) + return false; + bytes = xfarray_bytes(rx->xattr_records) + xfblob_bytes(rx->xattr_blobs); return bytes > XREP_XATTR_MAX_STASH_BYTES; } +/* + * Did we observe rename changing parent pointer xattrs while we were flushing + * salvaged attrs? + */ +static inline bool +xrep_xattr_saw_pptr_conflict( + struct xrep_xattr *rx) +{ + bool ret; + + ASSERT(rx->can_flush); + + if (!xfs_has_parent(rx->sc->mp)) + return false; + + ASSERT(xfs_isilocked(rx->sc->ip, XFS_ILOCK_EXCL)); + + mutex_lock(&rx->lock); + ret = xfarray_bytes(rx->pptr_recs) > 0; + mutex_unlock(&rx->lock); + + return ret; +} + +/* + * Reset the entire repair state back to initial conditions, now that we've + * detected a parent pointer update to the attr structure while we were + * flushing salvaged attrs. See the locking notes in dir_repair.c for more + * information on why this is all necessary. + */ +STATIC int +xrep_xattr_full_reset( + struct xrep_xattr *rx) +{ + struct xfs_scrub *sc = rx->sc; + struct xfs_attr_sf_hdr *hdr; + struct xfs_ifork *ifp = &sc->tempip->i_af; + int error; + + trace_xrep_xattr_full_reset(sc->ip, sc->tempip); + + /* The temporary file's data fork had better not be in btree format. */ + if (sc->tempip->i_df.if_format == XFS_DINODE_FMT_BTREE) { + ASSERT(0); + return -EIO; + } + + /* + * We begin in transaction context with sc->ip ILOCKed but not joined + * to the transaction. To reset to the initial state, we must hold + * sc->ip's ILOCK to prevent rename from updating parent pointer + * information and the tempfile's ILOCK to clear its contents. + */ + xchk_iunlock(rx->sc, XFS_ILOCK_EXCL); + xrep_tempfile_ilock_both(sc); + xfs_trans_ijoin(sc->tp, sc->ip, 0); + xfs_trans_ijoin(sc->tp, sc->tempip, 0); + + /* + * Free all the blocks of the attr fork of the temp file, and reset + * it back to local format. + */ + if (xfs_ifork_has_extents(&sc->tempip->i_af)) { + error = xrep_reap_ifork(sc, sc->tempip, XFS_ATTR_FORK); + if (error) + return error; + + ASSERT(ifp->if_bytes == 0); + ifp->if_format = XFS_DINODE_FMT_LOCAL; + xfs_idata_realloc(sc->tempip, sizeof(*hdr), XFS_ATTR_FORK); + } + + /* Reinitialize the attr fork to an empty shortform structure. */ + hdr = (struct xfs_attr_sf_hdr *)ifp->if_u1.if_data; + memset(hdr, 0, sizeof(*hdr)); + hdr->totsize = cpu_to_be16(sizeof(*hdr)); + xfs_trans_log_inode(sc->tp, sc->tempip, XFS_ILOG_CORE | XFS_ILOG_ADATA); + + /* + * Roll this transaction to commit our reset ondisk. The tempfile + * should no longer be joined to the transaction, so we drop its ILOCK. + * This should leave us in transaction context with sc->ip ILOCKed but + * not joined to the transaction. + */ + error = xrep_roll_trans(sc); + if (error) + return error; + xrep_tempfile_iunlock(sc); + + /* + * Erase any accumulated parent pointer updates now that we've erased + * the tempfile's attr fork. We're resetting the entire repair state + * back to where we were initially, except now we won't flush salvaged + * xattrs until the very end. + */ + mutex_lock(&rx->lock); + xfarray_truncate(rx->pptr_recs); + xfblob_truncate(rx->pptr_names); + mutex_unlock(&rx->lock); + + rx->can_flush = false; + rx->attrs_found = 0; + + ASSERT(xfarray_bytes(rx->xattr_records) == 0); + ASSERT(xfblob_bytes(rx->xattr_blobs) == 0); + return 0; +} + /* Extract as many attribute keys and values as we can. */ STATIC int xrep_xattr_recover( @@ -731,6 +895,7 @@ xrep_xattr_recover( int nmap; int error; +restart: /* * Iterate each xattr leaf block in the attr fork to scan them for any * attributes that we might salvage. @@ -769,6 +934,14 @@ xrep_xattr_recover( error = xrep_xattr_flush_stashed(rx); if (error) return error; + + if (xrep_xattr_saw_pptr_conflict(rx)) { + error = xrep_xattr_full_reset(rx); + if (error) + return error; + + goto restart; + } } } } @@ -929,6 +1102,195 @@ xrep_xattr_salvage_attributes( return xrep_xattr_flush_stashed(rx); } +/* + * Add this stashed incore parent pointer to the temporary file. + * The caller must hold the tempdir's IOLOCK, must not hold any ILOCKs, and + * must not be in transaction context. + */ +STATIC int +xrep_xattr_replay_pptr_update( + struct xrep_xattr *rx, + const struct xrep_xattr_pptr *pptr) +{ + struct xfs_scrub *sc = rx->sc; + int error; + + rx->pptr.p_ino = pptr->p_ino; + rx->pptr.p_gen = pptr->p_gen; + rx->pptr.p_namelen = pptr->namelen; + xfs_parent_irec_hashname(sc->mp, &rx->pptr); + + switch (pptr->action) { + case XREP_XATTR_PPTR_ADD: + /* Create parent pointer. */ + trace_xrep_xattr_replay_parentadd(sc->tempip, &rx->pptr); + + error = xfs_parent_set(sc->tempip, sc->ip->i_ino, &rx->pptr, + &rx->pptr_scratch); + if (error) { + ASSERT(error != -EEXIST); + return error; + } + break; + case XREP_XATTR_PPTR_REMOVE: + /* Remove parent pointer. */ + trace_xrep_xattr_replay_parentremove(sc->tempip, &rx->pptr); + + error = xfs_parent_unset(sc->tempip, sc->ip->i_ino, &rx->pptr, + &rx->pptr_scratch); + if (error) { + ASSERT(error != -ENOATTR); + return error; + } + break; + default: + ASSERT(0); + return -EIO; + } + + return 0; +} + +/* + * Flush stashed parent pointer updates that have been recorded by the scanner. + * This is done to reduce the memory requirements of the parent pointer + * rebuild, since files can have a lot of hardlinks and the fs can be busy. + * + * Caller must not hold transactions or ILOCKs. Caller must hold the tempfile + * IOLOCK. + */ +STATIC int +xrep_xattr_replay_pptr_updates( + struct xrep_xattr *rx) +{ + xfarray_idx_t array_cur; + int error; + + mutex_lock(&rx->lock); + foreach_xfarray_idx(rx->pptr_recs, array_cur) { + struct xrep_xattr_pptr pptr; + + error = xfarray_load(rx->pptr_recs, array_cur, &pptr); + if (error) + goto out_unlock; + + error = xfblob_load(rx->pptr_names, pptr.name_cookie, + rx->pptr.p_name, pptr.namelen); + if (error) + goto out_unlock; + rx->pptr.p_name[MAXNAMELEN - 1] = 0; + mutex_unlock(&rx->lock); + + error = xrep_xattr_replay_pptr_update(rx, &pptr); + if (error) + return error; + + mutex_lock(&rx->lock); + } + + /* Empty out both arrays now that we've added the entries. */ + xfarray_truncate(rx->pptr_recs); + xfblob_truncate(rx->pptr_names); + mutex_unlock(&rx->lock); + return 0; +out_unlock: + mutex_unlock(&rx->lock); + return error; +} + +/* + * Remember that we want to create a parent pointer in the tempfile. These + * stashed actions will be replayed later. + */ +STATIC int +xrep_xattr_stash_parentadd( + struct xrep_xattr *rx, + const struct xfs_name *name, + const struct xfs_inode *dp) +{ + struct xrep_xattr_pptr pptr = { + .action = XREP_XATTR_PPTR_ADD, + .namelen = name->len, + .p_ino = dp->i_ino, + .p_gen = VFS_IC(dp)->i_generation, + }; + int error; + + trace_xrep_xattr_stash_parentadd(rx->sc->tempip, dp, name); + + error = xfblob_store(rx->pptr_names, &pptr.name_cookie, name->name, + name->len); + if (error) + return error; + + return xfarray_append(rx->pptr_recs, &pptr); +} + +/* + * Remember that we want to remove a parent pointer from the tempfile. These + * stashed actions will be replayed later. + */ +STATIC int +xrep_xattr_stash_parentremove( + struct xrep_xattr *rx, + const struct xfs_name *name, + const struct xfs_inode *dp) +{ + struct xrep_xattr_pptr pptr = { + .action = XREP_XATTR_PPTR_REMOVE, + .namelen = name->len, + .p_ino = dp->i_ino, + .p_gen = VFS_IC(dp)->i_generation, + }; + int error; + + trace_xrep_xattr_stash_parentremove(rx->sc->tempip, dp, name); + + error = xfblob_store(rx->pptr_names, &pptr.name_cookie, name->name, + name->len); + if (error) + return error; + + return xfarray_append(rx->pptr_recs, &pptr); +} + +/* + * Capture dirent updates being made by other threads. We will have to replay + * the parent pointer updates before swapping attr forks. + */ +STATIC int +xrep_xattr_live_dirent_update( + struct notifier_block *nb, + unsigned long action, + void *data) +{ + struct xfs_dir_update_params *p = data; + struct xrep_xattr *rx; + struct xfs_scrub *sc; + int error; + + rx = container_of(nb, struct xrep_xattr, hooks.dirent_hook.nb); + sc = rx->sc; + + /* + * This thread updated a dirent that points to the file that we're + * repairing, so stash the update for replay against the temporary + * file. + */ + if (p->ip->i_ino != sc->ip->i_ino) + return NOTIFY_DONE; + + mutex_lock(&rx->lock); + if (p->delta > 0) + error = xrep_xattr_stash_parentadd(rx, p->name, p->dp); + else + error = xrep_xattr_stash_parentremove(rx, p->name, p->dp); + if (error) + rx->live_update_aborted = true; + mutex_unlock(&rx->lock); + return NOTIFY_DONE; +} + /* * Prepare both inodes' attribute forks for extent swapping. Promote the * tempfile from short format to leaf format, and if the file being repaired @@ -1031,6 +1393,45 @@ xrep_xattr_swap( return xrep_tempswap_contents(sc, tx); } +/* + * Finish replaying stashed parent pointer updates, allocate a transaction for + * swapping extents, and take the ILOCKs of both files before we commit the new + * extended attribute structure. + */ +STATIC int +xrep_xattr_finalize_tempfile( + struct xrep_xattr *rx) +{ + struct xfs_scrub *sc = rx->sc; + int error; + + if (!xfs_has_parent(sc->mp)) + return xrep_tempswap_trans_alloc(sc, XFS_ATTR_FORK, &rx->tx); + + /* + * Repair relies on the ILOCK to quiesce all possible xattr updates. + * Replay all queued parent pointer updates into the tempfile before + * swapping the contents, even if that means dropping the ILOCKs and + * the transaction. + */ + do { + error = xrep_xattr_replay_pptr_updates(rx); + if (error) + return error; + + error = xrep_tempswap_trans_alloc(sc, XFS_ATTR_FORK, &rx->tx); + if (error) + return error; + + if (xfarray_length(rx->pptr_recs) == 0) + break; + + xchk_trans_cancel(sc); + xrep_tempfile_iunlock_both(sc); + } while (!xchk_should_terminate(sc, &error)); + return error; +} + /* * Swap the new extended attribute data (which we created in the tempfile) into * the file being repaired. @@ -1082,8 +1483,12 @@ xrep_xattr_rebuild_tree( if (error) return error; - /* Allocate swapext transaction and lock both inodes. */ - error = xrep_tempswap_trans_alloc(rx->sc, XFS_ATTR_FORK, &rx->tx); + /* + * Allocate transaction, lock inodes, and make sure that we've replayed + * all the stashed parent pointer updates to the temp file. After this + * point, we're ready to swapext. + */ + error = xrep_xattr_finalize_tempfile(rx); if (error) return error; @@ -1124,8 +1529,15 @@ STATIC void xrep_xattr_teardown( struct xrep_xattr *rx) { + if (xfs_has_parent(rx->sc->mp)) + xfs_dir_hook_del(rx->sc->mp, &rx->hooks); + if (rx->pptr_names) + xfblob_destroy(rx->pptr_names); + if (rx->pptr_recs) + xfarray_destroy(rx->pptr_recs); xfblob_destroy(rx->xattr_blobs); xfarray_destroy(rx->xattr_records); + mutex_destroy(&rx->lock); kfree(rx); } @@ -1144,6 +1556,9 @@ xrep_xattr_setup_scan( if (!rx) return -ENOMEM; rx->sc = sc; + rx->can_flush = true; + + mutex_init(&rx->lock); /* * Allocate enough memory to handle loading local attr values from the @@ -1171,11 +1586,44 @@ xrep_xattr_setup_scan( if (error) goto out_keys; + if (xfs_has_parent(sc->mp)) { + ASSERT(sc->flags & XCHK_FSGATES_DIRENTS); + + descr = xchk_xfile_ino_descr(sc, + "xattr retained parent pointer entries"); + error = xfarray_create(descr, 0, + sizeof(struct xrep_xattr_pptr), + &rx->pptr_recs); + kfree(descr); + if (error) + goto out_values; + + descr = xchk_xfile_ino_descr(sc, + "xattr retained parent pointer names"); + error = xfblob_create(descr, &rx->pptr_names); + kfree(descr); + if (error) + goto out_pprecs; + + xfs_hook_setup(&rx->hooks.dirent_hook, + xrep_xattr_live_dirent_update); + error = xfs_dir_hook_add(sc->mp, &rx->hooks); + if (error) + goto out_ppnames; + } + *rxp = rx; return 0; +out_ppnames: + xfblob_destroy(rx->pptr_names); +out_pprecs: + xfarray_destroy(rx->pptr_recs); +out_values: + xfblob_destroy(rx->xattr_blobs); out_keys: xfarray_destroy(rx->xattr_records); out_rx: + mutex_destroy(&rx->lock); kfree(rx); return error; } @@ -1212,6 +1660,11 @@ xrep_xattr( if (error) goto out_scan; + if (rx->live_update_aborted) { + error = -EIO; + goto out_scan; + } + /* Last chance to abort before we start committing fixes. */ if (xchk_should_terminate(sc, &error)) goto out_scan; diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h index 34e54ebf0daba..ebd5a91064281 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -2726,6 +2726,43 @@ DEFINE_EVENT(xrep_xattr_class, name, \ TP_ARGS(ip, arg_ip)) DEFINE_XREP_XATTR_EVENT(xrep_xattr_rebuild_tree); DEFINE_XREP_XATTR_EVENT(xrep_xattr_reset_fork); +DEFINE_XREP_XATTR_EVENT(xrep_xattr_full_reset); + +DECLARE_EVENT_CLASS(xrep_xattr_pptr_scan_class, + TP_PROTO(struct xfs_inode *ip, const struct xfs_inode *dp, + const struct xfs_name *name), + TP_ARGS(ip, dp, name), + TP_STRUCT__entry( + __field(dev_t, dev) + __field(xfs_ino_t, ino) + __field(xfs_ino_t, parent_ino) + __field(unsigned int, parent_gen) + __field(unsigned int, namelen) + __dynamic_array(char, name, name->len) + ), + TP_fast_assign( + __entry->dev = ip->i_mount->m_super->s_dev; + __entry->ino = ip->i_ino; + __entry->parent_ino = dp->i_ino; + __entry->parent_gen = VFS_IC(dp)->i_generation; + __entry->namelen = name->len; + memcpy(__get_str(name), name->name, name->len); + ), + TP_printk("dev %d:%d ino 0x%llx parent_ino 0x%llx parent_gen 0x%x name '%.*s'", + MAJOR(__entry->dev), MINOR(__entry->dev), + __entry->ino, + __entry->parent_ino, + __entry->parent_gen, + __entry->namelen, + __get_str(name)) +) +#define DEFINE_XREP_XATTR_PPTR_SCAN_EVENT(name) \ +DEFINE_EVENT(xrep_xattr_pptr_scan_class, name, \ + TP_PROTO(struct xfs_inode *ip, const struct xfs_inode *dp, \ + const struct xfs_name *name), \ + TP_ARGS(ip, dp, name)) +DEFINE_XREP_XATTR_PPTR_SCAN_EVENT(xrep_xattr_stash_parentadd); +DEFINE_XREP_XATTR_PPTR_SCAN_EVENT(xrep_xattr_stash_parentremove); TRACE_EVENT(xrep_dir_recover_dirblock, TP_PROTO(struct xfs_inode *dp, xfs_dablk_t dabno, uint32_t magic, @@ -2872,6 +2909,40 @@ DEFINE_XREP_PARENT_SALVAGE_EVENT(xrep_dir_salvaged_parent); DEFINE_XREP_PARENT_SALVAGE_EVENT(xrep_findparent_dirent); DEFINE_XREP_PARENT_SALVAGE_EVENT(xrep_findparent_from_dcache); +DECLARE_EVENT_CLASS(xrep_pptr_class, + TP_PROTO(struct xfs_inode *ip, const struct xfs_parent_name_irec *pptr), + TP_ARGS(ip, pptr), + TP_STRUCT__entry( + __field(dev_t, dev) + __field(xfs_ino_t, ino) + __field(xfs_ino_t, parent_ino) + __field(unsigned int, parent_gen) + __field(unsigned int, namelen) + __dynamic_array(char, name, pptr->p_namelen) + ), + TP_fast_assign( + __entry->dev = ip->i_mount->m_super->s_dev; + __entry->ino = ip->i_ino; + __entry->parent_ino = pptr->p_ino; + __entry->parent_gen = pptr->p_gen; + __entry->namelen = pptr->p_namelen; + memcpy(__get_str(name), pptr->p_name, pptr->p_namelen); + ), + TP_printk("dev %d:%d ino 0x%llx parent_ino 0x%llx parent_gen 0x%x name '%.*s'", + MAJOR(__entry->dev), MINOR(__entry->dev), + __entry->ino, + __entry->parent_ino, + __entry->parent_gen, + __entry->namelen, + __get_str(name)) +) +#define DEFINE_XREP_PPTR_EVENT(name) \ +DEFINE_EVENT(xrep_pptr_class, name, \ + TP_PROTO(struct xfs_inode *ip, const struct xfs_parent_name_irec *pptr), \ + TP_ARGS(ip, pptr)) +DEFINE_XREP_PPTR_EVENT(xrep_xattr_replay_parentadd); +DEFINE_XREP_PPTR_EVENT(xrep_xattr_replay_parentremove); + TRACE_EVENT(xrep_nlinks_set_record, TP_PROTO(struct xfs_mount *mp, xfs_ino_t ino, const struct xchk_nlink *obs), From patchwork Sun Dec 31 20:56:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507524 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BB439BA31 for ; Sun, 31 Dec 2023 20:56:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Qvr6NC4q" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 219A0C433C8; Sun, 31 Dec 2023 20:56:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056211; bh=HO1u+oFo7gp4yE6IMyCLuNbQyRQ5Vc/4yko2rvZ9DHQ=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=Qvr6NC4qDD1hQQn7/IxkSPiZY+wOI9Bt1VBPRqIzdd0z1EFBvofB8JREBxfL/N8nv i+EJIEgGNewTgCaI6bfPL+I5CXrT73SL6bKVqgMcDYJW6Pm32/scmvVPsW1S6U4Qw0 doW7dEuAHWY60DK7YO4YHD/YQlpOoJV87TtAObrWP169me0aGLLg7r7vQ/quAObHVB 6jlfwoJkhi71pFTT6bin7uCC/W99YQq26zTvj8wFVjiXDxNVbRrGsyuqWmB9G4hRRo bA8nJ8RTFiT6OojnB+BXbdKXnGMjMdwvH8tSiKS29ruYdKP+px+zDD0WDmLKyaPMVL r0fe+Qp2NI8Jg== Date: Sun, 31 Dec 2023 12:56:50 -0800 Subject: [PATCH 14/22] xfs: replace namebuf with parent pointer in parent pointer repair From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841972.1757392.3061229459574731690.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong Replace the dirent name buffer at the end of struct xrep_parent with a xfs_parent_name_irec object. The namebuf and p_name usage do not overlap, so we can save 256 bytes of memory by allowing them to overlap. Doing so makes the code a bit more complex, so this is called out separately. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/parent_repair.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c index 099620fc119e9..68cc3aee1d5c8 100644 --- a/fs/xfs/scrub/parent_repair.c +++ b/fs/xfs/scrub/parent_repair.c @@ -24,6 +24,7 @@ #include "xfs_trans_space.h" #include "xfs_health.h" #include "xfs_swapext.h" +#include "xfs_parent.h" #include "scrub/xfs_scrub.h" #include "scrub/scrub.h" #include "scrub/common.h" @@ -63,8 +64,12 @@ struct xrep_parent { /* Orphanage reparenting request. */ struct xrep_adoption adoption; - /* Directory entry name, plus the trailing null. */ - unsigned char namebuf[MAXNAMELEN]; + /* + * Scratch buffer for scanning dirents to create pptr xattrs. At the + * very end of the repair, it can also be used to compute the + * lost+found filename if we need to reparent the file. + */ + struct xfs_parent_name_irec pptr; }; /* Tear down all the incore stuff we created. */ @@ -236,7 +241,7 @@ xrep_parent_move_to_orphanage( if (error) return error; - error = xrep_adoption_compute_name(&rp->adoption, rp->namebuf); + error = xrep_adoption_compute_name(&rp->adoption, rp->pptr.p_name); if (error) return error; From patchwork Sun Dec 31 20:57:06 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507525 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F3AFBC129 for ; Sun, 31 Dec 2023 20:57:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="RzE2d48v" Received: by smtp.kernel.org (Postfix) with ESMTPSA id BFB52C433C7; Sun, 31 Dec 2023 20:57:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056226; bh=UuEZH7byGuT6qLbaBK0q6iQP9MokYuq8Q6DdcRsRwD4=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=RzE2d48vmrMsXxU3K2Z+7QXNmwzlBoZ2DcF4AXQa2oAkTL0bOiaclRV2sROg15SXf 30fNBIixaBqaQj35jnr6s2hiWEtm1ceSoBEPVF+tuZ4sEa2WSG+mwl18iSIpqFDVfq EKZfm2xUFbbQDS1SmBvuCW3/2DnSA8fOPkjPPgsc6fbsk6sIGKUXH0cTFyvDvrw+jp bMCy2tCOQQEJvZX4Nq78moa2971VoL4M5pTqR1aogUeFNYf6HmtJigwdQ2fzwlFd0a nsvMj8VEdxBVsZujIOB6q2rVLzyRuo/yxet9LUAzc7LTfQKg/Jc99q1wz7aZ/nB8i9 iF7uuRD7u+p+g== Date: Sun, 31 Dec 2023 12:57:06 -0800 Subject: [PATCH 15/22] xfs: repair directory parent pointers by scanning for dirents From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404841988.1757392.13366690562684132094.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong If parent pointers are enabled on the filesystem, we can repair the entire dataset by walking the directories of the filesystem looking for dirents that we can turn into parent pointers. Once we have a full incore dataset, we'll figure out what to do with it, but that's for a subsequent patch. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/parent_repair.c | 425 ++++++++++++++++++++++++++++++++++++++++++ fs/xfs/scrub/trace.h | 36 ++++ 2 files changed, 458 insertions(+), 3 deletions(-) diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c index 68cc3aee1d5c8..8d83aab8caa20 100644 --- a/fs/xfs/scrub/parent_repair.c +++ b/fs/xfs/scrub/parent_repair.c @@ -35,6 +35,9 @@ #include "scrub/readdir.h" #include "scrub/tempfile.h" #include "scrub/orphanage.h" +#include "scrub/xfile.h" +#include "scrub/xfarray.h" +#include "scrub/xfblob.h" /* * Repairing The Directory Parent Pointer @@ -50,20 +53,71 @@ * See the section on locking issues in dir_repair.c for more information about * conflicts with the VFS. The findparent code wll keep our incore parent * inode up to date. + * + * If parent pointers are enabled, we instead reconstruct the parent pointer + * information by visiting every directory entry of every directory in the + * system and translating the relevant dirents into parent pointers. In this + * case, it is advantageous to stash all parent pointers created from dirents + * from a single parent file before replaying them into the temporary file. To + * save memory, the live filesystem scan reuses the findparent object. Parent + * pointer repair chooses either directory scanning or findparent, but not + * both. + * + * When salvaging completes, the remaining stashed entries are replayed to the + * temporary file. All non-parent pointer extended attributes are copied to + * the temporary file's extended attributes. An atomic extent swap is used to + * commit the new directory blocks to the directory being repaired. This will + * disrupt attrmulti cursors. */ +/* A stashed parent pointer update. */ +struct xrep_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; + + /* Length of the pptr name. */ + uint8_t namelen; +}; + +/* + * Stash up to 8 pages of recovered parent pointers in pptr_recs and + * pptr_names before we write them to the temp file. + */ +#define XREP_PARENT_MAX_STASH_BYTES (PAGE_SIZE * 8) + struct xrep_parent { struct xfs_scrub *sc; + /* Fixed-size array of xrep_pptr structures. */ + struct xfarray *pptr_recs; + + /* Blobs containing parent pointer names. */ + struct xfblob *pptr_names; + /* * Information used to scan the filesystem to find the inumber of the - * dotdot entry for this directory. + * dotdot entry for this directory. On filesystems without parent + * pointers, we use the findparent_* functions on this object and + * access only the parent_ino field directly. + * + * When parent pointers are enabled, the directory entry scanner uses + * the iscan, hooks, and lock fields of this object directly. + * @pscan.lock coordinates access to pptr_recs, pptr_names, pptr, and + * pptr_scratch. This reduces the memory requirements of this + * structure. */ struct xrep_parent_scan_info pscan; /* Orphanage reparenting request. */ struct xrep_adoption adoption; + /* xattr key and da args for parent pointer replay. */ + struct xfs_parent_scratch pptr_scratch; + /* * Scratch buffer for scanning dirents to create pptr xattrs. At the * very end of the repair, it can also be used to compute the @@ -78,6 +132,12 @@ xrep_parent_teardown( struct xrep_parent *rp) { xrep_findparent_scan_teardown(&rp->pscan); + if (rp->pptr_names) + xfblob_destroy(rp->pptr_names); + rp->pptr_names = NULL; + if (rp->pptr_recs) + xfarray_destroy(rp->pptr_recs); + rp->pptr_recs = NULL; } /* Set up for a parent repair. */ @@ -86,6 +146,7 @@ xrep_setup_parent( struct xfs_scrub *sc) { struct xrep_parent *rp; + int error; xchk_fsgates_enable(sc, XCHK_FSGATES_DIRENTS); @@ -95,6 +156,10 @@ xrep_setup_parent( rp->sc = sc; sc->buf = rp; + error = xrep_tempfile_create(sc, S_IFREG); + if (error) + return error; + return xrep_orphanage_try_create(sc); } @@ -150,6 +215,318 @@ xrep_parent_find_dotdot( return error; } +/* + * Add this stashed incore parent pointer to the temporary file. + * The caller must hold the tempdir's IOLOCK, must not hold any ILOCKs, and + * must not be in transaction context. + */ +STATIC int +xrep_parent_replay_update( + struct xrep_parent *rp, + const struct xrep_pptr *pptr) +{ + struct xfs_scrub *sc = rp->sc; + int error; + + rp->pptr.p_ino = pptr->p_ino; + rp->pptr.p_gen = pptr->p_gen; + rp->pptr.p_namelen = pptr->namelen; + xfs_parent_irec_hashname(sc->mp, &rp->pptr); + + /* Create parent pointer. */ + trace_xrep_parent_replay_parentadd(sc->tempip, &rp->pptr); + + error = xfs_parent_set(sc->tempip, sc->ip->i_ino, &rp->pptr, + &rp->pptr_scratch); + if (error) + return error; + + return 0; +} + +/* + * Flush stashed parent pointer updates that have been recorded by the scanner. + * This is done to reduce the memory requirements of the parent pointer + * rebuild, since files can have a lot of hardlinks and the fs can be busy. + * + * Caller must not hold transactions or ILOCKs. Caller must hold the tempfile + * IOLOCK. + */ +STATIC int +xrep_parent_replay_updates( + struct xrep_parent *rp) +{ + xfarray_idx_t array_cur; + int error; + + mutex_lock(&rp->pscan.lock); + foreach_xfarray_idx(rp->pptr_recs, array_cur) { + struct xrep_pptr pptr; + + error = xfarray_load(rp->pptr_recs, array_cur, &pptr); + if (error) + goto out_unlock; + + error = xfblob_load(rp->pptr_names, pptr.name_cookie, + rp->pptr.p_name, pptr.namelen); + if (error) + goto out_unlock; + rp->pptr.p_name[MAXNAMELEN - 1] = 0; + mutex_unlock(&rp->pscan.lock); + + error = xrep_parent_replay_update(rp, &pptr); + if (error) + return error; + + mutex_lock(&rp->pscan.lock); + } + + /* Empty out both arrays now that we've added the entries. */ + xfarray_truncate(rp->pptr_recs); + xfblob_truncate(rp->pptr_names); + mutex_unlock(&rp->pscan.lock); + return 0; +out_unlock: + mutex_unlock(&rp->pscan.lock); + return error; +} + +/* + * Remember that we want to create a parent pointer in the tempfile. These + * stashed actions will be replayed later. + */ +STATIC int +xrep_parent_stash_parentadd( + struct xrep_parent *rp, + const struct xfs_name *name, + const struct xfs_inode *dp) +{ + struct xrep_pptr pptr = { + .namelen = name->len, + .p_ino = dp->i_ino, + .p_gen = VFS_IC(dp)->i_generation, + }; + int error; + + trace_xrep_parent_stash_parentadd(rp->sc->tempip, dp, name); + + error = xfblob_store(rp->pptr_names, &pptr.name_cookie, name->name, + name->len); + if (error) + return error; + + return xfarray_append(rp->pptr_recs, &pptr); +} + +/* + * Examine an entry of a directory. If this dirent leads us back to the file + * whose parent pointers we're rebuilding, add a pptr to the temporary + * directory. + */ +STATIC int +xrep_parent_scan_dirent( + struct xfs_scrub *sc, + struct xfs_inode *dp, + xfs_dir2_dataptr_t dapos, + const struct xfs_name *name, + xfs_ino_t ino, + void *priv) +{ + struct xrep_parent *rp = priv; + int error; + + /* Dirent doesn't point to this directory. */ + if (ino != rp->sc->ip->i_ino) + return 0; + + /* No weird looking names. */ + if (name->len == 0 || !xfs_dir2_namecheck(name->name, name->len)) + return -EFSCORRUPTED; + + /* No mismatching ftypes. */ + if (name->type != xfs_mode_to_ftype(VFS_I(sc->ip)->i_mode)) + return -EFSCORRUPTED; + + /* Don't pick up dot or dotdot entries; we only want child dirents. */ + if (xfs_dir2_samename(name, &xfs_name_dotdot) || + xfs_dir2_samename(name, &xfs_name_dot)) + return 0; + + /* + * Transform this dirent into a parent pointer and queue it for later + * addition to the temporary file. + */ + mutex_lock(&rp->pscan.lock); + error = xrep_parent_stash_parentadd(rp, name, dp); + mutex_unlock(&rp->pscan.lock); + return error; +} + +/* + * Decide if we want to look for dirents in this directory. Skip the file + * being repaired and any files being used to stage repairs. + */ +static inline bool +xrep_parent_want_scan( + struct xrep_parent *rp, + const struct xfs_inode *ip) +{ + return ip != rp->sc->ip && !xrep_is_tempfile(ip); +} + +/* + * Take ILOCK on a file that we want to scan. + * + * Select ILOCK_EXCL if the file is a directory with an unloaded data bmbt. + * Otherwise, take ILOCK_SHARED. + */ +static inline unsigned int +xrep_parent_scan_ilock( + struct xrep_parent *rp, + struct xfs_inode *ip) +{ + uint lock_mode = XFS_ILOCK_SHARED; + + /* Still need to take the shared ILOCK to advance the iscan cursor. */ + if (!xrep_parent_want_scan(rp, ip)) + goto lock; + + if (S_ISDIR(VFS_I(ip)->i_mode) && xfs_need_iread_extents(&ip->i_df)) { + lock_mode = XFS_ILOCK_EXCL; + goto lock; + } + +lock: + xfs_ilock(ip, lock_mode); + return lock_mode; +} + +/* + * Scan this file for relevant child dirents that point to the file whose + * parent pointers we're rebuilding. + */ +STATIC int +xrep_parent_scan_file( + struct xrep_parent *rp, + struct xfs_inode *ip) +{ + unsigned int lock_mode; + int error = 0; + + lock_mode = xrep_parent_scan_ilock(rp, ip); + + if (!xrep_parent_want_scan(rp, ip)) + goto scan_done; + + if (S_ISDIR(VFS_I(ip)->i_mode)) { + /* + * If the directory looks as though it has been zapped by the + * inode record repair code, we cannot scan for child dirents. + */ + if (xchk_dir_looks_zapped(ip)) { + error = -EBUSY; + goto scan_done; + } + + error = xchk_dir_walk(rp->sc, ip, xrep_parent_scan_dirent, rp); + if (error) + goto scan_done; + } + +scan_done: + xchk_iscan_mark_visited(&rp->pscan.iscan, ip); + xfs_iunlock(ip, lock_mode); + return error; +} + +/* Decide if we've stashed too much pptr data in memory. */ +static inline bool +xrep_parent_want_flush_stashed( + struct xrep_parent *rp) +{ + unsigned long long bytes; + + bytes = xfarray_bytes(rp->pptr_recs) + xfblob_bytes(rp->pptr_names); + return bytes > XREP_PARENT_MAX_STASH_BYTES; +} + +/* + * Scan all directories in the filesystem to look for dirents that we can turn + * into parent pointers. + */ +STATIC int +xrep_parent_scan_dirtree( + struct xrep_parent *rp) +{ + struct xfs_scrub *sc = rp->sc; + struct xfs_inode *ip; + int error; + + /* + * Filesystem scans are time consuming. Drop the file ILOCK and all + * other resources for the duration of the scan and hope for the best. + * The live update hooks will keep our scan information up to date. + */ + xchk_trans_cancel(sc); + if (sc->ilock_flags & (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL)) + xchk_iunlock(sc, sc->ilock_flags & (XFS_ILOCK_SHARED | + XFS_ILOCK_EXCL)); + error = xchk_trans_alloc_empty(sc); + if (error) + return error; + + while ((error = xchk_iscan_iter(&rp->pscan.iscan, &ip)) == 1) { + bool flush; + + error = xrep_parent_scan_file(rp, ip); + xchk_irele(sc, ip); + if (error) + break; + + /* Flush stashed pptr updates to constrain memory usage. */ + mutex_lock(&rp->pscan.lock); + flush = xrep_parent_want_flush_stashed(rp); + mutex_unlock(&rp->pscan.lock); + if (flush) { + xchk_trans_cancel(sc); + + error = xrep_tempfile_iolock_polled(sc); + if (error) + break; + + error = xrep_parent_replay_updates(rp); + xrep_tempfile_iounlock(sc); + if (error) + break; + + error = xchk_trans_alloc_empty(sc); + if (error) + break; + } + + if (xchk_should_terminate(sc, &error)) + break; + } + xchk_iscan_iter_finish(&rp->pscan.iscan); + if (error) { + /* + * If we couldn't grab an inode that was busy with a state + * change, change the error code so that we exit to userspace + * as quickly as possible. + */ + if (error == -EBUSY) + return -ECANCELED; + return error; + } + + /* + * Cancel the empty transaction so that we can (later) use the atomic + * extent swap helpers to lock files and commit the new directory. + */ + xchk_trans_cancel(rp->sc); + return 0; +} + /* Reset a directory's dotdot entry, if needed. */ STATIC int xrep_parent_reset_dotdot( @@ -301,8 +678,39 @@ xrep_parent_setup_scan( struct xrep_parent *rp) { struct xfs_scrub *sc = rp->sc; + char *descr; + int error; - return xrep_findparent_scan_start(sc, &rp->pscan); + if (!xfs_has_parent(sc->mp)) + return xrep_findparent_scan_start(sc, &rp->pscan); + + /* Set up some staging memory for logging parent pointer updates. */ + descr = xchk_xfile_ino_descr(sc, "parent pointer entries"); + error = xfarray_create(descr, 0, sizeof(struct xrep_pptr), + &rp->pptr_recs); + kfree(descr); + if (error) + return error; + + descr = xchk_xfile_ino_descr(sc, "parent pointer names"); + error = xfblob_create(descr, &rp->pptr_names); + kfree(descr); + if (error) + goto out_recs; + + error = xrep_findparent_scan_start(sc, &rp->pscan); + if (error) + goto out_names; + + return 0; + +out_names: + xfblob_destroy(rp->pptr_names); + rp->pptr_names = NULL; +out_recs: + xfarray_destroy(rp->pptr_recs); + rp->pptr_recs = NULL; + return error; } int @@ -312,11 +720,22 @@ xrep_parent( struct xrep_parent *rp = sc->buf; int error; + /* + * When the parent pointers feature is enabled, repairs are committed + * by atomically committing a new xattr structure and reaping the old + * attr fork. Reaping requires rmap to be enabled. + */ + if (xfs_has_parent(sc->mp) && !xfs_has_rmapbt(sc->mp)) + return -EOPNOTSUPP; + error = xrep_parent_setup_scan(rp); if (error) return error; - error = xrep_parent_find_dotdot(rp); + if (xfs_has_parent(sc->mp)) + error = xrep_parent_scan_dirtree(rp); + else + error = xrep_parent_find_dotdot(rp); if (error) goto out_teardown; diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h index ebd5a91064281..def5d72d3c55c 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -2942,6 +2942,42 @@ DEFINE_EVENT(xrep_pptr_class, name, \ TP_ARGS(ip, pptr)) DEFINE_XREP_PPTR_EVENT(xrep_xattr_replay_parentadd); DEFINE_XREP_PPTR_EVENT(xrep_xattr_replay_parentremove); +DEFINE_XREP_PPTR_EVENT(xrep_parent_replay_parentadd); + +DECLARE_EVENT_CLASS(xrep_pptr_scan_class, + TP_PROTO(struct xfs_inode *ip, const struct xfs_inode *dp, + const struct xfs_name *name), + TP_ARGS(ip, dp, name), + TP_STRUCT__entry( + __field(dev_t, dev) + __field(xfs_ino_t, ino) + __field(xfs_ino_t, parent_ino) + __field(unsigned int, parent_gen) + __field(unsigned int, namelen) + __dynamic_array(char, name, name->len) + ), + TP_fast_assign( + __entry->dev = ip->i_mount->m_super->s_dev; + __entry->ino = ip->i_ino; + __entry->parent_ino = dp->i_ino; + __entry->parent_gen = VFS_IC(dp)->i_generation; + __entry->namelen = name->len; + memcpy(__get_str(name), name->name, name->len); + ), + TP_printk("dev %d:%d ino 0x%llx parent_ino 0x%llx parent_gen 0x%x name '%.*s'", + MAJOR(__entry->dev), MINOR(__entry->dev), + __entry->ino, + __entry->parent_ino, + __entry->parent_gen, + __entry->namelen, + __get_str(name)) +) +#define DEFINE_XREP_PPTR_SCAN_EVENT(name) \ +DEFINE_EVENT(xrep_pptr_scan_class, name, \ + TP_PROTO(struct xfs_inode *ip, const struct xfs_inode *dp, \ + const struct xfs_name *name), \ + TP_ARGS(ip, dp, name)) +DEFINE_XREP_PPTR_SCAN_EVENT(xrep_parent_stash_parentadd); TRACE_EVENT(xrep_nlinks_set_record, TP_PROTO(struct xfs_mount *mp, xfs_ino_t ino, From patchwork Sun Dec 31 20:57:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507526 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DA65FC126 for ; Sun, 31 Dec 2023 20:57:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="JWKk8b+g" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 64BEFC433C8; Sun, 31 Dec 2023 20:57:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056242; bh=+jjiU4bBCt0mOSKPZklVgtUrnbkx3opoDc3IpTa7PIA=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=JWKk8b+ggAfWWWQz8Y3WhbnyuAygCEDaPoOJGPWjbYx+vdPFtMn/uM6pYQKC4sNZB +H+W11VK79gtK1nCs/2k87khEdPTeLeZB8BNo6579bwOLRiWRcjGCjJ7HXQ2+1nW40 N9jmYlRl4D66pb/cAsU+HJZibJaxPVJtoJDJHZQrRkB2635KOdU2o3ytv5N4JAMEzz D+p1fNqn5OmJTcKDW/Hr6/WdjXBNzjjfheCADJVyjQyG0mg4N2prGduWq+elYt6S6p kFrsGPivXlbuQ7jIC7WXbxE5uB/FXmzXtYbYKCYeT3FKLpQeEbMTMilFJG9O3tdP+a HCOuGe12051gA== Date: Sun, 31 Dec 2023 12:57:21 -0800 Subject: [PATCH 16/22] xfs: implement live updates for parent pointer repairs From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404842004.1757392.8727074541747879565.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong While we're scanning the filesystem for dirents that we can turn into parent pointers, we cannot hold the IOLOCK or ILOCK of the file being repaired. Therefore, we need to set up a dirent hook so that we can keep the temporary file's parent pionters up to date with the rest of the filesystem. Hence we add the ability to *remove* pptrs from the temporary file. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/parent_repair.c | 111 +++++++++++++++++++++++++++++++++++++++--- fs/xfs/scrub/trace.h | 2 + 2 files changed, 106 insertions(+), 7 deletions(-) diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c index 8d83aab8caa20..b94eebec3cc79 100644 --- a/fs/xfs/scrub/parent_repair.c +++ b/fs/xfs/scrub/parent_repair.c @@ -70,6 +70,12 @@ * disrupt attrmulti cursors. */ +/* Create a parent pointer in the tempfile. */ +#define XREP_PPTR_ADD (1) + +/* Remove a parent pointer from the tempfile. */ +#define XREP_PPTR_REMOVE (2) + /* A stashed parent pointer update. */ struct xrep_pptr { /* Cookie for retrieval of the pptr name. */ @@ -81,6 +87,9 @@ struct xrep_pptr { /* Length of the pptr name. */ uint8_t namelen; + + /* XREP_PPTR_{ADD,REMOVE} */ + uint8_t action; }; /* @@ -233,13 +242,29 @@ xrep_parent_replay_update( rp->pptr.p_namelen = pptr->namelen; xfs_parent_irec_hashname(sc->mp, &rp->pptr); - /* Create parent pointer. */ - trace_xrep_parent_replay_parentadd(sc->tempip, &rp->pptr); + switch (pptr->action) { + case XREP_PPTR_ADD: + /* Create parent pointer. */ + trace_xrep_parent_replay_parentadd(sc->tempip, &rp->pptr); - error = xfs_parent_set(sc->tempip, sc->ip->i_ino, &rp->pptr, - &rp->pptr_scratch); - if (error) - return error; + error = xfs_parent_set(sc->tempip, sc->ip->i_ino, &rp->pptr, + &rp->pptr_scratch); + if (error) + return error; + break; + case XREP_PPTR_REMOVE: + /* Remove parent pointer. */ + trace_xrep_parent_replay_parentremove(sc->tempip, &rp->pptr); + + error = xfs_parent_unset(sc->tempip, sc->ip->i_ino, &rp->pptr, + &rp->pptr_scratch); + if (error) + return error; + break; + default: + ASSERT(0); + return -EIO; + } return 0; } @@ -302,6 +327,7 @@ xrep_parent_stash_parentadd( const struct xfs_inode *dp) { struct xrep_pptr pptr = { + .action = XREP_PPTR_ADD, .namelen = name->len, .p_ino = dp->i_ino, .p_gen = VFS_IC(dp)->i_generation, @@ -318,6 +344,34 @@ xrep_parent_stash_parentadd( return xfarray_append(rp->pptr_recs, &pptr); } +/* + * Remember that we want to remove a parent pointer from the tempfile. These + * stashed actions will be replayed later. + */ +STATIC int +xrep_parent_stash_parentremove( + struct xrep_parent *rp, + const struct xfs_name *name, + const struct xfs_inode *dp) +{ + struct xrep_pptr pptr = { + .action = XREP_PPTR_REMOVE, + .namelen = name->len, + .p_ino = dp->i_ino, + .p_gen = VFS_IC(dp)->i_generation, + }; + int error; + + trace_xrep_parent_stash_parentremove(rp->sc->tempip, dp, name); + + error = xfblob_store(rp->pptr_names, &pptr.name_cookie, name->name, + name->len); + if (error) + return error; + + return xfarray_append(rp->pptr_recs, &pptr); +} + /* * Examine an entry of a directory. If this dirent leads us back to the file * whose parent pointers we're rebuilding, add a pptr to the temporary @@ -527,6 +581,48 @@ xrep_parent_scan_dirtree( return 0; } +/* + * Capture dirent updates being made by other threads which are relevant to the + * file being repaired. + */ +STATIC int +xrep_parent_live_update( + struct notifier_block *nb, + unsigned long action, + void *data) +{ + struct xfs_dir_update_params *p = data; + struct xrep_parent *rp; + struct xfs_scrub *sc; + int error; + + rp = container_of(nb, struct xrep_parent, pscan.hooks.dirent_hook.nb); + sc = rp->sc; + + /* + * This thread updated a dirent that points to the file that we're + * repairing, so stash the update for replay against the temporary + * file. + */ + if (p->ip->i_ino == sc->ip->i_ino && + xchk_iscan_want_live_update(&rp->pscan.iscan, p->dp->i_ino)) { + mutex_lock(&rp->pscan.lock); + if (p->delta > 0) + error = xrep_parent_stash_parentadd(rp, p->name, p->dp); + else + error = xrep_parent_stash_parentremove(rp, p->name, + p->dp); + mutex_unlock(&rp->pscan.lock); + if (error) + goto out_abort; + } + + return NOTIFY_DONE; +out_abort: + xchk_iscan_abort(&rp->pscan.iscan); + return NOTIFY_DONE; +} + /* Reset a directory's dotdot entry, if needed. */ STATIC int xrep_parent_reset_dotdot( @@ -698,7 +794,8 @@ xrep_parent_setup_scan( if (error) goto out_recs; - error = xrep_findparent_scan_start(sc, &rp->pscan); + error = __xrep_findparent_scan_start(sc, &rp->pscan, + xrep_parent_live_update); if (error) goto out_names; diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h index def5d72d3c55c..b03e1e69740fa 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -2943,6 +2943,7 @@ DEFINE_EVENT(xrep_pptr_class, name, \ DEFINE_XREP_PPTR_EVENT(xrep_xattr_replay_parentadd); DEFINE_XREP_PPTR_EVENT(xrep_xattr_replay_parentremove); DEFINE_XREP_PPTR_EVENT(xrep_parent_replay_parentadd); +DEFINE_XREP_PPTR_EVENT(xrep_parent_replay_parentremove); DECLARE_EVENT_CLASS(xrep_pptr_scan_class, TP_PROTO(struct xfs_inode *ip, const struct xfs_inode *dp, @@ -2978,6 +2979,7 @@ DEFINE_EVENT(xrep_pptr_scan_class, name, \ const struct xfs_name *name), \ TP_ARGS(ip, dp, name)) DEFINE_XREP_PPTR_SCAN_EVENT(xrep_parent_stash_parentadd); +DEFINE_XREP_PPTR_SCAN_EVENT(xrep_parent_stash_parentremove); TRACE_EVENT(xrep_nlinks_set_record, TP_PROTO(struct xfs_mount *mp, xfs_ino_t ino, From patchwork Sun Dec 31 20:57:37 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507527 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8215FC126 for ; Sun, 31 Dec 2023 20:57:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="uaUmE5dP" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0F2D3C433C7; Sun, 31 Dec 2023 20:57:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056258; bh=n5x3wKcay7LxdKw+va7Yvw7Zg8gUhc8Mk30TNiEEPdk=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=uaUmE5dPc8iV7/s2apVRnbspP4nxbk5SmMb2G8clFSWLJ2RUnjid0BugFcI9SwIo8 w8JI7fuHAUjIDwdGWpB+O0ynzlRF3x34MZFQ4iAWc6eKpe9KKVTrtrKxSNjVfFg3fl eACi+6PHXdplJlAov30WLUdXwMaSAVmlEtuYz4DaSSmxSuKYWl4DTbi3PGn5kyi7Gs qaWqPYIQ/ItJO7GYhG/Glyy85Ea3bdvbso+cbbXYyrO1wBNH+5tSC5BIzjVtKDoAc1 ox5A6Q7UcTr4oZ2qnwu1tlpjK0aHtr4zVXgokzWQdhC2FmVFY2VVtkYyVcS4ui34+T 7arXCgZPzG/4g== Date: Sun, 31 Dec 2023 12:57:37 -0800 Subject: [PATCH 17/22] xfs: remove pointless unlocked assertion From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404842020.1757392.7312336360740998905.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong Remove this assertion about the inode not having an attr fork from xfs_bmap_add_attrfork because the function handles that case just fine. Weirder still, the function actually /requires/ the caller not to hold the ILOCK, which means that its accesses are not stabilized. Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_bmap.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index 44b8c315c5978..1dd1876a7b145 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -1023,8 +1023,6 @@ xfs_bmap_add_attrfork( int logflags; /* logging flags */ int error; /* error return value */ - ASSERT(xfs_inode_has_attr_fork(ip) == 0); - mp = ip->i_mount; ASSERT(!XFS_NOT_DQATTACHED(mp, ip)); From patchwork Sun Dec 31 20:57:53 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507528 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 43518C127 for ; Sun, 31 Dec 2023 20:57:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="b4zHo6Mc" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B2326C433C7; Sun, 31 Dec 2023 20:57:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056273; bh=1SCZXOSXy2/rgINEMXwbXP3iwBbazL9udDacGH5doAU=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=b4zHo6Mc5hFSfaED/M2nNuW0WZcdkSBLM5ofsrTtIAtuy26nxpCvgTMTxDfQSPLut vLfjGF8dhkeEOjS4pyNxJtE1DsvmqY9KaiBMF9gI0DpeEpZFhSq5MWXIZLJ7y0TXL1 EKvMMxLrfc3auo5FnavjTnmq2BvQiFWkMM8YBiQASKQ8oxpACSUeE7xS5Tgbxj3Zky EiidbcyN+K4scM5iWNlJw5Y1fcOZFrD1RCMOeCO1vw5FQnOe86muY6c3yD4QazRtup g9cWG8vWXpaWGAtTEBINdw3Vw9zVQYuzpHFgj5QhfbaZ548kQcVNXX5cWN2M+6YFIH f9pjki3lfZ6Qg== Date: Sun, 31 Dec 2023 12:57:53 -0800 Subject: [PATCH 18/22] xfs: split xfs_bmap_add_attrfork into two pieces From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404842036.1757392.5025398136396581980.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong Split this function into two pieces -- one to make the actual changes to the inode core to add the attr fork, and another one to deal with getting the transaction and locking the inodes. The next couple of patches will need this to be split into two. One patch implements committing new parent pointer recordsets to damaged files. If one file has an attr fork and the other does not, we have to create the missing attr fork before the atomic swap transaction, and can use the behavior encoded in the current xfs_bmap_add_attrfork. The second patch adapts /lost+found adoptions to handle parent pointers correctly. The adoption process will add a parent pointer to a child that is being moved to /lost+found, but this requires that the attr fork already exists. We don't know if we're actually going to commit the adoption until we've already reserved a transaction and taken the ILOCKs, which means that we must have a way to bypass the start of the current xfs_bmap_add_attrfork. Therefore, create xfs_attr_add_fork as the helper that creates a transaction and takes locks; and make xfs_bmap_add_attrfork the function that updates the inode core and allocates the incore attr fork. Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_attr.c | 39 ++++++++++++++++++++++++++++++++++++++- fs/xfs/libxfs/xfs_bmap.c | 36 ++++++++++-------------------------- fs/xfs/libxfs/xfs_bmap.h | 3 ++- 3 files changed, 50 insertions(+), 28 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index c13eb7b7b5b8f..ef32c2a22c617 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -935,6 +935,43 @@ xfs_attr_defer_add( trace_xfs_attr_defer_add(new->xattri_dela_state, args->dp); } +STATIC int +xfs_attr_add_fork( + struct xfs_inode *ip, /* incore inode pointer */ + int size, /* space new attribute needs */ + int rsvd) /* xact may use reserved blks */ +{ + struct xfs_mount *mp = ip->i_mount; + struct xfs_trans *tp; /* transaction pointer */ + unsigned int blks; /* space reservation */ + int error; /* error return value */ + + ASSERT(!XFS_NOT_DQATTACHED(mp, ip)); + + blks = XFS_ADDAFORK_SPACE_RES(mp); + + error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_addafork, blks, 0, + rsvd, &tp); + if (error) + return error; + + if (xfs_inode_has_attr_fork(ip)) + goto trans_cancel; + + error = xfs_bmap_add_attrfork(tp, ip, size, rsvd); + if (error) + goto trans_cancel; + + error = xfs_trans_commit(tp); + xfs_iunlock(ip, XFS_ILOCK_EXCL); + return error; + +trans_cancel: + xfs_trans_cancel(tp); + xfs_iunlock(ip, XFS_ILOCK_EXCL); + return error; +} + /* * Note: If args->value is NULL the attribute will be removed, just like the * Linux ->setattr API. @@ -986,7 +1023,7 @@ xfs_attr_set( xfs_attr_sf_entsize_byname(args->namelen, args->valuelen); - error = xfs_bmap_add_attrfork(dp, sf_size, rsvd); + error = xfs_attr_add_fork(dp, sf_size, rsvd); if (error) return error; } diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index 1dd1876a7b145..d34354d2cdd49 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -1007,38 +1007,29 @@ xfs_bmap_set_attrforkoff( } /* - * Convert inode from non-attributed to attributed. - * Must not be in a transaction, ip must not be locked. + * Convert inode from non-attributed to attributed. Caller must hold the + * ILOCK_EXCL and the file cannot have an attr fork. */ int /* error code */ xfs_bmap_add_attrfork( - xfs_inode_t *ip, /* incore inode pointer */ + struct xfs_trans *tp, + struct xfs_inode *ip, /* incore inode pointer */ int size, /* space new attribute needs */ int rsvd) /* xact may use reserved blks */ { - xfs_mount_t *mp; /* mount structure */ - xfs_trans_t *tp; /* transaction pointer */ - int blks; /* space reservation */ + struct xfs_mount *mp = tp->t_mountp; int version = 1; /* superblock attr version */ int logflags; /* logging flags */ int error; /* error return value */ - mp = ip->i_mount; + ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); ASSERT(!XFS_NOT_DQATTACHED(mp, ip)); - - blks = XFS_ADDAFORK_SPACE_RES(mp); - - error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_addafork, blks, 0, - rsvd, &tp); - if (error) - return error; - if (xfs_inode_has_attr_fork(ip)) - goto trans_cancel; + ASSERT(!xfs_inode_has_attr_fork(ip)); xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); error = xfs_bmap_set_attrforkoff(ip, size, &version); if (error) - goto trans_cancel; + return error; xfs_ifork_init_attr(ip, XFS_DINODE_FMT_EXTENTS, 0); logflags = 0; @@ -1059,7 +1050,7 @@ xfs_bmap_add_attrfork( if (logflags) xfs_trans_log_inode(tp, ip, logflags); if (error) - goto trans_cancel; + return error; if (!xfs_has_attr(mp) || (!xfs_has_attr2(mp) && version == 2)) { bool log_sb = false; @@ -1078,14 +1069,7 @@ xfs_bmap_add_attrfork( xfs_log_sb(tp); } - error = xfs_trans_commit(tp); - xfs_iunlock(ip, XFS_ILOCK_EXCL); - return error; - -trans_cancel: - xfs_trans_cancel(tp); - xfs_iunlock(ip, XFS_ILOCK_EXCL); - return error; + return 0; } /* diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h index 87633449c379a..c9e297dba88d0 100644 --- a/fs/xfs/libxfs/xfs_bmap.h +++ b/fs/xfs/libxfs/xfs_bmap.h @@ -174,7 +174,8 @@ int xfs_bmap_longest_free_extent(struct xfs_perag *pag, void xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno, xfs_filblks_t len); unsigned int xfs_bmap_compute_attr_offset(struct xfs_mount *mp); -int xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd); +int xfs_bmap_add_attrfork(struct xfs_trans *tp, struct xfs_inode *ip, + int size, int rsvd); void xfs_bmap_local_to_extents_empty(struct xfs_trans *tp, struct xfs_inode *ip, int whichfork); int xfs_bmap_local_to_extents(struct xfs_trans *tp, struct xfs_inode *ip, From patchwork Sun Dec 31 20:58:08 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507529 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CDB34C126 for ; Sun, 31 Dec 2023 20:58:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="XNciWu16" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 55489C433C8; Sun, 31 Dec 2023 20:58:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056289; bh=VaJZ+1/YANtXO1wdFb1ICPCYF2x0BtGvPxKhlpqHp6w=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=XNciWu16Er5kFmOuLStRfjPeZwXit1gzU78aaJfpcROlx/F0cYlUGWGSzf203RjGy SMhGGgHAdJT0dRVsu2031ZT8MoGFUVZRqsqs9OxtH4vd4kyyIB5ueVf5fLL7yq3n58 pETp9UpulQHqHgLLAQOkfVE/jkK99X/ODZIQYWbkgOBxeYLJh9qH0WyGUApNeL9xYn aS01YuBgNEFGU/0IrvVgwHqeXnTR3mPoWwtQVsfuOzmV1FsjXBhAEAOLcCgMXMdz45 UamdfIGXtifOw1ZyaJgtfoVn8FfVpvzAxuGmGIwriJx07r/PAAx+TuQzpI8YCQo9Vd Iu9k7dcwM+ksg== Date: Sun, 31 Dec 2023 12:58:08 -0800 Subject: [PATCH 19/22] xfs: actually rebuild the parent pointer xattrs From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404842052.1757392.17748089433238342217.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong Once we've assembled all the parent pointers for a file, we need to commit the new dataset atomically to that file. Parent pointer records are embedded in the xattr structure, which means that we must write a new extended attribute structure, again, atomically. Therefore, we must copy the non-parent-pointer attributes from the file being repaired into the temporary file's extended attributes and then call the atomic extent swap mechanism to exchange the blocks. Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_attr.c | 2 fs/xfs/libxfs/xfs_attr.h | 2 fs/xfs/scrub/attr.c | 2 fs/xfs/scrub/attr_repair.c | 4 fs/xfs/scrub/attr_repair.h | 4 fs/xfs/scrub/findparent.c | 2 fs/xfs/scrub/listxattr.c | 12 + fs/xfs/scrub/listxattr.h | 4 fs/xfs/scrub/parent.c | 2 fs/xfs/scrub/parent_repair.c | 696 +++++++++++++++++++++++++++++++++++++++++- fs/xfs/scrub/trace.h | 2 11 files changed, 707 insertions(+), 25 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index ef32c2a22c617..9cefaeca8f854 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -935,7 +935,7 @@ xfs_attr_defer_add( trace_xfs_attr_defer_add(new->xattri_dela_state, args->dp); } -STATIC int +int xfs_attr_add_fork( struct xfs_inode *ip, /* incore inode pointer */ int size, /* space new attribute needs */ diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h index 0204f62298cb5..a2cfe9e35fd43 100644 --- a/fs/xfs/libxfs/xfs_attr.h +++ b/fs/xfs/libxfs/xfs_attr.h @@ -650,4 +650,6 @@ xfs_attri_can_use_without_log_assistance( return false; } +int xfs_attr_add_fork(struct xfs_inode *ip, int size, int rsvd); + #endif /* __XFS_ATTR_H__ */ diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c index f213d745746fd..555e1b65c78aa 100644 --- a/fs/xfs/scrub/attr.c +++ b/fs/xfs/scrub/attr.c @@ -647,7 +647,7 @@ xchk_xattr( * iteration, which doesn't really follow the usual buffer * locking order. */ - error = xchk_xattr_walk(sc, sc->ip, xchk_xattr_actor, NULL); + error = xchk_xattr_walk(sc, sc->ip, xchk_xattr_actor, NULL, NULL); if (!xchk_fblock_process_error(sc, XFS_ATTR_FORK, 0, &error)) return error; diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c index cc964dc427e23..0c369a243a635 100644 --- a/fs/xfs/scrub/attr_repair.c +++ b/fs/xfs/scrub/attr_repair.c @@ -1036,7 +1036,7 @@ xrep_xattr_reset_fork( * fork. The caller must ILOCK the tempfile and join it to the transaction. * This function returns with the inode joined to a clean scrub transaction. */ -STATIC int +int xrep_xattr_reset_tempfile_fork( struct xfs_scrub *sc) { @@ -1356,7 +1356,7 @@ xrep_xattr_swap_prep( } /* Swap the temporary file's attribute fork with the one being repaired. */ -STATIC int +int xrep_xattr_swap( struct xfs_scrub *sc, struct xrep_tempswap *tx) diff --git a/fs/xfs/scrub/attr_repair.h b/fs/xfs/scrub/attr_repair.h index 0a9ffa7cfa906..ea73b8d498a24 100644 --- a/fs/xfs/scrub/attr_repair.h +++ b/fs/xfs/scrub/attr_repair.h @@ -6,6 +6,10 @@ #ifndef __XFS_SCRUB_ATTR_REPAIR_H__ #define __XFS_SCRUB_ATTR_REPAIR_H__ +struct xrep_tempswap; + +int xrep_xattr_swap(struct xfs_scrub *sc, struct xrep_tempswap *tx); int xrep_xattr_reset_fork(struct xfs_scrub *sc); +int xrep_xattr_reset_tempfile_fork(struct xfs_scrub *sc); #endif /* __XFS_SCRUB_ATTR_REPAIR_H__ */ diff --git a/fs/xfs/scrub/findparent.c b/fs/xfs/scrub/findparent.c index 9468029f73933..ceb76b26c6cd1 100644 --- a/fs/xfs/scrub/findparent.c +++ b/fs/xfs/scrub/findparent.c @@ -24,6 +24,7 @@ #include "xfs_trans_space.h" #include "xfs_health.h" #include "xfs_swapext.h" +#include "xfs_parent.h" #include "scrub/xfs_scrub.h" #include "scrub/scrub.h" #include "scrub/common.h" @@ -33,6 +34,7 @@ #include "scrub/findparent.h" #include "scrub/readdir.h" #include "scrub/tempfile.h" +#include "scrub/listxattr.h" /* * Finding the Parent of a Directory diff --git a/fs/xfs/scrub/listxattr.c b/fs/xfs/scrub/listxattr.c index dc893f2cdc1c3..44f9660ae544f 100644 --- a/fs/xfs/scrub/listxattr.c +++ b/fs/xfs/scrub/listxattr.c @@ -267,6 +267,7 @@ xchk_xattr_walk_node( struct xfs_scrub *sc, struct xfs_inode *ip, xchk_xattr_fn attr_fn, + xchk_xattrleaf_fn leaf_fn, void *priv) { struct xfs_attr3_icleaf_hdr leafhdr; @@ -298,6 +299,12 @@ xchk_xattr_walk_node( xfs_trans_brelse(sc->tp, leaf_bp); + if (leaf_fn) { + error = leaf_fn(sc, priv); + if (error) + goto out_bitmap; + } + /* Make sure we haven't seen this new leaf already. */ len = 1; if (xdab_bitmap_test(&seen_dablks, leafhdr.forw, &len)) @@ -332,6 +339,7 @@ xchk_xattr_walk( struct xfs_scrub *sc, struct xfs_inode *ip, xchk_xattr_fn attr_fn, + xchk_xattrleaf_fn leaf_fn, void *priv) { int error; @@ -352,7 +360,7 @@ xchk_xattr_walk( if (xfs_attr_is_leaf(ip)) return xchk_xattr_walk_leaf(sc, ip, attr_fn, priv); - return xchk_xattr_walk_node(sc, ip, attr_fn, priv); + return xchk_xattr_walk_node(sc, ip, attr_fn, leaf_fn, priv); } /* @@ -380,5 +388,5 @@ xchk_pptr_walk( ASSERT(xfs_has_parent(sc->mp)); - return xchk_xattr_walk(sc, ip, xchk_pptr_walk_attr, &pw); + return xchk_xattr_walk(sc, ip, xchk_pptr_walk_attr, NULL, &pw); } diff --git a/fs/xfs/scrub/listxattr.h b/fs/xfs/scrub/listxattr.h index 7e4bd3ae75e15..6bd41443c5f44 100644 --- a/fs/xfs/scrub/listxattr.h +++ b/fs/xfs/scrub/listxattr.h @@ -11,8 +11,10 @@ typedef int (*xchk_xattr_fn)(struct xfs_scrub *sc, struct xfs_inode *ip, unsigned int namelen, const void *value, unsigned int valuelen, void *priv); +typedef int (*xchk_xattrleaf_fn)(struct xfs_scrub *sc, void *priv); + int xchk_xattr_walk(struct xfs_scrub *sc, struct xfs_inode *ip, - xchk_xattr_fn attr_fn, void *priv); + xchk_xattr_fn attr_fn, xchk_xattrleaf_fn leaf_fn, void *priv); struct xfs_parent_name_irec; diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c index 555aee4b73b37..6af212540c631 100644 --- a/fs/xfs/scrub/parent.c +++ b/fs/xfs/scrub/parent.c @@ -768,7 +768,7 @@ xchk_parent_pptr( if (error) goto out_entries; - error = xchk_xattr_walk(sc, sc->ip, xchk_parent_scan_attr, pp); + error = xchk_xattr_walk(sc, sc->ip, xchk_parent_scan_attr, NULL, pp); if (error == -ECANCELED) { error = 0; goto out_names; diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c index b94eebec3cc79..00e6735717a37 100644 --- a/fs/xfs/scrub/parent_repair.c +++ b/fs/xfs/scrub/parent_repair.c @@ -25,6 +25,8 @@ #include "xfs_health.h" #include "xfs_swapext.h" #include "xfs_parent.h" +#include "xfs_attr.h" +#include "xfs_bmap.h" #include "scrub/xfs_scrub.h" #include "scrub/scrub.h" #include "scrub/common.h" @@ -34,10 +36,13 @@ #include "scrub/findparent.h" #include "scrub/readdir.h" #include "scrub/tempfile.h" +#include "scrub/tempswap.h" #include "scrub/orphanage.h" #include "scrub/xfile.h" #include "scrub/xfarray.h" #include "scrub/xfblob.h" +#include "scrub/attr_repair.h" +#include "scrub/listxattr.h" /* * Repairing The Directory Parent Pointer @@ -107,6 +112,23 @@ struct xrep_parent { /* Blobs containing parent pointer names. */ struct xfblob *pptr_names; + /* xattr keys */ + struct xfarray *xattr_records; + + /* xattr values */ + struct xfblob *xattr_blobs; + + /* Scratch buffers for saving extended attributes */ + unsigned char *xattr_name; + void *xattr_value; + unsigned int xattr_value_sz; + + /* + * Information used to swap the attr fork, if the fs supports parent + * pointers. + */ + struct xrep_tempswap tx; + /* * Information used to scan the filesystem to find the inumber of the * dotdot entry for this directory. On filesystems without parent @@ -118,12 +140,17 @@ struct xrep_parent { * @pscan.lock coordinates access to pptr_recs, pptr_names, pptr, and * pptr_scratch. This reduces the memory requirements of this * structure. + * + * The lock also controls access to xattr_records and xattr_blobs(?) */ struct xrep_parent_scan_info pscan; /* Orphanage reparenting request. */ struct xrep_adoption adoption; + /* Have we seen any live updates of parent pointers recently? */ + bool saw_pptr_updates; + /* xattr key and da args for parent pointer replay. */ struct xfs_parent_scratch pptr_scratch; @@ -135,12 +162,43 @@ struct xrep_parent { struct xfs_parent_name_irec pptr; }; +struct xrep_parent_xattr { + /* Cookie for retrieval of the xattr name. */ + xfblob_cookie name_cookie; + + /* Cookie for retrieval of the xattr value. */ + xfblob_cookie value_cookie; + + /* XFS_ATTR_* flags */ + int flags; + + /* Length of the value and name. */ + uint32_t valuelen; + uint16_t namelen; +}; + +/* + * Stash up to 8 pages of attrs in xattr_records/xattr_blobs before we write + * them to the temp file. + */ +#define XREP_PARENT_XATTR_MAX_STASH_BYTES (PAGE_SIZE * 8) + /* Tear down all the incore stuff we created. */ static void xrep_parent_teardown( struct xrep_parent *rp) { xrep_findparent_scan_teardown(&rp->pscan); + kvfree(rp->xattr_name); + rp->xattr_name = NULL; + kvfree(rp->xattr_value); + rp->xattr_value = NULL; + if (rp->xattr_blobs) + xfblob_destroy(rp->xattr_blobs); + rp->xattr_blobs = NULL; + if (rp->xattr_records) + xfarray_destroy(rp->xattr_records); + rp->xattr_records = NULL; if (rp->pptr_names) xfblob_destroy(rp->pptr_names); rp->pptr_names = NULL; @@ -574,10 +632,11 @@ xrep_parent_scan_dirtree( } /* - * Cancel the empty transaction so that we can (later) use the atomic - * extent swap helpers to lock files and commit the new directory. + * Retake sc->ip's ILOCK now that we're done flushing stashed parent + * pointers. We end this function with an empty transaction and the + * ILOCK. */ - xchk_trans_cancel(rp->sc); + xchk_ilock(rp->sc, XFS_ILOCK_EXCL); return 0; } @@ -612,6 +671,8 @@ xrep_parent_live_update( else error = xrep_parent_stash_parentremove(rp, p->name, p->dp); + if (!error) + rp->saw_pptr_updates = true; mutex_unlock(&rp->pscan.lock); if (error) goto out_abort; @@ -666,6 +727,45 @@ xrep_parent_reset_dotdot( return xfs_trans_roll(&sc->tp); } +/* Pass back the parent inumber if this a parent pointer */ +STATIC int +xrep_parent_lookup_pptr( + struct xfs_scrub *sc, + struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, + void *priv) +{ + xfs_ino_t *inop = priv; + + if (!xfs_parent_verify_irec(sc->mp, pptr)) + return -EFSCORRUPTED; + + *inop = pptr->p_ino; + return -ECANCELED; +} + +/* + * Find the first parent of the scrub target by walking parent pointers for + * the purpose of deciding if we're going to move it to the orphanage. + * We don't care if the attr fork is zapped. + */ +STATIC int +xrep_parent_lookup_pptrs( + struct xfs_scrub *sc, + xfs_ino_t *inop) +{ + struct xfs_parent_name_irec pptr; + int error; + + *inop = NULLFSINO; + + error = xchk_pptr_walk(sc, sc->ip, xrep_parent_lookup_pptr, &pptr, + inop); + if (error && error != -ECANCELED) + return error; + return 0; +} + /* * Move the current file to the orphanage. * @@ -682,14 +782,25 @@ xrep_parent_move_to_orphanage( xfs_ino_t orig_parent, new_parent; int error; - /* - * We are about to drop the ILOCK on sc->ip to lock the orphanage and - * prepare for the adoption. Therefore, look up the old dotdot entry - * for sc->ip so that we can compare it after we re-lock sc->ip. - */ - error = xchk_dir_lookup(sc, sc->ip, &xfs_name_dotdot, &orig_parent); - if (error) - return error; + if (S_ISDIR(VFS_I(sc->ip)->i_mode)) { + /* + * We are about to drop the ILOCK on sc->ip to lock the + * orphanage and prepare for the adoption. Therefore, look up + * the old dotdot entry for sc->ip so that we can compare it + * after we re-lock sc->ip. + */ + error = xchk_dir_lookup(sc, sc->ip, &xfs_name_dotdot, + &orig_parent); + if (error) + return error; + } else { + /* + * We haven't dropped the ILOCK since we swapped in the new + * parent pointers, which means that the file cannot have been + * moved in the directory tree, and there are no parents. + */ + orig_parent = NULLFSINO; + } /* * Drop the ILOCK on the scrub target and commit the transaction. @@ -722,9 +833,14 @@ xrep_parent_move_to_orphanage( * Now that we've reacquired the ILOCK on sc->ip, look up the dotdot * entry again. If the parent changed or the child was unlinked while * the child directory was unlocked, we don't need to move the child to - * the orphanage after all. + * the orphanage after all. For a non-directory, we have to scan for + * the first parent pointer to see if one has been added. */ - error = xchk_dir_lookup(sc, sc->ip, &xfs_name_dotdot, &new_parent); + if (S_ISDIR(VFS_I(sc->ip)->i_mode)) + error = xchk_dir_lookup(sc, sc->ip, &xfs_name_dotdot, + &new_parent); + else + error = xrep_parent_lookup_pptrs(sc, &new_parent); if (error) return error; @@ -751,6 +867,492 @@ xrep_parent_move_to_orphanage( return 0; } +/* Ensure that the xattr value buffer is large enough. */ +STATIC int +xrep_parent_alloc_xattr_value( + struct xrep_parent *rp, + size_t bufsize) +{ + void *new_val; + + if (rp->xattr_value_sz >= bufsize) + return 0; + + if (rp->xattr_value) { + kvfree(rp->xattr_value); + rp->xattr_value = NULL; + rp->xattr_value_sz = 0; + } + + new_val = kvmalloc(bufsize, XCHK_GFP_FLAGS); + if (!new_val) + return -ENOMEM; + + rp->xattr_value = new_val; + rp->xattr_value_sz = bufsize; + return 0; +} + +/* Retrieve the (remote) value of a non-pptr xattr. */ +STATIC int +xrep_parent_fetch_xattr_remote( + struct xrep_parent *rp, + struct xfs_inode *ip, + unsigned int attr_flags, + const unsigned char *name, + unsigned int namelen, + unsigned int valuelen) +{ + struct xfs_scrub *sc = rp->sc; + struct xfs_da_args args = { + .op_flags = XFS_DA_OP_NOTIME, + .attr_filter = attr_flags & XFS_ATTR_NSP_ONDISK_MASK, + .geo = sc->mp->m_attr_geo, + .whichfork = XFS_ATTR_FORK, + .dp = ip, + .name = name, + .namelen = namelen, + .hashval = xfs_da_hashname(name, namelen), + .trans = sc->tp, + .valuelen = valuelen, + .owner = ip->i_ino, + }; + int error; + + /* + * If we need a larger value buffer, try to allocate one. If that + * fails, return with -EDEADLOCK to try harder. + */ + error = xrep_parent_alloc_xattr_value(rp, valuelen); + if (error == -ENOMEM) + return -EDEADLOCK; + if (error) + return error; + + args.value = rp->xattr_value; + return xfs_attr_get_ilocked(&args); +} + +/* Stash non-pptr attributes for later replay into the temporary file. */ +STATIC int +xrep_parent_stash_xattr( + struct xfs_scrub *sc, + struct xfs_inode *ip, + unsigned int attr_flags, + const unsigned char *name, + unsigned int namelen, + const void *value, + unsigned int valuelen, + void *priv) +{ + struct xrep_parent_xattr key = { + .valuelen = valuelen, + .namelen = namelen, + .flags = attr_flags & XFS_ATTR_NSP_ONDISK_MASK, + }; + struct xrep_parent *rp = priv; + int error; + + if (attr_flags & (XFS_ATTR_INCOMPLETE | XFS_ATTR_PARENT)) + return 0; + + if (!value) { + error = xrep_parent_fetch_xattr_remote(rp, ip, attr_flags, + name, namelen, valuelen); + if (error) + return error; + + value = rp->xattr_value; + } + + trace_xrep_parent_stash_xattr(rp->sc->tempip, key.flags, (void *)name, + key.namelen, key.valuelen); + + error = xfblob_store(rp->xattr_blobs, &key.name_cookie, name, + key.namelen); + if (error) + return error; + + error = xfblob_store(rp->xattr_blobs, &key.value_cookie, value, + key.valuelen); + if (error) + return error; + + return xfarray_append(rp->xattr_records, &key); +} + +/* Insert one xattr key/value. */ +STATIC int +xrep_parent_insert_xattr( + struct xrep_parent *rp, + const struct xrep_parent_xattr *key) +{ + struct xfs_da_args args = { + .dp = rp->sc->tempip, + .attr_filter = key->flags, + .namelen = key->namelen, + .valuelen = key->valuelen, + .op_flags = XFS_DA_OP_NOTIME, + .owner = rp->sc->ip->i_ino, + }; + int error; + + ASSERT(!(key->flags & XFS_ATTR_PARENT)); + + /* + * Grab pointers to the scrub buffer so that we can use them to insert + * attrs into the temp file. + */ + args.name = rp->xattr_name; + args.value = rp->xattr_value; + + /* + * The attribute name is stored near the end of the in-core buffer, + * though we reserve one more byte to ensure null termination. + */ + rp->xattr_name[XATTR_NAME_MAX] = 0; + + error = xfblob_load(rp->xattr_blobs, key->name_cookie, rp->xattr_name, + key->namelen); + if (error) + return error; + + error = xfblob_free(rp->xattr_blobs, key->name_cookie); + if (error) + return error; + + error = xfblob_load(rp->xattr_blobs, key->value_cookie, args.value, + key->valuelen); + if (error) + return error; + + error = xfblob_free(rp->xattr_blobs, key->value_cookie); + if (error) + return error; + + rp->xattr_name[key->namelen] = 0; + + trace_xrep_parent_insert_xattr(rp->sc->tempip, key->flags, + rp->xattr_name, key->namelen, key->valuelen); + + error = xfs_attr_set(&args); + if (error) { + ASSERT(error != -EEXIST); + return error; + } + + return 0; +} + +/* + * Periodically flush salvaged attributes to the temporary file. This is done + * to reduce the memory requirements of the xattr rebuild because files can + * contain millions of attributes. + */ +STATIC int +xrep_parent_flush_xattrs( + struct xrep_parent *rp) +{ + xfarray_idx_t array_cur; + int error; + + /* + * Entering this function, the scrub context has a reference to the + * inode being repaired, the temporary file, and the empty scrub + * transaction that we created for the xattr scan. We hold ILOCK_EXCL + * on the inode being repaired. + * + * To constrain kernel memory use, we occasionally flush salvaged + * xattrs from the xfarray and xfblob structures into the temporary + * file in preparation for swapping the xattr structures at the end. + * Updating the temporary file requires a transaction, so we commit the + * scrub transaction and drop the ILOCK so that xfs_attr_set can + * allocate whatever transaction it wants. + * + * We still hold IOLOCK_EXCL on the inode being repaired, which + * prevents anyone from adding xattrs (or parent pointers) while we're + * flushing. + */ + xchk_trans_cancel(rp->sc); + xchk_iunlock(rp->sc, XFS_ILOCK_EXCL); + + /* + * Take the IOLOCK of the temporary file while we modify xattrs. This + * isn't strictly required because the temporary file is never revealed + * to userspace, but we follow the same locking rules. We still hold + * sc->ip's IOLOCK. + */ + error = xrep_tempfile_iolock_polled(rp->sc); + if (error) + return error; + + /* Add all the salvaged attrs to the temporary file. */ + foreach_xfarray_idx(rp->xattr_records, array_cur) { + struct xrep_parent_xattr key; + + error = xfarray_load(rp->xattr_records, array_cur, &key); + if (error) + return error; + + error = xrep_parent_insert_xattr(rp, &key); + if (error) + return error; + } + + /* Empty out both arrays now that we've added the entries. */ + xfarray_truncate(rp->xattr_records); + xfblob_truncate(rp->xattr_blobs); + + xrep_tempfile_iounlock(rp->sc); + + /* Recreate the empty transaction and relock the inode. */ + error = xchk_trans_alloc_empty(rp->sc); + if (error) + return error; + xchk_ilock(rp->sc, XFS_ILOCK_EXCL); + return 0; +} + +/* Decide if we've stashed too much xattr data in memory. */ +static inline bool +xrep_parent_want_flush_xattrs( + struct xrep_parent *rp) +{ + unsigned long long bytes; + + bytes = xfarray_bytes(rp->xattr_records) + + xfblob_bytes(rp->xattr_blobs); + return bytes > XREP_PARENT_XATTR_MAX_STASH_BYTES; +} + +/* Flush staged attributes to the temporary file if we're over the limit. */ +STATIC int +xrep_parent_try_flush_xattrs( + struct xfs_scrub *sc, + void *priv) +{ + struct xrep_parent *rp = priv; + int error; + + if (!xrep_parent_want_flush_xattrs(rp)) + return 0; + + error = xrep_parent_flush_xattrs(rp); + if (error) + return error; + + /* + * If there were any parent pointer updates to the xattr structure + * while we dropped the ILOCK, the xattr structure is now stale. + * Signal to the attr copy process that we need to start over, but + * this time without opportunistic attr flushing. + * + * This is unlikely to happen, so we're ok with restarting the copy. + */ + mutex_lock(&rp->pscan.lock); + if (rp->saw_pptr_updates) + error = -ESTALE; + mutex_unlock(&rp->pscan.lock); + return error; +} + +/* Copy all the non-pptr extended attributes into the temporary file. */ +STATIC int +xrep_parent_copy_xattrs( + struct xrep_parent *rp) +{ + struct xfs_scrub *sc = rp->sc; + int error; + + /* + * Clear the pptr updates flag. We hold sc->ip ILOCKed, so there + * can't be any parent pointer updates in progress. + */ + mutex_lock(&rp->pscan.lock); + rp->saw_pptr_updates = false; + mutex_unlock(&rp->pscan.lock); + + /* Copy xattrs, stopping periodically to flush the incore buffers. */ + error = xchk_xattr_walk(sc, sc->ip, xrep_parent_stash_xattr, + xrep_parent_try_flush_xattrs, rp); + if (error && error != -ESTALE) + return error; + + if (error == -ESTALE) { + /* + * The xattr copy collided with a parent pointer update. + * Restart the copy, but this time hold the ILOCK all the way + * to the end to lock out any directory parent pointer updates. + */ + error = xchk_xattr_walk(sc, sc->ip, xrep_parent_stash_xattr, + NULL, rp); + if (error) + return error; + } + + /* Flush any remaining stashed xattrs to the temporary file. */ + if (xfarray_bytes(rp->xattr_records) == 0) + return 0; + + return xrep_parent_flush_xattrs(rp); +} + +/* + * Ensure that @sc->ip and @sc->tempip both have attribute forks before we + * head into the attr fork swap transaction. All files on a filesystem with + * parent pointers must have an attr fork because the parent pointer code + * does not itself add attribute forks. + * + * Note: Unlinkable unlinked files don't need one, but the overhead of having + * an unnecessary attr fork is not justified by the additional code complexity + * that would be needed to track that state correctly. + */ +STATIC int +xrep_parent_ensure_attr_fork( + struct xrep_parent *rp) +{ + struct xfs_scrub *sc = rp->sc; + int error; + + error = xfs_attr_add_fork(sc->tempip, + sizeof(struct xfs_attr_sf_hdr), 1); + if (error) + return error; + return xfs_attr_add_fork(sc->ip, sizeof(struct xfs_attr_sf_hdr), 1); +} + +/* + * Finish replaying stashed parent pointer updates, allocate a transaction for + * swapping extents, and take the ILOCKs of both files before we commit the new + * attribute structure. + */ +STATIC int +xrep_parent_finalize_tempfile( + struct xrep_parent *rp) +{ + struct xfs_scrub *sc = rp->sc; + int error; + + /* + * Repair relies on the ILOCK to quiesce all possible xattr updates. + * Replay all queued parent pointer updates into the tempfile before + * swapping the contents, even if that means dropping the ILOCKs and + * the transaction. + */ + do { + error = xrep_parent_replay_updates(rp); + if (error) + return error; + + error = xrep_parent_ensure_attr_fork(rp); + if (error) + return error; + + error = xrep_tempswap_trans_alloc(sc, XFS_ATTR_FORK, &rp->tx); + if (error) + return error; + + if (xfarray_length(rp->pptr_recs) == 0) + break; + + xchk_trans_cancel(sc); + xrep_tempfile_iunlock_both(sc); + } while (!xchk_should_terminate(sc, &error)); + return error; +} + +/* + * Replay all the stashed parent pointers into the temporary file, copy all + * the non-pptr xattrs from the file being repaired into the temporary file, + * and swap the extents atomically. + */ +STATIC int +xrep_parent_rebuild_pptrs( + struct xrep_parent *rp) +{ + struct xfs_scrub *sc = rp->sc; + xfs_ino_t parent_ino = NULLFSINO; + int error; + + /* + * Copy non-ppttr xattrs from the file being repaired into the + * temporary file's xattr structure. We hold sc->ip's IOLOCK, which + * prevents setxattr/removexattr calls from occurring, but renames + * update the parent pointers without holding IOLOCK. If we detect + * stale attr structures, we restart the scan but only flush at the + * end. + */ + error = xrep_parent_copy_xattrs(rp); + if (error) + return error; + + /* + * Cancel the empty transaction that we used to walk and copy attrs, + * and drop the ILOCK so that we can take the IOLOCK on the temporary + * file. We still hold sc->ip's IOLOCK. + */ + xchk_trans_cancel(sc); + xchk_iunlock(sc, XFS_ILOCK_EXCL); + + error = xrep_tempfile_iolock_polled(sc); + if (error) + return error; + + /* + * Allocate transaction, lock inodes, and make sure that we've replayed + * all the stashed pptr updates to the tempdir. After this point, + * we're ready to swapext the attr fork. + */ + error = xrep_parent_finalize_tempfile(rp); + if (error) + return error; + + /* Last chance to abort before we start committing pptr fixes. */ + if (xchk_should_terminate(sc, &error)) + return error; + + if (xchk_iscan_aborted(&rp->pscan.iscan)) + return -ECANCELED; + + /* + * Swap the attr fork and junk the old attr fork contents, which are + * now in the tempfile. + */ + error = xrep_xattr_swap(sc, &rp->tx); + if (error) + return error; + error = xrep_xattr_reset_tempfile_fork(sc); + if (error) + return error; + + /* + * Roll to get a transaction without any inodes joined to it. Then we + * can drop the tempfile's ILOCK and IOLOCK before doing more work on + * the scrub target file. + */ + error = xfs_trans_roll(&sc->tp); + if (error) + return error; + xrep_tempfile_iunlock(sc); + xrep_tempfile_iounlock(sc); + + /* + * We've committed the new parent pointers. Find at least one parent + * so that we can decide if we're moving this file to the orphanage. + * For this purpose, root directories are their own parents. + */ + if (sc->ip == sc->mp->m_rootip) { + xrep_findparent_scan_found(&rp->pscan, sc->ip->i_ino); + } else { + error = xrep_parent_lookup_pptrs(sc, &parent_ino); + if (error) + return error; + if (parent_ino != NULLFSINO) + xrep_findparent_scan_found(&rp->pscan, parent_ino); + } + return 0; +} + /* * Commit the new parent pointer structure (currently only the dotdot entry) to * the file that we're repairing. @@ -759,13 +1361,24 @@ STATIC int xrep_parent_rebuild_tree( struct xrep_parent *rp) { + int error; + + if (xfs_has_parent(rp->sc->mp)) { + error = xrep_parent_rebuild_pptrs(rp); + if (error) + return error; + } + if (rp->pscan.parent_ino == NULLFSINO) { if (xrep_orphanage_can_adopt(rp->sc)) return xrep_parent_move_to_orphanage(rp); return -EFSCORRUPTED; } - return xrep_parent_reset_dotdot(rp); + if (S_ISDIR(VFS_I(rp->sc->ip)->i_mode)) + return xrep_parent_reset_dotdot(rp); + + return 0; } /* Set up the filesystem scan so we can look for parents. */ @@ -775,18 +1388,39 @@ xrep_parent_setup_scan( { struct xfs_scrub *sc = rp->sc; char *descr; + struct xfs_da_geometry *geo = sc->mp->m_attr_geo; + int max_len; int error; if (!xfs_has_parent(sc->mp)) return xrep_findparent_scan_start(sc, &rp->pscan); + /* Buffers for copying non-pptr attrs to the tempfile */ + rp->xattr_name = kvmalloc(XATTR_NAME_MAX + 1, XCHK_GFP_FLAGS); + if (!rp->xattr_name) + return -ENOMEM; + + /* + * Allocate enough memory to handle loading local attr values from the + * xfblob data while flushing stashed attrs to the temporary file. + * We only realloc the buffer when salvaging remote attr values, so + * TRY_HARDER means we allocate the maximal attr value size. + */ + if (sc->flags & XCHK_TRY_HARDER) + max_len = XATTR_SIZE_MAX; + else + max_len = xfs_attr_leaf_entsize_local_max(geo->blksize); + error = xrep_parent_alloc_xattr_value(rp, max_len); + if (error) + goto out_xattr_name; + /* Set up some staging memory for logging parent pointer updates. */ descr = xchk_xfile_ino_descr(sc, "parent pointer entries"); error = xfarray_create(descr, 0, sizeof(struct xrep_pptr), &rp->pptr_recs); kfree(descr); if (error) - return error; + goto out_xattr_value; descr = xchk_xfile_ino_descr(sc, "parent pointer names"); error = xfblob_create(descr, &rp->pptr_names); @@ -794,19 +1428,47 @@ xrep_parent_setup_scan( if (error) goto out_recs; + /* Set up some storage for copying attrs before the swap */ + descr = xchk_xfile_ino_descr(sc, + "parent pointer retained xattr entries"); + error = xfarray_create(descr, 0, sizeof(struct xrep_parent_xattr), + &rp->xattr_records); + kfree(descr); + if (error) + goto out_names; + + descr = xchk_xfile_ino_descr(sc, + "parent pointer retained xattr values"); + error = xfblob_create(descr, &rp->xattr_blobs); + kfree(descr); + if (error) + goto out_attr_keys; + error = __xrep_findparent_scan_start(sc, &rp->pscan, xrep_parent_live_update); if (error) - goto out_names; + goto out_attr_values; return 0; +out_attr_values: + xfblob_destroy(rp->xattr_blobs); + rp->xattr_blobs = NULL; +out_attr_keys: + xfarray_destroy(rp->xattr_records); + rp->xattr_records = NULL; out_names: xfblob_destroy(rp->pptr_names); rp->pptr_names = NULL; out_recs: xfarray_destroy(rp->pptr_recs); rp->pptr_recs = NULL; +out_xattr_value: + kvfree(rp->xattr_value); + rp->xattr_value = NULL; +out_xattr_name: + kvfree(rp->xattr_name); + rp->xattr_name = NULL; return error; } @@ -836,7 +1498,7 @@ xrep_parent( if (error) goto out_teardown; - /* Last chance to abort before we start committing fixes. */ + /* Last chance to abort before we start committing dotdot fixes. */ if (xchk_should_terminate(sc, &error)) goto out_teardown; diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h index b03e1e69740fa..10e2d6544c5ad 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -2661,6 +2661,8 @@ DEFINE_EVENT(xrep_xattr_salvage_class, name, \ TP_ARGS(ip, flags, name, namelen, valuelen)) DEFINE_XREP_XATTR_SALVAGE_EVENT(xrep_xattr_salvage_rec); DEFINE_XREP_XATTR_SALVAGE_EVENT(xrep_xattr_insert_rec); +DEFINE_XREP_XATTR_SALVAGE_EVENT(xrep_parent_stash_xattr); +DEFINE_XREP_XATTR_SALVAGE_EVENT(xrep_parent_insert_xattr); DECLARE_EVENT_CLASS(xrep_pptr_salvage_class, TP_PROTO(struct xfs_inode *ip, unsigned int flags, const void *name, From patchwork Sun Dec 31 20:58:24 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507530 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 322AEBA2B for ; Sun, 31 Dec 2023 20:58:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="tEfVyhyU" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0313AC433C8; Sun, 31 Dec 2023 20:58:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056305; bh=S7VSA4CrBU009tt9ZgsQ+nOp1XmxH3qzjcafMRv6a9s=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=tEfVyhyUvfxO64wLDgaNBoPoItRhqhJBpI0sCaJUL9c39ZTUd8t3lO79HqvkMbyJJ VgbvCIVD/mXSp+VNWG18vDpbWKUtYXndO56YMJbwjeSRR/VgFywtNaVU9YiF5QbQk4 632jK4yJfuqgb5gQQdDpt8wdGpVaV5R8YyE/Jegiu/iWRFvNC3kgUjeWtcu54CkZVf RjwOXbcxqUhfz/TWnvB86ihaUghmvMwK2Dr0t6U24Icjme5g8THfNzB9dudTGECMDt TVdK0ohMwxMYZ0BLH2IuuuuJZLYE/D6Mymr5ayA/oGyVCjUJxon5+csx0WpRxLV7ha qjUm2JrwSdpDA== Date: Sun, 31 Dec 2023 12:58:24 -0800 Subject: [PATCH 20/22] xfs: adapt the orphanage code to handle parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404842069.1757392.18360146955036269056.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong Adapt the orphanage's adoption code to update the child file's parent pointers as part of the reparenting process. Also ensure that the child has an attr fork to receive the parent pointer update, since the runtime code assumes one exists. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/orphanage.c | 42 ++++++++++++++++++++++++++++++++++++++++++ fs/xfs/scrub/orphanage.h | 3 +++ fs/xfs/scrub/scrub.c | 2 ++ 3 files changed, 47 insertions(+) diff --git a/fs/xfs/scrub/orphanage.c b/fs/xfs/scrub/orphanage.c index ace7a0f23e474..b894b807155a7 100644 --- a/fs/xfs/scrub/orphanage.c +++ b/fs/xfs/scrub/orphanage.c @@ -19,6 +19,8 @@ #include "xfs_icache.h" #include "xfs_bmap.h" #include "xfs_bmap_btree.h" +#include "xfs_parent.h" +#include "xfs_attr_sf.h" #include "scrub/scrub.h" #include "scrub/common.h" #include "scrub/repair.h" @@ -330,8 +332,12 @@ xrep_adoption_trans_alloc( if (S_ISDIR(VFS_I(sc->ip)->i_mode)) child_blkres = xfs_rename_space_res(mp, 0, false, xfs_name_dotdot.len, false); + if (xfs_has_parent(mp)) + child_blkres += XFS_ADDAFORK_SPACE_RES(mp); adopt->child_blkres = child_blkres; + xfs_parent_args_init(mp, &adopt->ppargs); + /* * Allocate a transaction to link the child into the parent, along with * enough disk space to handle expansion of both the orphanage and the @@ -500,6 +506,21 @@ xrep_adoption_zap_dcache( dput(d_orphanage); } +/* + * If we have to add an attr fork ahead of a parent pointer update, how much + * space should we ask for? + */ +static inline int +xrep_adoption_attr_sizeof( + const struct xrep_adoption *adopt) +{ + size_t res = sizeof(struct xfs_attr_sf_hdr); + + res += xfs_attr_sf_entsize_byname(sizeof(struct xfs_parent_name_rec), + adopt->xname.len); + return res; +} + /* * Move the current file to the orphanage under the computed name. * @@ -522,6 +543,19 @@ xrep_adoption_move( if (error) return error; + /* + * If this filesystem has parent pointers, ensure that the file being + * moved to the orphanage has an attribute fork. This is required + * because the parent pointer code does not itself add attr forks. + */ + if (!xfs_inode_has_attr_fork(sc->ip) && xfs_has_parent(sc->mp)) { + int sf_size = xrep_adoption_attr_sizeof(adopt); + + error = xfs_bmap_add_attrfork(sc->tp, sc->ip, sf_size, true); + if (error) + return error; + } + /* Create the new name in the orphanage. */ error = xfs_dir_createname(sc->tp, sc->orphanage, xname, sc->ip->i_ino, adopt->orphanage_blkres); @@ -546,6 +580,14 @@ xrep_adoption_move( return error; } + /* Add a parent pointer from the file back to the lost+found. */ + if (xfs_has_parent(sc->mp)) { + error = xfs_parent_addname(sc->tp, &adopt->ppargs, + sc->orphanage, xname, sc->ip); + if (error) + return error; + } + /* * Notify dirent hooks that we moved the file to /lost+found, and * finish all the deferred work so that we know the adoption is fully diff --git a/fs/xfs/scrub/orphanage.h b/fs/xfs/scrub/orphanage.h index 9d40992583b24..74ce0bc05c6f1 100644 --- a/fs/xfs/scrub/orphanage.h +++ b/fs/xfs/scrub/orphanage.h @@ -54,6 +54,9 @@ struct xrep_adoption { struct xfs_scrub *sc; + /* Parent pointer context tracking */ + struct xfs_parent_args ppargs; + /* Block reservations for orphanage and child (if directory). */ unsigned int orphanage_blkres; unsigned int child_blkres; diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c index 440b8cb1957f4..d9c6d54ffad7f 100644 --- a/fs/xfs/scrub/scrub.c +++ b/fs/xfs/scrub/scrub.c @@ -19,6 +19,8 @@ #include "xfs_rmap.h" #include "xfs_xchgrange.h" #include "xfs_swapext.h" +#include "xfs_dir2.h" +#include "xfs_parent.h" #include "scrub/scrub.h" #include "scrub/common.h" #include "scrub/trace.h" From patchwork Sun Dec 31 20:58:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507531 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0B104BA2E for ; Sun, 31 Dec 2023 20:58:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="M+5T9012" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 90C9BC433C7; Sun, 31 Dec 2023 20:58:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056320; bh=U+Im2sCPPHjfihruMxiNWQy+Y5fWU3ngHV97IGLHMI8=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=M+5T9012464MV8Xhx7/RTrjo9tf9fU3bimlpPLvWrhGWd043IUszjiUImnVVl/HX8 6eH/7MdiFqsMQXMSvwR0oqwBuy4llSKAq8DE7APbD4wVbQGjRejNo/nAxZYOaIFihT d9dwas03h0xYOhqOto+nGvdit6b9qJoJqt/YahvP8eZ38BUAXNAOlQ9vpAMFTWZSHG Sr69Yt0oTaL05VHtoEgaZBI1ZguDlpt6/kNShYO5s9vro8P4TdRbrWRGxvtyjO5a26 RH9iy0Xodxpki0/E3Fx4GTh0r2Poh6pKJemHP2s4u+iWxlpHoH2V0X31jEdsUaNace cCkQfAi8YFwJw== Date: Sun, 31 Dec 2023 12:58:40 -0800 Subject: [PATCH 21/22] xfs: repair link count of nondirectories after rebuilding parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404842085.1757392.9649595192868451417.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong Since the parent pointer scrubber does not exhaustively search the filesystem for missing parent pointers, it doesn't have a good way to determine that there are pointers missing from an otherwise uncorrupt xattr structure. Instead, for nondirectories it employs a heuristic of comparing the file link count to the number of parent pointers found. However, we don't want this heuristic flagging a false corruption after a repair has actually scanned the entire filesystem to rebuild the parent pointers. Therefore, reset the file link count in this one case because we actually know the correct link count. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/parent_repair.c | 97 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c index 00e6735717a37..04ea0b05ed088 100644 --- a/fs/xfs/scrub/parent_repair.c +++ b/fs/xfs/scrub/parent_repair.c @@ -27,6 +27,7 @@ #include "xfs_parent.h" #include "xfs_attr.h" #include "xfs_bmap.h" +#include "xfs_ag.h" #include "scrub/xfs_scrub.h" #include "scrub/scrub.h" #include "scrub/common.h" @@ -160,6 +161,9 @@ struct xrep_parent { * lost+found filename if we need to reparent the file. */ struct xfs_parent_name_irec pptr; + + /* Number of parents we found after all other repairs */ + unsigned long long parents; }; struct xrep_parent_xattr { @@ -1381,6 +1385,92 @@ xrep_parent_rebuild_tree( return 0; } +/* Count the number of parent pointers. */ +STATIC int +xrep_parent_count_pptr( + struct xfs_scrub *sc, + struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, + void *priv) +{ + struct xrep_parent *rp = priv; + + if (!xfs_parent_verify_irec(sc->mp, pptr)) + return -EFSCORRUPTED; + + rp->parents++; + return 0; +} + +/* + * After all parent pointer rebuilding and adoption activity completes, reset + * the link count of this nondirectory, having scanned the fs to rebuild all + * parent pointers. + */ +STATIC int +xrep_parent_set_nondir_nlink( + struct xrep_parent *rp) +{ + struct xfs_scrub *sc = rp->sc; + struct xfs_inode *ip = sc->ip; + struct xfs_perag *pag; + bool joined = false; + int error; + + /* Count parent pointers so we can reset the file link count. */ + rp->parents = 0; + error = xchk_pptr_walk(sc, ip, xrep_parent_count_pptr, &rp->pptr, rp); + if (error) + return error; + + if (rp->parents > 0 && xfs_inode_on_unlinked_list(ip)) { + xfs_trans_ijoin(sc->tp, sc->ip, 0); + joined = true; + + /* + * The file is on the unlinked list but we found parents. + * Remove the file from the unlinked list. + */ + pag = xfs_perag_get(sc->mp, XFS_INO_TO_AGNO(sc->mp, ip->i_ino)); + if (!pag) { + ASSERT(0); + return -EFSCORRUPTED; + } + + error = xfs_iunlink_remove(sc->tp, pag, ip); + xfs_perag_put(pag); + if (error) + return error; + } else if (rp->parents == 0 && !xfs_inode_on_unlinked_list(ip)) { + xfs_trans_ijoin(sc->tp, sc->ip, 0); + joined = true; + + /* + * The file is not on the unlinked list but we found no + * parents. Add the file to the unlinked list. + */ + error = xfs_iunlink(sc->tp, ip); + if (error) + return error; + } + + /* Set the correct link count. */ + if (VFS_I(ip)->i_nlink != rp->parents) { + if (!joined) { + xfs_trans_ijoin(sc->tp, sc->ip, 0); + joined = true; + } + + set_nlink(VFS_I(ip), min_t(unsigned long long, rp->parents, + XFS_NLINK_PINNED)); + } + + /* Log the inode to keep it moving forward if we dirtied anything. */ + if (joined) + xfs_trans_log_inode(sc->tp, ip, XFS_ILOG_CORE); + return 0; +} + /* Set up the filesystem scan so we can look for parents. */ STATIC int xrep_parent_setup_scan( @@ -1505,6 +1595,13 @@ xrep_parent( error = xrep_parent_rebuild_tree(rp); if (error) goto out_teardown; + if (xfs_has_parent(sc->mp) && !S_ISDIR(VFS_I(sc->ip)->i_mode)) { + error = xrep_parent_set_nondir_nlink(rp); + if (error) + goto out_teardown; + } + + error = xrep_defer_finish(sc); out_teardown: xrep_parent_teardown(rp); From patchwork Sun Dec 31 20:58:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13507532 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 69086BA30 for ; Sun, 31 Dec 2023 20:58:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="h6EeRbKy" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 385EEC433C8; Sun, 31 Dec 2023 20:58:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704056336; bh=7UxDsglGfR6WXtzyc/9m3k6nZKcXvb1YW9Cl3ejrEz0=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=h6EeRbKy05taoKq8/FH91XJbiO4otrgHyv8QhVQy+XyBP9JuyEFIaEv/jM5D4TS0G kv6+O/KA/G9MSZ6XRCMc4ldyDeCGPhloRHpP+ljryh5zIFM0cBUGGccTcmhL4PmOqq HLaKKi0oVOP3z8LQb6cgnumgdZGqVlUsmZu2cQKiFKn8zadOYsy12eFxqWjK8RwBE6 8HWFCD06+ttw36YgkKfTPN8Weoxp8ybTa8jgPCcQOWbzFX44iD+P6SfedCAB4S3jyW S9Dr41Vv8091+KU3ay9y9yIpJ5Meva94/1yFcntMrZ3zVyJLnVPyVH8qrKqXeSG6t5 7qoznOnfePmXQ== Date: Sun, 31 Dec 2023 12:58:55 -0800 Subject: [PATCH 22/22] xfs: inode repair should ensure there's an attr fork to store parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <170404842101.1757392.14755740630315379035.stgit@frogsfrogsfrogs> In-Reply-To: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> References: <170404841699.1757392.2057683072581072853.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Darrick J. Wong The runtime parent pointer update code expects that any file being moved around the directory tree already has an attr fork. However, if we had to rebuild an inode core record, there's a chance that we zeroed forkoff as part of the inode to pass the iget verifiers. Therefore, if we performed any repairs on an inode core, ensure that the inode has a nonzero forkoff before unlocking the inode. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/inode_repair.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c index 5867617c00cd8..187f782d51f22 100644 --- a/fs/xfs/scrub/inode_repair.c +++ b/fs/xfs/scrub/inode_repair.c @@ -1688,6 +1688,44 @@ xrep_inode_extsize( } } +/* Ensure this file has an attr fork if it needs to hold a parent pointer. */ +STATIC int +xrep_inode_pptr( + struct xfs_scrub *sc) +{ + struct xfs_mount *mp = sc->mp; + struct xfs_inode *ip = sc->ip; + struct inode *inode = VFS_I(ip); + + if (!xfs_has_parent(mp)) + return 0; + + /* + * Unlinked inodes that cannot be added to the directory tree will not + * have a parent pointer. + */ + if (inode->i_nlink == 0 && !(inode->i_state & I_LINKABLE)) + return 0; + + /* The root directory doesn't have a parent pointer. */ + if (ip == mp->m_rootip) + return 0; + + /* + * Metadata inodes are rooted in the superblock and do not have any + * parents. + */ + if (xfs_is_metadata_inode(ip)) + return 0; + + /* Inode already has an attr fork; no further work possible here. */ + if (xfs_inode_has_attr_fork(ip)) + return 0; + + return xfs_bmap_add_attrfork(sc->tp, ip, + sizeof(struct xfs_attr_sf_hdr), true); +} + /* Fix any irregularities in an inode that the verifiers don't catch. */ STATIC int xrep_inode_problems( @@ -1696,6 +1734,9 @@ xrep_inode_problems( int error; error = xrep_inode_blockcounts(sc); + if (error) + return error; + error = xrep_inode_pptr(sc); if (error) return error; xrep_inode_timestamps(sc->ip);