Make ZFS use the physical sector size when computing initial ashift

[Justin T. Gibbs]

On Jul 10, 2013, at 1:42 PM, "Steven Hartland" <killing at multiplay.co.uk> wrote:

> 
> ----- Original Message ----- From: "Justin T. Gibbs"
>> On Jul 10, 2013, at 1:06 PM, "Steven Hartland" wrote:
>>> ----- Original Message ----- From: "Justin T. Gibbs" 
>>>> I'm sure lots of folks have "some solution" to this.  Here is an
>>>> old version of what we use at Spectra:
>>>> http://people.freebsd.org/~gibbs/zfs_patches/zfs_auto_ashift.diff
>>>> The above patch is missing some cleanup that was motivated by my
>>>> discussions with George Wilson about this change in April.  I'll
>>>> dig that up later tonight.  Even if you don't read the full diff,
>>>> please read the included checkin comment since it explains the
>>>> motivation behind this particular solution.
>>>> This is on my list of things to upstream in the next week or so after
>>>> I add logic to the userspace tools to report whether or not the
>>>> TLVs in a pool are using an optimal allocation size.  This is only
>>>> possible if you actually make ZFS fully aware of logical, physical,
>>>> and the configured allocation size.  All of the other patches I've seen
>>>> just treat physical as logical.
>>> Reading through your patch it seems that your logical_ashift equates to
>>> the current ashift values which for geom devices is based off sectorsize
>>> and your physical_ashift is based stripesize.
>>> This is almost identical to the approach I used adding a "desired ashift",
>>> which equates to your physical_ashift, along side the standard ashift
>>> i.e. required aka logical_ashift value :)
>> 
>> Yes, the approaches are similar.  Our current version records the logical
>> access size in the vdev structure too, which might relate to the issue
>> below.
>> 
>> > One issue I did spot in your patch is that you currently expose
>> > zfs_max_auto_ashift as a sysctl but don't clamp its value which would
>> > cause problems should a user configure values > 13.
>> 
>> I would expect the zio pipeline to simply insert an ashift aligned thunking
>> buffer for these operations, but I haven't tried going past an ashift of 13 in
>> my tests.  If it is an issue, it seems the restriction should be based on
>> logical access size, not optimal access size.
> 
> Yes with your methodology you'll only see the issue if zfs_max_auto_ashift
> and physical_ashift are both > 13, but this can be the case for example
> on a RAID controller with large stripsize.

I'm not sure I follow.  logical_ashift is available in our latest code, as is the
physical_ashift.  But even without the logical_ashift, why doesn't the zio
pipeline properly thunk zio_phys_read() access based on the configured ashift?

--
Justin