All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] statfs: handle mount propagation
@ 2018-04-13 16:11 Christian Brauner
  2018-04-13 16:11 ` [PATCH 1/6] fs: use << for MS_* flags Christian Brauner
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Christian Brauner @ 2018-04-13 16:11 UTC (permalink / raw)
  To: viro, tglx, kstewart, gregkh, pombredanne, linux-fsdevel,
	linux-kernel, serge
  Cc: Christian Brauner

Hey,

This little series
- unifies the definition of constants in statfs.h and fs.h
- extends statfs to handle mount propagation. This will let userspace
  easily query a given mountpoint for MS_UNBINDABLE, MS_SHARED,
  MS_PRIVATE and MS_SLAVE without always having to do costly parsing of
  /proc/<pid>/mountinfo.
  To this end the flags:
  - ST_UNBINDABLE
  - ST_SHARED
  - ST_PRIVATE
  - ST_SLAVE
  are added. They have the same value as their MS_* counterparts.

The patchset was made against Al's vfs/for-next tree but they also apply
cleanly against current linus/master. So if they are deemed suitable for
inclusion in the current release that should work too.

Thanks!
Christian

Christian Brauner (6):
  fs: use << for MS_* flags
  statfs: use << to align with fs header
  statfs: add ST_UNBINDABLE
  statfs: add ST_SHARED
  statfs: add ST_PRIVATE
  statfs: add ST_SLAVE

 fs/statfs.c             | 16 +++++++++++++++-
 include/linux/statfs.h  | 30 +++++++++++++++++-------------
 include/uapi/linux/fs.h | 33 +++++++++++++++++----------------
 3 files changed, 49 insertions(+), 30 deletions(-)

-- 
2.17.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/6] fs: use << for MS_* flags
  2018-04-13 16:11 [PATCH 0/6] statfs: handle mount propagation Christian Brauner
@ 2018-04-13 16:11 ` Christian Brauner
  2018-04-13 16:45   ` Randy Dunlap
  2018-04-13 16:11 ` [PATCH 2/6] statfs: use << to align with fs header Christian Brauner
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Christian Brauner @ 2018-04-13 16:11 UTC (permalink / raw)
  To: viro, tglx, kstewart, gregkh, pombredanne, linux-fsdevel,
	linux-kernel, serge
  Cc: Christian Brauner

