On Tue, Oct 28, 2014 at 12:35:29PM -0500, Eric Sandeen wrote:
> Today, this geometry:
>
> # modprobe scsi_debug opt_blks=2048 dev_size_mb=2048
> # blockdev --getpbsz --getss --getiomin --getioopt /dev/sdd
> 512
> 512
> 512
> 1048576
>
> will result in a warning at mkfs time, like this:
>
> # mkfs.xfs -f -d su=64k,sw=12 -l su=64k /dev/sdd
> mkfs.xfs: Specified data stripe width 1536 is not the same as the volume stripe width 2048
>
> because our geometry discovery thinks it looks like a
> valid striping setup which the commandline is overriding.
> However, a stripe unit of 512 really isn't indicative of
> a proper stripe geometry.
>
So the assumption is that the storage reports a non-physical block size
for minimum and optimal I/O sizes for geometry detection. There was a
real world scenario of this, right? Any idea of the configuration
details (e.g., raid layout) that resulted in an increased optimal I/O
size but not minimum I/O size?
This seems reasonable to me and the code looks fine, save a trailing
whitespace instance below. I'm just curious if there are any weird
corner cases where there's value in the reported optimal I/O size or the
real world situation was just noise.
> Prior to this patch, we reset only sunit *or* swidth,
> if either was equal to physical block size, but not
> necessarily both.
>
> Change the heuristic so that if either the discovered
> sunit or the discovered swidth is physical block size,
> we reset *both* to zero and ignore the geom completely.
>
> While we're at it, don't pass &dummy in for multiple
> arguments to blkid_get_topology(); that'll mean that
> inside the function, the last assignment wins, and could
> lead to unexpected results.
>
> Reported-by: Stan Hoeppner <stan@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 4546e35..a0fed31 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -410,21 +410,27 @@ static void blkid_get_topology(
> *lsectorsize = val;
> val = blkid_topology_get_physical_sector_size(tp);
> *psectorsize = val;
> + val = blkid_topology_get_minimum_io_size(tp);
> + *sunit = val;
> + val = blkid_topology_get_optimal_io_size(tp);
> + *swidth = val;
>
> /*
> - * Blkid reports the information in terms of bytes, but we want it in
> - * terms of 512 bytes blocks (just to convert it to bytes later..)
> - *
> * If the reported values are the same as the physical sector size
> - * do not bother to report anything. It will just cause warnings
> + * do not bother to report anything. It will only cause warnings
> * if people specify larger stripe units or widths manually.
> */
> - val = blkid_topology_get_minimum_io_size(tp);
> - if (val > *psectorsize)
> - *sunit = val >> 9;
> - val = blkid_topology_get_optimal_io_size(tp);
> - if (val > *psectorsize)
> - *swidth = val >> 9;
> + if (*sunit == *psectorsize || *swidth == *psectorsize) {
> + *sunit = 0;
> + *swidth = 0;
> + }
> +
> + /*
> + * Blkid reports the information in terms of bytes, but we want it in
> + * terms of 512 bytes blocks (only to convert it to bytes later..)
> + */
> + *sunit = *sunit >> 9;
> + *swidth = *swidth >> 9;
Trailing whitespace here.
Brian
>
> if (blkid_topology_get_alignment_offset(tp) != 0) {
> fprintf(stderr,
> @@ -484,10 +490,10 @@ static void get_topology(
> }
>
> if (xi->rtname && !xi->risfile) {
> - int dummy;
> + int sunit, lsectorsize, psectorsize;
>
> - blkid_get_topology(xi->rtname, &dummy, &ft->rtswidth,
> - &dummy, &dummy, force_overwrite);
> + blkid_get_topology(xi->rtname, &sunit, &ft->rtswidth,
> + &lsectorsize, &psectorsize, force_overwrite);
> }
> }
> #else /* ENABLE_BLKID */
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
