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

[Nathan Whitehorn]

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  wrote:

Warner Losh  writes:

Allan Jude  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
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Nathan Whitehorn]

On 08/18/16 22:33, Warner Losh wrote:

On Aug 18, 2016, at 11:21 PM, Nathan Whitehorn  wrote:



On 08/18/16 21:15, Warner Losh wrote:

On Thu, Aug 18, 2016 at 6:56 AM, Allan Jude  wrote:

On 08/18/16 05:50 AM, Dag-Erling Smørgrav wrote:

Nathan Whitehorn  writes:

OK. In which configurations? My Dell servers, for instance, don't do
this. How are they set up? What drivers are being used? Is this
something that affects passthrough disks, RAIDs, disk images?

Most LSI MegaRAID controllers don't have real passthrough, only JBOD.
You can query the drive with "camcontrol identify passX", but the
controller does not report a stripe size for the volume (mfidY).


The point is that *if the reported stripe size is wrong*, more things
than partition alignment in the installer will suffer for it.

It's not wrong, it's non-existent, and I'm getting really tired of
repeating myself.


Fixing the installer with a bandaid in the run-up to a release is
fine, but *we need to fix the underlying problem*.

We can't, because hardware sucks, and I'm getting really tired of
repeating myself.

DES


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. You'll need to make this a tunable. Then the upper
layers wouldn't care. There's a small chance that some SD cards might be
reporting values that are too large. But I think it is confined to SD cards and
if I see too many more I'll do something specific in the SD driver.

Warner



That sounds good to me and I think can clean up a lot of code and potential 
foot-shooting. Who is planning to make the patch? I'm happy to do anything that 
would be helpful.

The patch is super-easy, but I need to get the concept validated and make sure 
that it does not have unintended side effects.

Warner

Sounds great. There is no urgency here -- we have a good solution for 
11.0 already -- so taking time to do it right sounds good. I believe 
stripesize is only consumed by disk formatting tools, so unintended side 
effects at least should be minimal.

-Nathan
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Allan Jude]

On 2016-08-19 10:13, Warner Losh wrote:
> On Fri, Aug 19, 2016 at 12:51 AM, Dag-Erling Smørgrav  wrote:
>> Warner Losh  writes:
>>> Allan Jude  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?

-- 
Allan Jude



signature.asc
Description: OpenPGP digital signature

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

[Allan Jude]

On 2016-08-19 00:15, Warner Losh wrote:
>> 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. You'll need to make this a tunable. Then the upper
> layers wouldn't care. There's a small chance that some SD cards might be
> reporting values that are too large. But I think it is confined to SD cards 
> and
> if I see too many more I'll do something specific in the SD driver.
> 
> Warner
> 

I think I mentioned this earlier in the thread,, but I do have a USB
thumbstick that reports an 8mb stripe size.

But I agree, I think a sysctl where you set the minimum (default 4096,
but settable to 512), that is a tunable that can be overwritten in
loader.conf

What do we want for the logic as to what values it can be set to?

Any power of 2 greater than or equal to 512?

-- 
Allan Jude



signature.asc
Description: OpenPGP digital signature

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

[Warner Losh]

On Fri, Aug 19, 2016 at 12:51 AM, Dag-Erling Smørgrav  wrote:
> Warner Losh  writes:
>> Allan Jude  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
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Dag-Erling Smørgrav]

Warner Losh  writes:
> Allan Jude  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 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.

[*] Apart from audio CDs.  We already know that it will break those.

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Glen Barber]

