| Message ID | 20191025174026.31878-6-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: > There is no real need for a local variables here - either the I/O size > is valid and gets applied to the mount structure, or it is invalid and > the mount will fail entirely. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/xfs/xfs_super.c | 17 +++++++---------- > 1 file changed, 7 insertions(+), 10 deletions(-) > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index ee2dde897fb7..1467f4bebc41 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -161,7 +161,6 @@ xfs_parseargs( > char *p; > substring_t args[MAX_OPT_ARGS]; > int iosize = 0; > - uint8_t iosizelog = 0; > > /* > * set up the mount name first so all the errors will refer to the > @@ -229,7 +228,8 @@ xfs_parseargs( > case Opt_biosize: > if (suffix_kstrtoint(args, 10, &iosize)) > return -EINVAL; > - iosizelog = ffs(iosize) - 1; > + mp->m_writeio_log = ffs(iosize) - 1; > + mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE; > break; > case Opt_grpid: > case Opt_bsdgroups: > @@ -397,17 +397,14 @@ xfs_parseargs( > return -EINVAL; > } > > - if (iosizelog) { > - if (iosizelog > XFS_MAX_IO_LOG || > - iosizelog < XFS_MIN_IO_LOG) { > + if (mp->m_flags & XFS_MOUNT_DFLT_IOSIZE) { > + if (mp->m_writeio_log > XFS_MAX_IO_LOG || > + mp->m_writeio_log < XFS_MIN_IO_LOG) { There's a slight change in behavior here. Before, "mount -o biosize=0" would pass, because iosizelog, though specified, did not satisfy "if (iosizelog)"and so it was never tested, so it was essentially ignored. Now the same specification will fail mount. To make this a completely cosmetic change, perhaps this should test mp->m_writeio_log rather than the flag. If a change in behavior is desired I think that should be an explicit 2nd change. > xfs_warn(mp, "invalid log iosize: %d [not %d-%d]", > - iosizelog, XFS_MIN_IO_LOG, > - XFS_MAX_IO_LOG); > + mp->m_writeio_log, > + XFS_MIN_IO_LOG, XFS_MAX_IO_LOG); > return -EINVAL; > } > - > - mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE; > - mp->m_writeio_log = iosizelog; > } > > return 0; >
On 10/25/19 1:35 PM, Eric Sandeen wrote: > There's a slight change in behavior here. > > Before, "mount -o biosize=0" would pass, because iosizelog, though specified, > did not satisfy "if (iosizelog)"and so it was never tested, so it was > essentially ignored. sorry make that "-o allocsize=0" Which I guess /is/ documented as being invalid, we just never rejected it before. Still a change in behavior and worth being explicit about I think. -Eric
On Fri, Oct 25, 2019 at 02:03:06PM -0500, Eric Sandeen wrote: > sorry make that "-o allocsize=0" > > Which I guess /is/ documented as being invalid, we just never rejected it > before. > > Still a change in behavior and worth being explicit about I think. I'll updated the changelog to mention that.
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index ee2dde897fb7..1467f4bebc41 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -161,7 +161,6 @@ xfs_parseargs( char *p; substring_t args[MAX_OPT_ARGS]; int iosize = 0; - uint8_t iosizelog = 0; /* * set up the mount name first so all the errors will refer to the @@ -229,7 +228,8 @@ xfs_parseargs( case Opt_biosize: if (suffix_kstrtoint(args, 10, &iosize)) return -EINVAL; - iosizelog = ffs(iosize) - 1; + mp->m_writeio_log = ffs(iosize) - 1; + mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE; break; case Opt_grpid: case Opt_bsdgroups: @@ -397,17 +397,14 @@ xfs_parseargs( return -EINVAL; } - if (iosizelog) { - if (iosizelog > XFS_MAX_IO_LOG || - iosizelog < XFS_MIN_IO_LOG) { + if (mp->m_flags & XFS_MOUNT_DFLT_IOSIZE) { + if (mp->m_writeio_log > XFS_MAX_IO_LOG || + mp->m_writeio_log < XFS_MIN_IO_LOG) { xfs_warn(mp, "invalid log iosize: %d [not %d-%d]", - iosizelog, XFS_MIN_IO_LOG, - XFS_MAX_IO_LOG); + mp->m_writeio_log, + XFS_MIN_IO_LOG, XFS_MAX_IO_LOG); return -EINVAL; } - - mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE; - mp->m_writeio_log = iosizelog; } return 0;
There is no real need for a local variables here - either the I/O size is valid and gets applied to the mount structure, or it is invalid and the mount will fail entirely. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_super.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)