[PATCHSET 0/2] xfs_io: small fixes to fsmap command

[PATCHSET 0/2] xfs_io: small fixes to fsmap command

[Darrick J. Wong]

Hi all,

A couple of small fixes to the fsmap command.

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=fsmap-fixes
---
 io/fsmap.c |   33 ++-------------------------------
 1 file changed, 2 insertions(+), 31 deletions(-)

[PATCH 1/2] xfs_io: only print the header once when dumping fsmap in csv format

[Darrick J. Wong]

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

Only print the column names once when we're dumping fsmap information in
csv format.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 io/fsmap.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


diff --git a/io/fsmap.c b/io/fsmap.c
index 4b217595..9f179fa8 100644
--- a/io/fsmap.c
+++ b/io/fsmap.c
@@ -116,7 +116,8 @@ dump_map_machine(
 	struct fsmap		*p;
 	char			*fork;
 
-	printf(_("EXT,MAJOR,MINOR,PSTART,PEND,OWNER,OSTART,OEND,LENGTH\n"));
+	if (*nr == 0)
+		printf(_("EXT,MAJOR,MINOR,PSTART,PEND,OWNER,OSTART,OEND,LENGTH\n"));
 	for (i = 0, p = head->fmh_recs; i < head->fmh_entries; i++, p++) {
 		printf("%llu,%u,%u,%lld,%lld,", i + (*nr),
 			major(p->fmr_device), minor(p->fmr_device),

[PATCH 2/2] xfs_io: don't count fsmaps before querying fsmaps

[Darrick J. Wong]

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

There's a bunch of code in fsmap.c that tries to count the GETFSMAP
records so that it can size the fsmap array appropriately for the
GETFSMAP call.  It's pointless to iterate the entire result set /twice/
(unlike the bmap command where the extent count is actually stored in
the fs metadata), so get rid of the duplicate walk.

In other words: Iterate over the records using the default chunk size
instead of doing one call to find the size and doing a giant allocation
and GETFSMAP call.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 io/fsmap.c |   30 ------------------------------
 1 file changed, 30 deletions(-)


diff --git a/io/fsmap.c b/io/fsmap.c
index 9f179fa8..f540a7c0 100644
--- a/io/fsmap.c
+++ b/io/fsmap.c
@@ -372,13 +372,11 @@ fsmap_f(
 	char			**argv)
 {
 	struct fsmap		*p;
-	struct fsmap_head	*nhead;
 	struct fsmap_head	*head;
 	struct fsmap		*l, *h;
 	struct xfs_fsop_geom	fsgeo;
 	long long		start = 0;
 	long long		end = -1;
-	int			nmap_size;
 	int			map_size;
 	int			nflag = 0;
 	int			vflag = 0;
@@ -492,34 +490,6 @@ fsmap_f(
 	h->fmr_flags = UINT_MAX;
 	h->fmr_offset = ULLONG_MAX;
 
-	/* Count mappings */
-	if (!nflag) {
-		head->fmh_count = 0;
-		i = ioctl(file->fd, FS_IOC_GETFSMAP, head);
-		if (i < 0) {
-			fprintf(stderr, _("%s: xfsctl(XFS_IOC_GETFSMAP)"
-				" iflags=0x%x [\"%s\"]: %s\n"),
-				progname, head->fmh_iflags, file->name,
-				strerror(errno));
-			exitcode = 1;
-			free(head);
-			return 0;
-		}
-		if (head->fmh_entries > map_size + 2) {
-			map_size = 11ULL * head->fmh_entries / 10;
-			nmap_size = map_size > (1 << 24) ? (1 << 24) : map_size;
-			nhead = realloc(head, fsmap_sizeof(nmap_size));
-			if (nhead == NULL) {
-				fprintf(stderr,
-					_("%s: cannot realloc %zu bytes\n"),
-					progname, fsmap_sizeof(nmap_size));
-			} else {
-				head = nhead;
-				map_size = nmap_size;
-			}
-		}
-	}
-
 	/*
 	 * If this is an XFS filesystem, remember the data device.
 	 * (We report AG number/block for data device extents on XFS).

Re: [PATCH 2/2] xfs_io: don't count fsmaps before querying fsmaps

[Darrick J. Wong]

On Mon, Jul 05, 2021 at 04:25:27PM +0100, Christoph Hellwig wrote:
> On Fri, Jul 02, 2021 at 07:58:20PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > There's a bunch of code in fsmap.c that tries to count the GETFSMAP
> > records so that it can size the fsmap array appropriately for the
> > GETFSMAP call.  It's pointless to iterate the entire result set /twice/
> > (unlike the bmap command where the extent count is actually stored in
> > the fs metadata), so get rid of the duplicate walk.
> 
> In otherwords:  just keep iterating over the records using the default
> chunk size instead of doing one call to find the size and then do
> a giant allocation and GETFSMAP call.

I'll paste this into the commit log, thanks.

--D

> I find the current commit log a little confusing, but the change itself
> looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/2] xfs_io: don't count fsmaps before querying fsmaps

[Christoph Hellwig]

On Fri, Jul 02, 2021 at 07:58:20PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> There's a bunch of code in fsmap.c that tries to count the GETFSMAP
> records so that it can size the fsmap array appropriately for the
> GETFSMAP call.  It's pointless to iterate the entire result set /twice/
> (unlike the bmap command where the extent count is actually stored in
> the fs metadata), so get rid of the duplicate walk.

In otherwords:  just keep iterating over the records using the default
chunk size instead of doing one call to find the size and then do
a giant allocation and GETFSMAP call.

I find the current commit log a little confusing, but the change itself
looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/2] xfs_io: don't count fsmaps before querying fsmaps

[Darrick J. Wong]

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

There's a bunch of code in fsmap.c that tries to count the GETFSMAP
records so that it can size the fsmap array appropriately for the
GETFSMAP call.  It's pointless to iterate the entire result set /twice/
(unlike the bmap command where the extent count is actually stored in
the fs metadata), so get rid of the duplicate walk.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 io/fsmap.c |   30 ------------------------------
 1 file changed, 30 deletions(-)


diff --git a/io/fsmap.c b/io/fsmap.c
index 9f179fa8..f540a7c0 100644
--- a/io/fsmap.c
+++ b/io/fsmap.c
@@ -372,13 +372,11 @@ fsmap_f(
 	char			**argv)
 {
 	struct fsmap		*p;
-	struct fsmap_head	*nhead;
 	struct fsmap_head	*head;
 	struct fsmap		*l, *h;
 	struct xfs_fsop_geom	fsgeo;
 	long long		start = 0;
 	long long		end = -1;
-	int			nmap_size;
 	int			map_size;
 	int			nflag = 0;
 	int			vflag = 0;
@@ -492,34 +490,6 @@ fsmap_f(
 	h->fmr_flags = UINT_MAX;
 	h->fmr_offset = ULLONG_MAX;
 
-	/* Count mappings */
-	if (!nflag) {
-		head->fmh_count = 0;
-		i = ioctl(file->fd, FS_IOC_GETFSMAP, head);
-		if (i < 0) {
-			fprintf(stderr, _("%s: xfsctl(XFS_IOC_GETFSMAP)"
-				" iflags=0x%x [\"%s\"]: %s\n"),
-				progname, head->fmh_iflags, file->name,
-				strerror(errno));
-			exitcode = 1;
-			free(head);
-			return 0;
-		}
-		if (head->fmh_entries > map_size + 2) {
-			map_size = 11ULL * head->fmh_entries / 10;
-			nmap_size = map_size > (1 << 24) ? (1 << 24) : map_size;
-			nhead = realloc(head, fsmap_sizeof(nmap_size));
-			if (nhead == NULL) {
-				fprintf(stderr,
-					_("%s: cannot realloc %zu bytes\n"),
-					progname, fsmap_sizeof(nmap_size));
-			} else {
-				head = nhead;
-				map_size = nmap_size;
-			}
-		}
-	}
-
 	/*
 	 * If this is an XFS filesystem, remember the data device.
 	 * (We report AG number/block for data device extents on XFS).