On Fri, Aug 19, 2016 at 08:21:49AM +0200, Dag-Erling Smørgrav wrote:
> Nathan Whitehorn  writes:
> > This is ridiculous. I've asked a series of technical questions about
> > generalizing a patch you made and that I think is a good idea. In
> > response, those questions have been met with a non-stop torrent of
> > insults and abuse instead of answers, with only one eventual nugget of
> > information in response to one of them -- that you ran into problems
> > with mfid -- to redeem it. I'm done with this discussion.
> 
> I suggest you go and read through the discussion again.  I gave you
> plenty of information, mentioning LSI and hypervisors, for instance, and
> even, as I grew increasingly frustrated by your stonewalling, showed you
> how to inspect the code to confirm that most RAID drivers do not even
> try to report a stripe size.  Meanwhile, you steadfastedly refused to
> address my arguments, simply repeating your position again and again,
> and insisting that the fact that you have never encountered any systems
> that get the stripe size wrong (which I don't believe, but I'm willing
> to believe that you've never noticed) invalidates the fact that I have.
> Not to mention your steadfast defense of the non-existent purity of the
> installer.  When Warner finally intervened, you switched to setting up
> strawmen, asking me to pick and defend one of two positions which I've
> never held.  I'm not going to play that game.
> 
> > Hopefully we can have a real conversation about this at this some
> > point after the release.
> 
> Likewise, but I'm not holding my breath.
> 

I'll mediate this then.  Not right now.  I think everyone needs to step
back for a bit.

Glen



signature.asc
Description: PGP signature

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

[Dag-Erling Smørgrav]

Nathan Whitehorn  writes:
> This is ridiculous. I've asked a series of technical questions about
> generalizing a patch you made and that I think is a good idea. In
> response, those questions have been met with a non-stop torrent of
> insults and abuse instead of answers, with only one eventual nugget of
> information in response to one of them -- that you ran into problems
> with mfid -- to redeem it. I'm done with this discussion.

I suggest you go and read through the discussion again.  I gave you
plenty of information, mentioning LSI and hypervisors, for instance, and
even, as I grew increasingly frustrated by your stonewalling, showed you
how to inspect the code to confirm that most RAID drivers do not even
try to report a stripe size.  Meanwhile, you steadfastedly refused to
address my arguments, simply repeating your position again and again,
and insisting that the fact that you have never encountered any systems
that get the stripe size wrong (which I don't believe, but I'm willing
to believe that you've never noticed) invalidates the fact that I have.
Not to mention your steadfast defense of the non-existent purity of the
installer.  When Warner finally intervened, you switched to setting up
strawmen, asking me to pick and defend one of two positions which I've
never held.  I'm not going to play that game.

> Hopefully we can have a real conversation about this at this some
> point after the release.

Likewise, but I'm not holding my breath.

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Warner Losh]

> On Aug 18, 2016, at 11:21 PM, Nathan Whitehorn  wrote:
> 
> 
> 
> On 08/18/16 21:15, Warner Losh wrote:
>> On Thu, Aug 18, 2016 at 6:56 AM, Allan Jude  wrote:
>>> On 08/18/16 05:50 AM, Dag-Erling Smørgrav wrote:
 Nathan Whitehorn  writes:
> OK. In which configurations? My Dell servers, for instance, don't do
> this. How are they set up? What drivers are being used? Is this
> something that affects passthrough disks, RAIDs, disk images?
 Most LSI MegaRAID controllers don't have real passthrough, only JBOD.
 You can query the drive with "camcontrol identify passX", but the
 controller does not report a stripe size for the volume (mfidY).
 
> The point is that *if the reported stripe size is wrong*, more things
> than partition alignment in the installer will suffer for it.
 It's not wrong, it's non-existent, and I'm getting really tired of
 repeating myself.
 
> Fixing the installer with a bandaid in the run-up to a release is
> fine, but *we need to fix the underlying problem*.
 We can't, because hardware sucks, and I'm getting really tired of
 repeating myself.
 
 DES
 
>>> 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. You'll need to make this a tunable. Then the upper
>> layers wouldn't care. There's a small chance that some SD cards might be
>> reporting values that are too large. But I think it is confined to SD cards 
>> and
>> if I see too many more I'll do something specific in the SD driver.
>> 
>> Warner
>> 
>> 
> 
> That sounds good to me and I think can clean up a lot of code and potential 
> foot-shooting. Who is planning to make the patch? I'm happy to do anything 
> that would be helpful.

The patch is super-easy, but I need to get the concept validated and make sure 
that it does not have unintended side effects.

Warner



signature.asc
Description: Message signed with OpenPGP using GPGMail

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

[Nathan Whitehorn]

On 08/18/16 21:15, Warner Losh wrote:

On Thu, Aug 18, 2016 at 6:56 AM, Allan Jude  wrote:

On 08/18/16 05:50 AM, Dag-Erling Smørgrav wrote:

Nathan Whitehorn  writes:

OK. In which configurations? My Dell servers, for instance, don't do
this. How are they set up? What drivers are being used? Is this
something that affects passthrough disks, RAIDs, disk images?

Most LSI MegaRAID controllers don't have real passthrough, only JBOD.
You can query the drive with "camcontrol identify passX", but the
controller does not report a stripe size for the volume (mfidY).


The point is that *if the reported stripe size is wrong*, more things
than partition alignment in the installer will suffer for it.

It's not wrong, it's non-existent, and I'm getting really tired of
repeating myself.


Fixing the installer with a bandaid in the run-up to a release is
fine, but *we need to fix the underlying problem*.

We can't, because hardware sucks, and I'm getting really tired of
repeating myself.

DES


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. You'll need to make this a tunable. Then the upper
layers wouldn't care. There's a small chance that some SD cards might be
reporting values that are too large. But I think it is confined to SD cards and
if I see too many more I'll do something specific in the SD driver.

Warner




That sounds good to me and I think can clean up a lot of code and 
potential foot-shooting. Who is planning to make the patch? I'm happy to 
do anything that would be helpful.

-Nathan

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Warner Losh]

On Thu, Aug 18, 2016 at 6:56 AM, Allan Jude  wrote:
> On 08/18/16 05:50 AM, Dag-Erling Smørgrav wrote:
>> Nathan Whitehorn  writes:
>>> OK. In which configurations? My Dell servers, for instance, don't do
>>> this. How are they set up? What drivers are being used? Is this
>>> something that affects passthrough disks, RAIDs, disk images?
>>
>> Most LSI MegaRAID controllers don't have real passthrough, only JBOD.
>> You can query the drive with "camcontrol identify passX", but the
>> controller does not report a stripe size for the volume (mfidY).
>>
>>> The point is that *if the reported stripe size is wrong*, more things
>>> than partition alignment in the installer will suffer for it.
>>
>> It's not wrong, it's non-existent, and I'm getting really tired of
>> repeating myself.
>>
>>> Fixing the installer with a bandaid in the run-up to a release is
>>> fine, but *we need to fix the underlying problem*.
>>
>> We can't, because hardware sucks, and I'm getting really tired of
>> repeating myself.
>>
>> DES
>>
>
> 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. You'll need to make this a tunable. Then the upper
layers wouldn't care. There's a small chance that some SD cards might be
reporting values that are too large. But I think it is confined to SD cards and
if I see too many more I'll do something specific in the SD driver.

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Nathan Whitehorn]

On 08/18/16 08:12, Dag-Erling Smørgrav wrote:

Nathan Whitehorn  writes:

We have a mechanism (GEOM stripe size) for drivers to supply a default
alignment to userland. If we think we can get that right, great. If we
don't think we can get it right, the default system policy in the
absence of real information from drivers should be modified to report
a number that we think is more likely to be safe than the current
defaults (the logical sector size, usually 512 bytes) and potentially
tunable by the user. Hacking the userland tools one-by-one to impose
their own default policies to override the systemwide one is, while a
perfectly valid stopgap right before a release, a ridiculous long-term
solution. Do you disagree with any of that?

I'll tell you whether I agree or disagree when you stop putting words in
my mouth.

DES


This is ridiculous. I've asked a series of technical questions about 
generalizing a patch you made and that I think is a good idea. In 
response, those questions have been met with a non-stop torrent of 
insults and abuse instead of answers, with only one eventual nugget of 
information in response to one of them -- that you ran into problems 
with mfid -- to redeem it. I'm done with this discussion.


Hopefully we can have a real conversation about this at this some point 
after the release.

-Nathan
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Nathan Whitehorn]

On 08/18/16 05:56, Allan Jude wrote:

On 08/18/16 05:50 AM, Dag-Erling Smørgrav wrote:

Nathan Whitehorn  writes:

OK. In which configurations? My Dell servers, for instance, don't do
this. How are they set up? What drivers are being used? Is this
something that affects passthrough disks, RAIDs, disk images?

Most LSI MegaRAID controllers don't have real passthrough, only JBOD.
You can query the drive with "camcontrol identify passX", but the
controller does not report a stripe size for the volume (mfidY).


