From patchwork Tue Apr 16 01:36:12 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: 13631053 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 592F5A92E for ; Tue, 16 Apr 2024 01:36:12 +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=1713231373; cv=none; b=CCCxbxxF7fgw9VUATIqYbqIOnh6bxeWcHVRIAmboie7VtRHALTFWlf8gOBvTV3JmVkU5Pxa2aBjaiRJAl4kG/3ei02USpNpwsmDqT029KFI/yNOGVsOjK+nrqmHSbKsQk44NFqVa0HPX/rmt1fmmS8Wk1L1ImnjyYM6r2QbQFwc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231373; c=relaxed/simple; bh=RCuT/CVacWZA21H8Sz/pMpLqkGpDzYTG6p0KpTucSos=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=n5aw9PvXH4BiA7UJE14pPkLDZcj4iN6UhaBMJHcTYkVAFIhetdA5pcAX+14AWRVdWjumBlAnmcAWXisZDYTrg6FgwoKWhnfqHRsPM6cV2DNJwbAlmx4ryAJQrIViXEaRCe53sIfDFqjzb1vfrJ+Xib+o9/9izVUhplB5n1CnYnw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=U/PR2H4z; 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="U/PR2H4z" Received: by smtp.kernel.org (Postfix) with ESMTPSA id CA9ADC113CC; Tue, 16 Apr 2024 01:36:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231372; bh=RCuT/CVacWZA21H8Sz/pMpLqkGpDzYTG6p0KpTucSos=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=U/PR2H4zvHZvqJYyXan8+mMC0QgtvsViHuAJpvMpnu7vx6XfQiniCSafBFmwb+juM 13ikhf4/XWXzDqf1rUMToZODS1gN9o+xCWTZ4c8I4VdRmrPDMht7si55nAbPd/Etbw mEUwGA3IC8O4DmvgVK8VrMQXAdw8hAFqfb9k5KLgcMPMZMIFCM5BemfJvGQObpyX7z hiodC2fjjXy/wt9Xv24xMi9agPfzV/kdMSck60L40yTDwKrFLbQLZpvnhdTD2e2FST x6fKliw6MClHcp025T7ueq+ARLdgAjgGVhoEJVzGLLwXg86ItWAh+UF76Q/1EVG7is pC9fdxk3Q5ULQ== Date: Mon, 15 Apr 2024 18:36:12 -0700 Subject: [PATCH 01/17] xfs: remove some boilerplate from xfs_attr_set From: "Darrick J. Wong" To: djwong@kernel.org Cc: allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029202.253068.8909364981150861497.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 In preparation for online/offline repair wanting to use xfs_attr_set, move some of the boilerplate out of this function into the callers. Repair can initialize the da_args completely, and the userspace flag handling/twisting goes away once we move it to xfs_attr_change. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- fs/xfs/libxfs/xfs_attr.c | 33 ++++++++++++--------------------- fs/xfs/scrub/attr_repair.c | 4 ++++ fs/xfs/xfs_xattr.c | 24 ++++++++++++++++++++++-- 3 files changed, 38 insertions(+), 23 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 8c283e5c24702..df8418671c379 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -948,6 +948,16 @@ xfs_attr_lookup( return error; } +/* + * Make a change to the xattr structure. + * + * The caller must have initialized @args, attached dquots, and must not hold + * any ILOCKs. + * + * Returns -EEXIST for XFS_ATTRUPDATE_CREATE if the name already exists. + * Returns -ENOATTR for XFS_ATTRUPDATE_REMOVE if the name does not exist. + * Returns 0 on success, or a negative errno if something else went wrong. + */ int xfs_attr_set( struct xfs_da_args *args, @@ -961,27 +971,7 @@ xfs_attr_set( int rmt_blks = 0; unsigned int total; - if (xfs_is_shutdown(dp->i_mount)) - return -EIO; - - error = xfs_qm_dqattach(dp); - if (error) - return error; - - if (!args->owner) - args->owner = args->dp->i_ino; - args->geo = mp->m_attr_geo; - args->whichfork = XFS_ATTR_FORK; - xfs_attr_sethash(args); - - /* - * We have no control over the attribute names that userspace passes us - * to remove, so we have to allow the name lookup prior to attribute - * removal to fail as well. Preserve the logged flag, since we need - * to pass that through to the logging code. - */ - args->op_flags = XFS_DA_OP_OKNOENT | - (args->op_flags & XFS_DA_OP_LOGGED); + ASSERT(!args->trans); switch (op) { case XFS_ATTRUPDATE_UPSERT: @@ -1076,6 +1066,7 @@ xfs_attr_set( error = xfs_trans_commit(args->trans); out_unlock: xfs_iunlock(dp, XFS_ILOCK_EXCL); + args->trans = NULL; return error; out_trans_cancel: diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c index 8b89c112c492f..67c0ec0d1dbba 100644 --- a/fs/xfs/scrub/attr_repair.c +++ b/fs/xfs/scrub/attr_repair.c @@ -558,6 +558,9 @@ xrep_xattr_insert_rec( .namelen = key->namelen, .valuelen = key->valuelen, .owner = rx->sc->ip->i_ino, + .geo = rx->sc->mp->m_attr_geo, + .whichfork = XFS_ATTR_FORK, + .op_flags = XFS_DA_OP_OKNOENT, }; struct xchk_xattr_buf *ab = rx->sc->buf; int error; @@ -602,6 +605,7 @@ xrep_xattr_insert_rec( * xfs_attr_set creates and commits its own transaction. If the attr * already exists, we'll just drop it during the rebuild. */ + xfs_attr_sethash(&args); error = xfs_attr_set(&args, XFS_ATTRUPDATE_CREATE); if (error == -EEXIST) error = 0; diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c index c34d09c998a05..bf0cbcd7567e8 100644 --- a/fs/xfs/xfs_xattr.c +++ b/fs/xfs/xfs_xattr.c @@ -17,6 +17,7 @@ #include "xfs_acl.h" #include "xfs_log.h" #include "xfs_xattr.h" +#include "xfs_quota.h" #include @@ -70,7 +71,9 @@ xfs_attr_want_log_assist( /* * Set or remove an xattr, having grabbed the appropriate logging resources - * prior to calling libxfs. + * prior to calling libxfs. Callers of this function are only required to + * initialize the inode, attr_filter, name, namelen, value, and valuelen fields + * of @args. */ int xfs_attr_change( @@ -80,7 +83,19 @@ xfs_attr_change( struct xfs_mount *mp = args->dp->i_mount; int error; - ASSERT(!(args->op_flags & XFS_DA_OP_LOGGED)); + if (xfs_is_shutdown(mp)) + return -EIO; + + error = xfs_qm_dqattach(args->dp); + if (error) + return error; + + /* + * We have no control over the attribute names that userspace passes us + * to remove, so we have to allow the name lookup prior to attribute + * removal to fail as well. + */ + args->op_flags = XFS_DA_OP_OKNOENT; if (xfs_attr_want_log_assist(mp)) { error = xfs_attr_grab_log_assist(mp); @@ -90,6 +105,11 @@ xfs_attr_change( args->op_flags |= XFS_DA_OP_LOGGED; } + args->owner = args->dp->i_ino; + args->geo = mp->m_attr_geo; + args->whichfork = XFS_ATTR_FORK; + xfs_attr_sethash(args); + return xfs_attr_set(args, op); } From patchwork Tue Apr 16 01:36:27 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: 13631054 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 9C620CA7A for ; Tue, 16 Apr 2024 01:36:28 +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=1713231388; cv=none; b=ThvnE7FCB3p9SjtdtgxllW4wzTOiRAxtl/a4FYP/fOGLVqPEUrIHAn0b/QmIWRNWheD9twgTYFdSBxeCIWL3aNiYGkXVZ8c3wWyI3BT0BETSEz/TBQUgM2m4pocn7RRTM6qeIQ8/SwQc0m9L/PpPHD/dbE109xTEzFzfvrLSHOM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231388; c=relaxed/simple; bh=LQR8aW4nbtsPVI/YFXIuETcqJMVOULWfjk1pWuj9lw0=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=kUVJOAk2364nqaVNOHMiohU74XNlqmpmHgZmCrlm9i2X/ZpnPmAi/ROMz7y6Yfe4wic3Kc+IS1wnJmtAML8XYG+P1nLfGeKg8YN2508DE23o5sWEfUHWIvVleBxlQY7uY3pilX+1SucXnJEZtiL3FbgT/wYZueWhQmZOPKbREvw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=lQZ9mhoY; 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="lQZ9mhoY" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 693F1C113CC; Tue, 16 Apr 2024 01:36:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231388; bh=LQR8aW4nbtsPVI/YFXIuETcqJMVOULWfjk1pWuj9lw0=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=lQZ9mhoY4JJO8W/w3gO0H9292MGsWy1udDKB4pPNNlk+GwYGRAbiA8M5DUUngJyC4 k/iXqerBb3cUjTA1LK+DyiDSGlTf+529oJ6kr/swumNIJSGXiwyn9xkfLSaa+/7chN /IGwrhZG238uVyViltlTdTsKdmjRfUo3QGDFT/Lif8fwimmtKmWxFfPXVN/9NMr3o8 mTuqeOv2Om2Xk/Vep6Ol+gmZjPQmb8I7fTOWNypywSlLBBLw2oHZqcWMo3pb7o0e+Z sAJ5ZQmZGHwdNniVgbiAdibM6U9g8+PlWUMh+pNylfvQ/Dn02zqmFLkGO5dkpS7B3+ WDZCAqTsk1blA== Date: Mon, 15 Apr 2024 18:36:27 -0700 Subject: [PATCH 02/17] xfs: make the reserved block permission flag explicit in xfs_attr_set From: "Darrick J. Wong" To: djwong@kernel.org Cc: allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029218.253068.3901233919366892527.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Make the use of reserved blocks an explicit parameter to xfs_attr_set. Userspace setting XFS_ATTR_ROOT attrs should continue to be able to use it, but for online repairs we can back out and therefore do not care. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- fs/xfs/libxfs/xfs_attr.c | 6 +++--- fs/xfs/libxfs/xfs_attr.h | 2 +- fs/xfs/scrub/attr_repair.c | 2 +- fs/xfs/xfs_xattr.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index df8418671c379..c98145596f029 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -952,7 +952,7 @@ xfs_attr_lookup( * Make a change to the xattr structure. * * The caller must have initialized @args, attached dquots, and must not hold - * any ILOCKs. + * any ILOCKs. Reserved data blocks may be used if @rsvd is set. * * Returns -EEXIST for XFS_ATTRUPDATE_CREATE if the name already exists. * Returns -ENOATTR for XFS_ATTRUPDATE_REMOVE if the name does not exist. @@ -961,12 +961,12 @@ xfs_attr_lookup( int xfs_attr_set( struct xfs_da_args *args, - enum xfs_attr_update op) + enum xfs_attr_update op, + bool rsvd) { struct xfs_inode *dp = args->dp; struct xfs_mount *mp = dp->i_mount; struct xfs_trans_res tres; - bool rsvd = (args->attr_filter & XFS_ATTR_ROOT); int error, local; int rmt_blks = 0; unsigned int total; diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h index d12583dd7eecb..43dee4cbaab25 100644 --- a/fs/xfs/libxfs/xfs_attr.h +++ b/fs/xfs/libxfs/xfs_attr.h @@ -558,7 +558,7 @@ enum xfs_attr_update { XFS_ATTRUPDATE_REPLACE, /* set value, fail if attr does not exist */ }; -int xfs_attr_set(struct xfs_da_args *args, enum xfs_attr_update op); +int xfs_attr_set(struct xfs_da_args *args, enum xfs_attr_update op, bool rsvd); int xfs_attr_set_iter(struct xfs_attr_intent *attr); int xfs_attr_remove_iter(struct xfs_attr_intent *attr); bool xfs_attr_check_namespace(unsigned int attr_flags); diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c index 67c0ec0d1dbba..cbcc446d5119b 100644 --- a/fs/xfs/scrub/attr_repair.c +++ b/fs/xfs/scrub/attr_repair.c @@ -606,7 +606,7 @@ xrep_xattr_insert_rec( * already exists, we'll just drop it during the rebuild. */ xfs_attr_sethash(&args); - error = xfs_attr_set(&args, XFS_ATTRUPDATE_CREATE); + error = xfs_attr_set(&args, XFS_ATTRUPDATE_CREATE, false); if (error == -EEXIST) error = 0; diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c index bf0cbcd7567e8..d7092efe284a7 100644 --- a/fs/xfs/xfs_xattr.c +++ b/fs/xfs/xfs_xattr.c @@ -110,7 +110,7 @@ xfs_attr_change( args->whichfork = XFS_ATTR_FORK; xfs_attr_sethash(args); - return xfs_attr_set(args, op); + return xfs_attr_set(args, op, args->attr_filter & XFS_ATTR_ROOT); } From patchwork Tue Apr 16 01:36:43 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: 13631055 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 8C1CB539C for ; Tue, 16 Apr 2024 01:36:44 +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=1713231404; cv=none; b=ORsBcZE45hGQZ1i6iHXFf1M/bQpnIDoQPbJQvFJZwQdViFrjnQKIQaRcSO0v24AOWjrY/ylLA2+nyMhlHuLraxRIpPzDR6DI6yeTAMSnUN/0GWny/W93M9+yuO8ttBMLvQV3hKVouqbfcs9sFxfShqc9lbbTzvhEWEbw6V2yoFI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231404; c=relaxed/simple; bh=D44BGYNjjDDc0fWg5nVBELt1yLVtNJgu9ghly+lbR4A=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=k81i1EOFELFxiO+ZaElLWieq+qp4Gwy9wvX3rnaM47wMlxg0GHQ+X0UYJ38U9nHYgsRHRVWtMqjzpBHsJdtNFHnfPM6Y0hO0FVrlE/+eCEq4OofdWjEuidIE3OwK8SoCdjHOR0OGJ5wKViEASRwK1nmKBpD6W6etpRTpa8sk614= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=pDuaqfry; 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="pDuaqfry" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1277AC113CC; Tue, 16 Apr 2024 01:36:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231404; bh=D44BGYNjjDDc0fWg5nVBELt1yLVtNJgu9ghly+lbR4A=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=pDuaqfrynEPuMAqC9YcRWP6vdhNyuBM+nicrlpJMS35tzB/S72cExJ4Z2SdroMPlt DV045dHQO7gc4nYS0QfhDeA1lgv7VoTI/oygu3tl39pc1V0A51VXZo6RrLFvU4xfnt xxIOaE9HrkjCoNUnyOwuSrQgO08jVgsBX5wYoW/RomIciyDbBPS3HppIWjAsaIXXYA 2bTG7LsSjLrYgcTKareIpYh+Nste/TIo0WnVQlvzsP//U0tTcxkxB5VdEruVAurWTO WDJImtC7FRxDUdgeTMIDT4kwkLL7l9NigNpQwx8aMm8kb5rJGhNF/+CNsVGRvjG0+3 xipRIo4b/8QAg== Date: Mon, 15 Apr 2024 18:36:43 -0700 Subject: [PATCH 03/17] xfs: use xfs_attr_defer_parent for calling xfs_attr_set on pptrs From: "Darrick J. Wong" To: djwong@kernel.org Cc: allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029234.253068.15430807629732077593.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 xfs_attr_set function so that repair code can use it to fix broken parent pointers. Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_attr.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index c98145596f029..b18f6c258a174 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -1027,14 +1027,21 @@ xfs_attr_set( case -EEXIST: if (op == XFS_ATTRUPDATE_REMOVE) { /* if no value, we are performing a remove operation */ - xfs_attr_defer_add(args, XFS_ATTR_DEFER_REMOVE); + if (args->attr_filter & XFS_ATTR_PARENT) + xfs_attr_defer_parent(args, + XFS_ATTR_DEFER_REMOVE); + else + xfs_attr_defer_add(args, XFS_ATTR_DEFER_REMOVE); break; } /* Pure create fails if the attr already exists */ if (op == XFS_ATTRUPDATE_CREATE) goto out_trans_cancel; - xfs_attr_defer_add(args, XFS_ATTR_DEFER_REPLACE); + if (args->attr_filter & XFS_ATTR_PARENT) + xfs_attr_defer_parent(args, XFS_ATTR_DEFER_REPLACE); + else + xfs_attr_defer_add(args, XFS_ATTR_DEFER_REPLACE); break; case -ENOATTR: /* Can't remove what isn't there. */ @@ -1044,7 +1051,10 @@ xfs_attr_set( /* Pure replace fails if no existing attr to replace. */ if (op == XFS_ATTRUPDATE_REPLACE) goto out_trans_cancel; - xfs_attr_defer_add(args, XFS_ATTR_DEFER_SET); + if (args->attr_filter & XFS_ATTR_PARENT) + xfs_attr_defer_parent(args, XFS_ATTR_DEFER_SET); + else + xfs_attr_defer_add(args, XFS_ATTR_DEFER_SET); break; default: goto out_trans_cancel; From patchwork Tue Apr 16 01:36:59 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: 13631056 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 DB9C77464 for ; Tue, 16 Apr 2024 01:36:59 +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=1713231419; cv=none; b=IrUdK/SBuVHAJMM1HEuj8mZgU49jw6+0pYXlYWZubs1ukI3N3JQ+xzcLhVV2rjpvJ/oTdSP6XSuME0XPy1GPpNDdDFRW8l0pCF3OXDD8WkiNRCit6TCI4d5D47Kbc9nZD/7hqXaiGZHbqZnLgJTT5CVScYnQ/y0xfTd2gWkUh48= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231419; c=relaxed/simple; bh=5yxQ5A6mb6f1G5fiP6A8Bht5p4TxZA3ta8rV05MAThU=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=aqwEVrU9J7KgTTX8YmXEWS1uihVi1fYQkuWEwX+NDcaRYNo7mmTE3kYy2hGalzsVa2sQ4zwioldiCk67YsQ9NaiMCUPmHbgmzEUUY/QER4B4q7E/9VOfVkYVUi0XVV46AcPOs/3Hj5k52j9IifhBHI/cl52BB5rYoOfHoZKP4Zc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=IerAb1vz; 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="IerAb1vz" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B7777C113CC; Tue, 16 Apr 2024 01:36:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231419; bh=5yxQ5A6mb6f1G5fiP6A8Bht5p4TxZA3ta8rV05MAThU=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=IerAb1vzf7qOoZAAtRCWIAcGe8JGQCTYkUJbGdkGKwjw7B++ka1aX2iYPRRs7ZC03 6rEpw0WasL9bt2SSWDO1jTYSE4RLGziyaWErU6hSrf/1cujCxB6h5lyMdjzOwCWw4b a2QKRd/7IxV6nfVtkjo51IV4gKCCI/UtyQaTSQGZhPTZ2pBgxtaOkkczHDUNsjW3Bh AU8FrZdFtqNV+jyp9gESr9yfts5hkOk62TA690cSyzurPNA28daGxvpQWL2V+zP+kz Cqvea8nBaKqoR3k1g9vZbFQcivQteBY5RRlHTytemi/av/aHfxfhR5B0PaeDbM0I/0 WneBu644urFGw== Date: Mon, 15 Apr 2024 18:36:59 -0700 Subject: [PATCH 04/17] xfs: salvage parent pointers when rebuilding xattr structures From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029250.253068.11514114320156722007.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 | 36 +++++++++++++++++++++++++++--------- fs/xfs/scrub/trace.h | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 9 deletions(-) diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c index cbcc446d5119b..48443e88e298b 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); @@ -598,8 +609,15 @@ 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); + args.op_flags |= XFS_DA_OP_LOGGED; + } 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), From patchwork Tue Apr 16 01:37:14 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: 13631057 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 79422FC1F for ; Tue, 16 Apr 2024 01:37:15 +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=1713231435; cv=none; b=LWl9WgXL5RTd1tLo6XEAAMt6zuf0wwc2/HT3VqQtSaIyq71c/p2lfykUxRVjqnlnR5hZoG5iNpKQIlPXubUIIkmMGUBPMjZ5JAp5R8p84nyYplZfqryRK2kqfFmDyWKRrYZSCAxmT+UoqjYVgIAJHVUK51Nc86WsgN5B+BFUbA4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231435; c=relaxed/simple; bh=gnU/7fayqT4AJKSUfnkciXEJ0YSijwBo5XMf3kshZ7o=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=GlX1ROLJWzKjQeOKtMultph4NJMWriedBXXN2eN9Zlv8eH2E+M1YGEzh3aw105/IlXmWJEDodtmDsV4YWZX4bc7UBPlZZ280tHlxfR6a6q7g313me03Fau4THipJ+0nGi7BnZ2yrAxP0AGog/D56URWFIHCXQTM3QsF7y2aiB/w= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=lrSoJZZi; 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="lrSoJZZi" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 54ED7C113CC; Tue, 16 Apr 2024 01:37:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231435; bh=gnU/7fayqT4AJKSUfnkciXEJ0YSijwBo5XMf3kshZ7o=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=lrSoJZZiVJCTj9qaIZ7OcUsNlTBDiqFXe/DvPANA+XpEUW+QZ/Rxx3FxYnv/PFdB0 caExWs35ZGUnIzlknLS/g6N5bvD+ln/QcNvXyJtD6k3YRL1Rvun+s9GdjT3X58ye/P VFNuKRl1gfgzaSvyC/BIDVw7+yY4xcAA8mE3zXSQKQ4/aajb8B/j6NrEqGipdXwgS/ 9S2CF/kOpSyBhqyRvfJcuWyuqJn7PHMibLTXVc+yLdUjHiCw6yjgmGCPh6F9xt9Owb RajtnI3T7WgnVJIjYWtRBNmSUT5MttLPg5ZA+AjzD+0fMXjjEWv/mUz8NTPpOVQgah Kwjo7eDTVyMjw== Date: Mon, 15 Apr 2024 18:37:14 -0700 Subject: [PATCH 05/17] xfs: add raw parent pointer apis to support repair From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029266.253068.2220619250735295856.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/libxfs/xfs_dir2.c | 2 + fs/xfs/libxfs/xfs_dir2.h | 2 + fs/xfs/libxfs/xfs_parent.c | 64 ++++++++++++++++++++++++++++++++++++++++++++ fs/xfs/libxfs/xfs_parent.h | 6 ++++ 4 files changed, 72 insertions(+), 2 deletions(-) diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c index 9da99fa20c759..7634344dc5153 100644 --- a/fs/xfs/libxfs/xfs_dir2.c +++ b/fs/xfs/libxfs/xfs_dir2.c @@ -434,7 +434,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 eb3a5c35025b5..b580a78bcf4fc 100644 --- a/fs/xfs/libxfs/xfs_dir2.h +++ b/fs/xfs/libxfs/xfs_dir2.h @@ -58,7 +58,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 381fe57b9124e..04d3ef2a9f330 100644 --- a/fs/xfs/libxfs/xfs_parent.c +++ b/fs/xfs/libxfs/xfs_parent.c @@ -313,3 +313,67 @@ xfs_parent_lookup( xfs_parent_da_args_init(scratch, tp, pptr, ip, ip->i_ino, parent_name); return xfs_attr_get_ilocked(scratch); } + +/* Sanity-check a parent pointer before we try to perform repairs. */ +static inline bool +xfs_parent_sanity_check( + struct xfs_mount *mp, + const struct xfs_name *parent_name, + const struct xfs_parent_rec *pptr) +{ + if (!xfs_parent_namecheck(XFS_ATTR_PARENT, parent_name->name, + parent_name->len)) + return false; + + if (!xfs_parent_valuecheck(mp, pptr, sizeof(*pptr))) + return false; + + return true; +} + + +/* + * Attach the parent pointer (@parent_name -> @pptr) 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, + xfs_ino_t owner, + const struct xfs_name *parent_name, + struct xfs_parent_rec *pptr, + struct xfs_da_args *scratch) +{ + if (!xfs_parent_sanity_check(ip->i_mount, parent_name, pptr)) { + ASSERT(0); + return -EFSCORRUPTED; + } + + memset(scratch, 0, sizeof(struct xfs_da_args)); + xfs_parent_da_args_init(scratch, NULL, pptr, ip, owner, parent_name); + return xfs_attr_set(scratch, XFS_ATTRUPDATE_UPSERT, false); +} + +/* + * Remove the parent pointer (@parent_name -> @pptr) 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, + xfs_ino_t owner, + const struct xfs_name *parent_name, + struct xfs_parent_rec *pptr, + struct xfs_da_args *scratch) +{ + if (!xfs_parent_sanity_check(ip->i_mount, parent_name, pptr)) { + ASSERT(0); + return -EFSCORRUPTED; + } + + memset(scratch, 0, sizeof(struct xfs_da_args)); + xfs_parent_da_args_init(scratch, NULL, pptr, ip, owner, parent_name); + return xfs_attr_set(scratch, XFS_ATTRUPDATE_REMOVE, false); +} diff --git a/fs/xfs/libxfs/xfs_parent.h b/fs/xfs/libxfs/xfs_parent.h index 97788582321a6..b8036527cdc73 100644 --- a/fs/xfs/libxfs/xfs_parent.h +++ b/fs/xfs/libxfs/xfs_parent.h @@ -100,5 +100,11 @@ int xfs_parent_from_attr(struct xfs_mount *mp, unsigned int attr_flags, 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); +int xfs_parent_set(struct xfs_inode *ip, xfs_ino_t owner, + const struct xfs_name *name, struct xfs_parent_rec *pptr, + struct xfs_da_args *scratch); +int xfs_parent_unset(struct xfs_inode *ip, xfs_ino_t owner, + const struct xfs_name *name, struct xfs_parent_rec *pptr, + struct xfs_da_args *scratch); #endif /* __XFS_PARENT_H__ */ From patchwork Tue Apr 16 01:37:30 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: 13631058 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 209D5107A6 for ; Tue, 16 Apr 2024 01:37:31 +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=1713231451; cv=none; b=Z/1VpohYITCjqFu+wVjkUDyQpBju8Mo67WQMvEPOrMSKu9bp/fiLv6zXGTqanJybNg9paS/h/+Efb0j+C7t6dU9wAyudr3TNkYVelCgZa9FlrKDc1EOtXuWeTSEVL1NYxlS1lANp9I5seqPmPFHUwnB5X/gqUgrvmM/UUVyTSq8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231451; c=relaxed/simple; bh=y9XFFVUUTSln471G3qbWqapRTljx+en28kHPki/J9no=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=JtKh82LNtvOnSo/z+1Xx5t8vYVtFCjPjhgDd5liHQX6cPaj2ggx2pHlPPunHIH58fnOYafQz+m2U+1oLWhvh72pFcukVVbQXH5FISMuVyGo6apvaflDQ+E5P9Zevsczv8Cs/pdN51vigFLtfNdK9zuAzOr5gcB4W3nTifD5JZT0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=jEkUwsgQ; 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="jEkUwsgQ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E8733C113CC; Tue, 16 Apr 2024 01:37:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231451; bh=y9XFFVUUTSln471G3qbWqapRTljx+en28kHPki/J9no=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=jEkUwsgQudLA+/rvqYPnWkcezszERBxwz5uDnmxKKYOhZmfcHdN0uViV/gkhIxTpq eosXBWSO/UPqz73lp3AzPeRz6zR8irtRMsmEvKJmIfFIC+cquW1TaDPcaHHvDRpkB3 Ec7c3iNlx8WBmdUa4wiUMVRYeDan4jwFh++dCfl7vZm3GFHSCb9sKPjKkJTf6JSR9O vnwCZo1IPlI5y7ckDjwB6akBgEuV0bWzAjHSsVIFIE1IPiT5ytTU9U5nPw4Rl/FIKV ZKTIds/SHtohOILjWViHqWwlZLFRU5gkIpbQdIgMKxaczVMp0/lnfi1C8aXcL+MmAE 4HBUEpX545/lw== Date: Mon, 15 Apr 2024 18:37:30 -0700 Subject: [PATCH 06/17] xfs: repair directories by scanning directory parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029282.253068.8332217458222249450.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/dir_repair.c | 347 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 341 insertions(+), 6 deletions(-) diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c index 575397aef1f7a..4e6b0e88b9960 100644 --- a/fs/xfs/scrub/dir_repair.c +++ b/fs/xfs/scrub/dir_repair.c @@ -28,6 +28,7 @@ #include "xfs_exchmaps.h" #include "xfs_exchrange.h" #include "xfs_ag.h" +#include "xfs_parent.h" #include "scrub/xfs_scrub.h" #include "scrub/scrub.h" #include "scrub/common.h" @@ -43,6 +44,7 @@ #include "scrub/reap.h" #include "scrub/findparent.h" #include "scrub/orphanage.h" +#include "scrub/listxattr.h" /* * Directory Repair @@ -57,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 mapping exchange @@ -112,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; @@ -763,28 +782,35 @@ xrep_dir_replay_updates( int error; /* Add all the salvaged dirents to the temporary directory. */ + mutex_lock(&rd->pscan.lock); foreach_xfarray_idx(rd->dir_entries, array_cur) { struct xrep_dirent dirent; error = xfarray_load(rd->dir_entries, array_cur, &dirent); if (error) - return error; + goto out_unlock; error = xfblob_loadname(rd->dir_names, dirent.name_cookie, &rd->xname, dirent.namelen); if (error) - return error; + goto out_unlock; rd->xname.type = dirent.ftype; + mutex_unlock(&rd->pscan.lock); error = xrep_dir_replay_update(rd, &rd->xname, &dirent); if (error) return error; + mutex_lock(&rd->pscan.lock); } /* Empty out both arrays now that we've added the entries. */ xfarray_truncate(rd->dir_entries); xfblob_truncate(rd->dir_names); + mutex_unlock(&rd->pscan.lock); return 0; +out_unlock: + mutex_unlock(&rd->pscan.lock); + return error; } /* @@ -995,6 +1021,269 @@ 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, + 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, + .type = xfs_mode_to_ftype(VFS_I(ip)->i_mode), + }; + xfs_ino_t parent_ino; + uint32_t parent_gen; + struct xrep_dir *rd = priv; + int error; + + if (!(attr_flags & XFS_ATTR_PARENT)) + return 0; + + /* + * Ignore parent pointers that point back to a different dir, list the + * wrong generation number, or are invalid. + */ + error = xfs_parent_from_attr(sc->mp, attr_flags, name, namelen, value, + valuelen, &parent_ino, &parent_gen); + if (error) + return error; + + if (parent_ino != sc->ip->i_ino || + parent_gen != VFS_I(sc->ip)->i_generation) + return 0; + + 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_xattr_walk(rd->sc, ip, xrep_dir_scan_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; + + error = xrep_dir_replay_updates(rd); + 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 + * file mapping exchange functions 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 @@ -1194,6 +1483,45 @@ xrep_dir_set_nlink( return 0; } +/* + * Finish replaying stashed dirent updates, allocate a transaction for + * exchanging data fork mappings, 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_tempexch_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 exchanging + * 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_tempexch_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; +} + /* Exchange the temporary directory's data fork with the one being repaired. */ STATIC int xrep_dir_swap( @@ -1296,8 +1624,12 @@ xrep_dir_rebuild_tree( if (error) return error; - /* Allocate transaction and ILOCK the scrub file and the temp file. */ - error = xrep_tempexch_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 exchange data fork mappings. + */ + error = xrep_dir_finalize_tempdir(rd); if (error) return error; @@ -1482,7 +1814,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 Tue Apr 16 01:37:46 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: 13631059 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 BFD634C98 for ; Tue, 16 Apr 2024 01:37:46 +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=1713231466; cv=none; b=GW/9RmzymsGZy0jUsQ/vJW5P7QMQC9hgl/1+JRS4HSAb1nhcLQTd1ZDWQRSoWU70cD89umnsJbrBJnRihCc0Be36WE/AD7QPqpRm2xVaQL1ljNq2RNA4/UMbE+IUWN4QgsbWPMaOa+Ix8TuvGvVAegKQ2pOCGPrqWTZUu1lLzi4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231466; c=relaxed/simple; bh=kFiaPFZDEMjoy1H4YsZGc8ijSjXurlg/TvIyJ6mCWXc=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=mLovw58QiZc934XjTvhl347lQfmiUaFF8R3eeJO9vOeFeVWwI1QRrD63Jmn7KubSOxxMnmK3PmDdV9LQzrIndS6sk+UmDu0XHGcgR+uKQu/UB8IeK8QYPEneWF5huPYkDKr4TIYjBpzoHVfH1pO4xp78o3qXcTuSlulYbwzY0vA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=LA+IpCFE; 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="LA+IpCFE" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 94651C113CC; Tue, 16 Apr 2024 01:37:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231466; bh=kFiaPFZDEMjoy1H4YsZGc8ijSjXurlg/TvIyJ6mCWXc=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=LA+IpCFEV1qFmh9GJjV2rup4G5JxaeJHYRPlWNALYPCWakhvR2UYovFiVmPV5gxjK qQ43T8LRRg8g4Unhxh2LRQzzszk0UolKRPLu8XjimHCVY9HCgYLXgqMJwptofPZgQI ygfNoEOkAD03qqJzCvqRTKK+1RzT2CB6yW5DbVzXi+BpZLlGd+x/Dx8HRNl1GMasf6 kB6aDsYVpoV9DYLUtY4qMdqahvjENtM0Fjfbx+wjK+VoMQsSb6hfUH3tbXJu7Jwzrm iyDRopcxba8BINLQ0lHZ325eVKXsSHj4ZuVR1j/wKQYfCuH6cSM0flOsp5KxEb9FdI waZsh1qGt9YDg== Date: Mon, 15 Apr 2024 18:37:46 -0700 Subject: [PATCH 07/17] xfs: implement live updates for directory repairs From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029298.253068.12218798352368474899.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/dir_repair.c | 220 +++++++++++++++++++++++++++++++++++++++++---- fs/xfs/scrub/findparent.c | 10 +- fs/xfs/scrub/findparent.h | 10 ++ fs/xfs/scrub/trace.h | 2 4 files changed, 219 insertions(+), 23 deletions(-) diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c index 4e6b0e88b9960..60e31da4451e8 100644 --- a/fs/xfs/scrub/dir_repair.c +++ b/fs/xfs/scrub/dir_repair.c @@ -85,6 +85,12 @@ * 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; }; /* @@ -339,6 +348,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, @@ -354,6 +364,33 @@ 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_storename(rd->dir_names, &dirent.name_cookie, name); + 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( @@ -705,6 +742,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 @@ -732,26 +806,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, xname, &ino); - if (error != -ENOENT) { - ASSERT(error != -ENOENT); + error = xchk_dir_lookup(rd->sc, rd->sc->tempip, xname, &ino); + if (error != -ENOENT) { + ASSERT(error != -ENOENT); + goto out_cancel; + } +#endif + + error = xrep_dir_replay_createname(rd, xname, dirent->ino, + resblks); + if (error) + goto out_cancel; + + if (xname->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, xname, &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, xname, resblks); + if (error) + goto out_cancel; + + if (xname->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, xname, dirent->ino, resblks); - if (error) - goto out_cancel; - - if (xname->type == XFS_DIR3_FT_DIR) - rd->subdirs++; - rd->dirents++; /* Commit and unlock. */ error = xrep_trans_commit(rd->sc); @@ -1284,6 +1396,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.dhook.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 @@ -1633,6 +1810,9 @@ xrep_dir_rebuild_tree( if (error) return error; + if (xchk_iscan_aborted(&rd->pscan.iscan)) + return -ECANCELED; + /* * Exchange the tempdir's data fork with the file being repaired. This * recreates the transaction and re-takes the ILOCK in the scrub @@ -1688,7 +1868,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 712dd73e4789f..c78422ad757bf 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,10 @@ xrep_findparent_scan_start( * ILOCK, which means that any in-progress inode updates will finish * before we can scan the inode. */ - xfs_dir_hook_setup(&pscan->dhook, xrep_findparent_live_update); + if (custom_fn) + xfs_dir_hook_setup(&pscan->dhook, custom_fn); + else + xfs_dir_hook_setup(&pscan->dhook, xrep_findparent_live_update); error = xfs_dir_hook_add(sc->mp, &pscan->dhook); if (error) goto out_iscan; diff --git a/fs/xfs/scrub/findparent.h b/fs/xfs/scrub/findparent.h index 501f99d3164ed..d998c7a88152c 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 4b968df3d840c..64db413b18884 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -2692,6 +2692,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 Tue Apr 16 01:38:01 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: 13631060 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 6DE5C4C98 for ; Tue, 16 Apr 2024 01:38:02 +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=1713231482; cv=none; b=JxDLxdfFaPx7eKO46yjpDhzKN6KYevdRapbcHNHNyduCoPh9JW5tC9a8PE9i4gQLefh9Q1XC7eVXwOMuTScn/dAbbgbe5QVrjO+JJ7uX6xaiGwtc+FIB+MwUhF7Qflctz2ZwvkbVMExyeSJ7EQoEM6SGV/v6T7t3vqGbkZbnemU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231482; c=relaxed/simple; bh=lOP9LstyJ30xe3QZOPvciTPmbvFGAJMaPFqQjmdSmIM=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=E2YSqi90lIez/O7RlRZsStjdw7OW2f3InMQDTsLvFQshy3C+FX/CRMo279Ax9y1OMAO0u2btGeV2KpkF8WDnqHasay1uwWZzA5jZ1DuVAz+B+X31pwOo+NmZIGvXXyjauUOFu2ubxCVHpkB7g1hQUD2LqnnhZgyCCbjiqZ4aCqU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=UJGP+rsv; 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="UJGP+rsv" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 45DB0C113CC; Tue, 16 Apr 2024 01:38:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231482; bh=lOP9LstyJ30xe3QZOPvciTPmbvFGAJMaPFqQjmdSmIM=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=UJGP+rsvcxG/7xrX0yhyt2geYijm16EVoB8TbeVzvqMmg6JF8mYE0VJSA5PO0RgLa JJg3EUdlGkz8xBbPp4lIKblihVBnYtoaqE1/wWRfK9cqkal4itcZasqZEywcOiaeO4 FHOUJS6c+6RS4L0WfxTSF+aAArJSxdny29QF8NNgaUJ7Kp2vQsiTS94t4Lygty9cyc +kj0t/AKTNXUeJXz8JgvaBUvOM5zOC0V3yLMP3FRFO4Za+T0O2/j+bqsn5DW7snNjx JwMHVWBsbPJH18EEx7LyT8TToPiPDbFHIQEsn2xghe4GSY1S97XDK0LB1A3bPQ/uEo N0ICXC03zKiXg== Date: Mon, 15 Apr 2024 18:38:01 -0700 Subject: [PATCH 08/17] xfs: replay unlocked parent pointer updates that accrue during xattr repair From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029314.253068.15685421835444086157.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/attr_repair.c | 438 ++++++++++++++++++++++++++++++++++++++++++++ fs/xfs/scrub/trace.h | 73 +++++++ 2 files changed, 509 insertions(+), 2 deletions(-) diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c index 48443e88e298b..87f6c9cb54eb6 100644 --- a/fs/xfs/scrub/attr_repair.c +++ b/fs/xfs/scrub/attr_repair.c @@ -96,6 +96,52 @@ 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 dhook; + + /* Scratch buffer for capturing parent pointers. */ + struct xfs_da_args pptr_args; + + /* Name buffer */ + struct xfs_name xname; + char namebuf[MAXNAMELEN]; +}; + +/* 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 record. */ + struct xfs_parent_rec pptr_rec; + + /* Length of the pptr name. */ + uint8_t namelen; + + /* XREP_XATTR_PPTR_{ADD,REMOVE} */ + uint8_t action; }; /* Set up to recreate the extended attributes. */ @@ -103,6 +149,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); } @@ -711,11 +760,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; + + xfs_assert_ilocked(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 = ifp->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( @@ -730,6 +890,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. @@ -768,6 +929,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; + } } } } @@ -927,6 +1096,180 @@ 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 xfs_name *xname, + struct xrep_xattr_pptr *pptr) +{ + struct xfs_scrub *sc = rx->sc; + int error; + + switch (pptr->action) { + case XREP_XATTR_PPTR_ADD: + /* Create parent pointer. */ + trace_xrep_xattr_replay_parentadd(sc->tempip, xname, + &pptr->pptr_rec); + + error = xfs_parent_set(sc->tempip, sc->ip->i_ino, xname, + &pptr->pptr_rec, &rx->pptr_args); + ASSERT(error != -EEXIST); + return error; + case XREP_XATTR_PPTR_REMOVE: + /* Remove parent pointer. */ + trace_xrep_xattr_replay_parentremove(sc->tempip, xname, + &pptr->pptr_rec); + + error = xfs_parent_unset(sc->tempip, sc->ip->i_ino, xname, + &pptr->pptr_rec, &rx->pptr_args); + ASSERT(error != -ENOATTR); + return error; + } + + ASSERT(0); + return -EIO; +} + +/* + * Flush stashed parent pointer updates that have been recorded by the scanner. + * This is done to reduce the memory requirements of the xattr 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_loadname(rx->pptr_names, pptr.name_cookie, + &rx->xname, pptr.namelen); + if (error) + goto out_unlock; + mutex_unlock(&rx->lock); + + error = xrep_xattr_replay_pptr_update(rx, &rx->xname, &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, + }; + int error; + + trace_xrep_xattr_stash_parentadd(rx->sc->tempip, dp, name); + + xfs_inode_to_parent_rec(&pptr.pptr_rec, dp); + error = xfblob_storename(rx->pptr_names, &pptr.name_cookie, name); + 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, + }; + int error; + + trace_xrep_xattr_stash_parentremove(rx->sc->tempip, dp, name); + + xfs_inode_to_parent_rec(&pptr.pptr_rec, dp); + error = xfblob_storename(rx->pptr_names, &pptr.name_cookie, name); + 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 exchanging 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, dhook.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 an exchange. Promote the tempfile * from short format to leaf format, and if the file being repaired has a short @@ -1030,6 +1373,45 @@ xrep_xattr_swap( return xrep_tempexch_contents(sc, tx); } +/* + * Finish replaying stashed parent pointer updates, allocate a transaction for + * exchanging extent mappings, 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_tempexch_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 + * exchanging 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_tempexch_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; +} + /* * Exchange the new extended attribute data (which we created in the tempfile) * with the file being repaired. @@ -1082,8 +1464,12 @@ xrep_xattr_rebuild_tree( if (error) return error; - /* Allocate exchange transaction and lock both inodes. */ - error = xrep_tempexch_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 exchange attr fork mappings. + */ + error = xrep_xattr_finalize_tempfile(rx); if (error) return error; @@ -1124,8 +1510,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->dhook); + 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 +1537,10 @@ xrep_xattr_setup_scan( if (!rx) return -ENOMEM; rx->sc = sc; + rx->can_flush = true; + rx->xname.name = rx->namebuf; + + mutex_init(&rx->lock); /* * Allocate enough memory to handle loading local attr values from the @@ -1171,11 +1568,43 @@ 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_dir_hook_setup(&rx->dhook, xrep_xattr_live_dirent_update); + error = xfs_dir_hook_add(sc->mp, &rx->dhook); + 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 +1641,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 64db413b18884..68532f686eeb1 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -2602,6 +2602,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, @@ -2748,6 +2785,42 @@ 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_name *name, + const struct xfs_parent_rec *pptr), + TP_ARGS(ip, name, 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, name->len) + ), + TP_fast_assign( + __entry->dev = ip->i_mount->m_super->s_dev; + __entry->ino = ip->i_ino; + __entry->parent_ino = be64_to_cpu(pptr->p_ino); + __entry->parent_gen = be32_to_cpu(pptr->p_gen); + __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_EVENT(name) \ +DEFINE_EVENT(xrep_pptr_class, name, \ + TP_PROTO(struct xfs_inode *ip, const struct xfs_name *name, \ + const struct xfs_parent_rec *pptr), \ + TP_ARGS(ip, name, 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 Tue Apr 16 01:38:17 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: 13631061 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 4AA544C98 for ; Tue, 16 Apr 2024 01:38:18 +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=1713231498; cv=none; b=NOrbDjVUa5VM69b6yVrykTBeukZyrNQYIUKeT03slh28xxcrBNuNmDAfoV2H1dTNMF4JDVkjClv/zwMwKDm1EdOwNUEhMNVVoir0w86HylVeE3PLv1wbuvtjQxeM6/QUgWCGsQgcrLV5f/sEJe+EhfenC5skmrYePbj94WUaPl0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231498; c=relaxed/simple; bh=6O7Ggf7JzqHZg7Q0ffJ/qrOXmfSvGaBoZbUNMz6M+MQ=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=r3padXhDArBxYfS5x96ZGROcGvQ0nHIF2Z5DnkvBSlTzOkV02OdvP8p8MlSEUVrzsDZdg8J7yi3FVqXumwBUj7dTgtWM3qEUGqF40Z4/jA96CPouCw/2aUecWKMi75m/bQ0ylORAVyMRF+mSiLVy7FJzOolATFwDzbfzEYHczaE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=RRAnJNDt; 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="RRAnJNDt" Received: by smtp.kernel.org (Postfix) with ESMTPSA id DC7C7C113CC; Tue, 16 Apr 2024 01:38:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231497; bh=6O7Ggf7JzqHZg7Q0ffJ/qrOXmfSvGaBoZbUNMz6M+MQ=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=RRAnJNDtUoBP74MvFRRIlEnfo563MFQ5I/KNK9KySPiBOTxtbp2QazoLZpWYtCVdj 7nWgfSoLIZtPPUGsAnijd8e+k+2ToLDocKNnuVOmLCGMkuQtSUKvxeoDx92NtdV6op +O4Cd6nyEXuXMzTotVukBnRVbzyB6ZJTvd9flMV5pfw6mTViSWqokaxp0rPVDakv8x 1yqVV1/+1trQ6seCh1FinbXj1gZM8oBcHqDSROG2MOo2IgnPHy5xd0f+mKMfTsy81w +ZYbis9nG+UMdJiBYnYj6HpvaVhBMmLbP8c9MxFIvVBL9LtZ7vqwOyeVe1TXACdpwx KvAyd+smrWM9Q== Date: Mon, 15 Apr 2024 18:38:17 -0700 Subject: [PATCH 09/17] xfs: repair directory parent pointers by scanning for dirents From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029330.253068.15711120554583959440.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/parent_repair.c | 414 ++++++++++++++++++++++++++++++++++++++++++ fs/xfs/scrub/trace.h | 36 ++++ 2 files changed, 447 insertions(+), 3 deletions(-) diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c index 63590e1b35060..b4084a9f0e9c8 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_exchmaps.h" +#include "xfs_parent.h" #include "scrub/xfs_scrub.h" #include "scrub/scrub.h" #include "scrub/common.h" @@ -34,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 @@ -49,14 +53,61 @@ * 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 record. */ + struct xfs_parent_rec pptr_rec; + + /* 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; @@ -66,6 +117,9 @@ struct xrep_parent { /* Directory entry name, plus the trailing null. */ struct xfs_name xname; unsigned char namebuf[MAXNAMELEN]; + + /* Scratch buffer for scanning pptr xattrs */ + struct xfs_da_args pptr_args; }; /* Tear down all the incore stuff we created. */ @@ -74,6 +128,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. */ @@ -82,6 +142,7 @@ xrep_setup_parent( struct xfs_scrub *sc) { struct xrep_parent *rp; + int error; xchk_fsgates_enable(sc, XCHK_FSGATES_DIRENTS); @@ -92,6 +153,10 @@ xrep_setup_parent( rp->xname.name = rp->namebuf; sc->buf = rp; + error = xrep_tempfile_create(sc, S_IFREG); + if (error) + return error; + return xrep_orphanage_try_create(sc); } @@ -147,6 +212,307 @@ 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 xfs_name *xname, + struct xrep_pptr *pptr) +{ + struct xfs_scrub *sc = rp->sc; + + /* Create parent pointer. */ + trace_xrep_parent_replay_parentadd(sc->tempip, xname, &pptr->pptr_rec); + + return xfs_parent_set(sc->tempip, sc->ip->i_ino, xname, + &pptr->pptr_rec, &rp->pptr_args); +} + +/* + * 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_loadname(rp->pptr_names, pptr.name_cookie, + &rp->xname, pptr.namelen); + if (error) + goto out_unlock; + rp->xname.len = pptr.namelen; + mutex_unlock(&rp->pscan.lock); + + error = xrep_parent_replay_update(rp, &rp->xname, &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, + }; + int error; + + trace_xrep_parent_stash_parentadd(rp->sc->tempip, dp, name); + + xfs_inode_to_parent_rec(&pptr.pptr_rec, dp); + error = xfblob_storename(rp->pptr_names, &pptr.name_cookie, name); + 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( @@ -298,8 +664,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 @@ -309,11 +706,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 68532f686eeb1..10c2a8d10058b 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -2820,6 +2820,42 @@ DEFINE_EVENT(xrep_pptr_class, name, \ TP_ARGS(ip, name, 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 Tue Apr 16 01:38:33 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: 13631062 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 032D15227 for ; Tue, 16 Apr 2024 01:38:33 +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=1713231514; cv=none; b=RPKK4X7WpkOVzz78etAIDwdQpL5UM3yItvpjM6mKpNyqWlAMcuiu9/vU1cy8aLnDV4Rdb2Ejf3qTeibTQgPFwRBMCFmUc1r95dTIOZ65YhzHgeWF81QAlKbIpPqmAH6F5/R5xQVDN8tcO4nYSqS+yeMDbFsOVPw2Lv3mRa+SEV4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231514; c=relaxed/simple; bh=lf641bR2nGki5UOLidLnZb7sJyyLHGQNxwKgf8hiOYg=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=QTyk8Bqln8vE7XZyvmdOWi3O/jPJwYoS9Fr+BNi4+7drgYSWmk2t3QVJ2fWFP+kmBremmPRCfbymqoxon9fXqyxQe0E43E7XzCw0z7qq4ode9nUKSd8sETRH2j48uUBM1CH+R4gy5jn+3MQIAw1gwztIyHhxCtC7Br/Fpjofdc0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=FIYPZb8O; 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="FIYPZb8O" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8F6B7C113CC; Tue, 16 Apr 2024 01:38:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231513; bh=lf641bR2nGki5UOLidLnZb7sJyyLHGQNxwKgf8hiOYg=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=FIYPZb8Oay3A5WIZtYIck3Wyk2fuVHcpowlM2Bv3pTFROXvQa9qMtcyPoIFsfkyP2 uTmCm8RdJL1E3Wx3uiHe9Rf/Kbec1YFAvcHo8Gx3BxvzwAcWSM5l4RK2G1d7cd+mAH CXL99VaILxUvlI0rN/jl5DBmQGZ+UAU71iToXOHhpujk3rSOc0fka9Aa6o7iQYS6tu CFm1U4Z+XKl0PyV3Zkmi9VePsJaEmlkM6sbPaVHNXElEyLyfHXtNRIHsM+Vvp8On7U vQ327iHoC0qOkV5UIYS0P8xCs/a6lPc0Syjug9OYsNCE+zO20yIvQZsKYqNul2TmeF f5ijG6JotXU8w== Date: Mon, 15 Apr 2024 18:38:33 -0700 Subject: [PATCH 10/17] xfs: implement live updates for parent pointer repairs From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029346.253068.16910470222703212133.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/parent_repair.c | 103 ++++++++++++++++++++++++++++++++++++++++-- fs/xfs/scrub/trace.h | 2 + 2 files changed, 100 insertions(+), 5 deletions(-) diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c index b4084a9f0e9c8..311bc7990d7c7 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. */ @@ -80,6 +86,9 @@ struct xrep_pptr { /* Length of the pptr name. */ uint8_t namelen; + + /* XREP_PPTR_{ADD,REMOVE} */ + uint8_t action; }; /* @@ -225,11 +234,25 @@ xrep_parent_replay_update( { struct xfs_scrub *sc = rp->sc; - /* Create parent pointer. */ - trace_xrep_parent_replay_parentadd(sc->tempip, xname, &pptr->pptr_rec); + switch (pptr->action) { + case XREP_PPTR_ADD: + /* Create parent pointer. */ + trace_xrep_parent_replay_parentadd(sc->tempip, xname, + &pptr->pptr_rec); - return xfs_parent_set(sc->tempip, sc->ip->i_ino, xname, - &pptr->pptr_rec, &rp->pptr_args); + return xfs_parent_set(sc->tempip, sc->ip->i_ino, xname, + &pptr->pptr_rec, &rp->pptr_args); + case XREP_PPTR_REMOVE: + /* Remove parent pointer. */ + trace_xrep_parent_replay_parentremove(sc->tempip, xname, + &pptr->pptr_rec); + + return xfs_parent_unset(sc->tempip, sc->ip->i_ino, xname, + &pptr->pptr_rec, &rp->pptr_args); + } + + ASSERT(0); + return -EIO; } /* @@ -290,6 +313,7 @@ xrep_parent_stash_parentadd( const struct xfs_inode *dp) { struct xrep_pptr pptr = { + .action = XREP_PPTR_ADD, .namelen = name->len, }; int error; @@ -304,6 +328,32 @@ 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, + }; + int error; + + trace_xrep_parent_stash_parentremove(rp->sc->tempip, dp, name); + + xfs_inode_to_parent_rec(&pptr.pptr_rec, dp); + error = xfblob_storename(rp->pptr_names, &pptr.name_cookie, name); + 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 @@ -513,6 +563,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.dhook.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( @@ -684,7 +776,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 10c2a8d10058b..3e0cd482379c6 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -2821,6 +2821,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, @@ -2856,6 +2857,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 Tue Apr 16 01:38:48 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: 13631063 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 A5417539C for ; Tue, 16 Apr 2024 01:38:49 +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=1713231529; cv=none; b=Nt9P30/Ev7CO3hiqgywpOmmL/POlSKAt4BpR4eC461+uWmZ/nIpwex2C0Yf5gj3Bgbjl64z+kU7F2sCCUfBIAo/K/2NmBwQDY8OJXwsCXz5JanI8xNhMr6ZagDl25mNPq8Go/6WDx2l87LRGYIIutkm/ZhT1AjO5HRut5UvHw+s= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231529; c=relaxed/simple; bh=w6Ud6u4itjPCpwyVUR2tWVspXLm6O0fKSYuVIzXDnR4=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=na8fIw4O+/cgmiBG+pDmOxMBexId1dWTHVKy8Wu+gbpaBhxhGky00es7wc5oD3c4u5DOtdO/0O1YbxuhHYsWSBAHtUg01w0hFQVoqnqVieDZS385rXw5UIYuraXqskopIA5rQwDIUt2BKJ/rINIXVjWFjGq+Nt6CCJ8cpWuUBho= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=GrvPfXvA; 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="GrvPfXvA" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2817EC113CC; Tue, 16 Apr 2024 01:38:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231529; bh=w6Ud6u4itjPCpwyVUR2tWVspXLm6O0fKSYuVIzXDnR4=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=GrvPfXvACkdU0FTJ3qlO0Tz1iSUX5MzcYt1GMl0skLo312yYjKJXZiXVxjGhsh69s NRhzBjZzwpkucNtt/b6N0wwJHv7IdMsr/3MLHGJsxGy6PE6OybkmdOOTDs8oD/poly QUaPIzAqDeIx6Z+I1jTTfmvjPYD+UeDhDqZ321yu2sXR2j5FewemGI+5zv2BqppMUu nRuTq1wwcPHlv5gx5H5gFGj0MFm3eOO09UmyE0edVhn63wpT+/sb8crGfh8LzP6R+i l7tqkh/gYClziWYckC5TXzCjoAAyzYPJnGeEne6gp0ZJ6OAUFXrOxEn1Mr3S1vNHMc BLGLuEofqjL+Q== Date: Mon, 15 Apr 2024 18:38:48 -0700 Subject: [PATCH 11/17] xfs: remove pointless unlocked assertion From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029362.253068.1784222365312393758.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Reviewed-by: Christoph Hellwig --- 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 59b8b9dc29ccf..55a64a72771f2 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -1041,8 +1041,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 Tue Apr 16 01:39:04 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: 13631064 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 E83226AB9 for ; Tue, 16 Apr 2024 01:39:04 +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=1713231545; cv=none; b=hh5dVjkPCw6/irzCDnl3ZPeMPv8EsLpEiVAHt4935WanJB7INGusGnSeJZF3BtJjD+hniY5IKbCuyobLn7oi3vInicDSfGvwDh/CFZsPVHTDyHpzXYD9Vztf75nu303SiQGT1b0KZRBnP/86Uf3drbiXYQ7JPNRQXm99VJVA6Cw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231545; c=relaxed/simple; bh=2F/EisuDIc6DUgsEiZwfGDyUBXgiKSCfvB1tOVnjy9A=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=aWu7UVVo6MtMuKHbX2QdBnODgEQJLfjXq9D/XYtnL0r+syBZl/DVLgf8i8SvwpU9gnAx0IHnFPrPMQG9q1jCqBIV002q4CCSPABfyja2Q053vhwoOj792r99d5DfErM8cK8xK0hwkgwOpsE/6wiEbOQDhRU8Eeq6wpRvO1OR4Ug= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=tDa85h+s; 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="tDa85h+s" Received: by smtp.kernel.org (Postfix) with ESMTPSA id C0A1EC113CC; Tue, 16 Apr 2024 01:39:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231544; bh=2F/EisuDIc6DUgsEiZwfGDyUBXgiKSCfvB1tOVnjy9A=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=tDa85h+scTgPjcbRxoowhUB2tki7rVRUReqEQF/zwnH7EPRbuANK4Dja4PRiz+naJ /akEVTwsXb+qjWRym3TaNwLMAWzHI26AuYZqA3SQoDy50Vwv/UBwA5n5aX0iJt/5XZ 64l34lbRWeXo+miuUf92nU4bUWAdlI8s/DsKB3bulc88Fp1jntxkFsKt+uPbox0w3g nBen8EGnBF0mwzBcJk2zg0yYceYxJ/ifWFAv7wFLpjRzf8/AJMwcqBRb06memyqq5q O/ftjrVbAH9O9FH5cbaKjd/dv2Q6ACNrlvT0REPg8edHhXmIKBLmp0l61f9DcP3iUU nG8QafRs9t2sA== Date: Mon, 15 Apr 2024 18:39:04 -0700 Subject: [PATCH 12/17] xfs: split xfs_bmap_add_attrfork into two pieces From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029378.253068.12251263442516159682.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Reviewed-by: Christoph Hellwig --- 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 b18f6c258a174..5afce91cd129f 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -948,6 +948,43 @@ xfs_attr_lookup( return error; } +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; +} + /* * Make a change to the xattr structure. * @@ -989,7 +1026,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 55a64a72771f2..b01d477d1cfbc 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -1025,38 +1025,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; + xfs_assert_ilocked(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; @@ -1077,7 +1068,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; @@ -1096,14 +1087,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 32fb2a455c294..e98849eb9bbae 100644 --- a/fs/xfs/libxfs/xfs_bmap.h +++ b/fs/xfs/libxfs/xfs_bmap.h @@ -176,7 +176,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 Tue Apr 16 01:39:19 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: 13631065 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 7B1754C98 for ; Tue, 16 Apr 2024 01:39:20 +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=1713231560; cv=none; b=Ogs2OpyToaSKZWScxqPGxP4Lg3njeuHeQH/o1EsfRLbpgz8hl2epK+niA9+OLQppgzg/4XTF+zwgKHhhaIV8nQVItq5JstISLVK+PeNDQQHWAZTrumQc2mX74GxI33ZM2qXELXM9eFTuaJggCnHfi7Jv903z66/bmUD3fPk+qJI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231560; c=relaxed/simple; bh=XHd2VRbwuAlDr/wegxPAl8xA9Jc1m9H6XB5oADDV8TA=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=sQgNF+s0j3Bxd1+JW/+64xDM7Gu3Fi8eo5w0VTb0+ZKsJmx+UI986qLpeoSmyRoD8eFtBpQzyDyKH1cx/Hrw1GEjKYHTxmNQmI7O5Yd9zndOC14z0hq9q8xtIxZmD+90SItiUkdaLDeIOf7iYtNxr4pMo7RJddJ4fNJhL/PIhhY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Sg0eGvQc; 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="Sg0eGvQc" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4F1DAC113CC; Tue, 16 Apr 2024 01:39:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231560; bh=XHd2VRbwuAlDr/wegxPAl8xA9Jc1m9H6XB5oADDV8TA=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=Sg0eGvQccvEYEV8rncS00BXepKDFD6Wy1a20Wn2oipV55/8TF9nxVj8ti2ErZk2m3 /ojetjJUTJpFPkjImpL4cHab5h201jbhVJUGUMHbPZQzd5wx0CmO8oFgtR729lHfVc gSevLwtud3eFwl43sLXq5L+whcfH3q3h+1+Rud3ok+j/HjbZC9sbp+GSzS4quSBYD9 XpqVcoo/FwrsIarjtnFJNSjHTeaRVNAam4A/VslhnzRjWa2yDc5/L9rGjeYF6s5d17 e3xbYRUNObagTB47gFKphDc0zqU6ZlFKqLOuvm++seLsEPM+8y6ViTOikMKDtCjbPx r+VFRt+GwFp4Q== Date: Mon, 15 Apr 2024 18:39:19 -0700 Subject: [PATCH 13/17] xfs: add a per-leaf block callback to xchk_xattr_walk From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029394.253068.10146384322966526219.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 second callback function to xchk_xattr_walk so that we can do something in between attr leaf blocks. This will be used by the next patch to see if we should flush cached parent pointer updates to constrain memory usage. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/attr.c | 2 +- fs/xfs/scrub/dir_repair.c | 2 +- fs/xfs/scrub/listxattr.c | 10 +++++++++- fs/xfs/scrub/listxattr.h | 4 +++- fs/xfs/scrub/nlinks.c | 3 ++- fs/xfs/scrub/parent.c | 7 ++++--- 6 files changed, 20 insertions(+), 8 deletions(-) diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c index b550f3e34ffc7..708334f9b2bd1 100644 --- a/fs/xfs/scrub/attr.c +++ b/fs/xfs/scrub/attr.c @@ -675,7 +675,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/dir_repair.c b/fs/xfs/scrub/dir_repair.c index 60e31da4451e8..e968150fe0f06 100644 --- a/fs/xfs/scrub/dir_repair.c +++ b/fs/xfs/scrub/dir_repair.c @@ -1288,7 +1288,7 @@ xrep_dir_scan_file( goto scan_done; } - error = xchk_xattr_walk(rd->sc, ip, xrep_dir_scan_pptr, rd); + error = xchk_xattr_walk(rd->sc, ip, xrep_dir_scan_pptr, NULL, rd); if (error) goto scan_done; diff --git a/fs/xfs/scrub/listxattr.c b/fs/xfs/scrub/listxattr.c index cbe5911ecbbcf..256ff7700c942 100644 --- a/fs/xfs/scrub/listxattr.c +++ b/fs/xfs/scrub/listxattr.c @@ -221,6 +221,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; @@ -252,6 +253,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)) { @@ -288,6 +295,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; @@ -308,5 +316,5 @@ 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); } diff --git a/fs/xfs/scrub/listxattr.h b/fs/xfs/scrub/listxattr.h index 48fe89d05946b..703cfb7b14cfd 100644 --- a/fs/xfs/scrub/listxattr.h +++ b/fs/xfs/scrub/listxattr.h @@ -11,7 +11,9 @@ 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); #endif /* __XFS_SCRUB_LISTXATTR_H__ */ diff --git a/fs/xfs/scrub/nlinks.c b/fs/xfs/scrub/nlinks.c index d27b32e6f33db..80aee30886c45 100644 --- a/fs/xfs/scrub/nlinks.c +++ b/fs/xfs/scrub/nlinks.c @@ -434,7 +434,8 @@ xchk_nlinks_collect_dir( goto out_unlock; } - error = xchk_xattr_walk(sc, dp, xchk_nlinks_collect_pptr, xnc); + error = xchk_xattr_walk(sc, dp, xchk_nlinks_collect_pptr, NULL, + xnc); if (error == -ECANCELED) { error = 0; goto out_unlock; diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c index 068691434be17..733c410a22797 100644 --- a/fs/xfs/scrub/parent.c +++ b/fs/xfs/scrub/parent.c @@ -317,7 +317,7 @@ xchk_parent_pptr_and_dotdot( return 0; /* Otherwise, walk the pptrs again, and check. */ - error = xchk_xattr_walk(sc, sc->ip, xchk_parent_scan_dotdot, pp); + error = xchk_xattr_walk(sc, sc->ip, xchk_parent_scan_dotdot, NULL, pp); if (error == -ECANCELED) { /* Found a parent pointer that matches dotdot. */ return 0; @@ -699,7 +699,8 @@ xchk_parent_count_pptrs( */ if (pp->need_revalidate) { pp->pptrs_found = 0; - error = xchk_xattr_walk(sc, sc->ip, xchk_parent_count_pptr, pp); + error = xchk_xattr_walk(sc, sc->ip, xchk_parent_count_pptr, + NULL, pp); if (error == -EFSCORRUPTED) { /* Found a bad parent pointer */ xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); @@ -758,7 +759,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; From patchwork Tue Apr 16 01:39:35 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: 13631066 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 464DA5227 for ; Tue, 16 Apr 2024 01:39:36 +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=1713231576; cv=none; b=OmdG5wlJTnH3YCjIMMoxElYKTb/JM9mWndaZiVIiDP2kRXAe8unuiHF9SGdc7fW4ejEME59P0VrVz8ltFJ+T/JCiNjd1XxwzqSK94f3OqYXiWnWEtfN0d0LnsIaFnTkToW6IuHtVo49qM8iAFoTcXRTySQhVclBAzN6sUK5Ylhs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231576; c=relaxed/simple; bh=sQMHpUX9X6xKizUr4WvsChz54p+G9mqv+JG+DIjz2no=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=YR0RtSvQWJi7CNcO3y4O1oJQuy9NmmKZEnd7VvkqSEOL51Ql6slPI+Sk5mtgsqqY7Rv9XTmoK14Tsdokl08lOWgxCFe8P/EQkodmtgMwPLkyBtb2L8aqnYZNYimf1S/ij4CzPJYZdJ79AOzqB7M8GSGP1lqGd7X5ek6ZOjRZu6U= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=kHJ/C4LS; 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="kHJ/C4LS" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0FD29C113CC; Tue, 16 Apr 2024 01:39:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231576; bh=sQMHpUX9X6xKizUr4WvsChz54p+G9mqv+JG+DIjz2no=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=kHJ/C4LSt8W5v26L4jDZRablpyyf/MMmdXPRq/vwQWRwdrz3kNJNBd7MCzzGnwaf5 /ku9SxRiesEomfe0JPEmY+RxGkq+XZl7Q+a8n9MIsTK/vK0V+3o26r2EbTOpJYTw7n Ue/YmBk1/xIvLnUsqxBWdqA9CsxhvCQuTqvNQeSt429aIcblNAv4siKtbdnWbTZsDu vNAVHTvtTHBuRvQVIm2Qkf4EHBeM07edSPrkrV7qOCE5B/PWHD6aXX0wuo7CZV1I+X 5si82JT5ZyAHtAn4l6w+C0qF92QfY2d9kGHt+/oo273J5dyKrnlPcgBZ2Hh7ykxbAo i82W7AOPAIdbQ== Date: Mon, 15 Apr 2024 18:39:35 -0700 Subject: [PATCH 14/17] xfs: actually rebuild the parent pointer xattrs From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029411.253068.9438512611837531447.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/libxfs/xfs_attr.c | 2 fs/xfs/libxfs/xfs_attr.h | 1 fs/xfs/scrub/attr_repair.c | 4 fs/xfs/scrub/attr_repair.h | 4 fs/xfs/scrub/findparent.c | 2 fs/xfs/scrub/parent_repair.c | 709 +++++++++++++++++++++++++++++++++++++++++- fs/xfs/scrub/trace.h | 2 7 files changed, 701 insertions(+), 23 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 5afce91cd129f..899c07c5b75f8 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -948,7 +948,7 @@ xfs_attr_lookup( return error; } -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 43dee4cbaab25..088cb7b301680 100644 --- a/fs/xfs/libxfs/xfs_attr.h +++ b/fs/xfs/libxfs/xfs_attr.h @@ -648,5 +648,6 @@ int __init xfs_attr_intent_init_cache(void); void xfs_attr_intent_destroy_cache(void); int xfs_attr_sf_totsize(struct xfs_inode *dp); +int xfs_attr_add_fork(struct xfs_inode *ip, int size, int rsvd); #endif /* __XFS_ATTR_H__ */ diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c index 87f6c9cb54eb6..e059813b92b70 100644 --- a/fs/xfs/scrub/attr_repair.c +++ b/fs/xfs/scrub/attr_repair.c @@ -1030,7 +1030,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) { @@ -1336,7 +1336,7 @@ xrep_xattr_swap_prep( } /* Exchange the temporary file's attribute fork with the one being repaired. */ -STATIC int +int xrep_xattr_swap( struct xfs_scrub *sc, struct xrep_tempexch *tx) diff --git a/fs/xfs/scrub/attr_repair.h b/fs/xfs/scrub/attr_repair.h index 0a9ffa7cfa906..979729bd4a5f8 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_tempexch; + +int xrep_xattr_swap(struct xfs_scrub *sc, struct xrep_tempexch *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 c78422ad757bf..01766041ba2cd 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_exchmaps.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/parent_repair.c b/fs/xfs/scrub/parent_repair.c index 311bc7990d7c7..28e9746c06631 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_exchmaps.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/tempexch.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 @@ -65,9 +70,9 @@ * * 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. + * the temporary file's extended attributes. An atomic file mapping exchange + * is used to commit the new xattr blocks to the file being repaired. This + * will disrupt attrmulti cursors. */ /* Create a parent pointer in the tempfile. */ @@ -106,6 +111,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 exchange the attr fork mappings, if the fs + * supports parent pointers. + */ + struct xrep_tempexch tx; + /* * Information used to scan the filesystem to find the inumber of the * dotdot entry for this directory. On filesystems without parent @@ -117,6 +139,8 @@ 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; @@ -129,14 +153,48 @@ struct xrep_parent { /* Scratch buffer for scanning pptr xattrs */ struct xfs_da_args pptr_args; + + /* Have we seen any live updates of parent pointers recently? */ + bool saw_pptr_updates; }; +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; @@ -556,10 +614,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; } @@ -594,6 +653,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; @@ -648,6 +709,55 @@ 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, + unsigned int attr_flags, + const unsigned char *name, + unsigned int namelen, + const void *value, + unsigned int valuelen, + void *priv) +{ + xfs_ino_t *inop = priv; + xfs_ino_t parent_ino; + int error; + + if (!(attr_flags & XFS_ATTR_PARENT)) + return 0; + + error = xfs_parent_from_attr(sc->mp, attr_flags, name, namelen, value, + valuelen, &parent_ino, NULL); + if (error) + return error; + + *inop = parent_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) +{ + int error; + + *inop = NULLFSINO; + + error = xchk_xattr_walk(sc, sc->ip, xrep_parent_lookup_pptr, NULL, + inop); + if (error && error != -ECANCELED) + return error; + return 0; +} + /* * Move the current file to the orphanage. * @@ -664,14 +774,26 @@ 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 committed the new + * xattr structure (and hence the new parent pointer records), + * 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. @@ -704,9 +826,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; @@ -733,6 +860,488 @@ 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 = { + .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, + .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; + xfs_attr_sethash(&args); + 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, + .owner = rp->sc->ip->i_ino, + .geo = rp->sc->mp->m_attr_geo, + .whichfork = XFS_ATTR_FORK, + .op_flags = XFS_DA_OP_OKNOENT, + }; + 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); + + xfs_attr_sethash(&args); + return xfs_attr_set(&args, XFS_ATTRUPDATE_UPSERT, false); +} + +/* + * 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 exchanging 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 exchange 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 + * exchanging extent mappings, 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 + * exchanging 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_tempexch_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 exchange the attr fork contents 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 exchange the attr fork mappings. + */ + 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; + + /* + * Exchange the attr fork contents 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. @@ -741,13 +1350,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. */ @@ -757,18 +1377,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); @@ -776,19 +1417,47 @@ xrep_parent_setup_scan( if (error) goto out_recs; + /* Set up some storage for copying attrs before the mapping exchange */ + 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; } @@ -818,7 +1487,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 3e0cd482379c6..ecfaa4b88910f 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -2539,6 +2539,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 Tue Apr 16 01:39:51 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: 13631067 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 E678E6AB9 for ; Tue, 16 Apr 2024 01:39:51 +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=1713231592; cv=none; b=LQRgo9ybp0sgn7f1pG6UY272d+XpM1CdQJk98ejixo2hBU+u2c6ZFjKJUi1hQYVM7aF8ft8HRFM/aCCKaUOzPDrpwkmGZIDyDZegMamZ5yrZ2BtZEJL5KP/bjfBJMuwRcYtkPNQ4vj6sSx/+OqsPLPWoMsaE9c03CC0lSn4+8vo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231592; c=relaxed/simple; bh=1fBwwpPeAMkgpISWM/e1oUSJlZrc987LpxKoEc6AI6g=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=r+tAR+EoL1LUZMDTfYNKWb8h7V5vCVkN+3r51EnG9IDS1HQEIjgNOjehdPjxNl6vQFeXzkatLshLHSZBhZzHRAM0L8RskEQoNerccEgJS9+ydHl/nE+r7ekm+9UZ/qaCcfIzRat1fpm3V/gpGrvaMe+2Jh9dA2bJeKl7ayg28OY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=ajn3+Tqz; 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="ajn3+Tqz" Received: by smtp.kernel.org (Postfix) with ESMTPSA id BD643C113CC; Tue, 16 Apr 2024 01:39:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231591; bh=1fBwwpPeAMkgpISWM/e1oUSJlZrc987LpxKoEc6AI6g=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=ajn3+TqzTeYPrtMoD8BSNdqg6EFAWvx5IJUN4iM7qg4lJ/7lXPk0/TZlJimtXP0fw IzO3Q9jTCgmgJ/p1/IhCkdJdiSRrzbOqjvLyI+bRSmqRf6zdnzIMqcx04ouUl1Ogwg e5sXjnhNgNpWYc8PYwzhOVpUeYm9MS/8c0PI3zuJEpEWXpZu86bYecWWgCMneHdkoO E/SKwYKxCDSDrF0tQCG0apQJNR8mc9oFN/YDB2c+1if3KgXSZQ0bAEacqXmCsmjE6j tlWTVm2FGty4ipq2HdTe5nnhRhNo5DUJOWbZbXz58mXMiF6ICcDKzGR32L3vcvfRNB XU41gBBkAX8iA== Date: Mon, 15 Apr 2024 18:39:51 -0700 Subject: [PATCH 15/17] xfs: adapt the orphanage code to handle parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029428.253068.10744719763665946711.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/orphanage.c | 38 ++++++++++++++++++++++++++++++++++++++ fs/xfs/scrub/orphanage.h | 3 +++ fs/xfs/scrub/scrub.c | 2 ++ 3 files changed, 43 insertions(+) diff --git a/fs/xfs/scrub/orphanage.c b/fs/xfs/scrub/orphanage.c index 94bcc2799188f..b2f905924d0d8 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,6 +332,8 @@ 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; /* @@ -503,6 +507,19 @@ 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) +{ + return sizeof(struct xfs_attr_sf_hdr) + + xfs_attr_sf_entsize_byname(sizeof(struct xfs_parent_rec), + adopt->xname->len); +} + /* * Move the current file to the orphanage under the computed name. * @@ -524,6 +541,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, adopt->xname, sc->ip->i_ino, adopt->orphanage_blkres); @@ -548,6 +578,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, adopt->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 319179ab788d3..beb6b686784e6 100644 --- a/fs/xfs/scrub/orphanage.h +++ b/fs/xfs/scrub/orphanage.h @@ -54,6 +54,9 @@ struct xrep_adoption { /* Name used for the adoption. */ struct xfs_name *xname; + /* 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 ebb06838c31be..7b1f1abdc7a98 100644 --- a/fs/xfs/scrub/scrub.c +++ b/fs/xfs/scrub/scrub.c @@ -19,6 +19,8 @@ #include "xfs_rmap.h" #include "xfs_exchrange.h" #include "xfs_exchmaps.h" +#include "xfs_dir2.h" +#include "xfs_parent.h" #include "scrub/scrub.h" #include "scrub/common.h" #include "scrub/trace.h" From patchwork Tue Apr 16 01:40:06 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: 13631068 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 9039C5227 for ; Tue, 16 Apr 2024 01:40: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=1713231607; cv=none; b=cpA5JPS4NqFd1apGgpITBVFeLsaXYbPBt3dJebpLNcn4UyTXT3xWHEeHVIwBhQAzPBeN15nRehHCpXq10rHAA57roxegkA2vxm9l3d5q0ZG/ierCax/C+k5zRQzSinDjzrvbX5fdAMlioQudy2/yGT1gtKpWv8YPTkX+kk3uv7M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231607; c=relaxed/simple; bh=H9JBaeYSF53k+WFhJH2TaCJLr4+/GzTJbreljwkOfeM=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=IMCqWvWQtkyXFA8KF3qlX2mhb3D3+5MaKNnIIL1Go/ORyJ7S58WOS26OLa52suxiUdthUDEXIAyW6LSjrm15yFNDruMXSzT9MeASwpJrgtOuPdoAYR9lXczf7fSmhJhyMV8ANvWWVPNDxxFflokdhWzXZjhMddRXzAfmBcTu2nc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=KxmJdGwn; 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="KxmJdGwn" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 674F2C113CC; Tue, 16 Apr 2024 01:40:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231607; bh=H9JBaeYSF53k+WFhJH2TaCJLr4+/GzTJbreljwkOfeM=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=KxmJdGwnU08o59BkGy8yoWXQMBLJ28Uq+wbuCUFHQYw9YGfKOD9WHu9za0AILCZF6 y5fzRIlY6MjMRrOWuCRrCpWrqZ2T5K5XpWxfa3+vaph0opHYAhJpVyiwNxPuTGv1i7 o2XlvkfWI+eXICMcpa/mSJe9ilThn/mlbsAD9pP/CBkogwHIeyFcNNx/YTqc+ZGxgD YfgPsJ12IxJtBMWbPiPOV3pPhhxUkLBsI5XTfnYiks2BEkDAOCyNi+GVPOsFFgLjEU B5C7W7Y+xxJHasaM8U5zq95Iyim0Z8yqE/Nkk2Y+6H/lk5n017MnMlN6JeFw+CjIvp hsJImtm4SwklQ== Date: Mon, 15 Apr 2024 18:40:06 -0700 Subject: [PATCH 16/17] xfs: repair link count of nondirectories after rebuilding parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029443.253068.2993570753880366430.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Reviewed-by: Christoph Hellwig --- fs/xfs/scrub/parent_repair.c | 107 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c index 28e9746c06631..ee88ce5a12b83 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" @@ -156,6 +157,9 @@ struct xrep_parent { /* Have we seen any live updates of parent pointers recently? */ bool saw_pptr_updates; + + /* Number of parents we found after all other repairs */ + unsigned long long parents; }; struct xrep_parent_xattr { @@ -1370,6 +1374,102 @@ 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, + unsigned int attr_flags, + const unsigned char *name, + unsigned int namelen, + const void *value, + unsigned int valuelen, + void *priv) +{ + struct xrep_parent *rp = priv; + int error; + + if (!(attr_flags & XFS_ATTR_PARENT)) + return 0; + + error = xfs_parent_from_attr(sc->mp, attr_flags, name, namelen, value, + valuelen, NULL, NULL); + if (error) + return error; + + 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_xattr_walk(sc, ip, xrep_parent_count_pptr, NULL, 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( @@ -1494,6 +1594,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 Tue Apr 16 01:40: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: 13631069 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 43BC5539C for ; Tue, 16 Apr 2024 01:40: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=1713231623; cv=none; b=naoVOmp/XCFXHyY6aB6QssxmzJ3L4SZ4uEmnHVI80NTxbeJzVmD66JK30icM9ntBsFVN/WahOAW6EzP6Y4ImuDC62bwg/OhOlTeUKPJxqo2PFqH/k/Ca5GeWBVJ4BweJ/qHZU8KdyBNlvWy5f8XshJ8sZHcZmEumMTe4ksN5QsA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713231623; c=relaxed/simple; bh=SJzOxMXrkZLLXLjdKF3/XHh1uC3NeCg9KX6bnlxvq7U=; h=Date:Subject:From:To:Cc:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=OuAyhWnsLmmmiGpDmBpiiTSv/3Q5MssCTe+VRVL/O/E7nHOmxmUoanZai7eD4cJtD6syo5NCgQPiOp2fHiU+ftQ9L6z6MPKjA3MaR9mzij2QkLAzvIHME147Uvf3xddDP62NG4OakDrxfGuaVY+RmROZwyRWm4LejLzAATbcBsg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=WZS5zGWd; 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="WZS5zGWd" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1467EC113CC; Tue, 16 Apr 2024 01:40:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713231623; bh=SJzOxMXrkZLLXLjdKF3/XHh1uC3NeCg9KX6bnlxvq7U=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=WZS5zGWdtd4eQ4pG2vTrGhpuFsSufNW8rcz2/yN8LuUEsvOAfdF9tosQ0VHA44SNw TMBwuQhKHRmULHfDtBEpLaKb1KF/A3BYyzglcVLhq88wSR+tgr52IK6J64alJDQbiT cbkCEBUMB1f6usHha/w5nmz0xoWGdS6q/LIeVS5UqAsa/bnig2GoCxETYmb0EUU7RB ZdgEtB/UY2MGA7Pi/1uj9sQidwBmWb6KR3y+X1G+bj5wciFcad5rNMS3O8Ty9lHjdu yUMoyFW/Z5kOCs67wtyUWjbJ9TgtEtls75Wd3QtaYhdV9dx7mSUe+hPIB88uQq4ldM W0TFKLV7JHDSQ== Date: Mon, 15 Apr 2024 18:40:22 -0700 Subject: [PATCH 17/17] xfs: inode repair should ensure there's an attr fork to store parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: Christoph Hellwig , allison.henderson@oracle.com, hch@infradead.org, linux-xfs@vger.kernel.org, catherine.hoang@oracle.com, hch@lst.de Message-ID: <171323029459.253068.17231638937736352624.stgit@frogsfrogsfrogs> In-Reply-To: <171323029141.253068.12138115574003345390.stgit@frogsfrogsfrogs> References: <171323029141.253068.12138115574003345390.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 Reviewed-by: Christoph Hellwig --- 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 e3b74ea50fdef..daf9f1ee7c2cb 100644 --- a/fs/xfs/scrub/inode_repair.c +++ b/fs/xfs/scrub/inode_repair.c @@ -1736,6 +1736,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( @@ -1744,6 +1782,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);