Re: [PATCH] xfsprogs: mkfs: don't default to the physical sector size if it's bigger than XFS_MAX_SECTORSIZE

From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Jeff Moyer <jmoyer@redhat.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH] xfsprogs: mkfs: don't default to the physical sector size if it's bigger than XFS_MAX_SECTORSIZE
Date: Tue, 28 Jan 2020 08:14:23 -0800	[thread overview]
Message-ID: <20200128161423.GO3447196@magnolia> (raw)
In-Reply-To: <x49h80ftviy.fsf@segfault.boston.devel.redhat.com>

On Tue, Jan 28, 2020 at 11:07:01AM -0500, Jeff Moyer wrote:
> Hi,
> 
> In testing on ppc64, I ran into the following error when making a file
> system:
> 
> # ./mkfs.xfs -b size=65536 -f /dev/ram0
> illegal sector size 65536
> 
> Which is odd, because I didn't specify a sector size!  The problem is
> that validate_sectorsize defaults to the physical sector size, but in
> this case, the physical sector size is bigger than XFS_MAX_SECTORSIZE.
> 
> # cat /sys/block/ram0/queue/physical_block_size 
> 65536
> 
> Fall back to the default (logical sector size) if the physical sector
> size is greater than XFS_MAX_SECTORSIZE.

Do we need to check that ft->lsectorsize <= XFS_MAX_SECTORSIZE too?

(Not that I really expect disks with 64k LBA units...)

--D

> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 606f79da..dc9858af 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1803,8 +1803,13 @@ validate_sectorsize(
>  		if (!ft->lsectorsize)
>  			ft->lsectorsize = XFS_MIN_SECTORSIZE;
>  
> -		/* Older kernels may not have physical/logical distinction */
> -		if (!ft->psectorsize)
> +		/*
> +		 * Older kernels may not have physical/logical distinction.
> +		 * Some architectures have a page size > XFS_MAX_SECTORSIZE.
> +		 * In that case, a ramdisk or persistent memory device may
> +		 * advertise a physical sector size that is too big to use.
> +		 */
> +		if (!ft->psectorsize || ft->psectorsize > XFS_MAX_SECTORSIZE)
>  			ft->psectorsize = ft->lsectorsize;
>  
>  		cfg->sectorsize = ft->psectorsize;
> 