The point is that *if the reported stripe size is wrong*, more things
than partition alignment in the installer will suffer for it.

It's not wrong, it's non-existent, and I'm getting really tired of
repeating myself.


Fixing the installer with a bandaid in the run-up to a release is
fine, but *we need to fix the underlying problem*.

We can't, because hardware sucks, and I'm getting really tired of
repeating myself.

DES


Which makes more sense:

A) If stripesize == 0, use some sane value like 4096

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)

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)


I think "B" does what we want here best, but any of these approaches 
would work well.

-Nathan
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Dag-Erling Smørgrav]

Nathan Whitehorn  writes:
> We have a mechanism (GEOM stripe size) for drivers to supply a default
> alignment to userland. If we think we can get that right, great. If we
> don't think we can get it right, the default system policy in the
> absence of real information from drivers should be modified to report
> a number that we think is more likely to be safe than the current
> defaults (the logical sector size, usually 512 bytes) and potentially
> tunable by the user. Hacking the userland tools one-by-one to impose
> their own default policies to override the systemwide one is, while a
> perfectly valid stopgap right before a release, a ridiculous long-term
> solution. Do you disagree with any of that?

I'll tell you whether I agree or disagree when you stop putting words in
my mouth.

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Nathan Whitehorn]

On 08/18/16 02:50, Dag-Erling Smørgrav wrote:

Nathan Whitehorn  writes:

OK. In which configurations? My Dell servers, for instance, don't do
this. How are they set up? What drivers are being used? Is this
something that affects passthrough disks, RAIDs, disk images?

Most LSI MegaRAID controllers don't have real passthrough, only JBOD.
You can query the drive with "camcontrol identify passX", but the
controller does not report a stripe size for the volume (mfidY).


OK, so it's mfid. That's good to know.



The point is that *if the reported stripe size is wrong*, more things
than partition alignment in the installer will suffer for it.

It's not wrong, it's non-existent, and I'm getting really tired of
repeating myself.


For some drivers, this interface is not implemented. This is a bug, 
which should be fixed.





Fixing the installer with a bandaid in the run-up to a release is
fine, but *we need to fix the underlying problem*.

We can't, because hardware sucks, and I'm getting really tired of
repeating myself.

DES


As am I. Here's the point:

We have a mechanism (GEOM stripe size) for drivers to supply a default 
alignment to userland. If we think we can get that right, great. If we 
don't think we can get it right, the default system policy in the 
absence of real information from drivers should be modified to report a 
number that we think is more likely to be safe than the current defaults 
(the logical sector size, usually 512 bytes) and potentially tunable by 
the user. Hacking the userland tools one-by-one to impose their own 
default policies to override the systemwide one is, while a perfectly 
valid stopgap right before a release, a ridiculous long-term solution. 
Do you disagree with any of that?

-Nathan
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Slawa Olhovchenkov]

On Thu, Aug 18, 2016 at 08:56:55AM -0400, Allan Jude wrote:

> A) If stripesize == 0, use some sane value like 4096
> 
> 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)
> 
> 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)

As I am already pointed default mus be 4096, user can reduce to 512
(or 256/128).
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Allan Jude]

On 08/18/16 05:50 AM, Dag-Erling Smørgrav wrote:
> Nathan Whitehorn  writes:
>> OK. In which configurations? My Dell servers, for instance, don't do
>> this. How are they set up? What drivers are being used? Is this
>> something that affects passthrough disks, RAIDs, disk images?
> 
> Most LSI MegaRAID controllers don't have real passthrough, only JBOD.
> You can query the drive with "camcontrol identify passX", but the
> controller does not report a stripe size for the volume (mfidY).
> 
>> The point is that *if the reported stripe size is wrong*, more things
>> than partition alignment in the installer will suffer for it.
> 
> It's not wrong, it's non-existent, and I'm getting really tired of
> repeating myself.
> 
>> Fixing the installer with a bandaid in the run-up to a release is
>> fine, but *we need to fix the underlying problem*.
> 
> We can't, because hardware sucks, and I'm getting really tired of
> repeating myself.
> 
> DES
> 

Which makes more sense:

A) If stripesize == 0, use some sane value like 4096

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)

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)



-- 
Allan Jude
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Dag-Erling Smørgrav]

Nathan Whitehorn  writes:
> OK. In which configurations? My Dell servers, for instance, don't do
> this. How are they set up? What drivers are being used? Is this
> something that affects passthrough disks, RAIDs, disk images?

Most LSI MegaRAID controllers don't have real passthrough, only JBOD.
You can query the drive with "camcontrol identify passX", but the
controller does not report a stripe size for the volume (mfidY).

> The point is that *if the reported stripe size is wrong*, more things
> than partition alignment in the installer will suffer for it.

It's not wrong, it's non-existent, and I'm getting really tired of
repeating myself.

> Fixing the installer with a bandaid in the run-up to a release is
> fine, but *we need to fix the underlying problem*.

We can't, because hardware sucks, and I'm getting really tired of
repeating myself.

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Andriy Gapon]

On 17/08/2016 19:36, Nathan Whitehorn wrote:
> OK, so then what is the solution here? We have a number of tools that need to
> know this information: gpart, sade, bsdinstall, zfs, graid, etc. If we want to
> have a consistent set of defaults -- for example, to use 4K across the board,
> which I think is a good idea -- there should be a central place to set this 
> that
> does not involve hacking a variety of independent tools. Do you disagree?
> 
> I don't care how that happens. It happens that the way we currently encode 
> this
> is geom stripesize. If we feel like we can't get this right in drivers, then 
> we
> should provide a tunable to set a minimum default alignment. You could 
> implement
> this in lots of different ways. But having static values hardcoded in random
> places that makes similar tools (sade, gpart) behave inconsistently cannot
> possibly be the answer.
> 
> This is my point, from beginning to end. Is there any reason -- at all -- that
> we should prefer per-tool one-off changes to fixing the central mechanism we
> already have to give consistent results that we think are reliable?

It would be perfect to get a correct description of a disk and to do that in
central place.  But still I, as a user / administrator, want to be able to
_force_ the alignment that I want when I partition a disk, create a filesystem,
etc.  That is, even if the kernel reports the perfectly correct information and
the tools know how to automatically do what's best for me, I still want to eb
able to override.  And I think that installers and administrative tools should
provide a way to do that.  And many already do, e.g. 'gpart add -a X'.
So, I do not see how striving for the better disk detection (in a central place)
and having more knobs in the administration tools are mutually exclusive or
conflicting goals.

