add f_flags to struct statfs(64)

Add a flags field to help glibc implementing statvfs(3) efficiently.

We copy the flag values from glibc, and add a new ST_VALID flag to denote that f_flags is implemented.

Index: linux-2.6/arch/mips/include/asm/statfs.h
===================================================================
@@ -33,7 +33,8 @@ struct statfs {
 	/* Linux specials */
 	__kernel_fsid_t	f_fsid;
 	long		f_namelen;
-	long		f_spare[6];
+	long		f_flags;
+	long		f_spare[5];
 };
 
 #if (_MIPS_SIM == _MIPS_SIM_ABI32) || (_MIPS_SIM == _MIPS_SIM_NABI32)

@@ -53,7 +54,8 @@ struct statfs64 {
 	__u64	f_bavail;
 	__kernel_fsid_t f_fsid;
 	__u32	f_namelen;
-	__u32	f_spare[6];
+	__u32	f_flags;
+	__u32	f_spare[5];
 };
 
 #endif /* _MIPS_SIM == _MIPS_SIM_ABI32 */

@@ -73,7 +75,8 @@ struct statfs64 {			/* Same as struct st
 	/* Linux specials */
 	__kernel_fsid_t	f_fsid;
 	long		f_namelen;
-	long		f_spare[6];
+	long		f_flags;
+	long		f_spare[5];
 };
 
 struct compat_statfs64 {

@@ -88,7 +91,8 @@ struct compat_statfs64 {
 	__u64	f_bavail;
 	__kernel_fsid_t f_fsid;
 	__u32	f_namelen;
-	__u32	f_spare[6];
+	__u32	f_flags;
+	__u32	f_spare[5];
 };
 
 #endif /* _MIPS_SIM == _MIPS_SIM_ABI64 */

Index: linux-2.6/arch/s390/include/asm/statfs.h
===================================================================
@@ -33,7 +33,8 @@ struct statfs {
 	__kernel_fsid_t f_fsid;
 	int  f_namelen;
 	int  f_frsize;
-	int  f_spare[5];
+	int  f_flags;
+	int  f_spare[4];
 };
 
 struct statfs64 {

@@ -47,7 +48,8 @@ struct statfs64 {
 	__kernel_fsid_t f_fsid;
 	int  f_namelen;
 	int  f_frsize;
-	int  f_spare[5];
+	int  f_flags;
+	int  f_spare[4];
 };
 
 struct compat_statfs64 {

@@ -61,7 +63,8 @@ struct compat_statfs64 {
 	__kernel_fsid_t f_fsid;
 	__u32 f_namelen;
 	__u32 f_frsize;
-	__u32 f_spare[5];
+	__u32 f_flags;
+	__u32 f_spare[4];
 };
 
 #endif /* __s390x__ */

Index: linux-2.6/include/asm-generic/statfs.h
===================================================================
@@ -33,7 +33,8 @@ struct statfs {
 	__kernel_fsid_t f_fsid;
 	__statfs_word f_namelen;
 	__statfs_word f_frsize;
-	__statfs_word f_spare[5];
+	__statfs_word f_flags;
+	__statfs_word f_spare[4];
 };
 
 /*

@@ -55,7 +56,8 @@ struct statfs64 {
 	__kernel_fsid_t f_fsid;
 	__statfs_word f_namelen;
 	__statfs_word f_frsize;
-	__statfs_word f_spare[5];
+	__statfs_word f_flags;
+	__statfs_word f_spare[4];
 } ARCH_PACK_STATFS64;
 
 /* 

@@ -77,7 +79,8 @@ struct compat_statfs64 {
 	__kernel_fsid_t f_fsid;
 	__u32 f_namelen;
 	__u32 f_frsize;
-	__u32 f_spare[5];
+	__u32 f_flags;
+	__u32 f_spare[4];
 } ARCH_PACK_COMPAT_STATFS64;
 
 #endif

Index: linux-2.6/include/linux/statfs.h
===================================================================
@@ -2,7 +2,6 @@
 #define _LINUX_STATFS_H
 
 #include <linux/types.h>
-
 #include <asm/statfs.h>
 
 struct kstatfs {

@@ -16,7 +15,29 @@ struct kstatfs {
 	__kernel_fsid_t f_fsid;
 	long f_namelen;
 	long f_frsize;
-	long f_spare[5];
+	long f_flags;
+	long f_spare[4];
 };
 
+/*
+ * Definitions for the flag in f_flag.
+ *
+ * Generally these flags are equivalent to the MS_ flags used in the mount
+ * 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 */
+
 #endif

Index: linux-2.6/fs/statfs.c
===================================================================
@@ -2,11 +2,40 @@
 #include <linux/module.h>
 #include <linux/fs.h>
 #include <linux/file.h>
+#include <linux/mount.h>
 #include <linux/namei.h>
 #include <linux/statfs.h>
 #include <linux/security.h>
 #include <linux/uaccess.h>
 
+static int calculate_f_flags(struct vfsmount *mnt)
+{
+	struct super_block *sb = mnt->mnt_sb;
+	long flags = ST_VALID;
+
+	if (mnt->mnt_flags & MNT_READONLY)
+		flags |= ST_RDONLY;
+	if (mnt->mnt_flags & MNT_NOSUID)
+		flags |= ST_NOSUID;
+	if (mnt->mnt_flags & MNT_NODEV)
+		flags |= ST_NODEV;
+	if (mnt->mnt_flags & MNT_NOEXEC)
+		flags |= ST_NOEXEC;
+	if (mnt->mnt_flags & MNT_NOATIME)
+		flags |= ST_NOATIME;
+	if (mnt->mnt_flags & MNT_NODIRATIME)
+		flags |= ST_NODIRATIME;
+	if (mnt->mnt_flags & MNT_RELATIME)
+		flags |= ST_RELATIME;
+
+	if (sb->s_flags & MS_SYNCHRONOUS)
+		flags |= ST_SYNCHRONOUS;
+	if (sb->s_flags & MS_MANDLOCK)
+		flags |= ST_MANDLOCK;
+
+	return flags;
+}
+
 int statfs_by_dentry(struct dentry *dentry, struct kstatfs *buf)
 {
 	int retval;

@@ -26,7 +55,12 @@ int statfs_by_dentry(struct dentry *dent
 
 int vfs_statfs(struct path *path, struct kstatfs *buf)
 {
-	return statfs_by_dentry(path->dentry, buf);
+	int error;
+
+	error = statfs_by_dentry(path->dentry, buf);
+	if (!error)
+		buf->f_flags = calculate_f_flags(path->mnt);
+	return error;
 }
 EXPORT_SYMBOL(vfs_statfs);
 

@@ -69,6 +103,7 @@ static int do_statfs_native(struct path
 		buf->f_fsid = st.f_fsid;
 		buf->f_namelen = st.f_namelen;
 		buf->f_frsize = st.f_frsize;
+		buf->f_flags = st.f_flags;
 		memset(buf->f_spare, 0, sizeof(buf->f_spare));
 	}
 	return 0;

@@ -96,6 +131,7 @@ static int do_statfs64(struct path *path
 		buf->f_fsid = st.f_fsid;
 		buf->f_namelen = st.f_namelen;
 		buf->f_frsize = st.f_frsize;
+		buf->f_flags = st.f_flags;
 		memset(buf->f_spare, 0, sizeof(buf->f_spare));
 	}
 	return 0;

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Some of the compat layers (and older kernels) don't copy the f_spare values, so user space should clear the field before doing the system call, and you should be ok.

There should be no actual _uninitialized_ values copied from the kernel. IOW, either the kernel writes zero, or it doesn't write anything at all. Anything else would be a security issue anyway (ie kernel stack data leak). Afaik, no kernel does that.

Problem solved.

Linus

Sometime before 2.4.0 (I posted the exact release in the previous thread) the kernel initalized unused fields to 0xff. So if we want to support these kernels it is an issue.

1.0     - doesn't touch spare fields
1.2.13  - doesn't touch spare fields
2.0.40  - copies spare fields from uninitialized kernel stack
2.2.26  - copies spare fields from uninitialized kernel stack

2.4 onward - zeroes spare fields

Thanks,
Miklos

I think at least some of the compat_ functions don't zero the fields even now (the kstatfs struct, yes, but then don't copy them to user space).

As to any uninitialized kernel stack copying, we really don't even want to care. Not only are those ancient kernels (and nobody is going to upgrade glibc if they haven't upgraded the kernel), but it's clearly a kernel bug, and the potential "wrong f_flags" problem is way smaller than the "leaks kernel data" problem.

Not that anybody even uses statvfs(). The first google hit on statvfs() I found was somebody implementing a compatibility statfs() on top of it, and obviously ignoring any f_flags field. So the only reason this is even an issue in the first place is just the tbench performance issue.

In other words: NOBODY CARES. It's that simple. So please, guys - just do the obvious thing, or shut the h*ll up about it. There is _no_ reason to care in the least about any of this.

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

Well we do want to be simple I think, but it's not *totally* just for dbench performance :) Actually dbench is supposed to be an fs syscall reply of samba running a file server workload. For one reason or another, it calls statvfs a lot.

Not sure if anything else is remotely performance critical, but statvfs is preferred these days for portability, so proper atomicity of f_flags is nice too.

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

It seems to call statvfs() mainly because it wants the disk free block information, afaik. I can't find anything that cares about f_flag.

In fact, the few f_flag usages I found on google code search have that access conditional, usually by some configure script thing (ie HAVE_STATVFS_F_FLAG). The samba code I found was to _generate_ f_flag (from internal samba code), not to actually use it.

Oh, I'm sure I missed some case, but quite frankly, from looking at the stuff I _did_ find, it looks like the real problem has always been the stupid glibc emulation code rather than anything else. Without that emulation code and without any f_flags member in user space, everything I saw would have been perfectly happy. They'd just have compiled the f_flag code away.

So from my quick grep, it really does look like the real problem was always that glibc made things overly complicated, and emulated some documented SuS interface (slowly) that nobody really ever cared about, and thus made up the performance problem in the first place. Yes, it's mentioned in SuS, but at the same time, judging by the config script hackery, different unixes seem to have called it "f_flag" and "f_flags" or not had it at all. So it can't be _that_ important.

And other uses are equally happy to use statfs() instead of statvfs(), again making it more or less clear that they certainly don't care about the magic f_flag field.

It looks like another "paper standard" thing, in other words.

And if this teaches us anything, that _should_ be that people should not try to emulate paper standards badly. If there is no native system call, don't emulate it with some random crap.

Linus

People calls statvfs instead of statfs because it's the version that got standardized in Posix.

And with the glibc emulation of the flags damage is done now, and given how easy it is to fix (see my patches) there is no reason not to fill out the flags. We just need to figure out how. I see three theoretical variants:

(1) add new system calls
(2) drop the ST_VALID flag and only look at the flags field for

     2.6.36+ kernels.

(3) keep the ST_VALID flag in my patches and only look at the flags field in glibc for 2.6 kernels

(1) is pretty ugly, and Linus already vetoed it, (2) is okayish, but prevents distros from backporting this easy speedup in a useful way, so I'd prefer variant (3). If Uli is okay with that we should put my second version of the patches into the vfs tree and pull them for 2.6.36. If version (2) is preferred I can update the patches again.

I certainly don't mind the ST_VALID field. I don't think it's strictly _necessary_, but there's nothing really wrong with it either. So personally I'm fine with either 2 or 3.

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

glibc could just set 2.4.0 as the oldest version to test ST_VALID against. I think most distros compile it to support no older than 2.6.something kernels anyway.

Should we not allow glibc to assume the mapping is 1:1 except ST_VALID? If other spare fields may be used in future, this could be the best place to do versioning or otherwise make use of some bits. Possibly a couple of bits could be used for version number.