On 10/25/19 3:53 PM, Dave Chinner wrote:
> On Fri, Oct 25, 2019 at 02:18:02PM -0500, Eric Sandeen wrote:
>> On 10/25/19 12:40 PM, Christoph Hellwig wrote:
>>> Fill in the default _log values in xfs_parseargs similar to other
>>> defaults, and open code the updates based on the on-disk superblock
>>> in xfs_mountfs now that they are completely trivial.
>>>
>>> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>>> ---
>>> fs/xfs/xfs_mount.c | 36 +++++-------------------------------
>>> fs/xfs/xfs_super.c | 5 +++++
>>> 2 files changed, 10 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
>>> index 9800401a7d6f..bae53fdd5d51 100644
>>> --- a/fs/xfs/xfs_mount.c
>>> +++ b/fs/xfs/xfs_mount.c
>>> @@ -425,35 +425,6 @@ xfs_update_alignment(xfs_mount_t *mp)
>>> return 0;
>>> }
>>>
>>> -/*
>>> - * Set the default minimum read and write sizes unless
>>> - * already specified in a mount option.
>>> - * We use smaller I/O sizes when the file system
>>> - * is being used for NFS service (wsync mount option).
>>> - */
>>> -STATIC void
>>> -xfs_set_rw_sizes(xfs_mount_t *mp)
>>> -{
>>> - xfs_sb_t *sbp = &(mp->m_sb);
>>> - int writeio_log;
>>> -
>>> - if (!(mp->m_flags & XFS_MOUNT_DFLT_IOSIZE)) {
>>> - if (mp->m_flags & XFS_MOUNT_WSYNC)
>>> - writeio_log = XFS_WRITEIO_LOG_WSYNC;
>>> - else
>>> - writeio_log = XFS_WRITEIO_LOG_LARGE;
>>> - } else {
>>> - writeio_log = mp->m_writeio_log;
>>> - }
>>> -
>>> - if (sbp->sb_blocklog > writeio_log) {
>>> - mp->m_writeio_log = sbp->sb_blocklog;
>>> - } else {
>>> - mp->m_writeio_log = writeio_log;
>>> - }
>>> - mp->m_writeio_blocks = 1 << (mp->m_writeio_log - sbp->sb_blocklog);
>>> -}
>>> -
>>> /*
>>> * precalculate the low space thresholds for dynamic speculative preallocation.
>>> */
>>> @@ -718,9 +689,12 @@ xfs_mountfs(
>>> goto out_remove_errortag;
>>>
>>> /*
>>> - * Set the minimum read and write sizes
>>> + * Update the preferred write size based on the information from the
>>> + * on-disk superblock.
>>> */
>>> - xfs_set_rw_sizes(mp);
>>> + mp->m_writeio_log =
>>> + max_t(uint32_t, mp->m_sb.sb_blocklog, mp->m_writeio_log);
>>> + mp->m_writeio_blocks = 1 << (mp->m_writeio_log - mp->m_sb.sb_blocklog);
>>>
>>> /* set the low space thresholds for dynamic preallocation */
>>> xfs_set_low_space_thresholds(mp);
>>> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
>>> index 1467f4bebc41..83dbfcc5a02d 100644
>>> --- a/fs/xfs/xfs_super.c
>>> +++ b/fs/xfs/xfs_super.c
>>> @@ -405,6 +405,11 @@ xfs_parseargs(
>>> XFS_MIN_IO_LOG, XFS_MAX_IO_LOG);
>>> return -EINVAL;
>>> }
>>> + } else {
>>> + if (mp->m_flags & XFS_MOUNT_WSYNC)
>>> + mp->m_writeio_log = XFS_WRITEIO_LOG_WSYNC;
>>> + else
>>> + mp->m_writeio_log = XFS_WRITEIO_LOG_LARGE;
>>> }
>>
>> Ok let's see, by here, if Opt_allocsize was specified, we set
>> mp->m_writeio_log to the specified value, else if Opt_wsync was set, we
>> set m_writeio_log to XFS_WRITEIO_LOG_WSYNC (14), otherwise we default to
>> XFS_WRITEIO_LOG_LARGE (16). So that's it for parseargs.
>>
>> AFAICT we can't escape parseargs w/ writeio_log less than PAGE_SHIFT
>> (i.e. page size).
>
> We can - 64k page size means XFS_WRITEIO_LOG_WSYNC is less than
> PAGE_SHIFT, so it can escape here.
Whoops my brain just hotwires PAGE to 4k occasionally, thanks.
>> Then in xfs_mountfs, you have it reset to the max of sb_blocklog and
>> m_writeio_log. i.e. it gets resized iff sb_blocklog is greater than
>> the current m_writeio_log, which has a minimum of page size.
>>
>> IOWS, it only gets a new value in mountfs if block size is > page size.
>
> Which will never be true on 64k page size machines, but that's not
> what this code does, anyway. It will rewrite m_writeio_log if the
> block size is larger than the m_writeio_log value, so it preserves
> the existing behaviour of setting the m_writeio_log value to a
> minimum of one filesystem block....
Right, ok, thanks for straightening me out.
-Eric
