Re: [PATCH 9/9] libfrog: refactor open-coded INUMBERS calls

Allison Collins <allison.henderson@xxxxxxxxxx> · Sun, 1 Sep 2019 13:55:30 -0700

On 8/29/19 9:21 PM, Darrick J. Wong wrote:
From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>

Refactor all the INUMBERS ioctl callsites into helper functions.

Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
Looks good!  I think it looks a lot cleaner.  Thanks!

Reviewed-by: Allison Collins <allison.henderson@xxxxxxxxxx>

---
  include/xfrog.h    |    4 ++++
  io/imap.c          |   33 ++++++++++++++++-----------------
  io/open.c          |   24 ++++++++++++------------
  libfrog/bulkstat.c |   26 ++++++++++++++++++++++++++
  scrub/fscounters.c |   20 ++++++++------------
  scrub/inodes.c     |   17 ++++++-----------
  6 files changed, 72 insertions(+), 52 deletions(-)

diff --git a/include/xfrog.h b/include/xfrog.h
index 3a49acc3..d16481ce 100644
--- a/include/xfrog.h
+++ b/include/xfrog.h
@@ -116,4 +116,8 @@ int xfrog_bulkstat_single(struct xfs_fd *xfd, uint64_t ino,
  int xfrog_bulkstat(struct xfs_fd *xfd, uint64_t *lastino, uint32_t icount,
  		struct xfs_bstat *ubuffer, uint32_t *ocount);
  
+struct xfs_inogrp;
+int xfrog_inumbers(struct xfs_fd *xfd, uint64_t *lastino, uint32_t icount,
+		struct xfs_inogrp *ubuffer, uint32_t *ocount);
+
  #endif	/* __XFROG_H__ */
diff --git a/io/imap.c b/io/imap.c
index 9667289a..053ac28e 100644
--- a/io/imap.c
+++ b/io/imap.c
@@ -8,18 +8,20 @@
  #include "input.h"
  #include "init.h"
  #include "io.h"
+#include "xfrog.h"
  
  static cmdinfo_t imap_cmd;
  
  static int
  imap_f(int argc, char **argv)
  {
-	int		count;
-	int		nent;
-	int		i;
-	__u64		last = 0;
-	struct xfs_inogrp *t;
-	struct xfs_fsop_bulkreq bulkreq;
+	struct xfs_fd		xfd = XFS_FD_INIT(file->fd);
+	struct xfs_inogrp	*t;
+	uint64_t		last = 0;
+	uint32_t		count;
+	uint32_t		nent;
+	int			i;
+	int			error;
  
  	if (argc != 2)
  		nent = 1;
@@ -30,14 +32,8 @@ imap_f(int argc, char **argv)
  	if (!t)
  		return 0;
  
-	bulkreq.lastip  = &last;
-	bulkreq.icount  = nent;
-	bulkreq.ubuffer = (void *)t;
-	bulkreq.ocount  = &count;
-
-	while (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS, &bulkreq) == 0) {
-		if (count == 0)
-			goto out_free;
+	while ((error = xfrog_inumbers(&xfd, &last, nent, t, &count)) == 0 &&
+	       count > 0) {
  		for (i = 0; i < count; i++) {
  			printf(_("ino %10llu count %2d mask %016llx\n"),
  				(unsigned long long)t[i].xi_startino,
@@ -45,9 +41,12 @@ imap_f(int argc, char **argv)
  				(unsigned long long)t[i].xi_allocmask);
  		}
  	}
-	perror("xfsctl(XFS_IOC_FSINUMBERS)");
-	exitcode = 1;
-out_free:
+
+	if (error) {
+		errno = error;
+		perror("xfsctl(XFS_IOC_FSINUMBERS)");
+		exitcode = 1;
+	}
  	free(t);
  	return 0;
  }
diff --git a/io/open.c b/io/open.c
index 35e6131b..c0d0f1e9 100644
--- a/io/open.c
+++ b/io/open.c
@@ -675,24 +675,24 @@ inode_help(void)
  "\n"));
  }
  
