Re: [PATCH 39/45] xfs_io: don't re-query geometry information in fsmap_f

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Apr 09, 2025 at 09:55:42AM +0200, Christoph Hellwig wrote:
> But use the information store in "file".
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  io/fsmap.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/io/fsmap.c b/io/fsmap.c
> index 6a87e8972f26..3cc1b510316c 100644
> --- a/io/fsmap.c
> +++ b/io/fsmap.c
> @@ -166,9 +166,9 @@ static void
>  dump_map_verbose(
>  	unsigned long long	*nr,
>  	struct fsmap_head	*head,
> -	bool			*dumped_flags,
> -	struct xfs_fsop_geom	*fsgeo)
> +	bool			*dumped_flags)
>  {
> +	struct xfs_fsop_geom	*fsgeo = &file->geom;
>  	unsigned long long	i;
>  	struct fsmap		*p;
>  	int			agno;
> @@ -395,7 +395,6 @@ fsmap_f(
>  	struct fsmap		*p;
>  	struct fsmap_head	*head;
>  	struct fsmap		*l, *h;
> -	struct xfs_fsop_geom	fsgeo;
>  	long long		start = 0;
>  	long long		end = -1;
>  	int			map_size;
> @@ -470,17 +469,6 @@ fsmap_f(
>  		end <<= BBSHIFT;
>  	}
>  
> -	if (vflag) {
> -		c = -xfrog_geometry(file->fd, &fsgeo);
> -		if (c) {
> -			fprintf(stderr,
> -				_("%s: can't get geometry [\"%s\"]: %s\n"),
> -				progname, file->name, strerror(c));
> -			exitcode = 1;
> -			return 0;
> -		}
> -	}

This leads to a regression on ext4:

# xfs_io -c 'fsmap -vvvvvv' /opt/
Floating point exception

which is a divide by zero in the line:

	agno = p->fmr_physical / bperag;

because bperag is 0 on ext4.  The current behavior is slightly less
unfriendly:

# xfs_io -c 'fsmap -vvvvvv' /opt/
xfs_io: can't get geometry ["/opt/"]: Inappropriate ioctl for device

because the xfrog_geometry() call here fails.

--D

> -
>  	map_size = nflag ? nflag : 131072 / sizeof(struct fsmap);
>  	head = malloc(fsmap_sizeof(map_size));
>  	if (head == NULL) {
> @@ -531,7 +519,7 @@ fsmap_f(
>  			break;
>  
>  		if (vflag)
> -			dump_map_verbose(&nr, head, &dumped_flags, &fsgeo);
> +			dump_map_verbose(&nr, head, &dumped_flags);
>  		else if (mflag)
>  			dump_map_machine(&nr, head);
>  		else
> -- 
> 2.47.2
> 
>