Consistenly use << to define MS_* constants.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 include/uapi/linux/fs.h | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index d2a8313fabd7..9662790a657c 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -105,22 +105,23 @@ struct inodes_stat_t {
 /*
  * These are the fs-independent mount-flags: up to 32 flags are supported
  */
-#define MS_RDONLY	 1	/* Mount read-only */
-#define MS_NOSUID	 2	/* Ignore suid and sgid bits */
-#define MS_NODEV	 4	/* Disallow access to device special files */
-#define MS_NOEXEC	 8	/* Disallow program execution */
-#define MS_SYNCHRONOUS	16	/* Writes are synced at once */
-#define MS_REMOUNT	32	/* Alter flags of a mounted FS */
-#define MS_MANDLOCK	64	/* Allow mandatory locks on an FS */
-#define MS_DIRSYNC	128	/* Directory modifications are synchronous */
-#define MS_NOATIME	1024	/* Do not update access times. */
-#define MS_NODIRATIME	2048	/* Do not update directory access times */
-#define MS_BIND		4096
-#define MS_MOVE		8192
-#define MS_REC		16384
-#define MS_VERBOSE	32768	/* War is peace. Verbosity is silence.
-				   MS_VERBOSE is deprecated. */
-#define MS_SILENT	32768
+#define MS_RDONLY	(1<<0)	/* Mount read-only */
+#define MS_NOSUID	(1<<1)	/* Ignore suid and sgid bits */
+#define MS_NODEV	(1<<2)	/* Disallow access to device special files */
+#define MS_NOEXEC	(1<<3)	/* Disallow program execution */
+#define MS_SYNCHRONOUS	(1<<4)	/* Writes are synced at once */
+#define MS_REMOUNT	(1<<5)	/* Alter flags of a mounted FS */
+#define MS_MANDLOCK	(1<<6)	/* Allow mandatory locks on an FS */
+#define MS_DIRSYNC	(1<<7)	/* Directory modifications are synchronous */
+#define MS_NOATIME	(1<<10)	/* Do not update access times. */
+#define MS_NODIRATIME	(1<<11)	/* Do not update directory access times */
+#define MS_BIND		(1<<12)
+#define MS_MOVE		(1<<13)
+#define MS_REC		(1<<14)
+#define MS_VERBOSE	(1<<15)	/* War is peace. Verbosity is silence.
+				 * MS_VERBOSE is deprecated.
+				 */
+#define MS_SILENT	(1<<15)
 #define MS_POSIXACL	(1<<16)	/* VFS does not apply the umask */
 #define MS_UNBINDABLE	(1<<17)	/* change to unbindable */
 #define MS_PRIVATE	(1<<18)	/* change to private */
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/6] statfs: use << to align with fs header
  2018-04-13 16:11 [PATCH 0/6] statfs: handle mount propagation Christian Brauner
  2018-04-13 16:11 ` [PATCH 1/6] fs: use << for MS_* flags Christian Brauner
@ 2018-04-13 16:11 ` Christian Brauner
  2018-04-13 16:47   ` Randy Dunlap
  2018-04-13 17:35   ` Andreas Dilger
  2018-04-13 16:11 ` [PATCH 3/6] statfs: add ST_UNBINDABLE Christian Brauner
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 13+ messages in thread
From: Christian Brauner @ 2018-04-13 16:11 UTC (permalink / raw)
  To: viro, tglx, kstewart, gregkh, pombredanne, linux-fsdevel,
	linux-kernel, serge
  Cc: Christian Brauner

Consistenly use << to define ST_* constants. This also aligns them with
their MS_* counterparts in fs.h

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 include/linux/statfs.h | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/linux/statfs.h b/include/linux/statfs.h
index 3142e98546ac..b336c04e793c 100644
--- a/include/linux/statfs.h
+++ b/include/linux/statfs.h
@@ -27,18 +27,18 @@ struct kstatfs {
  * ABI.  The exception is ST_VALID which has the same value as MS_REMOUNT
  * which doesn't make any sense for statfs.
  */
-#define ST_RDONLY	0x0001	/* mount read-only */
-#define ST_NOSUID	0x0002	/* ignore suid and sgid bits */
-#define ST_NODEV	0x0004	/* disallow access to device special files */
-#define ST_NOEXEC	0x0008	/* disallow program execution */
-#define ST_SYNCHRONOUS	0x0010	/* writes are synced at once */
-#define ST_VALID	0x0020	/* f_flags support is implemented */
-#define ST_MANDLOCK	0x0040	/* allow mandatory locks on an FS */
-/* 0x0080 used for ST_WRITE in glibc */
-/* 0x0100 used for ST_APPEND in glibc */
-/* 0x0200 used for ST_IMMUTABLE in glibc */
-#define ST_NOATIME	0x0400	/* do not update access times */
-#define ST_NODIRATIME	0x0800	/* do not update directory access times */
-#define ST_RELATIME	0x1000	/* update atime relative to mtime/ctime */
+#define ST_RDONLY	(1<<0) /* mount read-only */
+#define ST_NOSUID	(1<<1) /* ignore suid and sgid bits */
+#define ST_NODEV	(1<<2) /* disallow access to device special files */
+#define ST_NOEXEC	(1<<3) /* disallow program execution */
+#define ST_SYNCHRONOUS	(1<<4) /* writes are synced at once */
+#define ST_VALID	(1<<5) /* f_flags support is implemented */
+#define ST_MANDLOCK	(1<<6) /* allow mandatory locks on an FS */
+/* (1<<7) used for ST_WRITE in glibc */
+/* (1<<8) used for ST_APPEND in glibc */
+/* (1<<9) used for ST_IMMUTABLE in glibc */
+#define ST_NOATIME	(1<<10) /* do not update access times */
+#define ST_NODIRATIME	(1<<11) /* do not update directory access times */
+#define ST_RELATIME	(1<<12) /* update atime relative to mtime/ctime */
 
 #endif
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/6] statfs: add ST_UNBINDABLE
  2018-04-13 16:11 [PATCH 0/6] statfs: handle mount propagation Christian Brauner
  2018-04-13 16:11 ` [PATCH 1/6] fs: use << for MS_* flags Christian Brauner
  2018-04-13 16:11 ` [PATCH 2/6] statfs: use << to align with fs header Christian Brauner
@ 2018-04-13 16:11 ` Christian Brauner
  2018-04-13 16:11 ` [PATCH 4/6] statfs: add ST_SHARED Christian Brauner
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Christian Brauner @ 2018-04-13 16:11 UTC (permalink / raw)
  To: viro, tglx, kstewart, gregkh, pombredanne, linux-fsdevel,
	linux-kernel, serge
  Cc: Christian Brauner

This lets userspace query whether a mountpoint was made MS_UNBINDABLE.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/statfs.c            | 2 ++
 include/linux/statfs.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/fs/statfs.c b/fs/statfs.c
index 5b2a24f0f263..61b3063d3921 100644
--- a/fs/statfs.c
+++ b/fs/statfs.c
@@ -29,6 +29,8 @@ static int flags_by_mnt(int mnt_flags)
 		flags |= ST_NODIRATIME;
 	if (mnt_flags & MNT_RELATIME)
 		flags |= ST_RELATIME;
+	if (mnt_flags & MNT_UNBINDABLE)
+		flags |= ST_UNBINDABLE;
 	return flags;
 }
 
diff --git a/include/linux/statfs.h b/include/linux/statfs.h
index b336c04e793c..e1b84d0388c1 100644
--- a/include/linux/statfs.h
+++ b/include/linux/statfs.h
@@ -40,5 +40,6 @@ struct kstatfs {
 #define ST_NOATIME	(1<<10) /* do not update access times */
 #define ST_NODIRATIME	(1<<11) /* do not update directory access times */
 #define ST_RELATIME	(1<<12) /* update atime relative to mtime/ctime */
+#define ST_UNBINDABLE	(1<<17)	/* change to unbindable */
 
 #endif
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/6] statfs: add ST_SHARED
  2018-04-13 16:11 [PATCH 0/6] statfs: handle mount propagation Christian Brauner
                   ` (2 preceding siblings ...)
  2018-04-13 16:11 ` [PATCH 3/6] statfs: add ST_UNBINDABLE Christian Brauner
@ 2018-04-13 16:11 ` Christian Brauner
  2018-04-13 16:11 ` [PATCH 5/6] statfs: add ST_PRIVATE Christian Brauner
  2018-04-13 16:11 ` [PATCH 6/6] statfs: add ST_SLAVE Christian Brauner
  5 siblings, 0 replies; 13+ messages in thread
