On 6/21/18 5:36 PM, Dave Chinner wrote:
> On Thu, Jun 21, 2018 at 05:31:58PM -0400, Jeff Mahoney wrote:
>> 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.
>
> Ah, so it came from the hardware? In that case, we probably
> shouldn't zero sunit when blkid reports this whacky case. i.e. I
> think we should set swidth = sunit so that we retain allocation
> alignment to the minimum IO size the device specified.
Hrmph.
yeah, I'd really like to see
# blockdev --getiomin --getioopt --getss --getpbsz
for all the devices in the stack, I guess...
-Eric
--
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
