diff mbox

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

Message ID 20160616083807.GB7409@1wt.eu (mailing list archive)
State New, archived
Headers show

Commit Message

Willy Tarreau June 16, 2016, 8:38 a.m. UTC 
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) :

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Andy Lutomirski June 17, 2016, 3:12 a.m. UTC | #1 
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;
diff mbox

Patch

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;