Just my two bits.
-- 
Andriy Gapon
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Nathan Whitehorn]

On 08/17/16 10:56, Andriy Gapon wrote:

On 17/08/2016 19:36, Nathan Whitehorn wrote:

OK, so then what is the solution here? We have a number of tools that need to
know this information: gpart, sade, bsdinstall, zfs, graid, etc. If we want to
have a consistent set of defaults -- for example, to use 4K across the board,
which I think is a good idea -- there should be a central place to set this that
does not involve hacking a variety of independent tools. Do you disagree?

I don't care how that happens. It happens that the way we currently encode this
is geom stripesize. If we feel like we can't get this right in drivers, then we
should provide a tunable to set a minimum default alignment. You could implement
this in lots of different ways. But having static values hardcoded in random
places that makes similar tools (sade, gpart) behave inconsistently cannot
possibly be the answer.

This is my point, from beginning to end. Is there any reason -- at all -- that
we should prefer per-tool one-off changes to fixing the central mechanism we
already have to give consistent results that we think are reliable?

It would be perfect to get a correct description of a disk and to do that in
central place.  But still I, as a user / administrator, want to be able to
_force_ the alignment that I want when I partition a disk, create a filesystem,
etc.  That is, even if the kernel reports the perfectly correct information and
the tools know how to automatically do what's best for me, I still want to eb
able to override.  And I think that installers and administrative tools should
provide a way to do that.  And many already do, e.g. 'gpart add -a X'.
So, I do not see how striving for the better disk detection (in a central place)
and having more knobs in the administration tools are mutually exclusive or
conflicting goals.

Just my two bits.
Agreed 100%. The issue here is that this kind of patch unconditionally 
overrides the kernel in an unsettable way. I think the right scheme is:

1. Try to get the real values as much as possible.
2. Provide a global hint to all tools that you want some value (e.g. 4K) 
unless the drivers are *sure* it's the wrong answer.

3. Have options in individual tools to force other values.

We have (1), though it can probably be improved, and (3) and just need (2).
-Nathan
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Nathan Whitehorn]

On 08/17/16 08:57, Warner Losh wrote:

On Wed, Aug 17, 2016 at 9:26 AM, Nathan Whitehorn
 wrote:

Not true at all. All modern disks report their physical sector size, as
distinct from the logical one, in their ATA IDENTIFY data and ata_da.c uses
that.

You are correct that there are two fields in the ATA IDENTIFY. However,
you are incorrect in thinking that all modern disks report their actual
physical sector size instead of a 'compatibility' number. We have dozens
of quirks and are adding them at the rate of a couple a month to make the
current imperfect magic happen.


There is also a small quirks table for some older spinning disks and a
few SSDs that lie and mostly hasn't needed additions in quite some time.
camcontrol identify correctly reports 4096 for the physical sector size on 5
different random AF-512e disks I just checked (some of those are also,
redundantly, in the quirks table). Since this seems to have become the
standard, I can't imagine that the quirks table would need to grow much in
the future for this issue.

Any yet the table continues to grow. As someone who evaluates disks for a large
streaming media company, you cannot begin to imagine the number of things
that vendors get wrong... Life would be so much easier if you could actually
trust vendors to report things correctly in their ATA IDENTIFY command.
SCSI is better, but still not perfect.

You should really listen to people that have been on the front line here.
CAM does a decent job of getting things right. It isn't perfect and can never
be perfect. Expecting it to magically change to be perfect is unreasonable
and will literally never happen.

Warner



OK, so then what is the solution here? We have a number of tools that 
need to know this information: gpart, sade, bsdinstall, zfs, graid, etc. 
If we want to have a consistent set of defaults -- for example, to use 
4K across the board, which I think is a good idea -- there should be a 
central place to set this that does not involve hacking a variety of 
independent tools. Do you disagree?


I don't care how that happens. It happens that the way we currently 
encode this is geom stripesize. If we feel like we can't get this right 
in drivers, then we should provide a tunable to set a minimum default 
alignment. You could implement this in lots of different ways. But 
having static values hardcoded in random places that makes similar tools 
(sade, gpart) behave inconsistently cannot possibly be the answer.


This is my point, from beginning to end. Is there any reason -- at all 
-- that we should prefer per-tool one-off changes to fixing the central 
mechanism we already have to give consistent results that we think are 
reliable?

-Nathan

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Nathan Whitehorn]

On 08/17/16 08:45, Dag-Erling Smørgrav wrote:

Nathan Whitehorn  writes:

Dag-Erling Smørgrav  writes:

I have mentioned several examples to you, and even told you how to
confirm, by inspecting the source code, that most drivers do *not*
set the stripe size.

Most drivers? Yes, sure. Most drivers that people use? No.

Every.  Single.  Dell.  Server.

Probably every single HP server as well, I haven't checked.

VirtualBox gets it wrong.  Xen gets it wrong.  I believe KVM (including
RHEV) and VMWare get it wrong too, but haven't been able to verify.

I guess those aren't important to you?

DES


OK. In which configurations? My Dell servers, for instance, don't do 
this. How are they set up? What drivers are being used? Is this 
something that affects passthrough disks, RAIDs, disk images?


The point is that *if the reported stripe size is wrong*, more things 
than partition alignment in the installer will suffer for it. Fixing the 
installer with a bandaid in the run-up to a release is fine, but *we 
need to fix the underlying problem*.

-Nathan
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Warner Losh]

