[6/7] xfs: clean up setting m_readio_* / m_writeio_*
diff mbox series

Message ID 20191025174026.31878-7-hch@lst.de
State New
Headers show
Series
  • [1/7] xfs: cleanup stat blksize reporting
Related show

Commit Message

Christoph Hellwig Oct. 25, 2019, 5:40 p.m. UTC
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@lst.de>
---
 fs/xfs/xfs_mount.c | 36 +++++-------------------------------
 fs/xfs/xfs_super.c |  5 +++++
 2 files changed, 10 insertions(+), 31 deletions(-)

Comments

Eric Sandeen Oct. 25, 2019, 7:18 p.m. UTC | #1
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@lst.de>
> ---
>  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).

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 is a little surprising and nonobvious and it makes me wonder
if you're intentionally future-proofing here, or if that's just weird.
:)

-Eric


>  	return 0;
>
Dave Chinner Oct. 25, 2019, 8:53 p.m. UTC | #2
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@lst.de>
> > ---
> >  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.

> 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....

Cheers,

Dave.
Eric Sandeen Oct. 27, 2019, 2:23 a.m. UTC | #3
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@lst.de>
>>> ---
>>>  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

Patch
diff mbox series

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;
 	}
 
 	return 0;