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@xxxxxx>
> ---
> 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;
>
