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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/21/18 3:49 PM, Dave Chinner wrote:
> On Thu, Jun 21, 2018 at 02:26:45PM -0500, Eric Sandeen wrote:
>> On 6/21/18 2:15 PM, Jeff Mahoney wrote:
>>> 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.
> 
> Exactly what kind of dm device does that - we've never had anyone
> report this before? Also, if it's a dm device, shouldn't it
> also be fixed to output a sane set of IO characteristics in /sys?

It's multipath, so it just follows the stacking rules.  The underlying
SCSI devices report the same numbers.  The optimal io number is
documented as being optional, at least for the kernel, so we need to
handle it being 0 anyway.  I'm not sure if the device specifying a
minimum i/o size larger than the sector size and also not specifying an
optimal i/o size is valid SCSI.  I'll ask for more information since now
I'm also curious.

-Jeff


-- 
Jeff Mahoney
SUSE Labs

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux