diff mbox series

[10/18] libxfs: introduce libxfs_buf_read_uncached

Message ID 158216301746.602314.17789861786273491972.stgit@magnolia (mailing list archive)
State Superseded, archived
Headers show
Series xfsprogs: refactor buffer function names | expand

Commit Message

Darrick J. Wong Feb. 20, 2020, 1:43 a.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

Introduce an uncached read function so that userspace can handle them in
the same way as the kernel.  This also eliminates the need for some of
the libxfs_purgebuf calls (and two trips into the cache code).

Refactor the get/read uncached buffer functions to hide the details of
uncached buffer-ism in rdwr.c.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 copy/xfs_copy.c          |   13 +++++++--
 db/init.c                |    9 +++---
 libxfs/libxfs_api_defs.h |    2 +
 libxfs/libxfs_io.h       |   22 +++------------
 libxfs/rdwr.c            |   67 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 88 insertions(+), 25 deletions(-)

Comments

Christoph Hellwig Feb. 21, 2020, 2:48 p.m. UTC | #1 
On Wed, Feb 19, 2020 at 05:43:37PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Introduce an uncached read function so that userspace can handle them in
> the same way as the kernel.  This also eliminates the need for some of
> the libxfs_purgebuf calls (and two trips into the cache code).
> 
> Refactor the get/read uncached buffer functions to hide the details of
> uncached buffer-ism in rdwr.c.

Split this into one patch adding the functionality to libxfs and
one each to convert db and copy over with a good rationale for the
switch in each case?
Darrick J. Wong Feb. 21, 2020, 8:50 p.m. UTC | #2 
On Fri, Feb 21, 2020 at 06:48:33AM -0800, Christoph Hellwig wrote:
> On Wed, Feb 19, 2020 at 05:43:37PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Introduce an uncached read function so that userspace can handle them in
> > the same way as the kernel.  This also eliminates the need for some of
> > the libxfs_purgebuf calls (and two trips into the cache code).
> > 
> > Refactor the get/read uncached buffer functions to hide the details of
> > uncached buffer-ism in rdwr.c.
> 
> Split this into one patch adding the functionality to libxfs and
> one each to convert db and copy over with a good rationale for the
> switch in each case?

Both programs use the uncached read for the same purpose, which is to
read the superblock without polluting the buffer cache when we haven't
yet established the filesystem sector size.

The only reason why repair doesn't need patching is that reaches into
the buffer cache internals to call lseek and read by hand.  I guess that
should be fixed, separately.

--D
Darrick J. Wong Feb. 21, 2020, 8:59 p.m. UTC | #3 
On Fri, Feb 21, 2020 at 12:50:28PM -0800, Darrick J. Wong wrote:
> On Fri, Feb 21, 2020 at 06:48:33AM -0800, Christoph Hellwig wrote:
> > On Wed, Feb 19, 2020 at 05:43:37PM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > Introduce an uncached read function so that userspace can handle them in
> > > the same way as the kernel.  This also eliminates the need for some of
> > > the libxfs_purgebuf calls (and two trips into the cache code).
> > > 
> > > Refactor the get/read uncached buffer functions to hide the details of
> > > uncached buffer-ism in rdwr.c.
> > 
> > Split this into one patch adding the functionality to libxfs and
> > one each to convert db and copy over with a good rationale for the
> > switch in each case?
> 
> Both programs use the uncached read for the same purpose, which is to
> read the superblock without polluting the buffer cache when we haven't
> yet established the filesystem sector size.

HAH nope, despite the very similiar code, they do it for different
reasons.  Separate patches it is.

--D

> The only reason why repair doesn't need patching is that reaches into
> the buffer cache internals to call lseek and read by hand.  I guess that
> should be fixed, separately.
> 
> --D
diff mbox series

Patch

diff --git a/copy/xfs_copy.c b/copy/xfs_copy.c
index bb3ecd43..75c39407 100644
--- a/copy/xfs_copy.c
+++ b/copy/xfs_copy.c
@@ -562,6 +562,7 @@  main(int argc, char **argv)
 	libxfs_init_t	xargs;
 	thread_args	*tcarg;
 	struct stat	statbuf;
+	int		error;
 
 	progname = basename(argv[0]);
 
@@ -710,14 +711,20 @@  main(int argc, char **argv)
 
 	/* We don't yet know the sector size, so read maximal size */
 	libxfs_buftarg_init(&mbuf, xargs.ddev, xargs.logdev, xargs.rtdev);
-	sbp = libxfs_buf_read(mbuf.m_ddev_targp, XFS_SB_DADDR,
-			     1 << (XFS_MAX_SECTORSIZE_LOG - BBSHIFT), 0, NULL);
+	error = -libxfs_buf_read_uncached(mbuf.m_ddev_targp, XFS_SB_DADDR,
+			1 << (XFS_MAX_SECTORSIZE_LOG - BBSHIFT), 0, &sbp, NULL);
+	if (error) {
+		do_log(_("%s: couldn't read superblock, error=%d\n"),
+				progname, error);
+		exit(1);
+	}
+
 	sb = &mbuf.m_sb;
 	libxfs_sb_from_disk(sb, XFS_BUF_TO_SBP(sbp));
 
 	/* Do it again, now with proper length and verifier */
 	libxfs_buf_relse(sbp);
-	libxfs_purgebuf(sbp);
+
 	sbp = libxfs_buf_read(mbuf.m_ddev_targp, XFS_SB_DADDR,
 			     1 << (sb->sb_sectlog - BBSHIFT),
 			     0, &xfs_sb_buf_ops);
