svn commit: r304142 - head/usr.sbin/bsdinstall/partedit

Nathan Whitehorn nwhitehorn at freebsd.org
Fri Aug 19 16:37:28 UTC 2016



On 08/19/16 08:57, Allan Jude wrote:
> On 2016-08-19 10:13, Warner Losh wrote:
>> On Fri, Aug 19, 2016 at 12:51 AM, Dag-Erling Smørgrav <des at des.no> wrote:
>>> Warner Losh <imp at bsdimp.com> writes:
>>>> Allan Jude <allanjude at freebsd.org> writes:
>>>>> Which makes more sense:
>>>>>
>>>>> A) If stripesize == 0, use some sane value like 4096
>>>> I don't like this.
>>>>
>>>>> B) Some other combination that uses the reported stripe size, unless it
>>>>> is 0, in which case it uses 4096 (or some other value controlled by a
>>>>> different new sysctl)
>>>> Don't like this so much.
>>>>
>>>>> C) create kern.geom.min_stripe_size with a default of 512, but users can
>>>>> set 4096 if they use only 4k devices. (doesn't really solve the problem
>>>>> for the installer)
>>>> Default it to 4k, and allow users to set it to 512. If the drive
>>>> reports < this value
>>>> report this value instead.
>>> I don't like either option.  Option D (which I don't like either, but
>>> which should at least work in most cases) is a sysctl that specifies a
>>> minimum factor, and set the reported stripe size to the least common
>>> multiple of that number and the actual stripe or sector size.  This is
>>> what my bsdinstall patch does.  However, I think that pushing this down
>>> to a layer where it will affect all applications is a terrible idea,
>>> because we have no way of knowing what will break[*], and it can
>>> seriously mislead users and hinder troubleshooting - especially if it is
>>> enabled by default rather than only when necessary.
>> I took a look into the implications of doing a 4k stripesize 'automatically'
>> this morning. I found a few places in g_part where it would actively
>> hurt when coupled with gpart's insistence on aligning things. So I
>> now think it's a bad idea. This will make it harder for FreeBSD to
>> generate arbitrary disk layouts. And I'm not too sure about what
>> things like gstripe would report as a result and if this would actually
>> interfere if you had a large, but not power of two stripe size.
>>
>>> I don't think it's a good idea to enforce stripe alignment everywhere,
>>> either.  It works for partitions because they are very large relative to
>>> the stripe size, and at worst we will waste a few millionths of the disk
>>> on inter-partition gaps, which should only occur between the partition
>>> table and the boot partition, and possibly, if the stripe size is very
>>> large, between the boot partition and the swap partition.  But forcing
>>> filesystems to respect the stripe size will lead to no end of trouble,
>>> because RAID volumes can have stripe sizes of 16 kB or more.  I think it
>>> is important to align partitions during installation because of the huge
>>> performance impact of misaligned partitions on AF disks, but despite
>>> what Nathan claims, I never advocated applying the same logic
>>> everywhere.
>> Yea, having poked at it for just a little while, I agree. The installer is the
>> right place to make sure we don't cross-thread the 4k sectors. Stripe size
>> means too many other things to have it be useful in that context.
>>
>> Warner
>>
> Maybe instead we just change gpart to default to 4k alignment, but users
> can always override with -a 512 or some other value?
>
> Then the installer behaves the same as a user typing 'gpart', but we
> don't mess with the entire geom layer?
>

ZFS also looks at this, so we would need it there, and there might be a 
few others. At the very least, gpart and the installer should have the 
same behavior.

Warner, could you elaborate on what you mean by "cross-threading"? Are 
you worried about nested partition tables in which the outer one is 
misaligned? Having the disk drivers report a 4K stripesize seems to have 
worked so far; I'm not sure how reporting it more often would cause 
problems. You wouldn't want all drivers, or all GEOM layers, to do this, 
of course. If the problem is that "stripe size" has an overloaded 
meaning, we could easily add another GEOM property.
-Nathan


More information about the svn-src-head mailing list