Patchwork [02/12] container quota: lock/unlock mount namespace when performing quotactl.

login
register
mail settings
Submitter jeff.liu
Date May 30, 2012, 2:58 p.m.
Message ID <1338389946-13711-3-git-send-email-jeff.liu@oracle.com>
Download mbox | patch
Permalink /patch/161991/
State Not Applicable
Headers show

Comments

jeff.liu - May 30, 2012, 2:58 p.m.
Introduce two functions to lock/unlock a particular mount namespace
when container quota operations is performed.

Signed-off-by: Jie Liu <jeff.liu@oracle.com>
---
 fs/namespace.c                |   17 +++++++++++++++++
 include/linux/mnt_namespace.h |   13 +++++++++++++
 2 files changed, 30 insertions(+), 0 deletions(-)
Glauber Costa - May 31, 2012, 9:04 a.m.
On 05/30/2012 06:58 PM, jeff.liu@oracle.com wrote:
> Introduce two functions to lock/unlock a particular mount namespace
> when container quota operations is performed.
>
> Signed-off-by: Jie Liu<jeff.liu@oracle.com>
> ---
>   fs/namespace.c                |   17 +++++++++++++++++
>   include/linux/mnt_namespace.h |   13 +++++++++++++
>   2 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index e608199..8823ba8 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2633,3 +2633,20 @@ bool our_mnt(struct vfsmount *mnt)
>   {
>   	return check_mnt(real_mount(mnt));
>   }
> +
> +/*
> + * Lock the mount namespace when performing a particular container
> + * disk quotactl operation, especially to protect removing it.
> + */
> +void lock_mnt_ns(struct mnt_namespace *ns)
> +{
> +	down_write(&namespace_sem);
> +}
> +
> +/*
> + * Unlock the mount namespace after quotactl on that was done.
> + */
> +void unlock_mnt_ns(struct mnt_namespace *ns)
> +{
> +	up_write(&namespace_sem);
> +}
> diff --git a/include/linux/mnt_namespace.h b/include/linux/mnt_namespace.h
> index 5a8e390..ff72ed5 100644
> --- a/include/linux/mnt_namespace.h
> +++ b/include/linux/mnt_namespace.h
> @@ -9,6 +9,19 @@ extern struct mnt_namespace *copy_mnt_ns(unsigned long, struct mnt_namespace *,
>   		struct fs_struct *);
>   extern void put_mnt_ns(struct mnt_namespace *ns);
>
> +#ifdef CONFIG_NS_QUOTA
> +extern void lock_mnt_ns(struct mnt_namespace *);
> +extern void unlock_mnt_ns(struct mnt_namespace *);
> +#else
> +void lock_mnt_ns(struct mnt_namespace *)
> +{
> +}
> +
> +void unlock_mnt_ns(struct mnt_namespace *)
> +{
> +}
> +#endif
> +

Seems weird. There is nothing ns-quota specific in locking a namespace.
Also, just grabbing a lock from an outer subsystem is quite ugly. If you 
are grabbing a namespace lock, this suggests you are tweaking namespace 
data. So you should have that file provide you with a function that does 
the full operation.

Of course there are exceptions to that, but it rings a bell.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
jeff.liu - May 31, 2012, 12:40 p.m.
On 05/31/2012 05:04 PM, Glauber Costa wrote:

> On 05/30/2012 06:58 PM, jeff.liu@oracle.com wrote:
>> Introduce two functions to lock/unlock a particular mount namespace
>> when container quota operations is performed.
>>
>> Signed-off-by: Jie Liu<jeff.liu@oracle.com>
>> ---
>>   fs/namespace.c                |   17 +++++++++++++++++
>>   include/linux/mnt_namespace.h |   13 +++++++++++++
>>   2 files changed, 30 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/namespace.c b/fs/namespace.c
>> index e608199..8823ba8 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -2633,3 +2633,20 @@ bool our_mnt(struct vfsmount *mnt)
>>   {
>>       return check_mnt(real_mount(mnt));
>>   }
>> +
>> +/*
>> + * Lock the mount namespace when performing a particular container
>> + * disk quotactl operation, especially to protect removing it.
>> + */
>> +void lock_mnt_ns(struct mnt_namespace *ns)
>> +{
>> +    down_write(&namespace_sem);
>> +}
>> +
>> +/*
>> + * Unlock the mount namespace after quotactl on that was done.
>> + */
>> +void unlock_mnt_ns(struct mnt_namespace *ns)
>> +{
>> +    up_write(&namespace_sem);
>> +}
>> diff --git a/include/linux/mnt_namespace.h
>> b/include/linux/mnt_namespace.h
>> index 5a8e390..ff72ed5 100644
>> --- a/include/linux/mnt_namespace.h
>> +++ b/include/linux/mnt_namespace.h
>> @@ -9,6 +9,19 @@ extern struct mnt_namespace *copy_mnt_ns(unsigned
>> long, struct mnt_namespace *,
>>           struct fs_struct *);
>>   extern void put_mnt_ns(struct mnt_namespace *ns);
>>
>> +#ifdef CONFIG_NS_QUOTA
>> +extern void lock_mnt_ns(struct mnt_namespace *);
>> +extern void unlock_mnt_ns(struct mnt_namespace *);
>> +#else
>> +void lock_mnt_ns(struct mnt_namespace *)
>> +{
>> +}
>> +
>> +void unlock_mnt_ns(struct mnt_namespace *)
>> +{
>> +}
>> +#endif
>> +
> 
> Seems weird. There is nothing ns-quota specific in locking a namespace.
> Also, just grabbing a lock from an outer subsystem is quite ugly. If you
> are grabbing a namespace lock, this suggests you are tweaking namespace
> data. So you should have that file provide you with a function that does
> the full operation.

Maybe this can be safely removed, it is presented like a place holder,
which is suggests to protect changing data inner quota info of a mount
namespace.
However, both quota info initialization and destroy business are all
projected at mount init and put mount namespace stage, so that the
lock/unlock are make less sense here.

Thanks,
-Jeff

> 
> Of course there are exceptions to that, but it rings a bell.


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

Patch

diff --git a/fs/namespace.c b/fs/namespace.c
index e608199..8823ba8 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2633,3 +2633,20 @@  bool our_mnt(struct vfsmount *mnt)
 {
 	return check_mnt(real_mount(mnt));
 }
+
+/*
+ * Lock the mount namespace when performing a particular container
+ * disk quotactl operation, especially to protect removing it.
+ */
+void lock_mnt_ns(struct mnt_namespace *ns)
+{
+	down_write(&namespace_sem);
+}
+
+/*
+ * Unlock the mount namespace after quotactl on that was done.
+ */
+void unlock_mnt_ns(struct mnt_namespace *ns)
+{
+	up_write(&namespace_sem);
+}
diff --git a/include/linux/mnt_namespace.h b/include/linux/mnt_namespace.h
index 5a8e390..ff72ed5 100644
--- a/include/linux/mnt_namespace.h
+++ b/include/linux/mnt_namespace.h
@@ -9,6 +9,19 @@  extern struct mnt_namespace *copy_mnt_ns(unsigned long, struct mnt_namespace *,
 		struct fs_struct *);
 extern void put_mnt_ns(struct mnt_namespace *ns);
 
+#ifdef CONFIG_NS_QUOTA
+extern void lock_mnt_ns(struct mnt_namespace *);
+extern void unlock_mnt_ns(struct mnt_namespace *);
+#else
+void lock_mnt_ns(struct mnt_namespace *)
+{
+}
+
+void unlock_mnt_ns(struct mnt_namespace *)
+{
+}
+#endif
+
 extern const struct file_operations proc_mounts_operations;
 extern const struct file_operations proc_mountinfo_operations;
 extern const struct file_operations proc_mountstats_operations;