From: Christian Brauner @ 2018-04-13 16:11 UTC (permalink / raw)
  To: viro, tglx, kstewart, gregkh, pombredanne, linux-fsdevel,
	linux-kernel, serge
  Cc: Christian Brauner

This lets userspace query whether a mountpoint was made MS_SHARED.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/statfs.c            | 2 ++
 include/linux/statfs.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/fs/statfs.c b/fs/statfs.c
index 61b3063d3921..2fc6f9c3793c 100644
--- a/fs/statfs.c
+++ b/fs/statfs.c
@@ -31,6 +31,8 @@ static int flags_by_mnt(int mnt_flags)
 		flags |= ST_RELATIME;
 	if (mnt_flags & MNT_UNBINDABLE)
 		flags |= ST_UNBINDABLE;
+	if (mnt_flags & MNT_SHARED)
+		flags |= ST_SHARED;
 	return flags;
 }
 
diff --git a/include/linux/statfs.h b/include/linux/statfs.h
index e1b84d0388c1..5416b2936dd9 100644
--- a/include/linux/statfs.h
+++ b/include/linux/statfs.h
@@ -41,5 +41,6 @@ struct kstatfs {
 #define ST_NODIRATIME	(1<<11) /* do not update directory access times */
 #define ST_RELATIME	(1<<12) /* update atime relative to mtime/ctime */
 #define ST_UNBINDABLE	(1<<17)	/* change to unbindable */
+#define ST_SHARED	(1<<20)	/* change to shared */
 
 #endif
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/6] statfs: add ST_PRIVATE
  2018-04-13 16:11 [PATCH 0/6] statfs: handle mount propagation Christian Brauner
                   ` (3 preceding siblings ...)
  2018-04-13 16:11 ` [PATCH 4/6] statfs: add ST_SHARED Christian Brauner
@ 2018-04-13 16:11 ` Christian Brauner
  2018-04-13 16:11 ` [PATCH 6/6] statfs: add ST_SLAVE Christian Brauner
  5 siblings, 0 replies; 13+ messages in thread
