From patchwork Wed Apr 10 01:02:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13623413 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 C9011A55 for ; Wed, 10 Apr 2024 01:02:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712710927; cv=none; b=CyTEFTMsziZPrdUNTP+wA6LnIAJAYIzu4LGWRLgUPk0n5SuRExY4PTbFedB6RSZhFmXjUcgg9b/BgDaIXoNUs0GQ9opzoDx9EboHzVVbVQ03uDPMfnBOnqnJMQVz3ZAeS976Hg7AomN4AL1CgjYmGC63stXCjrONX1sLh9JYRCE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712710927; c=relaxed/simple; bh=Cv5SNt81jjSd0Z1cPwJiMil+3I6ch3ixDBMnc8l6TO0=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=ihySk2WHMES1mWUm+qX9mc/n4czVAYIR03KNCfrE4UHqvILhwGv5Qtr8snSBQ+WihXdZWpjJsKD26uiF9rd6iITV+kiPa/JH5tU0uzYfOMyYUI3nrR0FIaMh6CyjermWZTVlIXgaoxyyg0bvULovFcu12c6ZURQPAaiWJvzqG9Q= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=jtpr2DZE; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="jtpr2DZE" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9BFDAC433F1; Wed, 10 Apr 2024 01:02:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1712710927; bh=Cv5SNt81jjSd0Z1cPwJiMil+3I6ch3ixDBMnc8l6TO0=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=jtpr2DZENJdmxzUmddAthmmkcOVRyvUe66zoPEfVaJ4b5qbKCIxsUW7X5RBhsNOxz uM10nW/JVxJa/xdNSLDVa+dUDas/ECVzVQc/O8+YEw9jTIB9vgIWamAtPLbLigoP7c vnxedsIstCF43II880AP1tyAOGvUVKpV5DjgiOqChRSve9FOZXEa6AX4HQH4cLma0s W0OlwCGRg+aTmE2HyJDtSFZ8Fm3EgeSlAyiJcNMdh49hn6ZxIz2gTafjokn0Hq3U2V Z/h6SPKikZcs/9PeFsYLvo7l1hHmKWiPxFl67qP8Bnr1w32RzLvermIMjO66LPxEaz Pc1rDlcMPazWQ== Date: Tue, 09 Apr 2024 18:02:07 -0700 Subject: [PATCH 1/7] xfs: check dirents have parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, hch@lst.de, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <171270970483.3632713.2430218014853007760.stgit@frogsfrogsfrogs> In-Reply-To: <171270970449.3632713.16827511408040390390.stgit@frogsfrogsfrogs> References: <171270970449.3632713.16827511408040390390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/libxfs/xfs_parent.c | 22 +++++++++ fs/xfs/libxfs/xfs_parent.h | 5 ++ fs/xfs/scrub/dir.c | 112 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 1 deletion(-) diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c index db8cfad0b968e..5898dc1ebff02 100644 --- a/fs/xfs/libxfs/xfs_parent.c +++ b/fs/xfs/libxfs/xfs_parent.c @@ -294,3 +294,25 @@ xfs_parent_from_xattr( *parent_gen = be32_to_cpu(rec->p_gen); return 1; } + +/* + * Look up a parent pointer record (@parent_name -> @pptr) of @ip. + * + * Caller must hold at least ILOCK_SHARED. The scratchpad need not be + * initialized. + * + * Returns 0 if the pointer is found, -ENOATTR if there is no match, or a + * negative errno. + */ +int +xfs_parent_lookup( + struct xfs_trans *tp, + struct xfs_inode *ip, + const struct xfs_name *parent_name, + struct xfs_parent_rec *pptr, + struct xfs_da_args *scratch) +{ + memset(scratch, 0, sizeof(struct xfs_da_args)); + xfs_parent_da_args_init(scratch, tp, pptr, ip, ip->i_ino, parent_name); + return xfs_attr_get_ilocked(scratch); +} diff --git a/fs/xfs/libxfs/xfs_parent.h b/fs/xfs/libxfs/xfs_parent.h index 3003ab496f854..b063312a61acb 100644 --- a/fs/xfs/libxfs/xfs_parent.h +++ b/fs/xfs/libxfs/xfs_parent.h @@ -96,4 +96,9 @@ int xfs_parent_from_xattr(struct xfs_mount *mp, unsigned int attr_flags, const void *value, unsigned int valuelen, xfs_ino_t *parent_ino, uint32_t *parent_gen); +/* Repair functions */ +int xfs_parent_lookup(struct xfs_trans *tp, struct xfs_inode *ip, + const struct xfs_name *name, struct xfs_parent_rec *pptr, + struct xfs_da_args *scratch); + #endif /* __XFS_PARENT_H__ */ diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c index 3fe6ffcf9c062..e11d73eb89352 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,14 @@ xchk_setup_directory( /* Directories */ +struct xchk_dir { + struct xfs_scrub *sc; + + /* information for parent pointer validation. */ + struct xfs_parent_rec pptr_rec; + struct xfs_da_args pptr_args; +}; + /* Scrub a directory entry. */ /* Check that an inode's mode matches a given XFS_DIR3_FT_* type. */ @@ -63,6 +73,90 @@ 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; + + xfs_inode_to_parent_rec(&sd->pptr_rec, sc->ip); + error = xfs_parent_lookup(sc->tp, ip, name, &sd->pptr_rec, + &sd->pptr_args); + 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 +174,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 +241,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 +870,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 +903,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 Wed Apr 10 01:02:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13623414 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 737B9A55 for ; Wed, 10 Apr 2024 01:02:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712710943; cv=none; b=Mfggta5zJpLAu1QznS2Wm2djh93/dKe7bGbAEeCmGbj2A/TnFnzfC0nK006xy/mKwDIcDTgArZsFkikRsYGMpwghLc+9BKuGQ1bpBf674Re9L5OOKEYxj+A4zeO4JnRdAD+v97O8+/c53Wi1WQJLnu1xhP3I3eUeq6NKMqB6v3g= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712710943; c=relaxed/simple; bh=yYr7PnDaLwJa9AdiHM8T3GpqnpuE6OsQoa5OYanYHtA=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=s7lHPR52k6R30TeCjD0AFPcfgBFEZFj9q+zPF9SuIAn5ehdHY7l5ZhJ+GxqcbZ+Al5FlZXn8kPSU/b9L4LZ3eCJPgq9lD3wrdWNxnatT6figEME35pP1ci2Hv4WIxCovROxz0AjEckvwEKaJ9W7G3R8/CmY60v1gTCCoWQoU+iY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=jT+pAiaw; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="jT+pAiaw" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 45B8CC433C7; Wed, 10 Apr 2024 01:02:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1712710943; bh=yYr7PnDaLwJa9AdiHM8T3GpqnpuE6OsQoa5OYanYHtA=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=jT+pAiaweC0t38TYNrakM0VlFDHRsPV6MBLHVnxV83TYh7Nj9F7ZFV6auOFDrzJ7G /2/JnskDVwkOQt/jjYzsLspiDXG0uYnZ7MS9rI/tE6w9aBFS/n/AU+17EH8Avy4VVj /Us5m1dKPL03kKP0SlTo2hwSRRgxbmYTDb8pw/S2aqc6tr6+fhPA34uU3t+Of93xOn wx90MTR3odj8RNodMS7CZWD0Vw/ASw8qGsjF1SjinVf7NGZGQWPiqK40qiCNOdm9sB VqWxnXN8lp7mzmKgxk9f9rnIyRkzdKEPZWBSztkXYtpcgRUKb7WD8e6hz/wN3iD+Zf oBnxcL53P/ZZQ== Date: Tue, 09 Apr 2024 18:02:22 -0700 Subject: [PATCH 2/7] xfs: deferred scrub of dirents From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, hch@lst.de, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <171270970500.3632713.13655651505954944042.stgit@frogsfrogsfrogs> In-Reply-To: <171270970449.3632713.16827511408040390390.stgit@frogsfrogsfrogs> References: <171270970449.3632713.16827511408040390390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/dir.c | 234 +++++++++++++++++++++++++++++++++++++++++++++++- fs/xfs/scrub/readdir.c | 78 ++++++++++++++++ fs/xfs/scrub/readdir.h | 3 + fs/xfs/scrub/trace.h | 34 +++++++ 4 files changed, 346 insertions(+), 3 deletions(-) diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c index e11d73eb89352..62474d0557c41 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,12 +47,37 @@ 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; /* information for parent pointer validation. */ struct xfs_parent_rec pptr_rec; struct xfs_da_args pptr_args; + + /* 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. */ + struct xfs_name xname; + uint8_t namebuf[MAXNAMELEN]; }; /* Scrub a directory entry. */ @@ -148,8 +177,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, ip->i_ino); + + error = xfblob_storename(sd->dir_names, &save_de.name_cookie, + name); + 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); @@ -865,6 +912,142 @@ 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, + const struct xfs_name *xname) +{ + 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, 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, 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_loadname(sd->dir_names, dirent.name_cookie, + &sd->xname, dirent.namelen); + if (error) + return error; + + error = xchk_dir_slow_dirent(sd, &dirent, &sd->xname); + if (error) + return error; + } + + return 0; +} + /* Scrub a whole directory. */ int xchk_directory( @@ -907,11 +1090,56 @@ xchk_directory( if (!sd) return -ENOMEM; sd->sc = sc; + sd->xname.name = sd->namebuf; + + 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 028690761c629..28a94c78b0b19 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. */ @@ -380,3 +381,80 @@ xchk_dir_lookup( *ino = args.inumber; return error; } + +/* + * Try to grab the IOLOCK and ILOCK of sc->ip and ip, returning @ip's lock + * state. The caller may have a transaction, so we must use trylock for both + * IOLOCKs. + */ +static inline unsigned int +xchk_dir_trylock_both( + struct xfs_scrub *sc, + struct xfs_inode *ip) +{ + if (!xchk_ilock_nowait(sc, XFS_IOLOCK_EXCL)) + return 0; + + if (!xfs_ilock_nowait(ip, XFS_IOLOCK_SHARED)) + goto parent_iolock; + + xchk_ilock(sc, XFS_ILOCK_EXCL); + if (!xfs_ilock_nowait(ip, XFS_ILOCK_EXCL)) + goto parent_ilock; + + return XFS_IOLOCK_SHARED | XFS_ILOCK_EXCL; + +parent_ilock: + xchk_iunlock(sc, XFS_ILOCK_EXCL); + xfs_iunlock(ip, XFS_IOLOCK_SHARED); +parent_iolock: + xchk_iunlock(sc, XFS_IOLOCK_EXCL); + return 0; +} + +/* + * 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); + + for (nr = 0; nr < HZ; nr++) { + *lockmode = xchk_dir_trylock_both(sc, ip); + if (*lockmode) + return 0; + + 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 814db1d1747a0..4db762480b8d4 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -1511,6 +1511,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 struct xfs_name *name, + xfs_ino_t far_ino), + TP_ARGS(ip, name, far_ino), + TP_STRUCT__entry( + __field(dev_t, dev) + __field(xfs_ino_t, ino) + __field(unsigned int, namelen) + __dynamic_array(char, name, name->len) + __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 = name->len; + memcpy(__get_str(name), name, name->len); + __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 struct xfs_name *name, \ + xfs_ino_t far_ino), \ + TP_ARGS(ip, name, 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 Wed Apr 10 01:02:38 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13623415 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 68445EC2 for ; Wed, 10 Apr 2024 01:02:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712710959; cv=none; b=TJT8zoMjdRYC2zNEPMFapX/nU1IOuSapyBOEzPFLegpbsXzkxiqQeRul2p1UAxIUgOojCO22rBKocZcPu/ds22BxJOs6pm19pDZZIJiXjvIvbnn7ZdyYO56QKia0uT9s39TgEKSmfEFWODlshLyVdAKY8OtW61WSSzDHeXvfDHQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712710959; c=relaxed/simple; bh=NeHtEgvVqOyxaPWjOAfw+wsV5WkN/y9mg1X24axCWk0=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=lDroYNjxTEgn5DK67k2bVFEoqaUf24B3CEN7PA3TomT6LJ8D1y1LfdwfPLJo3J4/Ewju4wSuOyDxCfUfkwuNXX/VnsOUh4iSCHaD1yXgOJo6gCJnRb8Wi1q6KNPfNeWtWUyihjd6ITMDMeFHEHCPzMgfeAVKqFPlVITBC2A8/IQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=cj97Q2re; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="cj97Q2re" Received: by smtp.kernel.org (Postfix) with ESMTPSA id DBB2DC433F1; Wed, 10 Apr 2024 01:02:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1712710958; bh=NeHtEgvVqOyxaPWjOAfw+wsV5WkN/y9mg1X24axCWk0=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=cj97Q2reOJSQ5HSRvhGcfGRVhfs7wxhOv7HFgpnwagdQGeDPkEVNkL89ZglUMS4WV DyOyrVfumQdvjMm9YSYmBh9znV5/jR3C0aKH6H/VN9NWxG0V0puum5PXnFIP/8uull /aw1MyoQceZXItdPQCWeE18L0pOvCxfggYgqHweXA9N/Lq+UYghuaQHu1Y8D+QrQoS B1IW+33giuWOEH9qijT5oRmslxGL9pXaW6IlxWlVsR0Eq1hmJnv6gvmEQD0eCl4clv pPij74XoTZesCmMQDDFBxxqq2vvIIUE4KJslbrCtrJp6xJRQqxi4qYVgxJsCwqj1xW I4HN6hRuee81A== Date: Tue, 09 Apr 2024 18:02:38 -0700 Subject: [PATCH 3/7] xfs: scrub parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, hch@lst.de, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <171270970517.3632713.14790309685305343360.stgit@frogsfrogsfrogs> In-Reply-To: <171270970449.3632713.16827511408040390390.stgit@frogsfrogsfrogs> References: <171270970449.3632713.16827511408040390390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/parent.c | 367 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 367 insertions(+) diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c index acb6282c3d148..966e106c1fe6d 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,366 @@ 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; + + /* 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, + unsigned int attr_flags, + const unsigned char *name, + unsigned int namelen, + const void *value, + unsigned int valuelen, + void *priv) +{ + struct xchk_pptrs *pp = priv; + xfs_ino_t parent_ino; + int ret; + + ret = xfs_parent_from_xattr(sc->mp, attr_flags, name, namelen, + value, valuelen, &parent_ino, NULL); + if (ret < 0) + return ret; + + if (ret == 1 && pp->parent_ino == parent_ino) + 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_xattr_walk(sc, sc->ip, xchk_parent_scan_dotdot, 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, + const struct xfs_name *xname, + struct xfs_inode *dp) +{ + 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, + const struct xfs_parent_rec *pptr, + struct xfs_inode **dpp) +{ + struct xfs_scrub *sc = pp->sc; + struct xfs_inode *ip; + xfs_ino_t parent_ino = be64_to_cpu(pptr->p_ino); + int error; + + /* Validate inode number. */ + error = xfs_dir_ino_validate(sc->mp, parent_ino); + if (error) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + + error = xchk_iget(sc, parent_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 != be32_to_cpu(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 xname = { + .name = name, + .len = namelen, + }; + struct xchk_pptrs *pp = priv; + struct xfs_inode *dp = NULL; + const struct xfs_parent_rec *pptr_rec = value; + xfs_ino_t parent_ino; + unsigned int lockmode; + int ret; + + ret = xfs_parent_from_xattr(sc->mp, attr_flags, name, namelen, + value, valuelen, &parent_ino, NULL); + if (ret < 0) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + if (ret != 1) + return 0; + + /* No self-referential parent pointers. */ + if (parent_ino == sc->ip->i_ino) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + + pp->pptrs_found++; + + ret = xchk_parent_iget(pp, pptr_rec, &dp); + if (ret) + return ret; + if (!dp) + return 0; + + /* Try to lock the inode. */ + lockmode = xchk_parent_lock_dir(sc, dp); + if (!lockmode) { + xchk_set_incomplete(sc); + ret = -ECANCELED; + goto out_rele; + } + + ret = xchk_parent_dirent(pp, &xname, dp); + if (ret) + goto out_unlock; + +out_unlock: + xfs_iunlock(dp, lockmode); +out_rele: + xchk_irele(sc, dp); + return ret; +} + +/* + * 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 +570,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 Wed Apr 10 01:02:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13623416 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 B43C010E3 for ; Wed, 10 Apr 2024 01:02:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712710974; cv=none; b=H7KQdcmxoNeDnaOnQ3HCvTbaNUehqiufCvrUnavXDvgn+0z17CZasGYcWS99tQcDYHpTFJkcS/8NHAbMmcMcGCqXbc23JDPYNbVKglhFUq6qTUzSvAmUmr9XmfTKlHBIGllMQ53Zm8na/3bnVG/n9Cm8zxqw7BeP5Tz8ZYxXYTM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712710974; c=relaxed/simple; bh=KhQcc2+Qzs4Vx4LUEHftIo0HU+DiAmMAx7jwVrBllX0=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=rZD6AjuvCjLi4/W+q7glB2+JWRmpK7dVs197cn15dD4GxMZTIHwc0o6p2EYN23VG+JJxcMCJsCED3Gx7bf8T236KegLbNlRhTOBPkc9ZWt+hjBR6BF8c5axiFtCe8xk8uzrLW7THElBFaTe4U2s/z0JSh3Nuu/Ai3AOVah4tm8M= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=tWM0icSb; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="tWM0icSb" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 89018C433F1; Wed, 10 Apr 2024 01:02:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1712710974; bh=KhQcc2+Qzs4Vx4LUEHftIo0HU+DiAmMAx7jwVrBllX0=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=tWM0icSbMfcHFJ8+n/yg1pDJCJaj9+UK/IUHxcN5SsviTYal3FI8947Nz9VLG97LZ Ch/NTMyy7quMYdptylKxAuoxf9R2l1NklMGI8IV5KDVWdFZC+Okm6+9CRhsfb7GGQF kgB3gRnQ6vG6vTuPo8VlSjLR8340HgpIA3NYcvWjVN/GwVjB5Ojo1IbdK8kQIPn7HE iX6B/QcNeytDNzRLiXxJvFAIG9iCrLj5usltWUpp7Rs6CLrb5B6G2WesMku3Gf8C81 q/AsoCLt78Ti69dpQWSF+aFc3X+wQbeLLoePqJLZdTZQ8hyPENiZ1njYHBMVqfJM33 xiGiNRi9opeJQ== Date: Tue, 09 Apr 2024 18:02:54 -0700 Subject: [PATCH 4/7] xfs: deferred scrub of parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, hch@lst.de, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <171270970533.3632713.1334905881423990253.stgit@frogsfrogsfrogs> In-Reply-To: <171270970449.3632713.16827511408040390390.stgit@frogsfrogsfrogs> References: <171270970449.3632713.16827511408040390390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/Makefile | 2 fs/xfs/scrub/parent.c | 264 ++++++++++++++++++++++++++++++++++++++++++++++++- fs/xfs/scrub/trace.h | 3 + 3 files changed, 261 insertions(+), 8 deletions(-) diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile index c969b11ce0f47..af99a455ce4db 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 \ ) xfs-$(CONFIG_XFS_RT) += $(addprefix scrub/, \ diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c index 966e106c1fe6d..cb50e89e7ee64 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,6 +214,18 @@ 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 record. */ + struct xfs_parent_rec pptr_rec; + + /* Length of the pptr name. */ + uint8_t namelen; +}; + struct xchk_pptrs { struct xfs_scrub *sc; @@ -219,6 +234,22 @@ struct xchk_pptrs { /* Parent of this directory. */ xfs_ino_t parent_ino; + + /* Fixed-size array of xchk_pptr structures. */ + struct xfarray *pptr_entries; + + /* Blobs containing parent pointer names. */ + struct xfblob *pptr_names; + + /* Scratch buffer for scanning pptr xattrs */ + struct xfs_da_args pptr_args; + + /* If we've cycled the ILOCK, we must revalidate all deferred pptrs. */ + bool need_revalidate; + + /* Name buffer */ + struct xfs_name xname; + char namebuf[MAXNAMELEN]; }; /* Does this parent pointer match the dotdot entry? */ @@ -457,8 +488,25 @@ xchk_parent_scan_attr( /* Try to lock the inode. */ lockmode = xchk_parent_lock_dir(sc, dp); if (!lockmode) { - xchk_set_incomplete(sc); - ret = -ECANCELED; + struct xchk_pptr save_pp = { + .pptr_rec = *pptr_rec, /* struct copy */ + .namelen = namelen, + }; + + /* Couldn't lock the inode, so save the pptr for later. */ + trace_xchk_parent_defer(sc->ip, &xname, dp->i_ino); + + ret = xfblob_storename(pp->pptr_names, &save_pp.name_cookie, + &xname); + if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, + &ret)) + goto out_rele; + + ret = xfarray_append(pp->pptr_entries, &save_pp); + if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, + &ret)) + goto out_rele; + goto out_rele; } @@ -473,6 +521,159 @@ xchk_parent_scan_attr( return ret; } +/* + * 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, + const struct xfs_name *xname, + struct xfs_parent_rec *pptr) +{ + struct xfs_scrub *sc = pp->sc; + int error; + + error = xfs_parent_lookup(sc->tp, sc->ip, xname, pptr, &pp->pptr_args); + 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, + const struct xfs_name *xname, + struct xfs_parent_rec *pptr) +{ + struct xfs_scrub *sc = pp->sc; + struct xfs_inode *dp = NULL; + unsigned int lockmode; + int error; + + /* Check that the deferred parent pointer still exists. */ + if (pp->need_revalidate) { + error = xchk_parent_revalidate_pptr(pp, xname, pptr); + if (error == -ENOENT) + return 0; + if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, + &error)) + return error; + } + + error = xchk_parent_iget(pp, pptr, &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, xname, 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, xname, 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, xname, pptr); + 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, xname, 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 = xfblob_loadname(pp->pptr_names, pptr.name_cookie, + &pp->xname, pptr.namelen); + if (error) + return error; + + error = xchk_parent_slow_pptr(pp, &pp->xname, &pptr.pptr_rec); + 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, + unsigned int attr_flags, + const unsigned char *name, + unsigned int namelen, + const void *value, + unsigned int valuelen, + void *priv) +{ + struct xchk_pptrs *pp = priv; + int ret; + + ret = xfs_parent_from_xattr(sc->mp, attr_flags, name, namelen, + value, valuelen, NULL, NULL); + if (ret != 1) + return ret; + + 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 @@ -483,6 +684,23 @@ 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_xattr_walk(sc, sc->ip, xchk_parent_count_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) @@ -507,23 +725,51 @@ xchk_parent_pptr( struct xfs_scrub *sc) { struct xchk_pptrs *pp; + char *descr; int error; pp = kvzalloc(sizeof(struct xchk_pptrs), XCHK_GFP_FLAGS); if (!pp) return -ENOMEM; pp->sc = sc; + pp->xname.name = pp->namebuf; + + /* + * 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 @@ -541,7 +787,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) @@ -554,8 +800,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 4db762480b8d4..97a106519b531 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -1544,6 +1544,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 Wed Apr 10 01:03:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13623417 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 BD1E81854 for ; Wed, 10 Apr 2024 01:03:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712710990; cv=none; b=RrEhkC7NywGFEOZ3nc+BsYcJFPxpHH/IHxWKry78oSOZKOthnoRXNxZEgmfTV42uhDyuHaH8YjVkRSy5yJsyoEDNJuVHZ91AvI9Fw6/eEo6o4JwKcZtaWyjvDke52GyHZI5T+RBH3bQukduSKjp0d7Tevk7Z5rXwwHZ4QLww5nw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712710990; c=relaxed/simple; bh=j5e3khGmG+8B0kzMmItAACsoBcrHYaQZXDLXnWMquhE=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=M0zxKhr0mkIWo1ldLxedWS9bFJqexPI5HddskGHCB9bIhWChldWa8DKOZQxUG13R3CRgf5hT5/Z3dTbm4iRmo4kAkoLMyXSULd26KQrnEkJj/ola9GEmg6qDs4//58iH8yINP+D1L6LMG9pJ+jUSC5MVQ0KhQJtKgdY/8P1AfWU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=moyac4e+; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="moyac4e+" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3CF27C433F1; Wed, 10 Apr 2024 01:03:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1712710990; bh=j5e3khGmG+8B0kzMmItAACsoBcrHYaQZXDLXnWMquhE=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=moyac4e+YbEYmfYLquWFmowTD2XCzenS1DMdZEMizCSXBakB0Gvj21s1QDGStk9ij 6Xl+bZX13MmQVYai0Yra33b5c5P7dcwuHvLq74UBh6b+Sr6JaaRRqbHLVlfNmY9FyF O5/meIC9oonYCwz40ILVlHL6KS2FSJuwR26GIkKIarE25dDyzq9TGDQgwtMuuuUQF3 zyqt8gZjBkAg/3Pq9E4Y8Ncju0tCesKIhJGRyZYr34qTgY+P+HhrTt/RIi8/FExgB3 RLb34yEtThzVxMTDqDXaJdhmScEIKY1bQvS2RKDfBwhsEmuTyXIBGR5AKdknotWaE2 qDzZQIS7NpPKA== Date: Tue, 09 Apr 2024 18:03:09 -0700 Subject: [PATCH 5/7] xfs: walk directory parent pointers to determine backref count From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, hch@lst.de, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <171270970550.3632713.9842307177705801451.stgit@frogsfrogsfrogs> In-Reply-To: <171270970449.3632713.16827511408040390390.stgit@frogsfrogsfrogs> References: <171270970449.3632713.16827511408040390390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/common.h | 1 + fs/xfs/scrub/nlinks.c | 82 +++++++++++++++++++++++++++++++++++++++++- fs/xfs/scrub/nlinks_repair.c | 2 + fs/xfs/scrub/parent.c | 61 +++++++++++++++++++++++++++++++ fs/xfs/scrub/trace.c | 1 + fs/xfs/scrub/trace.h | 28 ++++++++++++++ 6 files changed, 174 insertions(+), 1 deletion(-) diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h index 89f7bbec887ed..e00466f404829 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 fcb9c473f372e..a733e4e178de4 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 @@ -272,12 +274,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; } @@ -314,6 +321,58 @@ 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, + unsigned int attr_flags, + const unsigned char *name, + unsigned int namelen, + const void *value, + unsigned int valuelen, + void *priv) +{ + struct xfs_name xname = { + .name = name, + .len = namelen, + }; + struct xchk_nlink_ctrs *xnc = priv; + const struct xfs_parent_rec *pptr_rec = value; + xfs_ino_t parent_ino; + int ret; + + /* Update the shadow link counts if we haven't already failed. */ + + if (xchk_iscan_aborted(&xnc->collect_iscan)) { + ret = -ECANCELED; + goto out_incomplete; + } + + ret = xfs_parent_from_xattr(sc->mp, attr_flags, name, namelen, + value, valuelen, &parent_ino, NULL); + if (ret != 1) + return ret; + + trace_xchk_nlinks_collect_pptr(sc->mp, ip, &xname, pptr_rec); + + mutex_lock(&xnc->lock); + + ret = xchk_nlinks_update_incore(xnc, parent_ino, 0, 1, 0); + if (ret) + 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 ret; +} + /* Walk a directory to bump the observed link counts of the children. */ STATIC int xchk_nlinks_collect_dir( @@ -360,6 +419,27 @@ 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_xattr_walk(sc, dp, xchk_nlinks_collect_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_repair.c b/fs/xfs/scrub/nlinks_repair.c index 83f8637bb08fd..78d0f650fe897 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 cb50e89e7ee64..57b49fbf97a30 100644 --- a/fs/xfs/scrub/parent.c +++ b/fs/xfs/scrub/parent.c @@ -866,3 +866,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 b2ce7b22cad34..4a8cc2c98d997 100644 --- a/fs/xfs/scrub/trace.c +++ b/fs/xfs/scrub/trace.c @@ -19,6 +19,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 97a106519b531..3e726610b9e32 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -26,6 +26,7 @@ struct xchk_iscan; struct xchk_nlink; struct xchk_fscounters; struct xfs_rmap_update_params; +struct xfs_parent_rec; /* * ftrace's __print_symbolic requires that all enum values be wrapped in the @@ -1363,6 +1364,33 @@ 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_name *name, + const struct xfs_parent_rec *pptr), + TP_ARGS(mp, dp, name, 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, name->len) + ), + TP_fast_assign( + __entry->dev = mp->m_super->s_dev; + __entry->dir = dp->i_ino; + __entry->ino = be64_to_cpu(pptr->p_ino); + __entry->namelen = name->len; + memcpy(__get_str(name), name->name, name->len); + ), + 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 Wed Apr 10 01:03:25 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13623418 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 5AC3F391 for ; Wed, 10 Apr 2024 01:03:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712711006; cv=none; b=AGhvSzW2LUNV6SHpTGHuckXE0r7+sErh865Mvk+q6H4CpQ2PvzOwiHBnbm2ooMBb94ltbejzWOSc0GTZzL1uXUhDXQgVPWUb3nNnU8Bwp7ersSYAzdkOqqZUoQj+95bIm2GK/eZBS1GLMgrfNa2us8BQAWLyH9KuVIRkQHb4C6Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712711006; c=relaxed/simple; bh=q6exp7NnZBLsf4xo43lDb7Y3SZNtQQlJqRCE1Erm6GM=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=ZqlE5y3g6o90lL4xn42DLhVJOc0zS6Wb8WJ68K895IW5lW9rkggLv6LVZEu6x4HR+arjSonoWk6VXOy9dAEl/Iae9KfWqfIYa5eItiCNkIKbaMr4SDi9D8JBe7Am3dC1YPCGlvlF1uWy0vSOCddnidI69tteZZZU718g1HQ4fBg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=XDx+9JgJ; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="XDx+9JgJ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E2BD6C433F1; Wed, 10 Apr 2024 01:03:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1712711005; bh=q6exp7NnZBLsf4xo43lDb7Y3SZNtQQlJqRCE1Erm6GM=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=XDx+9JgJFhg+281SJL/gIX35PCC31tKyxQVvsffUDJmBXkcw9RVEFQUiXEg9/Uf4o a8D5ScXAbKh+26MQop1lzOgIpMhCt4euHHAVtaTN86eoWXxbc9LP1O4obDuDdx2lVq wEnbRYE/C+sn3eVRol6sAA4IJUnxkGlA6sd1uGCDUMZ+NgAryOLAervnXEuRmH8Nzy LzQpBLc4vEgbPFrMBAF36rkQnMqvtXBj29xGhjr7VRpQoQO7Jyuow195l1QYOrMTy3 u2YDOuNVlUyuSiM3R46DcD7GPxC/cmx2p9UdUmNXRctsuIwiznM8SmZVt8vkQjPWKS sDudvBjVovtHA== Date: Tue, 09 Apr 2024 18:03:25 -0700 Subject: [PATCH 6/7] xfs: check parent pointer xattrs when scrubbing From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, hch@lst.de, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <171270970567.3632713.8784234263889465896.stgit@frogsfrogsfrogs> In-Reply-To: <171270970449.3632713.16827511408040390390.stgit@frogsfrogsfrogs> References: <171270970449.3632713.16827511408040390390.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 Check parent pointer xattrs as part of scrubbing xattrs. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/attr.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c index fe51a17661831..b91234bbd58aa 100644 --- a/fs/xfs/scrub/attr.c +++ b/fs/xfs/scrub/attr.c @@ -17,6 +17,7 @@ #include "xfs_attr.h" #include "xfs_attr_leaf.h" #include "xfs_attr_sf.h" +#include "xfs_parent.h" #include "scrub/scrub.h" #include "scrub/common.h" #include "scrub/dabtree.h" @@ -208,6 +209,13 @@ xchk_xattr_actor( return -ECANCELED; } + /* Check parent pointer record. */ + if ((attr_flags & XFS_ATTR_PARENT) && + !xfs_parent_valuecheck(sc->mp, value, valuelen)) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, args.blkno); + return -ECANCELED; + } + /* * Local and shortform xattr values are stored in the attr leaf block, * so we don't need to retrieve the value from a remote block to detect From patchwork Wed Apr 10 01:03:41 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13623419 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 DEC21621 for ; Wed, 10 Apr 2024 01:03:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712711022; cv=none; b=FSx2CIVN7kV365Ux3lpzY9aUdvHbmvO6tny+66UeCwg50PNY0u/8VTGrKxFLj/3ntsCzZ0OY0ojOKobsGqJ6UNhaNnv57Wjs9+t9M8fUNBwGR2aPZxR6tq/QeOb3/4lGa/QCK92g3CvW9xFT4iPdld0+JUombIPZxdqmWRXiP2w= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712711022; c=relaxed/simple; bh=9uxYOvA/TDMS6M7Yb+Qzg/SxrYeMPsUXyXDGau5bePE=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=AophBSV1hM5SbVTQSEEnuduZKGDnDmiKbdwRvXsPrRhc6iF0h7chDOf9anwG6lxmXQTZ7DocpLJT2IsPW4/CfQ1221aILtG4bI5MQliP8x0nOsnggN9DQUZqRb/fg6jC9ohv9kZl4RlnK5AUug/vHD8hoo13CqFTpl4u0+aM7Rg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=UB8KFoE5; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="UB8KFoE5" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 81662C433C7; Wed, 10 Apr 2024 01:03:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1712711021; bh=9uxYOvA/TDMS6M7Yb+Qzg/SxrYeMPsUXyXDGau5bePE=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=UB8KFoE5svXQWJlr4Y40o6/zfTOgmIfMON3M59kL/zqtKb1tV3gK4/IPJwhZ2BXiM JEp15cG5mAzhPhttWePivd4Ld/4rCz5zXO3ZTrDJBOJEPYNfrLyKm/u3bCeUTlgzkh Uc48TNRxPtxLzMo7854wkfpQS8Iwas1Aq5IMAfjvi4vNlk/Iie0ZlhR0Hyo6B6sgxr JDCeO8i2sUK2QqqVvg/No8ODOoWyDWjVGgQDa9B0Ntz4ccp5sG6b66f95wp0g0OmnW JlmNorVWBPf95igvyTpro5F9Q14QC4Wjp9srIJJ9bggJFNueql9oXZoCjdwh+W0aZ6 S2gr6Z5uT0IZA== Date: Tue, 09 Apr 2024 18:03:41 -0700 Subject: [PATCH 7/7] xfs: salvage parent pointers when rebuilding xattr structures From: "Darrick J. Wong" To: djwong@kernel.org Cc: catherine.hoang@oracle.com, hch@lst.de, allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <171270970583.3632713.16977275276286469285.stgit@frogsfrogsfrogs> In-Reply-To: <171270970449.3632713.16827511408040390390.stgit@frogsfrogsfrogs> References: <171270970449.3632713.16827511408040390390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/attr_repair.c | 34 +++++++++++++++++++++++++--------- fs/xfs/scrub/trace.h | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c index 7228758c2da1a..091cef077cdde 100644 --- a/fs/xfs/scrub/attr_repair.c +++ b/fs/xfs/scrub/attr_repair.c @@ -28,6 +28,7 @@ #include "xfs_exchmaps.h" #include "xfs_exchrange.h" #include "xfs_acl.h" +#include "xfs_parent.h" #include "scrub/xfs_scrub.h" #include "scrub/scrub.h" #include "scrub/common.h" @@ -127,6 +128,9 @@ xrep_xattr_want_salvage( return false; if (valuelen > XATTR_SIZE_MAX || valuelen < 0) return false; + if (attr_flags & XFS_ATTR_PARENT) + return xfs_parent_valuecheck(rx->sc->mp, value, valuelen); + return true; } @@ -154,14 +158,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); @@ -596,8 +607,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 3e726610b9e32..4b968df3d840c 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -2540,6 +2540,44 @@ 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, namelen) + ), + TP_fast_assign( + const struct xfs_parent_rec *rec = value; + + __entry->dev = ip->i_mount->m_super->s_dev; + __entry->ino = ip->i_ino; + __entry->parent_ino = be64_to_cpu(rec->p_ino); + __entry->parent_gen = be32_to_cpu(rec->p_gen); + __entry->namelen = namelen; + memcpy(__get_str(name), name, 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),