Re: [git pull] new mount API

[Miklos Szeredi <miklos@szeredi.hu>]

On Fri, Aug 24, 2018 at 10:45:31AM +0100, David Howells wrote:
> Miklos Szeredi <miklos@szeredi.hu> wrote:
> 
> > I'm not against merging this patchset (have nits about resuing the
> > MS_* constants for the new API that I've complained about, but that's
> > really easy to fix before -final),
> 
> Can you send me a patch that does what you want here?  They're only used by
> fsmount() for creating a vfsmount and not used for the superblock creation or
> reconfiguration.

I'm bothered by the fact that we use the same MS_ prefix in the old mount(2) api
and the new fsmount(2) api.  What happens if we introduce new flags for
fsmount(2) and are already out of flags for mount(2)?  I see a big mess that
way.

Also notice, how the old code just totally ignored MS_RELATIME?  Bit of rot in
the new interface already.

So here's a patch. The MNT_ prefix is already used by libmount, and we don't
want any confusion arising from that.  So how about M_*?  Short and sweet, just
like O_* for open(2).

Reusing the same values as MNT_ isn't important, but it does reduce the code
size a bit.  Especially if we'd switch to MNT_STRICTATIME internally as well.


Thanks,
Miklos
---

diff --git a/fs/namespace.c b/fs/namespace.c
index e34e3fd064b0..a4b8503f80b8 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3351,7 +3351,7 @@ EXPORT_SYMBOL_GPL(kern_mount);
  * Create a kernel mount representation for a new, prepared superblock
  * (specified by fs_fd) and attach to an open_tree-like file descriptor.
  */
-SYSCALL_DEFINE3(fsmount, int, fs_fd, unsigned int, flags, unsigned int, ms_flags)
+SYSCALL_DEFINE3(fsmount, int, fs_fd, unsigned int, flags, unsigned int, m_flags)
 {
 	struct fs_context *fc;
 	struct file *file;
@@ -3366,27 +3366,21 @@ SYSCALL_DEFINE3(fsmount, int, fs_fd, unsigned int, flags, unsigned int, ms_flags
 	if ((flags & ~(FSMOUNT_CLOEXEC)) != 0)
 		return -EINVAL;
 
-	if (ms_flags & ~(MS_RDONLY | MS_NOSUID | MS_NODEV | MS_NOEXEC |
-			 MS_NOATIME | MS_NODIRATIME | MS_RELATIME |
-			 MS_STRICTATIME))
+	if (m_flags & ~(M_NOSUID | M_NODEV | M_NOEXEC |	M_NOATIME |
+			M_NODIRATIME | M_STRICTATIME | M_RDONLY))
 		return -EINVAL;
 
-	if (ms_flags & MS_RDONLY)
-		mnt_flags |= MNT_READONLY;
-	if (ms_flags & MS_NOSUID)
-		mnt_flags |= MNT_NOSUID;
-	if (ms_flags & MS_NODEV)
-		mnt_flags |= MNT_NODEV;
-	if (ms_flags & MS_NOEXEC)
-		mnt_flags |= MNT_NOEXEC;
-	if (ms_flags & MS_NODIRATIME)
-		mnt_flags |= MNT_NODIRATIME;
+	BUILD_BUG_ON(M_NOSUID != MNT_NOSUID || M_NODEV != MNT_NODEV ||
+		     M_NOEXEC != MNT_NOEXEC || M_NOATIME != MNT_NOATIME ||
+		     M_NODIRATIME != MNT_NODIRATIME ||
+		     M_STRICTATIME != MNT_RELATIME || M_RDONLY != MNT_READONLY);
+
+	/* Relatime is the default, but internally that's what we flag */
+	mnt_flags = m_flags & ~M_STRICTATIME;
 
-	if (ms_flags & MS_STRICTATIME) {
-		if (ms_flags & MS_NOATIME)
+	if (m_flags & M_STRICTATIME) {
+		if (m_flags & M_NOATIME)
 			return -EINVAL;
-	} else if (ms_flags & MS_NOATIME) {
-		mnt_flags |= MNT_NOATIME;
 	} else {
 		mnt_flags |= MNT_RELATIME;
 	}
diff --git a/include/uapi/linux/mount.h b/include/uapi/linux/mount.h
index 3634e065836c..331e69f5a7bf 100644
--- a/include/uapi/linux/mount.h
+++ b/include/uapi/linux/mount.h
@@ -1,6 +1,15 @@
 #ifndef _UAPI_LINUX_MOUNT_H
 #define _UAPI_LINUX_MOUNT_H
 
+/* Mount flags passed to fsmount(2) */
+#define M_NOSUID	0x01
+#define M_NODEV		0x02
+#define M_NOEXEC	0x04
+#define M_NOATIME	0x08
+#define M_NODIRATIME	0x10
+#define M_STRICTATIME	0x20
+#define M_RDONLY	0x40
+
 /*
  * These are the fs-independent mount-flags: up to 32 flags are supported
  *