From: Christian Brauner @ 2018-04-13 16:11 UTC (permalink / raw)
  To: viro, tglx, kstewart, gregkh, pombredanne, linux-fsdevel,
	linux-kernel, serge
  Cc: Christian Brauner

This lets userspace query whether a mountpoint was made MS_PRIVATE.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/statfs.c            | 2 ++
 include/linux/statfs.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/fs/statfs.c b/fs/statfs.c
index 2fc6f9c3793c..26cda2586d7e 100644
--- a/fs/statfs.c
+++ b/fs/statfs.c
@@ -33,6 +33,8 @@ static int flags_by_mnt(int mnt_flags)
 		flags |= ST_UNBINDABLE;
 	if (mnt_flags & MNT_SHARED)
 		flags |= ST_SHARED;
+	else
+		flags |= ST_PRIVATE;
 	return flags;
 }
 
diff --git a/include/linux/statfs.h b/include/linux/statfs.h
index 5416b2936dd9..1ea4a45aa6c3 100644
--- a/include/linux/statfs.h
+++ b/include/linux/statfs.h
@@ -41,6 +41,7 @@ struct kstatfs {
 #define ST_NODIRATIME	(1<<11) /* do not update directory access times */
 #define ST_RELATIME	(1<<12) /* update atime relative to mtime/ctime */
 #define ST_UNBINDABLE	(1<<17)	/* change to unbindable */
+#define ST_PRIVATE	(1<<18)	/* change to private */
 #define ST_SHARED	(1<<20)	/* change to shared */
 
 #endif
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/6] statfs: add ST_SLAVE
  2018-04-13 16:11 [PATCH 0/6] statfs: handle mount propagation Christian Brauner
                   ` (4 preceding siblings ...)
  2018-04-13 16:11 ` [PATCH 5/6] statfs: add ST_PRIVATE Christian Brauner
@ 2018-04-13 16:11 ` Christian Brauner
  5 siblings, 0 replies; 13+ messages in thread
From: Christian Brauner @ 2018-04-13 16:11 UTC (permalink / raw)
  To: viro, tglx, kstewart, gregkh, pombredanne, linux-fsdevel,
	linux-kernel, serge
  Cc: Christian Brauner

This lets userspace query whether a mountpoint was made MS_SLAVE.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/statfs.c            | 10 +++++++++-
 include/linux/statfs.h |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/statfs.c b/fs/statfs.c
index 26cda2586d7e..86e957d16a68 100644
--- a/fs/statfs.c
+++ b/fs/statfs.c
@@ -10,6 +10,7 @@
 #include <linux/uaccess.h>
 #include <linux/compat.h>
 #include "internal.h"
+#include "pnode.h"
 
 static int flags_by_mnt(int mnt_flags)
 {
@@ -52,8 +53,15 @@ static int flags_by_sb(int s_flags)
 
 static int calculate_f_flags(struct vfsmount *mnt)
 {
-	return ST_VALID | flags_by_mnt(mnt->mnt_flags) |
+	int flags = 0;
+
+	flags = ST_VALID | flags_by_mnt(mnt->mnt_flags) |
 		flags_by_sb(mnt->mnt_sb->s_flags);
+
+	if (IS_MNT_SLAVE(real_mount(mnt)))
+		flags |= ST_SLAVE;
+
+	return flags;
 }
 
 static int statfs_by_dentry(struct dentry *dentry, struct kstatfs *buf)
diff --git a/include/linux/statfs.h b/include/linux/statfs.h
index 1ea4a45aa6c3..663fa5498a7d 100644
--- a/include/linux/statfs.h
+++ b/include/linux/statfs.h
@@ -42,6 +42,7 @@ struct kstatfs {
 #define ST_RELATIME	(1<<12) /* update atime relative to mtime/ctime */
 #define ST_UNBINDABLE	(1<<17)	/* change to unbindable */
 #define ST_PRIVATE	(1<<18)	/* change to private */
+#define ST_SLAVE	(1<<19)	/* change to slave */
 #define ST_SHARED	(1<<20)	/* change to shared */
 
 #endif
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] fs: use << for MS_* flags
  2018-04-13 16:11 ` [PATCH 1/6] fs: use << for MS_* flags Christian Brauner