On Wed, Aug 17, 2016 at 10:04 AM, Warner Losh  wrote:
> On Wed, Aug 17, 2016 at 9:45 AM, Dag-Erling Smørgrav  wrote:
>> Nathan Whitehorn  writes:
>>> Dag-Erling Smørgrav  writes:
>>> > I have mentioned several examples to you, and even told you how to
>>> > confirm, by inspecting the source code, that most drivers do *not*
>>> > set the stripe size.
>>> Most drivers? Yes, sure. Most drivers that people use? No.
>>
>> Every.  Single.  Dell.  Server.
>>
>> Probably every single HP server as well, I haven't checked.
>>
>> VirtualBox gets it wrong.  Xen gets it wrong.  I believe KVM (including
>> RHEV) and VMWare get it wrong too, but haven't been able to verify.
>>
>> I guess those aren't important to you?
>
> Also, SD cards get it wrong. It is reported, true enough, but it is often
> wrong in the sense it can be suboptimal. Different SD cards report
> different things for the number of sectors to erase at a time. Some
> vendors do an excellent job of reporting the optimal amount. Others
> do not (often because

[stupid gmail]

often because they copy from last year's design. I've had to override
the reported size numerous times to get decent performance from
SD cards in the past. Almost all of the recent embedded boards
that FreeBSD supports use SD cards. While we provide images
to just boot off of, there are several designs that have both a SD
card and an embedded eMMC or similar card that we should be
able to run the installer on.

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Warner Losh]

On Wed, Aug 17, 2016 at 9:45 AM, Dag-Erling Smørgrav  wrote:
> Nathan Whitehorn  writes:
>> Dag-Erling Smørgrav  writes:
>> > I have mentioned several examples to you, and even told you how to
>> > confirm, by inspecting the source code, that most drivers do *not*
>> > set the stripe size.
>> Most drivers? Yes, sure. Most drivers that people use? No.
>
> Every.  Single.  Dell.  Server.
>
> Probably every single HP server as well, I haven't checked.
>
> VirtualBox gets it wrong.  Xen gets it wrong.  I believe KVM (including
> RHEV) and VMWare get it wrong too, but haven't been able to verify.
>
> I guess those aren't important to you?

Also, SD cards get it wrong. It is reported, true enough, but it is often
wrong in the sense it can be suboptimal. Different SD cards report
different things for the number of sectors to erase at a time. Some
vendors do an excellent job of reporting the optimal amount. Others
do not (often because the

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Warner Losh]

On Wed, Aug 17, 2016 at 9:26 AM, Nathan Whitehorn
 wrote:
> Not true at all. All modern disks report their physical sector size, as
> distinct from the logical one, in their ATA IDENTIFY data and ata_da.c uses
> that.

You are correct that there are two fields in the ATA IDENTIFY. However,
you are incorrect in thinking that all modern disks report their actual
physical sector size instead of a 'compatibility' number. We have dozens
of quirks and are adding them at the rate of a couple a month to make the
current imperfect magic happen.

> There is also a small quirks table for some older spinning disks and a
> few SSDs that lie and mostly hasn't needed additions in quite some time.
> camcontrol identify correctly reports 4096 for the physical sector size on 5
> different random AF-512e disks I just checked (some of those are also,
> redundantly, in the quirks table). Since this seems to have become the
> standard, I can't imagine that the quirks table would need to grow much in
> the future for this issue.

Any yet the table continues to grow. As someone who evaluates disks for a large
streaming media company, you cannot begin to imagine the number of things
that vendors get wrong... Life would be so much easier if you could actually
trust vendors to report things correctly in their ATA IDENTIFY command.
SCSI is better, but still not perfect.

You should really listen to people that have been on the front line here.
CAM does a decent job of getting things right. It isn't perfect and can never
be perfect. Expecting it to magically change to be perfect is unreasonable
and will literally never happen.

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Warner Losh]

On Wed, Aug 17, 2016 at 8:36 AM, Nathan Whitehorn
 wrote:

> As for grepping, the CAM disk drivers are all in sys/cam, not sys/dev, as
> I'm sure you know, and you will find all the code that handles this there.

There's at least a dozen disk drivers that aren't CAM.

The code in CAM that handles this is, at best, a set of heuristics
that can and does get it wrong.

These two facts alone severely undermine your argument.

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Dag-Erling Smørgrav]

Nathan Whitehorn  writes:
> Dag-Erling Smørgrav  writes:
> > I have mentioned several examples to you, and even told you how to
> > confirm, by inspecting the source code, that most drivers do *not*
> > set the stripe size.
> Most drivers? Yes, sure. Most drivers that people use? No.

Every.  Single.  Dell.  Server.

Probably every single HP server as well, I haven't checked.

VirtualBox gets it wrong.  Xen gets it wrong.  I believe KVM (including
RHEV) and VMWare get it wrong too, but haven't been able to verify.

I guess those aren't important to you?

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Dag-Erling Smørgrav]

Hans Petter Selasky  writes:
> My intention is not to install FreeBSD on a 3K disk. My question is
> pure mathematical, if the bsdinstall will segfault, division by zero
> or anything like that, if one should try to install FreeBSD on a 3K
> disk, because you round up the size of the disk to be bigger than it
> actually is.

The code in question computes the offset and size of the largest block
of contiguous free space on a disk that already has a partition table.
If you even got this far, it would round the offset up to 4096 and the
size down by and equivalent amount and you'd get an error box saying "No
free space left on device".

Say you have a 3584-byte provider with 512-byte sectors formatted with
MBR, leaving 3072 bytes (6 sectors) free at offset 512 (1); stripe size
is unreported and is arbitrarily set to the smallest common multiple of
512 and 4096, which is 4096; misalignment is 512 % 4096 == 512 bytes;
adjustment is (4096 - 512) / 512 == 7 sectors; adjusted offset is 8
sectors, adjusted size is -1 sectors which the caller rejects (and yes,
the variables are unsigned and the check is for <= 0, not == 0, so
everything works as intended).

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Nathan Whitehorn]

On 08/17/16 08:03, Dag-Erling Smørgrav wrote:

Nathan Whitehorn  writes:

Dag-Erling Smørgrav  writes:

[...]  And you keep refusing to address the fact that most drivers
don't report a stripe size, except by repeating your claim that they
do, with no evidence to back it up.  Feel free to 'grep -r
stripesize /usr/src/sys/dev'.  Go on, I'll wait.

And yet, if you look at the GEOM XML, it is reported and there. And,
look, it's even right for the AF 512e disks in my machine!

Yes, that is one of the few cases where we get it right, as previously
mentioned.  But it only works because it's known to us (listed in the
quirk table) and directly attached.  If you replace that drive with a
brand new one a year from now, you have no guarantee that the new drive
will be recognized as an AF drive.


