Re: [PATCH v2 1/2] fs: Improve and simplify copy_mount_options

Andy Lutomirski
2016-06-17T03:12:08Z

On Thu, Jun 16, 2016 at 1:38 AM, Willy Tarreau <w@1wt.eu> wrote:
> On Thu, Jun 16, 2016 at 10:20:38AM +0200, Willy Tarreau wrote:
>> However if the initial point was to avoid reading extra
>> pages most of the time, we could possibly consider that the string
>> copy is an optimization for the case where we have the information
>> available. Thus if !fstype || !(type->fs_flags & FS_BINARY_MOUNTDATA)
>> then we use copy_mount_options() otherwise we use copy_mount_string().
>> It will only leave the full page copy for mount --bind or -o remount
>> then.
>
> For example like this (not compile-tested either) :

I like this approach. Al, is this okay with you?

--Andy

>
> diff --git a/fs/compat.c b/fs/compat.c
> index be6e48b..6407d40 100644
> --- a/fs/compat.c
> +++ b/fs/compat.c
> @@ -795,18 +795,26 @@ COMPAT_SYSCALL_DEFINE5(mount, const char __user *, dev_name,
> void *options;
> char *kernel_dev;
> int retval;
> + bool use_str = false;
> + struct file_system_type *fstype;
>
> kernel_type = copy_mount_string(type);
> retval = PTR_ERR(kernel_type);
> if (IS_ERR(kernel_type))
> goto out;
>
> + if (kernel_type && (fstype = get_fs_type(kernel_type)) {
> + if (!(fstype->fs_flags & FS_BINARY_MOUNTDATA))
> + use_str = true;
> + put_filesystem(fstype);
> + }
> +
> kernel_dev = copy_mount_string(dev_name);
> retval = PTR_ERR(kernel_dev);
> if (IS_ERR(kernel_dev))
> goto out1;
>
> - options = copy_mount_options(data);
> + options = use_str ? copy_mount_string(data) : copy_mount_options(data);
> retval = PTR_ERR(options);
> if (IS_ERR(options))
> goto out2;
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 4fb1691..f14089d 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2906,18 +2906,26 @@ SYSCALL_DEFINE5(mount, char __user *, dev_name, char __user *, dir_name,
> char *kernel_type;
> char *kernel_dev;
> void *options;
> + bool use_str = false;
> + struct file_system_type *fstype;
>
> kernel_type = copy_mount_string(type);
> ret = PTR_ERR(kernel_type);
> if (IS_ERR(kernel_type))
> goto out_type;
>
> + if (kernel_type && (fstype = get_fs_type(kernel_type)) {
> + if (!(fstype->fs_flags & FS_BINARY_MOUNTDATA))
> + use_str = true;
> + put_filesystem(fstype);
> + }
> +
> kernel_dev = copy_mount_string(dev_name);
> ret = PTR_ERR(kernel_dev);
> if (IS_ERR(kernel_dev))
> goto out_dev;
>
> - options = copy_mount_options(data);
> + options = use_str ? copy_mount_string(data) : copy_mount_options(data);
> ret = PTR_ERR(options);
> if (IS_ERR(options))
> goto out_data;

--
Andy Lutomirski
AMA Capital Management, LLC