@ 2018-04-13 16:45   ` Randy Dunlap
  2018-04-13 20:19     ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: Randy Dunlap @ 2018-04-13 16:45 UTC (permalink / raw)
  To: Christian Brauner, viro, tglx, kstewart, gregkh, pombredanne,
	linux-fsdevel, linux-kernel, serge

On 04/13/2018 09:11 AM, Christian Brauner wrote:
> Consistenly use << to define MS_* constants.
> 
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
>  include/uapi/linux/fs.h | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index d2a8313fabd7..9662790a657c 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -105,22 +105,23 @@ struct inodes_stat_t {
>  /*
>   * These are the fs-independent mount-flags: up to 32 flags are supported
>   */
> -#define MS_RDONLY	 1	/* Mount read-only */
> -#define MS_NOSUID	 2	/* Ignore suid and sgid bits */
> -#define MS_NODEV	 4	/* Disallow access to device special files */
> -#define MS_NOEXEC	 8	/* Disallow program execution */
> -#define MS_SYNCHRONOUS	16	/* Writes are synced at once */
> -#define MS_REMOUNT	32	/* Alter flags of a mounted FS */
> -#define MS_MANDLOCK	64	/* Allow mandatory locks on an FS */
> -#define MS_DIRSYNC	128	/* Directory modifications are synchronous */
> -#define MS_NOATIME	1024	/* Do not update access times. */
> -#define MS_NODIRATIME	2048	/* Do not update directory access times */
> -#define MS_BIND		4096
> -#define MS_MOVE		8192
> -#define MS_REC		16384
> -#define MS_VERBOSE	32768	/* War is peace. Verbosity is silence.
> -				   MS_VERBOSE is deprecated. */
> -#define MS_SILENT	32768
> +#define MS_RDONLY	(1<<0)	/* Mount read-only */

Why not just use BIT(n) instead?

#include <linux/bitops.h>

#define MS_RDONLY	BIT(0)	/* Mount read-only */

etc.

> +#define MS_NOSUID	(1<<1)	/* Ignore suid and sgid bits */
> +#define MS_NODEV	(1<<2)	/* Disallow access to device special files */
> +#define MS_NOEXEC	(1<<3)	/* Disallow program execution */
> +#define MS_SYNCHRONOUS	(1<<4)	/* Writes are synced at once */
> +#define MS_REMOUNT	(1<<5)	/* Alter flags of a mounted FS */
> +#define MS_MANDLOCK	(1<<6)	/* Allow mandatory locks on an FS */
> +#define MS_DIRSYNC	(1<<7)	/* Directory modifications are synchronous */
> +#define MS_NOATIME	(1<<10)	/* Do not update access times. */
> +#define MS_NODIRATIME	(1<<11)	/* Do not update directory access times */
> +#define MS_BIND		(1<<12)
> +#define MS_MOVE		(1<<13)
> +#define MS_REC		(1<<14)
> +#define MS_VERBOSE	(1<<15)	/* War is peace. Verbosity is silence.
> +				 * MS_VERBOSE is deprecated.
> +				 */
> +#define MS_SILENT	(1<<15)
>  #define MS_POSIXACL	(1<<16)	/* VFS does not apply the umask */
>  #define MS_UNBINDABLE	(1<<17)	/* change to unbindable */
>  #define MS_PRIVATE	(1<<18)	/* change to private */
> 


-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/6] statfs: use << to align with fs header
  2018-04-13 16:11 ` [PATCH 2/6] statfs: use << to align with fs header Christian Brauner
@ 2018-04-13 16:47   ` Randy Dunlap
  2018-04-13 17:35   ` Andreas Dilger
  1 sibling, 0 replies; 13+ messages in thread
From: Randy Dunlap @ 2018-04-13 16:47 UTC (permalink / raw)
  To: Christian Brauner, viro, tglx, kstewart, gregkh, pombredanne,
	linux-fsdevel, linux-kernel, serge

On 04/13/2018 09:11 AM, Christian Brauner wrote:
> Consistenly use << to define ST_* constants. This also aligns them with
> their MS_* counterparts in fs.h
> 
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
>  include/linux/statfs.h | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)

Lots of opportunities to use BIT(n) macro.
Is there some reason not to do that?