Not true at all. All modern disks report their physical sector size, as 
distinct from the logical one, in their ATA IDENTIFY data and ata_da.c 
uses that. There is also a small quirks table for some older spinning 
disks and a few SSDs that lie and mostly hasn't needed additions in 
quite some time. camcontrol identify correctly reports 4096 for the 
physical sector size on 5 different random AF-512e disks I just checked 
(some of those are also, redundantly, in the quirks table). Since this 
seems to have become the standard, I can't imagine that the quirks table 
would need to grow much in the future for this issue.





I've literally never seen a case where we don't already do the right
thing here.

The I can only conclude that you have very little real-world experience.
I have mentioned several examples to you, and even told you how to
confirm, by inspecting the source code, that most drivers do *not* set
the stripe size.


Most drivers? Yes, sure. Most drivers that people use? No. And I am 
aware of how to use grep, thanks.





It's correct, as far as I can tell, 100% of the time on all possible
variants of AF disks.

You keep repeating this, as if it somehow proves me wrong.  It doesn't.


One could argue that calling this the "stripesize" is a hack, and I
would agree,

One could, but one would be wrong.

As for grepping, the CAM disk drivers are all in sys/cam, not sys/dev,
as I'm sure you know, and you will find all the code that handles this
there.

Only for directly attached drives, and most drives do not report the
correct physical sector size, which is why we have quirk tables.


Which disks? Your original email said that VMware reports too-small 
values and that there is a bug in the LSI MegaRAID driver. I'm not sure 
which actual drivers those were (e.g. mfid or through CAM), or how they 
were set up (RAID or passthrough), but that's a small list and those 
issues can presumably be fixed. I would be happy to help make those 
changes if you can provide some more information.



We should just fix the driver for whatever weird disk you have in your
machine (what is it, by the way?).

Oh please.  Now you're just being an .


I somehow still don't know what problem, on what hardware, you are 
actually trying to fix. Instead, I get delightful personal invective.


The patch is fine for 11.0. I would like to know what bugs it was 
fixing, so we can fix them more broadly after the release. Is that so 
much to ask?

-Nathan


DES


___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Hans Petter Selasky]

On 08/17/16 17:05, Dag-Erling Smørgrav wrote:

Hans Petter Selasky  writes:

Not sure if it is an issue, but what will happen if a magic disk has a
size less than 4K and uses a block size of 512bytes and the disk
alignment gets rounded up to 4K. Will any of logic in this patch fail
or hang?


What is a magic disk, and why would you want to install FreeBSD on a
drive with less than 4096 bytes available?



Hi,

My intention is not to install FreeBSD on a 3K disk. My question is pure 
mathematical, if the bsdinstall will segfault, division by zero or 
anything like that, if one should try to install FreeBSD on a 3K disk, 
because you round up the size of the disk to be bigger than it actually is.


Should there be a check for too small disks in there?

--HPS
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Slawa Olhovchenkov]

On Wed, Aug 17, 2016 at 05:00:16PM +0200, Hans Petter Selasky wrote:

> On 08/17/16 16:49, Slawa Olhovchenkov wrote:
> > On Wed, Aug 17, 2016 at 07:36:00AM -0700, Nathan Whitehorn wrote:
> >
> 
> > In long term, prefered aligment is forsing 4k (or may be more):
> > install system on 512b [mirror] disk aligment now may be need required 
> > replace
> > disk to 4k aligment. For more flexsible in future now best chois is 4k
> > or more.
> 
> Hi,
> 
> Not sure if it is an issue, but what will happen if a magic disk has a 
> size less than 4K and uses a block size of 512bytes and the disk 
> alignment gets rounded up to 4K. Will any of logic in this patch fail or 
> hang?

I am not sure about understunding you.
Do you talk about disk total size of 1536 bytes and less?
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Dag-Erling Smørgrav]

Hans Petter Selasky  writes:
> Not sure if it is an issue, but what will happen if a magic disk has a
> size less than 4K and uses a block size of 512bytes and the disk
> alignment gets rounded up to 4K. Will any of logic in this patch fail
> or hang?

What is a magic disk, and why would you want to install FreeBSD on a
drive with less than 4096 bytes available?

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Nathan Whitehorn]

On 08/17/16 07:49, Slawa Olhovchenkov wrote:

On Wed, Aug 17, 2016 at 07:36:00AM -0700, Nathan Whitehorn wrote:


Your contention that the installer does not make policy decisions is
equally spurious.  The installer makes many policy decisions, including
the disk layout, the size of the swap partition, the name of the pool,
the use of boot environments (which I dislike but am not allowed to
override), the number of filesets and their mountpoints (which I also
dislike and am not allowed to override either), etc.  The Unix
philosophy is to push such decisions up the stack, not down.  The
decision to align partitions on 4096-byte boundaries because we're not
sure of the correct number but know for a fact that using a smaller
number can have a huge impact on performance is the installer's to make.

Those are all things that the operating system does not have defaults
for: there are no tools like, say, gpart or newfs that layout disks in
any even vaguely automated way, and so no tools that would ever have
defaults for, say, the size of a swap partition except for the
installer. As such, the defaults are quite properly in the installer.
This is quite different: there are many tools that care about disk
alignment (say, gpart) and, by default, use the GEOM stripesize. The
installer is, after this patch, overriding what was meant to be a
system-wide default.

My concern is that pushing this into the installer means that newfs,
zfs, gpart, etc., which all look at the GEOM stripesize for preferred
alignment, will still have suboptimal behavior on systems affected by
your patch. If we identified which drivers are reporting the wrong
alignment, we could fix the whole system at a go by changing it there.
As it is, we now have inconsistent default behavior for partitions
between tools (the installer and sade will now use a different alignment
than gpart on whatever systems you were trying to fix here) and between
pre- and post-installation environments.

In long term, prefered aligment is forsing 4k (or may be more):
install system on 512b [mirror] disk aligment now may be need required replace
disk to 4k aligment. For more flexsible in future now best chois is 4k
or more.


For future-proofing in such circumstances, it might be worth expanding 
the vfs.zfs.min_auto_ashift tunable into some global thing that the disk 
drivers (or geom_disk) read to round up the physical sector sizes they 
would otherwise report. That would ensure that gpart, zfs, etc. all 
behave consistently in such cases.

-Nathan

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Dag-Erling Smørgrav]

