Linux-XFS Archive mirror
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: djwong@kernel.org
Cc: hch@infradead.org, linux-xfs@vger.kernel.org, hch@lst.de
Subject: [PATCH 2/2] xfs: only iget the file once when doing vectored scrub-by-handle
Date: Wed, 17 Apr 2024 16:14:07 -0700	[thread overview]
Message-ID: <171339555598.1999874.4695876291132839484.stgit@frogsfrogsfrogs> (raw)
In-Reply-To: <171339555559.1999874.4456227116424200314.stgit@frogsfrogsfrogs>

From: Darrick J. Wong <djwong@kernel.org>

If a program wants us to perform a scrub on a file handle and the fd
passed to ioctl() is not the file referenced in the handle, iget the
file once and pass it into the scrub code.  This amortizes the untrusted
iget lookup over /all/ the scrubbers mentioned in the scrubv call.

When running fstests in "rebuild all metadata after each test" mode, I
observed a 10% reduction in runtime on account of avoiding repeated
inobt lookups.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/scrub/scrub.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)


diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c
index 78b00ab85c9c9..43af5ce1f99f0 100644
--- a/fs/xfs/scrub/scrub.c
+++ b/fs/xfs/scrub/scrub.c
@@ -792,6 +792,37 @@ xfs_scrubv_check_barrier(
 	return 0;
 }
 
+/*
+ * If the caller provided us with a nonzero inode number that isn't the ioctl
+ * file, try to grab a reference to it to eliminate all further untrusted inode
+ * lookups.  If we can't get the inode, let each scrub function try again.
+ */
+STATIC struct xfs_inode *
+xchk_scrubv_open_by_handle(
+	struct xfs_mount		*mp,
+	const struct xfs_scrub_vec_head	*head)
+{
+	struct xfs_trans		*tp;
+	struct xfs_inode		*ip;
+	int				error;
+
+	error = xfs_trans_alloc_empty(mp, &tp);
+	if (error)
+		return NULL;
+
+	error = xfs_iget(mp, tp, head->svh_ino, XCHK_IGET_FLAGS, 0, &ip);
+	xfs_trans_cancel(tp);
+	if (error)
+		return NULL;
+
+	if (VFS_I(ip)->i_generation != head->svh_gen) {
+		xfs_irele(ip);
+		return NULL;
+	}
+
+	return ip;
+}
+
 /* Vectored scrub implementation to reduce ioctl calls. */
 int
 xfs_ioc_scrubv_metadata(
@@ -804,6 +835,7 @@ xfs_ioc_scrubv_metadata(
 	struct xfs_scrub_vec		__user *uvectors;
 	struct xfs_inode		*ip_in = XFS_I(file_inode(file));
 	struct xfs_mount		*mp = ip_in->i_mount;
+	struct xfs_inode		*handle_ip = NULL;
 	struct xfs_scrub_vec		*v;
 	size_t				vec_bytes;
 	unsigned int			i;
@@ -848,6 +880,17 @@ xfs_ioc_scrubv_metadata(
 		trace_xchk_scrubv_item(mp, &head, i, v);
 	}
 
+	/*
+	 * If the caller wants us to do a scrub-by-handle and the file used to
+	 * call the ioctl is not the same file, load the incore inode and pin
+	 * it across all the scrubv actions to avoid repeated UNTRUSTED
+	 * lookups.  The reference is not passed to deeper layers of scrub
+	 * because each scrubber gets to decide its own strategy and return
+	 * values for getting an inode.
+	 */
+	if (head.svh_ino && head.svh_ino != ip_in->i_ino)
+		handle_ip = xchk_scrubv_open_by_handle(mp, &head);
+
 	/* Run all the scrubbers. */
 	for (i = 0, v = vectors; i < head.svh_nr; i++, v++) {
 		struct xfs_scrub_metadata	sm = {
@@ -895,6 +938,8 @@ xfs_ioc_scrubv_metadata(
 	}
 
 out_free:
+	if (handle_ip)
+		xfs_irele(handle_ip);
 	kfree(vectors);
 	return error;
 }


  parent reply	other threads:[~2024-04-17 23:14 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-17 23:10 [PATCHBOMB v13.3] xfs: tweaks and fixes to online repair, part 2 Darrick J. Wong
2024-04-17 23:13 ` [PATCHSET v13.3 1/2] xfs: reduce iget overhead in scrub Darrick J. Wong
2024-04-17 23:13   ` [PATCH 1/2] xfs: use dontcache for grabbing inodes during scrub Darrick J. Wong
2024-04-18  4:22     ` Christoph Hellwig
2024-04-17 23:14   ` Darrick J. Wong [this message]
2024-04-18  4:22     ` [PATCH 2/2] xfs: only iget the file once when doing vectored scrub-by-handle Christoph Hellwig
2024-04-17 23:13 ` [PATCHSET v13.3 2/2] xfs: minor fixes to online repair Darrick J. Wong
2024-04-17 23:14   ` [PATCH 1/4] xfs: drop the scrub file's iolock when transaction allocation fails Darrick J. Wong
2024-04-18  4:23     ` Christoph Hellwig
2024-04-17 23:14   ` [PATCH 2/4] xfs: fix iunlock calls in xrep_adoption_trans_alloc Darrick J. Wong
2024-04-18  4:23     ` Christoph Hellwig
2024-04-17 23:14   ` [PATCH 3/4] xfs: exchange-range for repairs is no longer dynamic Darrick J. Wong
2024-04-18  4:24     ` Christoph Hellwig
2024-04-17 23:15   ` [PATCH 4/4] xfs: invalidate dentries for a file before moving it to the orphanage Darrick J. Wong
2024-04-18  4:25     ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2024-04-24  3:07 [PATCHSET v13.4 8/9] xfs: reduce iget overhead in scrub Darrick J. Wong
2024-04-24  3:28 ` [PATCH 2/2] xfs: only iget the file once when doing vectored scrub-by-handle Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=171339555598.1999874.4695876291132839484.stgit@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=hch@infradead.org \
    --cc=hch@lst.de \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).