> diff --git a/include/linux/statfs.h b/include/linux/statfs.h
> index 3142e98546ac..b336c04e793c 100644
> --- a/include/linux/statfs.h
> +++ b/include/linux/statfs.h
> @@ -27,18 +27,18 @@ struct kstatfs {
>   * ABI.  The exception is ST_VALID which has the same value as MS_REMOUNT
>   * which doesn't make any sense for statfs.
>   */
> -#define ST_RDONLY	0x0001	/* mount read-only */
> -#define ST_NOSUID	0x0002	/* ignore suid and sgid bits */
> -#define ST_NODEV	0x0004	/* disallow access to device special files */
> -#define ST_NOEXEC	0x0008	/* disallow program execution */
> -#define ST_SYNCHRONOUS	0x0010	/* writes are synced at once */
> -#define ST_VALID	0x0020	/* f_flags support is implemented */
> -#define ST_MANDLOCK	0x0040	/* allow mandatory locks on an FS */
> -/* 0x0080 used for ST_WRITE in glibc */
> -/* 0x0100 used for ST_APPEND in glibc */
> -/* 0x0200 used for ST_IMMUTABLE in glibc */
> -#define ST_NOATIME	0x0400	/* do not update access times */
> -#define ST_NODIRATIME	0x0800	/* do not update directory access times */
> -#define ST_RELATIME	0x1000	/* update atime relative to mtime/ctime */
> +#define ST_RDONLY	(1<<0) /* mount read-only */
> +#define ST_NOSUID	(1<<1) /* ignore suid and sgid bits */
> +#define ST_NODEV	(1<<2) /* disallow access to device special files */
> +#define ST_NOEXEC	(1<<3) /* disallow program execution */
> +#define ST_SYNCHRONOUS	(1<<4) /* writes are synced at once */
> +#define ST_VALID	(1<<5) /* f_flags support is implemented */
> +#define ST_MANDLOCK	(1<<6) /* allow mandatory locks on an FS */
> +/* (1<<7) used for ST_WRITE in glibc */
> +/* (1<<8) used for ST_APPEND in glibc */
> +/* (1<<9) used for ST_IMMUTABLE in glibc */
> +#define ST_NOATIME	(1<<10) /* do not update access times */
> +#define ST_NODIRATIME	(1<<11) /* do not update directory access times */
> +#define ST_RELATIME	(1<<12) /* update atime relative to mtime/ctime */
>  
>  #endif
> 


-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/6] statfs: use << to align with fs header
  2018-04-13 16:11 ` [PATCH 2/6] statfs: use << to align with fs header Christian Brauner
  2018-04-13 16:47   ` Randy Dunlap
@ 2018-04-13 17:35   ` Andreas Dilger
  2018-04-13 17:55     ` Randy Dunlap
  1 sibling, 1 reply; 13+ messages in thread
From: Andreas Dilger @ 2018-04-13 17:35 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Alexander Viro, Thomas Gleixner, Kate Stewart,
	Greg Kroah-Hartman, Philippe Ombredanne, Linux FS Devel,
	linux-kernel, serge

