On 7/19/18 4:23 PM, Jeff Mahoney wrote:
> Commit 051b4e37f5e (mkfs: factor AG alignment) factored out the
> AG alignment code into a separate function. It got rid of
> redundant checks for dswidth != 0 since calc_stripe_factors was
> supposed to guarantee that if dsunit is non-zero dswidth will be
> as well. Unfortunately, there's hardware out there that reports its
> optimal i/o size as larger than the maximum i/o size, which the kernel
> treats as broken and zeros out the optimal i/o size. We'll accept
> the multi-sector dsunit but have a zero dswidth and hit a divide-by-zero
> in align_ag_geometry.
>
> To resolve this we can check the topology before consuming it, default
> to using the stripe unit as the stripe width, and warn the user about it.
>
> Fixes: 051b4e37f5e (mkfs: factor AG alignment)
> Signed-off-by: Jeff Mahoney <jeffm@xxxxxxxx>
Looks fine to me logically. Sorry for nitpicking a patch again (it's
a character flaw) but I'd like to massage this slightly:
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 1074886..231542f 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -2281,6 +2281,16 @@ _("data stripe width (%d) must be a multiple of the data stripe unit (%d)\n"),
/* if no stripe config set, use the device default */
if (!dsunit) {
+ /* Watch out for nonsense from device */
+ if (ft->dsunit && ft->dswidth == 0) {
+ fprintf(stderr,
+_("%s: Volume reports stripe unit of %d bytes but stripe width of 0.\n"),
+ progname, ft->dsunit << 9);
+ fprintf(stderr,
+_("Using stripe width of %d bytes, which may not be optimal.\n"),
+ ft->dsunit << 9);
+ ft->dswidth = ft->dsunit;
+ }
dsunit = ft->dsunit;
dswidth = ft->dswidth;
use_dev = true;
to make it a bit more clear that we're checking the /device/-reported
topology (by looking at ft before using it) and also to break up
the long warning message into < 80 char lines. OK?
This all seems a little messy yet (an inherited mess) but that's slightly
clearer to me.
Thanks,
-Eric
> ---
> mkfs/xfs_mkfs.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index a135e06e..35542e57 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -2295,6 +2295,12 @@ _("data stripe width (%d) must be a multiple of the data stripe unit (%d)\n"),
> if (!dsunit) {
> dsunit = ft->dsunit;
> dswidth = ft->dswidth;
> + if (dsunit && dswidth == 0) {
> + fprintf(stderr,
> +_("%s: Volume reports stripe unit of %d bytes but stripe width of 0. Using stripe width of %d bytes, which may not be optimal.\n"),
> + progname, dsunit << 9, dsunit << 9);
> + dswidth = dsunit;
> + }
> use_dev = true;
> } else {
> /* check and warn is alignment is sub-optimal */
>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