Nathan Whitehorn  writes:
> Dag-Erling Smørgrav  writes:
> > [...]  And you keep refusing to address the fact that most drivers
> > don't report a stripe size, except by repeating your claim that they
> > do, with no evidence to back it up.  Feel free to 'grep -r
> > stripesize /usr/src/sys/dev'.  Go on, I'll wait.
> And yet, if you look at the GEOM XML, it is reported and there. And,
> look, it's even right for the AF 512e disks in my machine!

Yes, that is one of the few cases where we get it right, as previously
mentioned.  But it only works because it's known to us (listed in the
quirk table) and directly attached.  If you replace that drive with a
brand new one a year from now, you have no guarantee that the new drive
will be recognized as an AF drive.

> I've literally never seen a case where we don't already do the right
> thing here.

The I can only conclude that you have very little real-world experience.
I have mentioned several examples to you, and even told you how to
confirm, by inspecting the source code, that most drivers do *not* set
the stripe size.

> It's correct, as far as I can tell, 100% of the time on all possible
> variants of AF disks.

You keep repeating this, as if it somehow proves me wrong.  It doesn't.

> One could argue that calling this the "stripesize" is a hack, and I
> would agree,

One could, but one would be wrong.

> As for grepping, the CAM disk drivers are all in sys/cam, not sys/dev,
> as I'm sure you know, and you will find all the code that handles this
> there.

Only for directly attached drives, and most drives do not report the
correct physical sector size, which is why we have quirk tables.

> We should just fix the driver for whatever weird disk you have in your
> machine (what is it, by the way?).

Oh please.  Now you're just being an .

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Hans Petter Selasky]

On 08/17/16 16:49, Slawa Olhovchenkov wrote:

On Wed, Aug 17, 2016 at 07:36:00AM -0700, Nathan Whitehorn wrote:




In long term, prefered aligment is forsing 4k (or may be more):
install system on 512b [mirror] disk aligment now may be need required replace
disk to 4k aligment. For more flexsible in future now best chois is 4k
or more.


Hi,

Not sure if it is an issue, but what will happen if a magic disk has a 
size less than 4K and uses a block size of 512bytes and the disk 
alignment gets rounded up to 4K. Will any of logic in this patch fail or 
hang?


--HPS

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Slawa Olhovchenkov]

On Wed, Aug 17, 2016 at 07:36:00AM -0700, Nathan Whitehorn wrote:

> > Your contention that the installer does not make policy decisions is
> > equally spurious.  The installer makes many policy decisions, including
> > the disk layout, the size of the swap partition, the name of the pool,
> > the use of boot environments (which I dislike but am not allowed to
> > override), the number of filesets and their mountpoints (which I also
> > dislike and am not allowed to override either), etc.  The Unix
> > philosophy is to push such decisions up the stack, not down.  The
> > decision to align partitions on 4096-byte boundaries because we're not
> > sure of the correct number but know for a fact that using a smaller
> > number can have a huge impact on performance is the installer's to make.
> 
> Those are all things that the operating system does not have defaults 
> for: there are no tools like, say, gpart or newfs that layout disks in 
> any even vaguely automated way, and so no tools that would ever have 
> defaults for, say, the size of a swap partition except for the 
> installer. As such, the defaults are quite properly in the installer. 
> This is quite different: there are many tools that care about disk 
> alignment (say, gpart) and, by default, use the GEOM stripesize. The 
> installer is, after this patch, overriding what was meant to be a 
> system-wide default.
> 
> My concern is that pushing this into the installer means that newfs, 
> zfs, gpart, etc., which all look at the GEOM stripesize for preferred 
> alignment, will still have suboptimal behavior on systems affected by 
> your patch. If we identified which drivers are reporting the wrong 
> alignment, we could fix the whole system at a go by changing it there. 
> As it is, we now have inconsistent default behavior for partitions 
> between tools (the installer and sade will now use a different alignment 
> than gpart on whatever systems you were trying to fix here) and between 
> pre- and post-installation environments.

In long term, prefered aligment is forsing 4k (or may be more):
install system on 512b [mirror] disk aligment now may be need required replace
disk to 4k aligment. For more flexsible in future now best chois is 4k
or more.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Nathan Whitehorn]

On 08/17/16 03:07, Dag-Erling Smørgrav wrote:

Nathan Whitehorn  writes:

As a note for people who weren't paying attention to the bug, we need
to fix this in a better way outside of the constraints of getting 11.0
out the door. The system (gpart, the installer, ZFS, etc.) uses the
reported GEOM stripesize for partition alignment and IO block size
selection. If that is wrong, we should identify devices on which it is
wrong and fix them, and maybe also add some global tunable that sets a
floor on the numbers reported by GEOM_DISK. Hacking the installer like
this is triage, which is fine, but not viable as a permanent solution
to anything.

Modifying GEOM to report a bogus number when none is provided by the
lower layer(s) is absolutely not going to happen.  You have absolutely
no idea what your proposed change will break.  And you keep refusing to
address the fact that most drivers don't report a stripe size, except by
repeating your claim that they do, with no evidence to back it up.  Feel
free to 'grep -r stripesize /usr/src/sys/dev'.  Go on, I'll wait.


And yet, if you look at the GEOM XML, it is reported and there. And, 
look, it's even right for the AF 512e disks in my machine!


  512
  4096

I've literally never seen a case where we don't already do the right 
thing here. The GEOM stripesize is, whether you like it or not, the way 
we have long ago decided to communicate the information about optimal 
alignment from the disk drivers to userland. We do a good job of it, 
too. It's correct, as far as I can tell, 100% of the time on all 
possible variants of AF disks. One could argue that calling this the 
"stripesize" is a hack, and I would agree, but it's what the operating 
system does and has done for many years.


As for grepping, the CAM disk drivers are all in sys/cam, not sys/dev, 
as I'm sure you know, and you will find all the code that handles this 
there.



Your contention that the installer does not make policy decisions is
equally spurious.  The installer makes many policy decisions, including
the disk layout, the size of the swap partition, the name of the pool,
the use of boot environments (which I dislike but am not allowed to
override), the number of filesets and their mountpoints (which I also
dislike and am not allowed to override either), etc.  The Unix
philosophy is to push such decisions up the stack, not down.  The
decision to align partitions on 4096-byte boundaries because we're not
sure of the correct number but know for a fact that using a smaller
number can have a huge impact on performance is the installer's to make.