[-- Attachment #1: Type: text/plain, Size: 2856 bytes --]

On Apr 13, 2018, at 10:11 AM, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> 
> Consistenly use << to define ST_* constants. This also aligns them with
> their MS_* counterparts in fs.h

IMHO, using (1 << 10) makes the code harder to debug.  If you see a field
in a structure like 0x8354, it is non-trivial to map this to the ST_*
flags if they are declared in the form (1 << 10) or BIT(10).  If they are
declared in the form 0x100 (as they are now) then it is trivial that the
ST_APPEND flag is set in 0x8354, and easy to understand the other flags.

So, my preference would be to NOT land this or the previous patch.

Cheers, Andreas

> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> include/linux/statfs.h | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/statfs.h b/include/linux/statfs.h
> index 3142e98546ac..b336c04e793c 100644
> --- a/include/linux/statfs.h
> +++ b/include/linux/statfs.h
> @@ -27,18 +27,18 @@ struct kstatfs {
>  * ABI.  The exception is ST_VALID which has the same value as MS_REMOUNT
>  * which doesn't make any sense for statfs.
>  */
> -#define ST_RDONLY	0x0001	/* mount read-only */
> -#define ST_NOSUID	0x0002	/* ignore suid and sgid bits */
> -#define ST_NODEV	0x0004	/* disallow access to device special files */
> -#define ST_NOEXEC	0x0008	/* disallow program execution */
> -#define ST_SYNCHRONOUS	0x0010	/* writes are synced at once */
> -#define ST_VALID	0x0020	/* f_flags support is implemented */
> -#define ST_MANDLOCK	0x0040	/* allow mandatory locks on an FS */
> -/* 0x0080 used for ST_WRITE in glibc */
> -/* 0x0100 used for ST_APPEND in glibc */
> -/* 0x0200 used for ST_IMMUTABLE in glibc */
> -#define ST_NOATIME	0x0400	/* do not update access times */
> -#define ST_NODIRATIME	0x0800	/* do not update directory access times */
> -#define ST_RELATIME	0x1000	/* update atime relative to mtime/ctime */
> +#define ST_RDONLY	(1<<0) /* mount read-only */
> +#define ST_NOSUID	(1<<1) /* ignore suid and sgid bits */
> +#define ST_NODEV	(1<<2) /* disallow access to device special files */
> +#define ST_NOEXEC	(1<<3) /* disallow program execution */
> +#define ST_SYNCHRONOUS	(1<<4) /* writes are synced at once */
> +#define ST_VALID	(1<<5) /* f_flags support is implemented */
> +#define ST_MANDLOCK	(1<<6) /* allow mandatory locks on an FS */
> +/* (1<<7) used for ST_WRITE in glibc */
> +/* (1<<8) used for ST_APPEND in glibc */
> +/* (1<<9) used for ST_IMMUTABLE in glibc */
> +#define ST_NOATIME	(1<<10) /* do not update access times */
> +#define ST_NODIRATIME	(1<<11) /* do not update directory access times */
> +#define ST_RELATIME	(1<<12) /* update atime relative to mtime/ctime */
> 
> #endif
> --
> 2.17.0
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/6] statfs: use << to align with fs header
  2018-04-13 17:35   ` Andreas Dilger
@ 2018-04-13 17:55     ` Randy Dunlap
  2018-04-13 18:32       ` Christian Brauner
  0 siblings, 1 reply; 13+ messages in thread
From: Randy Dunlap @ 2018-04-13 17:55 UTC (permalink / raw)
  To: Andreas Dilger, Christian Brauner
  Cc: Alexander Viro, Thomas Gleixner, Kate Stewart,
	Greg Kroah-Hartman, Philippe Ombredanne, Linux FS Devel,
	linux-kernel, serge

On 04/13/2018 10:35 AM, Andreas Dilger wrote:
> On Apr 13, 2018, at 10:11 AM, Christian Brauner <christian.brauner@ubuntu.com> wrote:
>>
>> Consistenly use << to define ST_* constants. This also aligns them with
>> their MS_* counterparts in fs.h
> 
> IMHO, using (1 << 10) makes the code harder to debug.  If you see a field
> in a structure like 0x8354, it is non-trivial to map this to the ST_*
> flags if they are declared in the form (1 << 10) or BIT(10).  If they are
> declared in the form 0x100 (as they are now) then it is trivial that the
> ST_APPEND flag is set in 0x8354, and easy to understand the other flags.
> 
> So, my preference would be to NOT land this or the previous patch.

That makes sense to me.

-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/6] statfs: use << to align with fs header
  2018-04-13 17:55     ` Randy Dunlap
@ 2018-04-13 18:32       ` Christian Brauner
  0 siblings, 0 replies; 13+ messages in thread
From: Christian Brauner @ 2018-04-13 18:32 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Andreas Dilger, Alexander Viro, Thomas Gleixner, Kate Stewart,
	Greg Kroah-Hartman, Philippe Ombredanne, Linux FS Devel,
	linux-kernel, serge

On Fri, Apr 13, 2018 at 10:55:23AM -0700, Randy Dunlap wrote:
> On 04/13/2018 10:35 AM, Andreas Dilger wrote:
> > On Apr 13, 2018, at 10:11 AM, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> >>
> >> Consistenly use << to define ST_* constants. This also aligns them with
> >> their MS_* counterparts in fs.h
> > 
> > IMHO, using (1 << 10) makes the code harder to debug.  If you see a field
> > in a structure like 0x8354, it is non-trivial to map this to the ST_*
> > flags if they are declared in the form (1 << 10) or BIT(10).  If they are
> > declared in the form 0x100 (as they are now) then it is trivial that the
> > ST_APPEND flag is set in 0x8354, and easy to understand the other flags.
> > 
> > So, my preference would be to NOT land this or the previous patch.

All higher values are already initialized with bit-shifts for MS_*
constants starting with (1<<16) as you can see from the patch and in
fs.h:

> +#define MS_VERBOSE     (1<<15) /* War is peace. Verbosity is silence.
> +                                * MS_VERBOSE is deprecated.
> +                                */
> +#define MS_SILENT      (1<<15)
>  #define MS_POSIXACL    (1<<16) /* VFS does not apply the umask */
>  #define MS_UNBINDABLE  (1<<17) /* change to unbindable */
>  #define MS_PRIVATE     (1<<18) /* change to private */

This just makes it uniform which imho has merit on its own.

If using shifts is considered a valid counter argument because for lack
of ease to analyze struct fields then the values for MS_* flags in fs.h
should probably all be hex values.

In any case, I'm not going to bikeshed over this. The two patches can
simply be left out when applying or I can change it all over to hex
values.

Christian

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] fs: use << for MS_* flags
  2018-04-13 16:45   ` Randy Dunlap
@ 2018-04-13 20:19     ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2018-04-13 20:19 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Christian Brauner, viro, tglx, kstewart, pombredanne,
	linux-fsdevel, linux-kernel, serge

On Fri, Apr 13, 2018 at 09:45:01AM -0700, Randy Dunlap wrote:
> On 04/13/2018 09:11 AM, Christian Brauner wrote:
> > Consistenly use << to define MS_* constants.
> > 
> > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> > ---
> >  include/uapi/linux/fs.h | 33 +++++++++++++++++----------------
> >  1 file changed, 17 insertions(+), 16 deletions(-)
> > 
> > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> > index d2a8313fabd7..9662790a657c 100644
> > --- a/include/uapi/linux/fs.h
> > +++ b/include/uapi/linux/fs.h
> > @@ -105,22 +105,23 @@ struct inodes_stat_t {
> >  /*
> >   * These are the fs-independent mount-flags: up to 32 flags are supported
> >   */
> > -#define MS_RDONLY	 1	/* Mount read-only */
> > -#define MS_NOSUID	 2	/* Ignore suid and sgid bits */
> > -#define MS_NODEV	 4	/* Disallow access to device special files */
> > -#define MS_NOEXEC	 8	/* Disallow program execution */
> > -#define MS_SYNCHRONOUS	16	/* Writes are synced at once */
> > -#define MS_REMOUNT	32	/* Alter flags of a mounted FS */
> > -#define MS_MANDLOCK	64	/* Allow mandatory locks on an FS */
> > -#define MS_DIRSYNC	128	/* Directory modifications are synchronous */
> > -#define MS_NOATIME	1024	/* Do not update access times. */
> > -#define MS_NODIRATIME	2048	/* Do not update directory access times */
> > -#define MS_BIND		4096
> > -#define MS_MOVE		8192
> > -#define MS_REC		16384
> > -#define MS_VERBOSE	32768	/* War is peace. Verbosity is silence.
> > -				   MS_VERBOSE is deprecated. */
> > -#define MS_SILENT	32768
> > +#define MS_RDONLY	(1<<0)	/* Mount read-only */
> 
> Why not just use BIT(n) instead?
> 
> #include <linux/bitops.h>
> 
> #define MS_RDONLY	BIT(0)	/* Mount read-only */

BIT() is not exported to uapi files :(

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-04-13 20:19 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-13 16:11 [PATCH 0/6] statfs: handle mount propagation Christian Brauner
2018-04-13 16:11 ` [PATCH 1/6] fs: use << for MS_* flags Christian Brauner
2018-04-13 16:45   ` Randy Dunlap
2018-04-13 20:19     ` Greg KH
2018-04-13 16:11 ` [PATCH 2/6] statfs: use << to align with fs header Christian Brauner
2018-04-13 16:47   ` Randy Dunlap
2018-04-13 17:35   ` Andreas Dilger
2018-04-13 17:55     ` Randy Dunlap
2018-04-13 18:32       ` Christian Brauner
2018-04-13 16:11 ` [PATCH 3/6] statfs: add ST_UNBINDABLE Christian Brauner
2018-04-13 16:11 ` [PATCH 4/6] statfs: add ST_SHARED Christian Brauner
2018-04-13 16:11 ` [PATCH 5/6] statfs: add ST_PRIVATE Christian Brauner
2018-04-13 16:11 ` [PATCH 6/6] statfs: add ST_SLAVE Christian Brauner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.