From patchwork Thu Apr 6 19:28:43 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: 13203940 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3FF84C7618D for ; Thu, 6 Apr 2023 19:29:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235677AbjDFT3F (ORCPT ); Thu, 6 Apr 2023 15:29:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57336 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236590AbjDFT2w (ORCPT ); Thu, 6 Apr 2023 15:28:52 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DEC7E9029 for ; Thu, 6 Apr 2023 12:28:44 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 443B964B89 for ; Thu, 6 Apr 2023 19:28:44 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A75DFC433D2; Thu, 6 Apr 2023 19:28:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1680809323; bh=Liu5OtqVpxnpRl51yMOPYjCNUAmvojWBFZ+Z5BKen7c=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=JBnQsWVenbE7SAKAlqn4X/XIOIbrL5BUFyqR2HvmJ267tLL+joLk6mQbDdyH/OQ+2 l/qjuhtFZqchoOUARIwpSktv5w+o/m8XL6qHANInhMtF97ixOvxvLckYSvKvvdVOV5 /0p8OgzyFTLEeHdk5B5eQGm7MlCjYY/xJzxoIK8ww1rk7WKyayyPChZajIiSUtEk+V pvf6oi72LgeDh6W8XBj9EMh8vZlNUsM7ucvXd8X2oIwDORfOAxf1Sr4rBl6nmlAldB UlqUf6pGtCwzX/d1F6eYDs8HRWyG6JqXyoMhYTEKDXzPSGYowOoeyIZv+UVvpqc2Aj TWB92UC7cBqpw== Date: Thu, 06 Apr 2023 12:28:43 -0700 Subject: [PATCH 1/2] xfs: check dirents have parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <168080826307.616123.6377624538863249633.stgit@frogsfrogsfrogs> In-Reply-To: <168080826292.616123.18366076398528767455.stgit@frogsfrogsfrogs> References: <168080826292.616123.18366076398528767455.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org 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/scrub/dir.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+), 1 deletion(-) diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c index e30624dc35b3..9ae3afc4661a 100644 --- a/fs/xfs/scrub/dir.c +++ b/fs/xfs/scrub/dir.c @@ -15,6 +15,8 @@ #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/dabtree.h" @@ -39,6 +41,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. */ @@ -61,6 +77,88 @@ 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 (!strncmp(".", name->name, name->len) || + !strncmp("..", name->name, name->len)) + return 0; + + /* 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. * @@ -78,6 +176,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; @@ -144,6 +243,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) @@ -759,6 +866,7 @@ int xchk_directory( struct xfs_scrub *sc) { + struct xchk_dir *sd; int error; if (!S_ISDIR(VFS_I(sc->ip)->i_mode)) @@ -786,9 +894,16 @@ 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); if (error == -ECANCELED) error = 0; + + kvfree(sd); return error; } From patchwork Thu Apr 6 19:28: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: 13203951 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 21D79C76196 for ; Thu, 6 Apr 2023 19:29:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229892AbjDFT3P (ORCPT ); Thu, 6 Apr 2023 15:29:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57698 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237592AbjDFT3D (ORCPT ); Thu, 6 Apr 2023 15:29:03 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 57F525FC8 for ; Thu, 6 Apr 2023 12:29:00 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D9FAA64B3A for ; Thu, 6 Apr 2023 19:28:59 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 40991C433EF; Thu, 6 Apr 2023 19:28:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1680809339; bh=hjY53V57PqHAAGqN0UZ49rDFXPB1/b2XKSKOD2GgeCM=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=o0Jkl9KaovyGDtc7suAZNiqzWRh0qA0tp54M5gH6viBBl2V4hoN75rDXO6TPjTDFE SnixRIHft8GjH9M/aQyNJEOXoLIph+nBkytNlDqHjHxSODJgJdgtc4s7/LikPWWJ+X nIxYiltTJ/ioRSuS38WUCF+km5ufZQ0kaK3jCGoNmxRXONAn50rOcwDweCr3jpClTN /y1+HnOloG47SXssIZWa8Fd9eJPKgKZGyoMWHH5oJ9GrcDwrtymRRfmu/iXJ1G2jjY kfHayuyznCFXr63SAIjA3kbELOj1PH10Q5P81Mgnp4fLHXUYzqmCIp1WbOluA2ikQp 1JJ9Z1sI3VYgg== Date: Thu, 06 Apr 2023 12:28:58 -0700 Subject: [PATCH 2/2] xfs: deferred scrub of dirents From: "Darrick J. Wong" To: djwong@kernel.org Cc: allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <168080826321.616123.6991708692627732159.stgit@frogsfrogsfrogs> In-Reply-To: <168080826292.616123.18366076398528767455.stgit@frogsfrogsfrogs> References: <168080826292.616123.18366076398528767455.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org 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 | 225 ++++++++++++++++++++++++++++++++++++++++++++++++++ fs/xfs/scrub/trace.h | 2 2 files changed, 225 insertions(+), 2 deletions(-) diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c index 9ae3afc4661a..5c28adca97f1 100644 --- a/fs/xfs/scrub/dir.c +++ b/fs/xfs/scrub/dir.c @@ -22,6 +22,10 @@ #include "scrub/dabtree.h" #include "scrub/readdir.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 @@ -41,6 +45,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; @@ -50,6 +66,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]; @@ -150,8 +175,24 @@ 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_process_error(sc, XFS_DATA_FORK, 0, &error)) + return error; + + error = xfarray_append(sd->dir_entries, &save_de); + if (xchk_fblock_process_error(sc, XFS_DATA_FORK, 0, &error)) + return error; + + return 0; } error = xchk_dir_parent_pointer(sd, name, ip); @@ -861,6 +902,156 @@ 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_ATTR_FORK, 0); + return 0; + } + if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_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) + 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_slowpath(sc->ip, xname.name, xname.len, ip->i_ino); + + while (true) { + 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)) { + break; + } + xchk_iunlock(sc, XFS_ILOCK_EXCL); + } + xchk_iunlock(sc, XFS_IOLOCK_EXCL); + + if (xchk_should_terminate(sc, &error)) + goto out_rele; + + delay(1); + } + lockmode = XFS_IOLOCK_SHARED | XFS_ILOCK_EXCL; + + /* Revalidate, since we just cycled the locks. */ + error = xchk_dir_revalidate_dirent(sd, &xname, dirent->ino); + if (error == -ENOENT) + 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( @@ -899,11 +1090,41 @@ xchk_directory( return -ENOMEM; sd->sc = sc; + if (xfs_has_parent(sc->mp)) { + /* + * Set up some staging memory for dirents that we can't check + * due to locking contention. + */ + error = xfarray_create(sc->mp, "directory entries", 0, + sizeof(struct xchk_dirent), &sd->dir_entries); + if (error) + goto out_sd; + + error = xfblob_create(sc->mp, "dirent names", &sd->dir_names); + 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) + 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); return error; } diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h index 87e400096245..1b73fa20b6a1 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -987,6 +987,8 @@ DEFINE_EVENT(xchk_pptr_class, name, \ TP_ARGS(ip, name, namelen, parent_ino)) DEFINE_XCHK_PPTR_CLASS(xchk_parent_defer); DEFINE_XCHK_PPTR_CLASS(xchk_parent_slowpath); +DEFINE_XCHK_PPTR_CLASS(xchk_dir_defer); +DEFINE_XCHK_PPTR_CLASS(xchk_dir_slowpath); /* repair tracepoints */ #if IS_ENABLED(CONFIG_XFS_ONLINE_REPAIR)