| Message ID | 20191025174026.31878-7-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] xfs: cleanup stat blksize reporting | expand |
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; >
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.
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
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;
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(-)