Re: [PATCH] mkfs: fix divide-by-zero in align_ag_geometry

[Jeff Mahoney <jeffm@suse.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2037 bytes --]

On 6/20/18 11:57 PM, Dave Chinner wrote:
> On Wed, Jun 20, 2018 at 10:55:20PM -0400, jeffm@suse.com wrote:
>> From: Jeff Mahoney <jeffm@suse.com>
>>
>> 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]