+#define IGROUP_NR	(1024)
  static __u64
  get_last_inode(void)
  {
-	__u64			lastip = 0;
-	__u64			lastgrp = 0;
-	__s32			ocount = 0;
+	struct xfs_fd		xfd = XFS_FD_INIT(file->fd);
+	uint64_t		lastip = 0;
+	uint32_t		lastgrp = 0;
+	uint32_t		ocount = 0;
  	__u64			last_ino;
-	struct xfs_inogrp	igroup[1024];
-	struct xfs_fsop_bulkreq	bulkreq;
-
-	bulkreq.lastip = &lastip;
-	bulkreq.ubuffer = &igroup;
-	bulkreq.icount = sizeof(igroup) / sizeof(struct xfs_inogrp);
-	bulkreq.ocount = &ocount;
+	struct xfs_inogrp	igroup[IGROUP_NR];
  
  	for (;;) {
-		if (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS,
-				&bulkreq)) {
+		int		ret;
+
+		ret = xfrog_inumbers(&xfd, &lastip, IGROUP_NR, igroup,
+				&ocount);
+		if (ret) {
+			errno = ret;
  			perror("XFS_IOC_FSINUMBERS");
  			return 0;
  		}
diff --git a/libfrog/bulkstat.c b/libfrog/bulkstat.c
index 0e11ccea..18fffb20 100644
--- a/libfrog/bulkstat.c
+++ b/libfrog/bulkstat.c
@@ -50,3 +50,29 @@ xfrog_bulkstat(
  		return errno;
  	return 0;
  }
+
+/*
+ * Query inode allocation bitmask information.  Returns zero or a positive
+ * error code.
+ */
+int
+xfrog_inumbers(
+	struct xfs_fd		*xfd,
+	uint64_t		*lastino,
+	uint32_t		icount,
+	struct xfs_inogrp	*ubuffer,
+	uint32_t		*ocount)
+{
+	struct xfs_fsop_bulkreq	bulkreq = {
+		.lastip		= (__u64 *)lastino,
+		.icount		= icount,
+		.ubuffer	= ubuffer,
+		.ocount		= (__s32 *)ocount,
+	};
+	int			ret;
+
+	ret = ioctl(xfd->fd, XFS_IOC_FSINUMBERS, &bulkreq);
+	if (ret)
+		return errno;
+	return 0;
+}
diff --git a/scrub/fscounters.c b/scrub/fscounters.c
index ea6af156..3915822e 100644
--- a/scrub/fscounters.c
+++ b/scrub/fscounters.c
@@ -15,6 +15,7 @@
  #include "xfs_scrub.h"
  #include "common.h"
  #include "fscounters.h"
+#include "xfrog.h"
  
  /*
   * Filesystem counter collection routines.  We can count the number of
@@ -41,30 +42,25 @@ xfs_count_inodes_range(
  	uint64_t		last_ino,
  	uint64_t		*count)
  {
-	struct xfs_fsop_bulkreq	igrpreq = {NULL};
  	struct xfs_inogrp	inogrp;
-	__u64			igrp_ino;
+	uint64_t		igrp_ino;
  	uint64_t		nr = 0;
-	__s32			igrplen = 0;
+	uint32_t		igrplen = 0;
  	int			error;
  
  	ASSERT(!(first_ino & (XFS_INODES_PER_CHUNK - 1)));
  	ASSERT((last_ino & (XFS_INODES_PER_CHUNK - 1)));
  
-	igrpreq.lastip  = &igrp_ino;
-	igrpreq.icount  = 1;
-	igrpreq.ubuffer = &inogrp;
-	igrpreq.ocount  = &igrplen;
-
  	igrp_ino = first_ino;
-	error = ioctl(ctx->mnt.fd, XFS_IOC_FSINUMBERS, &igrpreq);
-	while (!error && igrplen && inogrp.xi_startino < last_ino) {
+	while (!(error = xfrog_inumbers(&ctx->mnt, &igrp_ino, 1, &inogrp,
+			&igrplen))) {
+		if (igrplen == 0 || inogrp.xi_startino >= last_ino)
+			break;
  		nr += inogrp.xi_alloccount;
-		error = ioctl(ctx->mnt.fd, XFS_IOC_FSINUMBERS, &igrpreq);
  	}
  
  	if (error) {
-		str_errno(ctx, descr);
+		str_liberror(ctx, error, descr);
  		return false;
  	}
  
diff --git a/scrub/inodes.c b/scrub/inodes.c
index 413037d8..acad9b16 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -90,16 +90,15 @@ xfs_iterate_inodes_range(
  	xfs_inode_iter_fn	fn,
  	void			*arg)
  {
-	struct xfs_fsop_bulkreq	igrpreq = {NULL};
  	struct xfs_handle	handle;
  	struct xfs_inogrp	inogrp;
  	struct xfs_bstat	bstat[XFS_INODES_PER_CHUNK];
  	char			idescr[DESCR_BUFSZ];
  	struct xfs_bstat	*bs;
-	__u64			igrp_ino;
+	uint64_t		igrp_ino;
  	uint64_t		ino;
  	uint32_t		bulklen = 0;
-	__s32			igrplen = 0;
+	uint32_t		igrplen = 0;
  	bool			moveon = true;
  	int			i;
  	int			error;
@@ -108,11 +107,6 @@ xfs_iterate_inodes_range(
  
  	memset(bstat, 0, XFS_INODES_PER_CHUNK * sizeof(struct xfs_bstat));
  
-	igrpreq.lastip  = &igrp_ino;
-	igrpreq.icount  = 1;
-	igrpreq.ubuffer = &inogrp;
-	igrpreq.ocount  = &igrplen;
-
  	memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
  	handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
  			sizeof(handle.ha_fid.fid_len);
@@ -120,7 +114,7 @@ xfs_iterate_inodes_range(
  
  	/* Find the inode chunk & alloc mask */
  	igrp_ino = first_ino;
-	error = ioctl(ctx->mnt.fd, XFS_IOC_FSINUMBERS, &igrpreq);
+	error = xfrog_inumbers(&ctx->mnt, &igrp_ino, 1, &inogrp, &igrplen);
  	while (!error && igrplen) {
  		/* Load the inodes. */
  		ino = inogrp.xi_startino - 1;
@@ -176,12 +170,13 @@ _("Changed too many times during scan; giving up."));
  
  		stale_count = 0;
  igrp_retry:
-		error = ioctl(ctx->mnt.fd, XFS_IOC_FSINUMBERS, &igrpreq);
+		error = xfrog_inumbers(&ctx->mnt, &igrp_ino, 1, &inogrp,
+				&igrplen);
  	}
  
  err:
  	if (error) {
-		str_errno(ctx, descr);
+		str_liberror(ctx, error, descr);
  		moveon = false;
  	}
  out:







