[PATCH v2 0/4] xfs_spaceman: use runtime support library

[PATCH v2 0/4] xfs_spaceman: use runtime support library

[Darrick J. Wong]

Hi all,

Convert xfs_spaceman to use the XFS online runtime support library.  The
first two patches clean up a few things.  Patches 3 converts spaceman to
use struct xfs_fd, and patch 4 establishishes libfrog unit conversion
helpers to replace open-coded conversion code.

This series comes after the xfsprogs 5.3 fixes series.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=spaceman-xfrog-conversion

[PATCH 1/4] xfs_spaceman: remove typedef usage

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Kill off the struct typedefs inside spaceman.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
---
 spaceman/file.c   |   10 +++++-----
 spaceman/freesp.c |    8 ++++----
 spaceman/space.h  |    8 ++++----
 3 files changed, 13 insertions(+), 13 deletions(-)


diff --git a/spaceman/file.c b/spaceman/file.c
index f96a29e5..5647ca7d 100644
--- a/spaceman/file.c
+++ b/spaceman/file.c
@@ -16,13 +16,13 @@
 
 static cmdinfo_t print_cmd;
 
-fileio_t	*filetable;
+struct fileio	*filetable;
 int		filecount;
-fileio_t	*file;
+struct fileio	*file;
 
 static void
 print_fileio(
-	fileio_t	*file,
+	struct fileio	*file,
 	int		index,
 	int		braces)
 {
@@ -101,8 +101,8 @@ addfile(
 	}
 
 	/* Extend the table of currently open files */
-	filetable = (fileio_t *)realloc(filetable,	/* growing */
-					++filecount * sizeof(fileio_t));
+	filetable = (struct fileio *)realloc(filetable,	/* growing */
+					++filecount * sizeof(struct fileio));
 	if (!filetable) {
 		perror("realloc");
 		filecount = 0;
diff --git a/spaceman/freesp.c b/spaceman/freesp.c
index 034f2340..527ecb52 100644
--- a/spaceman/freesp.c
+++ b/spaceman/freesp.c
@@ -14,17 +14,17 @@
 #include "space.h"
 #include "input.h"
 
-typedef struct histent
+struct histent
 {
 	long long	low;
 	long long	high;
 	long long	count;
 	long long	blocks;
-} histent_t;
+};
 
 static int		agcount;
 static xfs_agnumber_t	*aglist;
-static histent_t	*hist;
+static struct histent	*hist;
 static int		dumpflag;
 static long long	equalsize;
 static long long	multsize;
@@ -82,7 +82,7 @@ hcmp(
 	const void	*a,
 	const void	*b)
 {
-	return ((histent_t *)a)->low - ((histent_t *)b)->low;
+	return ((struct histent *)a)->low - ((struct histent *)b)->low;
 }
 
 static void
diff --git a/spaceman/space.h b/spaceman/space.h
index b246f602..8b224aca 100644
--- a/spaceman/space.h
+++ b/spaceman/space.h
@@ -6,16 +6,16 @@
 #ifndef XFS_SPACEMAN_SPACE_H_
 #define XFS_SPACEMAN_SPACE_H_
 
-typedef struct fileio {
+struct fileio {
 	struct xfs_fsop_geom geom;		/* XFS filesystem geometry */
 	struct fs_path	fs_path;	/* XFS path information */
 	char		*name;		/* file name at time of open */
 	int		fd;		/* open file descriptor */
-} fileio_t;
+};
 
-extern fileio_t		*filetable;	/* open file table */
+extern struct fileio	*filetable;	/* open file table */
 extern int		filecount;	/* number of open files */
-extern fileio_t		*file;		/* active file in file table */
+extern struct fileio	*file;		/* active file in file table */
 
 extern int	openfile(char *, struct xfs_fsop_geom *, struct fs_path *);
 extern int	addfile(char *, int , struct xfs_fsop_geom *, struct fs_path *);

[PATCH 2/4] xfs_spaceman: remove unnecessary test in openfile()

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

xfs_spaceman always records fs_path information for an open file because
spaceman requires running on XFS and it always passes a non-null fs_path
to openfile.  Therefore, openfile doesn't need the fs_path null check.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
---
 spaceman/file.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)


diff --git a/spaceman/file.c b/spaceman/file.c
index 5647ca7d..29b7d9ce 100644
--- a/spaceman/file.c
+++ b/spaceman/file.c
@@ -71,16 +71,15 @@ _("%s: Not on a mounted XFS filesystem.\n"),
 		return -1;
 	}
 
-	if (fs_path) {
-		fsp = fs_table_lookup(path, FS_MOUNT_POINT);
-		if (!fsp) {
-			fprintf(stderr, _("%s: cannot find mount point."),
-				path);
-			close(fd);
-			return -1;
-		}
-		memcpy(fs_path, fsp, sizeof(struct fs_path));
+	fsp = fs_table_lookup(path, FS_MOUNT_POINT);
+	if (!fsp) {
+		fprintf(stderr, _("%s: cannot find mount point."),
+			path);
+		close(fd);
+		return -1;
 	}
+	memcpy(fs_path, fsp, sizeof(struct fs_path));
+
 	return fd;
 }
 

[PATCH 3/4] xfs_spaceman: embed struct xfs_fd in struct fileio

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Replace the open-coded fd and geometry fields of struct fileio with a
single xfs_fd, which will enable us to use it natively throughout
xfs_spaceman in upcoming patches.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 spaceman/file.c     |   31 +++++++++++--------------------
 spaceman/freesp.c   |   30 +++++++++++++++++-------------
 spaceman/info.c     |   19 +++----------------
 spaceman/init.c     |   11 +++++++----
 spaceman/prealloc.c |   15 ++++++++-------
 spaceman/space.h    |    9 +++++----
 spaceman/trim.c     |   40 +++++++++++++++++++++-------------------
 7 files changed, 72 insertions(+), 83 deletions(-)


diff --git a/spaceman/file.c b/spaceman/file.c
index 29b7d9ce..43b87e14 100644
--- a/spaceman/file.c
+++ b/spaceman/file.c
@@ -11,8 +11,8 @@
 #include "input.h"
 #include "init.h"
 #include "libfrog/paths.h"
