Looks good to me. I'm fine falling back to zeroed values. Thanks for
following up on this.
Reviewed-by: Jeff Mahoney <jeffm@xxxxxxxx>
-Jeff
On 8/1/18 4:49 PM, Eric Sandeen wrote:
From: Jeff Mahoney <jeffm@xxxxxxxx>
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.
To resolve this we can check the topology before consuming it, and
ignore the bad stripe geometry.
[sandeen: remove guessing heuristic, just warn and ignore bad data.]
Fixes: 051b4e37f5e (mkfs: factor AG alignment)
Signed-off-by: Jeff Mahoney <jeffm@xxxxxxxx>
Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
---
so, I rewrote this a bit. I'm not a fan of guessing what the kernel
really must have meant, becaue next time the root cause may be differnt.
In other cases we ignore bad geometry, I think we should in this case as
well. This will also let me go forward with a factored-out geometry checker,
and for user-specified badness we'll warn and exit, for kernel-provided
badness we'll warn and ignore.
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 1074886..2e53c1e 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -2281,11 +2281,20 @@ _("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) {
- dsunit = ft->dsunit;
- dswidth = ft->dswidth;
- use_dev = true;
+ /* Ignore nonsense from device. XXX add more validation */
+ if (ft->dsunit && ft->dswidth == 0) {
+ fprintf(stderr,
+_("%s: Volume reports stripe unit of %d bytes and stripe width of 0, ignoring.\n"),
+ progname, BBTOB(ft->dsunit));
+ ft->dsunit = 0;
+ ft->dswidth = 0;
+ } else {
+ dsunit = ft->dsunit;
+ dswidth = ft->dswidth;
+ use_dev = true;
+ }
} else {
- /* check and warn is alignment is sub-optimal */
+ /* check and warn if user-specified alignment is sub-optimal */
if (ft->dsunit && ft->dsunit != dsunit) {
fprintf(stderr,
_("%s: Specified data stripe unit %d is not the same as the volume stripe unit %d\n"),
--
Jeff Mahoney
SUSE Labs
--
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