diff --git a/db/init.c b/db/init.c
index acab349c..eb53d672 100644
--- a/db/init.c
+++ b/db/init.c
@@ -47,6 +47,7 @@  init(
 	struct xfs_buf	*bp;
 	unsigned int	agcount;
 	int		c;
+	int		error;
 
 	setlocale(LC_ALL, "");
 	bindtextdomain(PACKAGE, LOCALEDIR);
@@ -112,10 +113,9 @@  init(
 	 */
 	memset(&xmount, 0, sizeof(struct xfs_mount));
 	libxfs_buftarg_init(&xmount, x.ddev, x.logdev, x.rtdev);
-	bp = libxfs_buf_read(xmount.m_ddev_targp, XFS_SB_DADDR,
-			    1 << (XFS_MAX_SECTORSIZE_LOG - BBSHIFT), 0, NULL);
-
-	if (!bp || bp->b_error) {
+	error = -libxfs_buf_read_uncached(xmount.m_ddev_targp, XFS_SB_DADDR,
+			1 << (XFS_MAX_SECTORSIZE_LOG - BBSHIFT), 0, &bp, NULL);
+	if (error) {
 		fprintf(stderr, _("%s: %s is invalid (cannot read first 512 "
 			"bytes)\n"), progname, fsdevice);
 		exit(1);
@@ -124,7 +124,6 @@  init(
 	/* copy SB from buffer to in-core, converting architecture as we go */
 	libxfs_sb_from_disk(&xmount.m_sb, XFS_BUF_TO_SBP(bp));
 	libxfs_buf_relse(bp);
-	libxfs_purgebuf(bp);
 
 	sbp = &xmount.m_sb;
 	if (sbp->sb_magicnum != XFS_SB_MAGIC) {
diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index df267c98..1149e301 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -45,7 +45,9 @@ 
 #define xfs_btree_init_block		libxfs_btree_init_block
 #define xfs_buf_delwri_submit		libxfs_buf_delwri_submit
 #define xfs_buf_get			libxfs_buf_get
+#define xfs_buf_get_uncached		libxfs_buf_get_uncached
 #define xfs_buf_read			libxfs_buf_read
+#define xfs_buf_read_uncached		libxfs_buf_read_uncached
 #define xfs_buf_relse			libxfs_buf_relse
 #define xfs_bunmapi			libxfs_bunmapi
 #define xfs_bwrite			libxfs_bwrite
diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index b01f2896..546b7710 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -257,23 +257,11 @@  xfs_buf_associate_memory(struct xfs_buf *bp, void *mem, size_t len)
 	return 0;
 }
 
-/*
- * Allocate an uncached buffer that points nowhere.  The refcount will be 1,
- * and the cache node hash list will be empty to indicate that it's uncached.
- */
-static inline struct xfs_buf *
-xfs_buf_get_uncached(struct xfs_buftarg *targ, size_t bblen, int flags)
-{
-	struct xfs_buf	*bp;
-
-	bp = libxfs_getbufr(targ, XFS_BUF_DADDR_NULL, bblen);
-	if (!bp)
-		return NULL;
-
-	INIT_LIST_HEAD(&bp->b_node.cn_hash);
-	bp->b_node.cn_count = 1;
-	return bp;
-}
+struct xfs_buf *libxfs_buf_get_uncached(struct xfs_buftarg *targ, size_t bblen,
+		int flags);
+int libxfs_buf_read_uncached(struct xfs_buftarg *targ, xfs_daddr_t daddr,
+		size_t bblen, int flags, struct xfs_buf **bpp,
+		const struct xfs_buf_ops *ops);
 
 /* Push a single buffer on a delwri queue. */
 static inline void
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 51f93c3e..ada20dd9 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -1070,6 +1070,73 @@  libxfs_readbuf_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
 	return bp;
 }
 
+/* Allocate a raw uncached buffer. */
+static inline struct xfs_buf *
+libxfs_getbufr_uncached(
+	struct xfs_buftarg	*targ,
+	xfs_daddr_t		daddr,
+	size_t			bblen)
+{
+	struct xfs_buf		*bp;
+
+	bp = libxfs_getbufr(targ, daddr, bblen);
+	if (!bp)
+		return NULL;
+
+	INIT_LIST_HEAD(&bp->b_node.cn_hash);
+	bp->b_node.cn_count = 1;
+	return bp;
+}
+
+/*
+ * Allocate an uncached buffer that points nowhere.  The refcount will be 1,
+ * and the cache node hash list will be empty to indicate that it's uncached.
+ */
+struct xfs_buf *
+libxfs_buf_get_uncached(
+	struct xfs_buftarg	*targ,
+	size_t			bblen,
+	int			flags)
+{
+	return libxfs_getbufr_uncached(targ, XFS_BUF_DADDR_NULL, bblen);
+}
+
+/*
+ * Allocate and read an uncached buffer.  The refcount will be 1, and the cache
+ * node hash list will be empty to indicate that it's uncached.
+ */
+int
+libxfs_buf_read_uncached(
+	struct xfs_buftarg	*targ,
+	xfs_daddr_t		daddr,
+	size_t			bblen,
+	int			flags,
+	struct xfs_buf		**bpp,
+	const struct xfs_buf_ops *ops)
+{
+	struct xfs_buf		*bp;
+	int			error;
+
+	*bpp = NULL;
+	bp = libxfs_getbufr_uncached(targ, daddr, bblen);
+	if (!bp)
+		return -ENOMEM;
+
+	error = libxfs_readbufr(targ, daddr, bp, bblen, flags);
+	if (error)
+		goto err;
+
+	error = libxfs_readbuf_verify(bp, ops);
+	if (error)
+		goto err;
+
+	*bpp = bp;
+	return 0;
+err:
+	libxfs_buf_relse(bp);
+	return error;
+}
+
 static int
 __write_buf(int fd, void *buf, int len, off64_t offset, int flags)
 {