-#include "space.h"
 #include "libfrog/fsgeom.h"
+#include "space.h"
 
 static cmdinfo_t print_cmd;
 
@@ -45,19 +45,13 @@ print_f(
 int
 openfile(
 	char		*path,
-	struct xfs_fsop_geom *geom,
+	struct xfs_fd	*xfd,
 	struct fs_path	*fs_path)
 {
 	struct fs_path	*fsp;
-	int		fd, ret;
-
-	fd = open(path, 0);
-	if (fd < 0) {
-		perror(path);
-		return -1;
-	}
+	int		ret;
 
-	ret = xfrog_geometry(fd, geom);
+	ret = xfd_open(xfd, path, O_RDONLY);
 	if (ret) {
 		if (ret == ENOTTY)
 			fprintf(stderr,
@@ -65,9 +59,8 @@ _("%s: Not on a mounted XFS filesystem.\n"),
 					path);
 		else {
 			errno = ret;
-			perror("XFS_IOC_FSGEOMETRY");
+			perror(path);
 		}
-		close(fd);
 		return -1;
 	}
 
@@ -75,19 +68,18 @@ _("%s: Not on a mounted XFS filesystem.\n"),
 	if (!fsp) {
 		fprintf(stderr, _("%s: cannot find mount point."),
 			path);
-		close(fd);
+		xfd_close(xfd);
 		return -1;
 	}
 	memcpy(fs_path, fsp, sizeof(struct fs_path));
 
-	return fd;
+	return xfd->fd;
 }
 
 int
 addfile(
 	char		*name,
-	int		fd,
-	struct xfs_fsop_geom *geometry,
+	struct xfs_fd	*xfd,
 	struct fs_path	*fs_path)
 {
 	char		*filename;
@@ -95,7 +87,7 @@ addfile(
 	filename = strdup(name);
 	if (!filename) {
 		perror("strdup");
-		close(fd);
+		xfd_close(xfd);
 		return -1;
 	}
 
@@ -106,15 +98,14 @@ addfile(
 		perror("realloc");
 		filecount = 0;
 		free(filename);
-		close(fd);
+		xfd_close(xfd);
 		return -1;
 	}
 
 	/* Finally, make this the new active open file */
 	file = &filetable[filecount - 1];
-	file->fd = fd;
 	file->name = filename;
-	file->geom = *geometry;
+	memcpy(&file->xfd, xfd, sizeof(struct xfs_fd));
 	memcpy(&file->fs_path, fs_path, sizeof(file->fs_path));
 	return 0;
 }
diff --git a/spaceman/freesp.c b/spaceman/freesp.c
index 527ecb52..f30171f0 100644
--- a/spaceman/freesp.c
+++ b/spaceman/freesp.c
@@ -8,6 +8,7 @@
 
 #include "libxfs.h"
 #include <linux/fiemap.h>
+#include "libfrog/fsgeom.h"
 #include "command.h"
 #include "init.h"
 #include "libfrog/paths.h"
@@ -149,7 +150,8 @@ scan_ag(
 	struct fsmap		*extent;
 	struct fsmap		*l, *h;
 	struct fsmap		*p;
-	off64_t			blocksize = file->geom.blocksize;
+	struct xfs_fsop_geom	*fsgeom = &file->xfd.fsgeom;
+	off64_t			blocksize = fsgeom->blocksize;
 	off64_t			bperag;
 	off64_t			aglen;
 	xfs_agblock_t		agbno;
@@ -158,7 +160,7 @@ scan_ag(
 	int			ret;
 	int			i;
 
-	bperag = (off64_t)file->geom.agblocks * blocksize;
+	bperag = (off64_t)fsgeom->agblocks * blocksize;
 
 	fsmap = malloc(fsmap_sizeof(NR_EXTENTS));
 	if (!fsmap) {
@@ -185,7 +187,7 @@ scan_ag(
 	h->fmr_offset = ULLONG_MAX;
 
 	while (true) {
-		ret = ioctl(file->fd, FS_IOC_GETFSMAP, fsmap);
+		ret = ioctl(file->xfd.fd, FS_IOC_GETFSMAP, fsmap);
 		if (ret < 0) {
 			fprintf(stderr, _("%s: FS_IOC_GETFSMAP [\"%s\"]: %s\n"),
 				progname, file->name, strerror(errno));
@@ -248,12 +250,13 @@ aglistadd(
 
 static int
 init(
-	int		argc,
-	char		**argv)
+	int			argc,
+	char			**argv)
 {
-	long long	x;
-	int		c;
-	int		speced = 0;	/* only one of -b -e -h or -m */
+	struct xfs_fsop_geom	*fsgeom = &file->xfd.fsgeom;
+	long long		x;
+	int			c;
+	int			speced = 0;	/* only one of -b -e -h or -m */
 
 	agcount = dumpflag = equalsize = multsize = optind = gflag = 0;
 	histcount = seen1 = summaryflag = 0;
@@ -321,7 +324,7 @@ init(
 		return 0;
 	if (!speced)
 		multsize = 2;
-	histinit(file->geom.agblocks);
+	histinit(fsgeom->agblocks);
 	return 1;
 many_spec:
 	return command_usage(&freesp_cmd);
@@ -332,10 +335,11 @@ init(
  */
 static int
 freesp_f(
-	int		argc,
-	char		**argv)
+	int			argc,
+	char			**argv)
 {
-	xfs_agnumber_t	agno;
+	struct xfs_fsop_geom	*fsgeom = &file->xfd.fsgeom;
+	xfs_agnumber_t		agno;
 
 	if (!init(argc, argv))
 		return 0;
@@ -343,7 +347,7 @@ freesp_f(
 		printf(_("        AG    extents     blocks\n"));
 	if (rtflag)
 		scan_ag(NULLAGNUMBER);
-	for (agno = 0; !rtflag && agno < file->geom.agcount; agno++)  {
+	for (agno = 0; !rtflag && agno < fsgeom->agcount; agno++) {
 		if (inaglist(agno))
 			scan_ag(agno);
 	}
diff --git a/spaceman/info.c b/spaceman/info.c
index f563cf1e..f6234c4c 100644
--- a/spaceman/info.c
+++ b/spaceman/info.c
@@ -7,8 +7,8 @@
 #include "command.h"
 #include "init.h"
 #include "libfrog/paths.h"
-#include "space.h"
 #include "libfrog/fsgeom.h"
+#include "space.h"
 
 static void
 info_help(void)
@@ -28,26 +28,13 @@ info_f(
 	int			argc,
 	char			**argv)
 {
-	struct xfs_fsop_geom	geo;
-	int			error;
-
 	if (fs_table_lookup_mount(file->name) == NULL) {
 		fprintf(stderr, _("%s: Not a XFS mount point.\n"), file->name);
 		return 1;
 	}
 
-	/* get the current filesystem size & geometry */
-	error = xfrog_geometry(file->fd, &geo);
-	if (error) {
-		fprintf(stderr,
-	_("%s: cannot determine geometry of filesystem mounted at %s: %s\n"),
-			progname, file->name, strerror(error));
-		exitcode = 1;
-		return 0;
-	}
-
-	xfs_report_geom(&geo, file->fs_path.fs_name, file->fs_path.fs_log,
-			file->fs_path.fs_rt);
+	xfs_report_geom(&file->xfd.fsgeom, file->fs_path.fs_name,
+			file->fs_path.fs_log, file->fs_path.fs_rt);
 	return 0;
 }
 
diff --git a/spaceman/init.c b/spaceman/init.c
index fa0397ab..4afdb386 100644
--- a/spaceman/init.c
+++ b/spaceman/init.c
@@ -5,6 +5,7 @@
  */
 
 #include "libxfs.h"
+#include "libfrog/fsgeom.h"
 #include "command.h"
 #include "input.h"
 #include "init.h"
@@ -60,7 +61,7 @@ init(
 	char		**argv)
 {
 	int		c;
-	struct xfs_fsop_geom geometry = { 0 };
+	struct xfs_fd	xfd = XFS_FD_INIT_EMPTY;
 	struct fs_path	fsp;
 
 	progname = basename(argv[0]);
@@ -88,11 +89,13 @@ init(
 	if (optind != argc - 1)
 		usage();
 
-	if ((c = openfile(argv[optind], &geometry, &fsp)) < 0)
+	c = openfile(argv[optind], &xfd, &fsp);
+	if (c < 0)
 		exit(1);
-	if (!platform_test_xfs_fd(c))
+	if (!platform_test_xfs_fd(xfd.fd))
 		printf(_("Not an XFS filesystem!\n"));
-	if (addfile(argv[optind], c, &geometry, &fsp) < 0)
+	c = addfile(argv[optind], &xfd, &fsp);
+	if (c < 0)
 		exit(1);
 
 	init_commands();
diff --git a/spaceman/prealloc.c b/spaceman/prealloc.c
index b223010c..e5d857bd 100644
--- a/spaceman/prealloc.c
+++ b/spaceman/prealloc.c
@@ -5,6 +5,7 @@
  */
 
 #include "libxfs.h"
+#include "libfrog/fsgeom.h"
 #include "command.h"
 #include "input.h"
 #include "init.h"
@@ -18,11 +19,12 @@ static cmdinfo_t prealloc_cmd;
  */
 static int
 prealloc_f(
-	int	argc,
-	char	**argv)
+	int			argc,
+	char			**argv)
 {
 	struct xfs_fs_eofblocks eofb = {0};
-	int	c;
+	struct xfs_fsop_geom	*fsgeom = &file->xfd.fsgeom;
+	int			c;
 
 	eofb.eof_version = XFS_EOFBLOCKS_VERSION;
 
@@ -51,9 +53,8 @@ prealloc_f(
 			break;
 		case 'm':
 			eofb.eof_flags |= XFS_EOF_FLAGS_MINFILESIZE;
-			eofb.eof_min_file_size = cvtnum(file->geom.blocksize,
-							file->geom.sectsize,
-							optarg);
+			eofb.eof_min_file_size = cvtnum(fsgeom->blocksize,
+					fsgeom->sectsize, optarg);
 			break;
 		case '?':
 		default:
@@ -63,7 +64,7 @@ prealloc_f(
 	if (optind != argc)
 		return command_usage(&prealloc_cmd);
 
-	if (ioctl(file->fd, XFS_IOC_FREE_EOFBLOCKS, &eofb) < 0) {
+	if (ioctl(file->xfd.fd, XFS_IOC_FREE_EOFBLOCKS, &eofb) < 0) {
 		fprintf(stderr, _("%s: XFS_IOC_FREE_EOFBLOCKS on %s: %s\n"),
 			progname, file->name, strerror(errno));
 	}
diff --git a/spaceman/space.h b/spaceman/space.h
index 8b224aca..2c26884a 100644
--- a/spaceman/space.h
+++ b/spaceman/space.h
@@ -7,18 +7,19 @@
 #define XFS_SPACEMAN_SPACE_H_
 
 struct fileio {
-	struct xfs_fsop_geom geom;		/* XFS filesystem geometry */
+	struct xfs_fd	xfd;		/* XFS runtime support context */
 	struct fs_path	fs_path;	/* XFS path information */
 	char		*name;		/* file name at time of open */
-	int		fd;		/* open file descriptor */
 };
 
 extern struct fileio	*filetable;	/* open file table */
 extern int		filecount;	/* number of open files */
 extern struct fileio	*file;		/* active file in file table */
 
-extern int	openfile(char *, struct xfs_fsop_geom *, struct fs_path *);
-extern int	addfile(char *, int , struct xfs_fsop_geom *, struct fs_path *);
+extern int	openfile(char *path, struct xfs_fd *xfd,
+			 struct fs_path *fs_path);
+extern int	addfile(char *path, struct xfs_fd *xfd,
+			struct fs_path *fs_path);
 
 extern void	print_init(void);
 extern void	help_init(void);
diff --git a/spaceman/trim.c b/spaceman/trim.c
index b23e2bf9..daf4e427 100644
--- a/spaceman/trim.c
+++ b/spaceman/trim.c
@@ -5,6 +5,7 @@
  */
 
 #include "libxfs.h"
+#include "libfrog/fsgeom.h"
 #include "command.h"
 #include "init.h"
 #include "libfrog/paths.h"
@@ -18,18 +19,19 @@ static cmdinfo_t trim_cmd;
  */
 static int
 trim_f(
-	int		argc,
-	char		**argv)
+	int			argc,
+	char			**argv)
 {
-	struct fstrim_range trim = {0};
-	xfs_agnumber_t	agno = 0;
-	off64_t		offset = 0;
-	ssize_t		length = 0;
-	ssize_t		minlen = 0;
-	int		aflag = 0;
-	int		fflag = 0;
-	int		ret;
-	int		c;
+	struct fstrim_range	trim = {0};
+	struct xfs_fsop_geom	*fsgeom = &file->xfd.fsgeom;
+	xfs_agnumber_t		agno = 0;
+	off64_t			offset = 0;
+	ssize_t			length = 0;
+	ssize_t			minlen = 0;
+	int			aflag = 0;
+	int			fflag = 0;
+	int			ret;
+	int			c;
 
 	while ((c = getopt(argc, argv, "a:fm:")) != EOF) {
 		switch (c) {
@@ -45,8 +47,8 @@ trim_f(
 			fflag = 1;
 			break;
 		case 'm':
-			minlen = cvtnum(file->geom.blocksize,
-					file->geom.sectsize, optarg);
+			minlen = cvtnum(fsgeom->blocksize, fsgeom->sectsize,
+					optarg);
 			break;
 		default:
 			return command_usage(&trim_cmd);
@@ -59,23 +61,23 @@ trim_f(
 	if (optind != argc - 2 && !(aflag || fflag))
 		return command_usage(&trim_cmd);
 	if (optind != argc) {
-		offset = cvtnum(file->geom.blocksize, file->geom.sectsize,
+		offset = cvtnum(fsgeom->blocksize, fsgeom->sectsize,
 				argv[optind]);
-		length = cvtnum(file->geom.blocksize, file->geom.sectsize,
+		length = cvtnum(fsgeom->blocksize, fsgeom->sectsize,
 				argv[optind + 1]);
 	} else if (agno) {
-		offset = (off64_t)agno * file->geom.agblocks * file->geom.blocksize;
-		length = file->geom.agblocks * file->geom.blocksize;
+		offset = (off64_t)agno * fsgeom->agblocks * fsgeom->blocksize;
+		length = fsgeom->agblocks * fsgeom->blocksize;
 	} else {
 		offset = 0;
-		length = file->geom.datablocks * file->geom.blocksize;
+		length = fsgeom->datablocks * fsgeom->blocksize;
 	}
 
 	trim.start = offset;
 	trim.len = length;
 	trim.minlen = minlen;
 
-	ret = ioctl(file->fd, FITRIM, (unsigned long)&trim);
+	ret = ioctl(file->xfd.fd, FITRIM, (unsigned long)&trim);
 	if (ret < 0) {
 		fprintf(stderr, "%s: ioctl(FITRIM) [\"%s\"]: %s\n",
 			progname, file->name, strerror(errno));

[PATCH 4/4] xfs_spaceman: convert open-coded unit conversions to helpers

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Create xfrog analogues of the libxfs byte/sector/block conversion
functions and convert spaceman to use them instead of open-coded
arithmatic we do now.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libfrog/fsgeom.c  |    1 +
 libfrog/fsgeom.h  |   85 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 spaceman/freesp.c |   15 +++------
 spaceman/trim.c   |    9 +++---
 4 files changed, 96 insertions(+), 14 deletions(-)


diff --git a/libfrog/fsgeom.c b/libfrog/fsgeom.c
index bc872834..631286cd 100644
--- a/libfrog/fsgeom.c
+++ b/libfrog/fsgeom.c
@@ -113,6 +113,7 @@ xfd_prepare_geometry(
 	xfd->inodelog = highbit32(xfd->fsgeom.inodesize);
 	xfd->inopblog = xfd->blocklog - xfd->inodelog;
 	xfd->aginolog = xfd->agblklog + xfd->inopblog;
+	xfd->blkbb_log = xfd->blocklog - BBSHIFT;
 	return 0;
 }
 
diff --git a/libfrog/fsgeom.h b/libfrog/fsgeom.h
index 6993dafb..5dcfc1bb 100644
--- a/libfrog/fsgeom.h
+++ b/libfrog/fsgeom.h
@@ -35,6 +35,9 @@ struct xfs_fd {
 
 	/* bits for agino in inum */
 	unsigned int		aginolog;
+
+	/* log2 of sb_blocksize / sb_sectsize */
+	unsigned int		blkbb_log;
 };
 
 /* Static initializers */
@@ -99,4 +102,86 @@ cvt_b_to_off_fsbt(
 	return bytes >> xfd->blocklog;
 }
 
+/* Convert sector number to bytes. */
+static inline uint64_t
+cvt_bbtob(
+	uint64_t		daddr)
+{
+	return daddr << BBSHIFT;
+}
+
+/* Convert bytes to sector number, rounding down. */
+static inline uint64_t
+cvt_btobbt(
+	uint64_t		bytes)
+{
+	return bytes >> BBSHIFT;
+}
+
+/* Convert fs block number to sector number. */
+static inline uint64_t
+cvt_off_fsb_to_bb(
+	struct xfs_fd		*xfd,
+	uint64_t		fsbno)
+{
+	return fsbno << xfd->blkbb_log;
+}
+
+/* Convert sector number to fs block number, rounded down. */
+static inline uint64_t
+cvt_bb_to_off_fsbt(
+	struct xfs_fd		*xfd,
+	uint64_t		daddr)
+{
+	return daddr >> xfd->blkbb_log;
+}
+
+/* Convert AG number and AG block to fs block number */
+static inline uint64_t
+cvt_agb_to_daddr(
+	struct xfs_fd		*xfd,
+	uint32_t		agno,
+	uint32_t		agbno)
+{
+	return cvt_off_fsb_to_bb(xfd,
+			(uint64_t)agno * xfd->fsgeom.agblocks + agbno);
+}
+
+/* Convert sector number to AG number. */
+static inline uint32_t
+cvt_daddr_to_agno(
+	struct xfs_fd		*xfd,
+	uint64_t		daddr)
+{
+	return cvt_bb_to_off_fsbt(xfd, daddr) / xfd->fsgeom.agblocks;
+}
+
+/* Convert sector number to AG block number. */
+static inline uint32_t
+cvt_daddr_to_agbno(
+	struct xfs_fd		*xfd,
+	uint64_t		daddr)
+{
+	return cvt_bb_to_off_fsbt(xfd, daddr) % xfd->fsgeom.agblocks;
+}
+
+/* Convert AG number and AG block to a byte location on disk. */
+static inline uint64_t
+cvt_agbno_to_b(
+	struct xfs_fd		*xfd,
+	xfs_agnumber_t		agno,
+	xfs_agblock_t		agbno)
+{
+	return cvt_bbtob(cvt_agb_to_daddr(xfd, agno, agbno));
+}
+
+/* Convert byte location on disk to AG block. */
+static inline xfs_agblock_t
+cvt_b_to_agbno(
+	struct xfs_fd		*xfd,
+	uint64_t		byteno)
+{
+	return cvt_daddr_to_agbno(xfd, cvt_btobbt(byteno));
+}
+
 #endif /* __LIBFROG_FSGEOM_H__ */
diff --git a/spaceman/freesp.c b/spaceman/freesp.c
index f30171f0..92cdb743 100644
--- a/spaceman/freesp.c
+++ b/spaceman/freesp.c
@@ -150,9 +150,7 @@ scan_ag(
 	struct fsmap		*extent;
 	struct fsmap		*l, *h;
 	struct fsmap		*p;
-	struct xfs_fsop_geom	*fsgeom = &file->xfd.fsgeom;
-	off64_t			blocksize = fsgeom->blocksize;
-	off64_t			bperag;
+	struct xfs_fd		*xfd = &file->xfd;
 	off64_t			aglen;
 	xfs_agblock_t		agbno;
 	unsigned long long	freeblks = 0;
@@ -160,8 +158,6 @@ scan_ag(
 	int			ret;
 	int			i;
 
-	bperag = (off64_t)fsgeom->agblocks * blocksize;
-
 	fsmap = malloc(fsmap_sizeof(NR_EXTENTS));
 	if (!fsmap) {
 		fprintf(stderr, _("%s: fsmap malloc failed.\n"), progname);
@@ -174,8 +170,8 @@ scan_ag(
 	l = fsmap->fmh_keys;
 	h = fsmap->fmh_keys + 1;
 	if (agno != NULLAGNUMBER) {
-		l->fmr_physical = agno * bperag;
-		h->fmr_physical = ((agno + 1) * bperag) - 1;
+		l->fmr_physical = cvt_agbno_to_b(xfd, agno, 0);
+		h->fmr_physical = cvt_agbno_to_b(xfd, agno + 1, 0);
 		l->fmr_device = h->fmr_device = file->fs_path.fs_datadev;
 	} else {
 		l->fmr_physical = 0;
@@ -206,9 +202,8 @@ scan_ag(
 			if (!(extent->fmr_flags & FMR_OF_SPECIAL_OWNER) ||
 			    extent->fmr_owner != XFS_FMR_OWN_FREE)
 				continue;
-			agbno = (extent->fmr_physical - (bperag * agno)) /
-								blocksize;
-			aglen = extent->fmr_length / blocksize;
+			agbno = cvt_b_to_agbno(xfd, extent->fmr_physical);
+			aglen = cvt_b_to_off_fsbt(xfd, extent->fmr_length);
 			freeblks += aglen;
 			freeexts++;
 
diff --git a/spaceman/trim.c b/spaceman/trim.c
index daf4e427..e9ed47e4 100644
--- a/spaceman/trim.c
+++ b/spaceman/trim.c
@@ -23,7 +23,8 @@ trim_f(
 	char			**argv)
 {
 	struct fstrim_range	trim = {0};
-	struct xfs_fsop_geom	*fsgeom = &file->xfd.fsgeom;
+	struct xfs_fd		*xfd = &file->xfd;
+	struct xfs_fsop_geom	*fsgeom = &xfd->fsgeom;
 	xfs_agnumber_t		agno = 0;
 	off64_t			offset = 0;
 	ssize_t			length = 0;
@@ -66,11 +67,11 @@ trim_f(
 		length = cvtnum(fsgeom->blocksize, fsgeom->sectsize,
 				argv[optind + 1]);
 	} else if (agno) {
-		offset = (off64_t)agno * fsgeom->agblocks * fsgeom->blocksize;
-		length = fsgeom->agblocks * fsgeom->blocksize;
+		offset = cvt_agbno_to_b(xfd, agno, 0);
+		length = cvt_off_fsb_to_b(xfd, fsgeom->agblocks);
 	} else {
 		offset = 0;
-		length = fsgeom->datablocks * fsgeom->blocksize;
+		length = cvt_off_fsb_to_b(xfd, fsgeom->datablocks);
 	}
 
 	trim.start = offset;

Re: [PATCH 3/4] xfs_spaceman: embed struct xfs_fd in struct fileio

[Dave Chinner]

On Tue, Sep 03, 2019 at 09:38:35PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Replace the open-coded fd and geometry fields of struct fileio with a
> single xfs_fd, which will enable us to use it natively throughout
> xfs_spaceman in upcoming patches.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  spaceman/file.c     |   31 +++++++++++--------------------
>  spaceman/freesp.c   |   30 +++++++++++++++++-------------
>  spaceman/info.c     |   19 +++----------------
>  spaceman/init.c     |   11 +++++++----
>  spaceman/prealloc.c |   15 ++++++++-------
>  spaceman/space.h    |    9 +++++----
>  spaceman/trim.c     |   40 +++++++++++++++++++++-------------------
>  7 files changed, 72 insertions(+), 83 deletions(-)

Nice! Really starts to simplify the file and geometry handling.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 4/4] xfs_spaceman: convert open-coded unit conversions to helpers

[Dave Chinner]

On Tue, Sep 03, 2019 at 09:38:47PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Create xfrog analogues of the libxfs byte/sector/block conversion
> functions and convert spaceman to use them instead of open-coded
> arithmatic we do now.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  libfrog/fsgeom.c  |    1 +
>  libfrog/fsgeom.h  |   85 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  spaceman/freesp.c |   15 +++------
>  spaceman/trim.c   |    9 +++---
>  4 files changed, 96 insertions(+), 14 deletions(-)

Looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 4/4] xfs_spaceman: convert open-coded unit conversions to helpers

[Darrick J. Wong]

On Tue, Aug 27, 2019 at 06:15:28PM +1000, Dave Chinner wrote:
> On Mon, Aug 26, 2019 at 02:20:45PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Create xfrog analogues of the libxfs byte/sector/block conversion
> > functions and convert spaceman to use them instead of open-coded
> > arithmatic we do now.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  include/xfrog.h   |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  libfrog/fsgeom.c  |    1 +
> >  spaceman/freesp.c |   18 ++++++--------
> >  spaceman/trim.c   |    9 ++++---
> >  4 files changed, 80 insertions(+), 14 deletions(-)
> 
> ....
> > +/* Convert fs block number to sector number. */
> > +static inline uint64_t
> > +xfrog_fsb_to_bb(
> > +	struct xfs_fd		*xfd,
> > +	uint64_t		fsbno)
> > +{
> > +	return fsbno << xfd->blkbb_log;
> > +}
> > +
> > +/* Convert sector number to fs block number, rounded down. */
> > +static inline uint64_t
> > +xfrog_bb_to_fsbt(
> > +	struct xfs_fd		*xfd,
> > +	uint64_t		daddr)
> > +{
> > +	return daddr >> xfd->blkbb_log;
> > +}
> 
> Same comment as previous ones about off_fsb_to_<foo> and vice versa.
> 
> And the more I see it, the less "xfrog" really means in these unit
> conversion functions. How about we prefix them "cvt_"?
> 
> Then the name of the function actually does exactly what is says.
> i.e. "convert basic blocks to offset in filesystem blocks"

Ok.  prefix conversion done.

> > @@ -174,8 +170,10 @@ scan_ag(
> >  	l = fsmap->fmh_keys;
> >  	h = fsmap->fmh_keys + 1;
> >  	if (agno != NULLAGNUMBER) {
> > -		l->fmr_physical = agno * bperag;
> > -		h->fmr_physical = ((agno + 1) * bperag) - 1;
> > +		l->fmr_physical = xfrog_bbtob(
> > +				xfrog_agb_to_daddr(xfd, agno, 0));
> > +		h->fmr_physical = xfrog_bbtob(
> > +				xfrog_agb_to_daddr(xfd, agno + 1, 0));
> >  		l->fmr_device = h->fmr_device = file->fs_path.fs_datadev;
> >  	} else {
> >  		l->fmr_physical = 0;
> 
> This is why - that's quite hard to read. A simple wrapper might be
> better:
> 
> static inline uint64_t
> cvt_agbno_to_off_b(
> 	struct xfs_fd		*xfd,
> 	xfs_agnumber_t		agno,
> 	xfs_agblock_t		agbno)
> {
> 	return cvt_bbtob(cvt_agbno_to_daddr(xfd, agno, agbno));
> }
> 
> And then we have:
> 
> 		l->fmr_physical = cvt_agbno_to_off_b(xfd, agno, 0);
> 		h->fmr_physical = cvt_agbno_to_off_b(xfd, agno + 1, 0);
> 
> 
> > @@ -206,9 +204,9 @@ scan_ag(
> >  			if (!(extent->fmr_flags & FMR_OF_SPECIAL_OWNER) ||
> >  			    extent->fmr_owner != XFS_FMR_OWN_FREE)
> >  				continue;
> > -			agbno = (extent->fmr_physical - (bperag * agno)) /
> > -								blocksize;
> > -			aglen = extent->fmr_length / blocksize;
> > +			agbno = xfrog_daddr_to_agbno(xfd,
> > +					xfrog_btobbt(extent->fmr_physical));
> 
> That's the reverse - cvt_off_b_to_agbno().
> 
> > +			aglen = xfrog_b_to_fsbt(xfd, extent->fmr_length);
> >  			freeblks += aglen;
> >  			freeexts++;
> >  
> > diff --git a/spaceman/trim.c b/spaceman/trim.c
> > index ea1308f7..8741bab2 100644
> > --- a/spaceman/trim.c
> > +++ b/spaceman/trim.c
> > @@ -23,7 +23,8 @@ trim_f(
> >  	char			**argv)
> >  {
> >  	struct fstrim_range	trim = {0};
> > -	struct xfs_fsop_geom	*fsgeom = &file->xfd.fsgeom;
> > +	struct xfs_fd		*xfd = &file->xfd;
> > +	struct xfs_fsop_geom	*fsgeom = &xfd->fsgeom;
> >  	xfs_agnumber_t		agno = 0;
> >  	off64_t			offset = 0;
> >  	ssize_t			length = 0;
> > @@ -66,11 +67,11 @@ trim_f(
> >  		length = cvtnum(fsgeom->blocksize, fsgeom->sectsize,
> >  				argv[optind + 1]);
> >  	} else if (agno) {
> > -		offset = (off64_t)agno * fsgeom->agblocks * fsgeom->blocksize;
> > -		length = fsgeom->agblocks * fsgeom->blocksize;
> > +		offset = xfrog_bbtob(xfrog_agb_to_daddr(xfd, agno, 0));
> > +		length = xfrog_fsb_to_b(xfd, fsgeom->agblocks);
> 
> cvt_agbno_to_off_b() again...

Done.

--D

> Cheers,
> 
> Dave.
> 
> -- 
> Dave Chinner
> david@fromorbit.com

Re: [PATCH 4/4] xfs_spaceman: convert open-coded unit conversions to helpers

[Dave Chinner]

On Mon, Aug 26, 2019 at 02:20:45PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Create xfrog analogues of the libxfs byte/sector/block conversion
> functions and convert spaceman to use them instead of open-coded
> arithmatic we do now.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  include/xfrog.h   |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libfrog/fsgeom.c  |    1 +
>  spaceman/freesp.c |   18 ++++++--------
>  spaceman/trim.c   |    9 ++++---
>  4 files changed, 80 insertions(+), 14 deletions(-)

....
> +/* Convert fs block number to sector number. */
> +static inline uint64_t
> +xfrog_fsb_to_bb(
> +	struct xfs_fd		*xfd,
> +	uint64_t		fsbno)
> +{
> +	return fsbno << xfd->blkbb_log;
> +}
> +
> +/* Convert sector number to fs block number, rounded down. */
> +static inline uint64_t
> +xfrog_bb_to_fsbt(
> +	struct xfs_fd		*xfd,
> +	uint64_t		daddr)
> +{
> +	return daddr >> xfd->blkbb_log;
> +}

Same comment as previous ones about off_fsb_to_<foo> and vice versa.

And the more I see it, the less "xfrog" really means in these unit
conversion functions. How about we prefix them "cvt_"?

Then the name of the function actually does exactly what is says.
i.e. "convert basic blocks to offset in filesystem blocks"

> @@ -174,8 +170,10 @@ scan_ag(
>  	l = fsmap->fmh_keys;
>  	h = fsmap->fmh_keys + 1;
>  	if (agno != NULLAGNUMBER) {
> -		l->fmr_physical = agno * bperag;
> -		h->fmr_physical = ((agno + 1) * bperag) - 1;
> +		l->fmr_physical = xfrog_bbtob(
> +				xfrog_agb_to_daddr(xfd, agno, 0));
> +		h->fmr_physical = xfrog_bbtob(
> +				xfrog_agb_to_daddr(xfd, agno + 1, 0));
>  		l->fmr_device = h->fmr_device = file->fs_path.fs_datadev;
>  	} else {
>  		l->fmr_physical = 0;

This is why - that's quite hard to read. A simple wrapper might be
better:

static inline uint64_t
cvt_agbno_to_off_b(
	struct xfs_fd		*xfd,
	xfs_agnumber_t		agno,
	xfs_agblock_t		agbno)
{
	return cvt_bbtob(cvt_agbno_to_daddr(xfd, agno, agbno));
}

And then we have:

		l->fmr_physical = cvt_agbno_to_off_b(xfd, agno, 0);
		h->fmr_physical = cvt_agbno_to_off_b(xfd, agno + 1, 0);


> @@ -206,9 +204,9 @@ scan_ag(
>  			if (!(extent->fmr_flags & FMR_OF_SPECIAL_OWNER) ||
>  			    extent->fmr_owner != XFS_FMR_OWN_FREE)
>  				continue;
> -			agbno = (extent->fmr_physical - (bperag * agno)) /
> -								blocksize;
> -			aglen = extent->fmr_length / blocksize;
> +			agbno = xfrog_daddr_to_agbno(xfd,
> +					xfrog_btobbt(extent->fmr_physical));

That's the reverse - cvt_off_b_to_agbno().

> +			aglen = xfrog_b_to_fsbt(xfd, extent->fmr_length);
>  			freeblks += aglen;
>  			freeexts++;
>  
> diff --git a/spaceman/trim.c b/spaceman/trim.c
> index ea1308f7..8741bab2 100644
> --- a/spaceman/trim.c
> +++ b/spaceman/trim.c
> @@ -23,7 +23,8 @@ trim_f(
>  	char			**argv)
>  {
>  	struct fstrim_range	trim = {0};
> -	struct xfs_fsop_geom	*fsgeom = &file->xfd.fsgeom;
> +	struct xfs_fd		*xfd = &file->xfd;
> +	struct xfs_fsop_geom	*fsgeom = &xfd->fsgeom;
>  	xfs_agnumber_t		agno = 0;
>  	off64_t			offset = 0;
>  	ssize_t			length = 0;
> @@ -66,11 +67,11 @@ trim_f(
>  		length = cvtnum(fsgeom->blocksize, fsgeom->sectsize,
>  				argv[optind + 1]);
>  	} else if (agno) {
> -		offset = (off64_t)agno * fsgeom->agblocks * fsgeom->blocksize;
> -		length = fsgeom->agblocks * fsgeom->blocksize;
> +		offset = xfrog_bbtob(xfrog_agb_to_daddr(xfd, agno, 0));
> +		length = xfrog_fsb_to_b(xfd, fsgeom->agblocks);

cvt_agbno_to_off_b() again...

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com

[PATCH 4/4] xfs_spaceman: convert open-coded unit conversions to helpers

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Create xfrog analogues of the libxfs byte/sector/block conversion
functions and convert spaceman to use them instead of open-coded
arithmatic we do now.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 include/xfrog.h   |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 libfrog/fsgeom.c  |    1 +
 spaceman/freesp.c |   18 ++++++--------
 spaceman/trim.c   |    9 ++++---
 4 files changed, 80 insertions(+), 14 deletions(-)


diff --git a/include/xfrog.h b/include/xfrog.h
index 6b605dcf..5748e967 100644
--- a/include/xfrog.h
+++ b/include/xfrog.h
@@ -45,6 +45,9 @@ struct xfs_fd {
 
 	/* bits for agino in inum */
 	unsigned int		aginolog;
+
+	/* log2 of sb_blocksize / sb_sectsize */
+	unsigned int		blkbb_log;
 };
 
 /* Static initializers */
@@ -100,6 +103,69 @@ xfrog_b_to_fsbt(
 	return bytes >> xfd->blocklog;
 }
 
+/* Convert sector number to bytes. */
+static inline uint64_t
+xfrog_bbtob(
+	uint64_t		daddr)
+{
+	return daddr << BBSHIFT;
+}
+
+/* Convert bytes to sector number, rounding down. */
+static inline uint64_t
+xfrog_btobbt(
+	uint64_t		bytes)
+{
+	return bytes >> BBSHIFT;
+}
+
+/* Convert fs block number to sector number. */
+static inline uint64_t
+xfrog_fsb_to_bb(
+	struct xfs_fd		*xfd,
+	uint64_t		fsbno)
+{
+	return fsbno << xfd->blkbb_log;
+}
+
+/* Convert sector number to fs block number, rounded down. */
+static inline uint64_t
+xfrog_bb_to_fsbt(
+	struct xfs_fd		*xfd,
+	uint64_t		daddr)
+{
+	return daddr >> xfd->blkbb_log;
+}
+
+/* Convert AG number and AG block to fs block number */
+static inline uint64_t
+xfrog_agb_to_daddr(
+	struct xfs_fd		*xfd,
+	uint32_t		agno,
+	uint32_t		agbno)
+{
+	return xfrog_fsb_to_bb(xfd,
+			(uint64_t)agno * xfd->fsgeom.agblocks + agbno);
+}
+
+/* Convert sector number to AG number. */
+static inline uint32_t
+xfrog_daddr_to_agno(
+	struct xfs_fd		*xfd,
+	uint64_t		daddr)
+{
+	return xfrog_bb_to_fsbt(xfd, daddr) / xfd->fsgeom.agblocks;
+}
+
+/* Convert sector number to AG block number. */
+static inline uint32_t
+xfrog_daddr_to_agbno(
+	struct xfs_fd		*xfd,
+	uint64_t		daddr)
+{
+	return xfrog_bb_to_fsbt(xfd, daddr) % xfd->fsgeom.agblocks;
+}
+
 /* Bulkstat wrappers */
 struct xfs_bstat;
 int xfrog_bulkstat_single(struct xfs_fd *xfd, uint64_t ino,
diff --git a/libfrog/fsgeom.c b/libfrog/fsgeom.c
index 159738c5..17479e4a 100644
--- a/libfrog/fsgeom.c
+++ b/libfrog/fsgeom.c
@@ -114,6 +114,7 @@ xfrog_prepare_geometry(
 	xfd->inodelog = highbit32(xfd->fsgeom.inodesize);
 	xfd->inopblog = xfd->blocklog - xfd->inodelog;
 	xfd->aginolog = xfd->agblklog + xfd->inopblog;
+	xfd->blkbb_log = xfd->blocklog - BBSHIFT;
 	return 0;
 }
 
diff --git a/spaceman/freesp.c b/spaceman/freesp.c
index 83cbecbd..9e9f32a2 100644
--- a/spaceman/freesp.c
+++ b/spaceman/freesp.c
@@ -150,9 +150,7 @@ scan_ag(
 	struct fsmap		*extent;
 	struct fsmap		*l, *h;
 	struct fsmap		*p;
-	struct xfs_fsop_geom	*fsgeom = &file->xfd.fsgeom;
-	off64_t			blocksize = fsgeom->blocksize;
-	off64_t			bperag;
+	struct xfs_fd		*xfd = &file->xfd;
 	off64_t			aglen;
 	xfs_agblock_t		agbno;
 	unsigned long long	freeblks = 0;
@@ -160,8 +158,6 @@ scan_ag(
 	int			ret;
 	int			i;
 
-	bperag = (off64_t)fsgeom->agblocks * blocksize;
-
 	fsmap = malloc(fsmap_sizeof(NR_EXTENTS));
 	if (!fsmap) {
 		fprintf(stderr, _("%s: fsmap malloc failed.\n"), progname);
@@ -174,8 +170,10 @@ scan_ag(
 	l = fsmap->fmh_keys;
 	h = fsmap->fmh_keys + 1;
 	if (agno != NULLAGNUMBER) {
-		l->fmr_physical = agno * bperag;
-		h->fmr_physical = ((agno + 1) * bperag) - 1;
+		l->fmr_physical = xfrog_bbtob(
+				xfrog_agb_to_daddr(xfd, agno, 0));
+		h->fmr_physical = xfrog_bbtob(
+				xfrog_agb_to_daddr(xfd, agno + 1, 0));
 		l->fmr_device = h->fmr_device = file->fs_path.fs_datadev;
 	} else {
 		l->fmr_physical = 0;
@@ -206,9 +204,9 @@ scan_ag(
 			if (!(extent->fmr_flags & FMR_OF_SPECIAL_OWNER) ||
 			    extent->fmr_owner != XFS_FMR_OWN_FREE)
 				continue;
-			agbno = (extent->fmr_physical - (bperag * agno)) /
-								blocksize;
-			aglen = extent->fmr_length / blocksize;
+			agbno = xfrog_daddr_to_agbno(xfd,
+					xfrog_btobbt(extent->fmr_physical));
+			aglen = xfrog_b_to_fsbt(xfd, extent->fmr_length);
 			freeblks += aglen;
 			freeexts++;
 
diff --git a/spaceman/trim.c b/spaceman/trim.c
index ea1308f7..8741bab2 100644
--- a/spaceman/trim.c
+++ b/spaceman/trim.c
@@ -23,7 +23,8 @@ trim_f(
 	char			**argv)
 {
 	struct fstrim_range	trim = {0};
-	struct xfs_fsop_geom	*fsgeom = &file->xfd.fsgeom;
+	struct xfs_fd		*xfd = &file->xfd;
+	struct xfs_fsop_geom	*fsgeom = &xfd->fsgeom;
 	xfs_agnumber_t		agno = 0;
 	off64_t			offset = 0;
 	ssize_t			length = 0;
@@ -66,11 +67,11 @@ trim_f(
 		length = cvtnum(fsgeom->blocksize, fsgeom->sectsize,
 				argv[optind + 1]);
 	} else if (agno) {
-		offset = (off64_t)agno * fsgeom->agblocks * fsgeom->blocksize;
-		length = fsgeom->agblocks * fsgeom->blocksize;
+		offset = xfrog_bbtob(xfrog_agb_to_daddr(xfd, agno, 0));
+		length = xfrog_fsb_to_b(xfd, fsgeom->agblocks);
 	} else {
 		offset = 0;
-		length = fsgeom->datablocks * fsgeom->blocksize;
+		length = xfrog_fsb_to_b(xfd, fsgeom->datablocks);
 	}
 
 	trim.start = offset;