Those are all things that the operating system does not have defaults 
for: there are no tools like, say, gpart or newfs that layout disks in 
any even vaguely automated way, and so no tools that would ever have 
defaults for, say, the size of a swap partition except for the 
installer. As such, the defaults are quite properly in the installer. 
This is quite different: there are many tools that care about disk 
alignment (say, gpart) and, by default, use the GEOM stripesize. The 
installer is, after this patch, overriding what was meant to be a 
system-wide default.


My concern is that pushing this into the installer means that newfs, 
zfs, gpart, etc., which all look at the GEOM stripesize for preferred 
alignment, will still have suboptimal behavior on systems affected by 
your patch. If we identified which drivers are reporting the wrong 
alignment, we could fix the whole system at a go by changing it there. 
As it is, we now have inconsistent default behavior for partitions 
between tools (the installer and sade will now use a different alignment 
than gpart on whatever systems you were trying to fix here) and between 
pre- and post-installation environments.


You are papering over a bug in some unspecified driver with some 
unspecified disks by hacking the installer. This is fine as an expedient 
for 11.0, but is a ridiculous solution to the problem otherwise. We 
should just fix the driver for whatever weird disk you have in your 
machine (what is it, by the way?). Since making the reported "stripe 
size" match the physical sector size is what GEOM has done since at 
least 2009, I doubt very much that fixing any bugs in that reported 
value would have the weird unintended consequences you imply it might.

-Nathan



DES


___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Dag-Erling Smørgrav]

Nathan Whitehorn  writes:
> As a note for people who weren't paying attention to the bug, we need
> to fix this in a better way outside of the constraints of getting 11.0
> out the door. The system (gpart, the installer, ZFS, etc.) uses the
> reported GEOM stripesize for partition alignment and IO block size
> selection. If that is wrong, we should identify devices on which it is
> wrong and fix them, and maybe also add some global tunable that sets a
> floor on the numbers reported by GEOM_DISK. Hacking the installer like
> this is triage, which is fine, but not viable as a permanent solution
> to anything.

Modifying GEOM to report a bogus number when none is provided by the
lower layer(s) is absolutely not going to happen.  You have absolutely
no idea what your proposed change will break.  And you keep refusing to
address the fact that most drivers don't report a stripe size, except by
repeating your claim that they do, with no evidence to back it up.  Feel
free to 'grep -r stripesize /usr/src/sys/dev'.  Go on, I'll wait.

Your contention that the installer does not make policy decisions is
equally spurious.  The installer makes many policy decisions, including
the disk layout, the size of the swap partition, the name of the pool,
the use of boot environments (which I dislike but am not allowed to
override), the number of filesets and their mountpoints (which I also
dislike and am not allowed to override either), etc.  The Unix
philosophy is to push such decisions up the stack, not down.  The
decision to align partitions on 4096-byte boundaries because we're not
sure of the correct number but know for a fact that using a smaller
number can have a huge impact on performance is the installer's to make.

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

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

[Nathan Whitehorn]

As a note for people who weren't paying attention to the bug, we need to 
fix this in a better way outside of the constraints of getting 11.0 out 
the door. The system (gpart, the installer, ZFS, etc.) uses the reported 
GEOM stripesize for partition alignment and IO block size selection. If 
that is wrong, we should identify devices on which it is wrong and fix 
them, and maybe also add some global tunable that sets a floor on the 
numbers reported by GEOM_DISK. Hacking the installer like this is 
triage, which is fine, but not viable as a permanent solution to anything.

-Nathan

On 08/15/16 02:30, Dag-Erling Smørgrav wrote:

Author: des
Date: Mon Aug 15 09:30:21 2016
New Revision: 304142
URL: https://svnweb.freebsd.org/changeset/base/304142

Log:
   Ensure that the sector size is a multiple of 4096 to avoid creating
   unaligned partitions when the actual sector size is hidden from us.
   
   PR:		211361

   MFC after:   3 days

Modified:
   head/usr.sbin/bsdinstall/partedit/gpart_ops.c

Modified: head/usr.sbin/bsdinstall/partedit/gpart_ops.c
==
--- head/usr.sbin/bsdinstall/partedit/gpart_ops.c   Mon Aug 15 09:27:15 
2016(r304141)
+++ head/usr.sbin/bsdinstall/partedit/gpart_ops.c   Mon Aug 15 09:30:21 
2016(r304142)
@@ -795,6 +795,7 @@ gpart_max_free(struct ggeom *geom, intma
  {
struct gconfig *gc;
struct gprovider *pp, **providers;
+   intmax_t sectorsize, stripesize, offset;
intmax_t lastend;
intmax_t start, end;
intmax_t maxsize, maxstart;
@@ -845,12 +846,25 @@ gpart_max_free(struct ggeom *geom, intma
  
  	pp = LIST_FIRST(>lg_consumer)->lg_provider;
  
-	/* Compute beginning of new partition and maximum available space */

-   if (pp->lg_stripesize > 0 &&
-   (maxstart*pp->lg_sectorsize % pp->lg_stripesize) != 0) {
-   intmax_t offset = (pp->lg_stripesize -
-   ((maxstart*pp->lg_sectorsize) % pp->lg_stripesize)) /
-   pp->lg_sectorsize;
+   /*
+* Round the start and size of the largest available space up to
+* the nearest multiple of the adjusted stripe size.
+*
+* The adjusted stripe size is the least common multiple of the
+* actual stripe size, or the sector size if no stripe size was
+* reported, and 4096.  The reason for this is that contemporary
+* disks often have 4096-byte physical sectors but report 512
+* bytes instead for compatibility with older / broken operating
+* systems and BIOSes.  For the same reasons, virtualized storage
+* may also report a 512-byte stripe size, or none at all.
+*/
+   sectorsize = pp->lg_sectorsize;
+   if ((stripesize = pp->lg_stripesize) == 0)
+   stripesize = sectorsize;
+   while (stripesize % 4096 != 0)
+   stripesize *= 2;
+   if ((offset = maxstart * sectorsize % stripesize) != 0) {
+   offset = (stripesize - offset) / sectorsize;
maxstart += offset;
maxsize -= offset;
}



___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"