2018-04-13 16:16:07

by Christian Brauner

[permalink] [raw]
Subject: [PATCH 0/6] statfs: handle mount propagation

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



2018-04-13 16:13:44

by Christian Brauner

[permalink] [raw]
Subject: [PATCH 4/6] statfs: add ST_SHARED

This lets userspace query whether a mountpoint was made MS_SHARED.

Signed-off-by: Christian Brauner <[email protected]>
---
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


2018-04-13 16:13:49

by Christian Brauner

[permalink] [raw]
Subject: [PATCH 5/6] statfs: add ST_PRIVATE

This lets userspace query whether a mountpoint was made MS_PRIVATE.

Signed-off-by: Christian Brauner <[email protected]>
---
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


2018-04-13 16:14:20

by Christian Brauner

[permalink] [raw]
Subject: [PATCH 6/6] statfs: add ST_SLAVE

This lets userspace query whether a mountpoint was made MS_SLAVE.

Signed-off-by: Christian Brauner <[email protected]>
---
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


2018-04-13 16:14:28

by Christian Brauner

[permalink] [raw]
Subject: [PATCH 2/6] statfs: use << to align with fs header

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

Signed-off-by: Christian Brauner <[email protected]>
---
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


2018-04-13 16:14:45

by Christian Brauner

[permalink] [raw]
Subject: [PATCH 3/6] statfs: add ST_UNBINDABLE

This lets userspace query whether a mountpoint was made MS_UNBINDABLE.

Signed-off-by: Christian Brauner <[email protected]>
---
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


2018-04-13 16:15:27

by Christian Brauner

[permalink] [raw]
Subject: [PATCH 1/6] fs: use << for MS_* flags

Consistenly use << to define MS_* constants.

Signed-off-by: Christian Brauner <[email protected]>
---
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


2018-04-13 16:46:33

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 1/6] fs: use << for MS_* flags

On 04/13/2018 09:11 AM, Christian Brauner wrote:
> Consistenly use << to define MS_* constants.
>
> Signed-off-by: Christian Brauner <[email protected]>
> ---
> 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

2018-04-13 16:49:07

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 2/6] statfs: use << to align with fs header

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 <[email protected]>
> ---
> 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

2018-04-13 17:38:26

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH 2/6] statfs: use << to align with fs header

On Apr 13, 2018, at 10:11 AM, Christian Brauner <[email protected]> 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 <[email protected]>
> ---
> 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






Attachments:
signature.asc (890.00 B)
Message signed with OpenPGP

2018-04-13 17:58:22

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 2/6] statfs: use << to align with fs header

On 04/13/2018 10:35 AM, Andreas Dilger wrote:
> On Apr 13, 2018, at 10:11 AM, Christian Brauner <[email protected]> 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

2018-04-13 18:34:21

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH 2/6] statfs: use << to align with fs header

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 <[email protected]> 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

2018-04-13 20:21:22

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/6] fs: use << for MS_* flags

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 <[email protected]>
> > ---
> > 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 :(