On 6/20/18 11:57 PM, Dave Chinner wrote: > On Wed, Jun 20, 2018 at 10:55:20PM -0400, jeffm@xxxxxxxx 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 but did too good a job since now >> it doesn't check at all. > > Of course they got removed - we've already validated the CLI input > and guaranteed that cfg->dswidth can only be zero iff cfg->dsunit is > zero in calc_stripe_factors(). > > i.e. We do input validation of CLI paramters before anything else so > that later users (like align_ag_geometry()) can assume the > parameters they are using are valid. In this case, the assumption is > that either both dsunit and dswidth are zero or that both are > non-zero and dswidth an integer multple of dsunit. It's not coming from the CLI parameters. It's coming from the topology. The blkid topology stuff is returning 8k for minimal i/o and 0 for optimal. Without a CLI config, we have dunit=0 in calc_stripe_factors, which takes it from the device. We set cfg->dsunit=16 and cfg->dswidth=0, and then head down to align_ag_geometry. The topology on this system looks like: ft = {dsunit = 16, dswidth = 0, rtswidth = 0, lsectorsize = 512, psectorsize = 512} That matches with a few of the dm targets I see reported on this system. Since minimal io size isn't sector size, we don't clear it. Talking with Eric, we should probably just extent that check in blkid_get_topology to cover that case since it's not like we can just reject what blkid gives us. >> When we hit the check to see if agsize >> is a multiple of stripe width: (cfg->agsize % cfg->dswidth), we crash >> on a divide by zero. > > What CLI config did you use to hit this? I'd like to reproduce it so > I can see where calc_stripe_factors() is going wrong.... It was just "mkfs.xfs <path-to-mpath-device>" -Jeff -- Jeff Mahoney SUSE Labs
Attachment:
signature.asc
Description: OpenPGP digital signature