[PATCH 2/2] add f_flags to struct statfs(64)

[PATCH 2/2] add f_flags to struct statfs(64)

[Christoph Hellwig]

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.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/arch/mips/include/asm/statfs.h
===================================================================
--- linux-2.6.orig/arch/mips/include/asm/statfs.h	2010-06-27 11:18:56.228261526 +0200
+++ linux-2.6/arch/mips/include/asm/statfs.h	2010-06-29 16:58:13.036004011 +0200
@@ -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
===================================================================
--- linux-2.6.orig/arch/s390/include/asm/statfs.h	2010-06-27 11:18:56.241254542 +0200
+++ linux-2.6/arch/s390/include/asm/statfs.h	2010-06-29 16:58:13.037004081 +0200
@@ -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
===================================================================
--- linux-2.6.orig/include/asm-generic/statfs.h	2010-06-27 11:18:56.257254961 +0200
+++ linux-2.6/include/asm-generic/statfs.h	2010-06-29 16:58:13.045157664 +0200
@@ -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
===================================================================
--- linux-2.6.orig/include/linux/statfs.h	2010-06-27 11:18:56.269254612 +0200
+++ linux-2.6/include/linux/statfs.h	2010-06-29 16:58:13.054173308 +0200
@@ -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
===================================================================
--- linux-2.6.orig/fs/statfs.c	2010-06-29 16:58:12.548255583 +0200
+++ linux-2.6/fs/statfs.c	2010-06-29 16:58:13.059033694 +0200
@@ -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;

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Ulrich Drepper]

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

On 07/07/2010 09:53 AM, Christoph Hellwig wrote:
> +	long flags = ST_VALID;

How does this work with old kernels which didn't initialize f_spare?

- -- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)

iEYEARECAAYFAkw0tToACgkQ2ijCOnn/RHS9DwCgwJpVPgf4yEdecLFZEIaW+oDJ
B+sAniphc4K83MWufH8cPCA2QepOSz/A
=gFVr
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Linus Torvalds]

On Wed, Jul 7, 2010 at 10:11 AM, Ulrich Drepper <drepper@redhat.com> wrote:
>
> How does this work with old kernels which didn't initialize f_spare?

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

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Christoph Hellwig]

On Wed, Jul 07, 2010 at 10:31:37AM -0700, Linus Torvalds wrote:
> On Wed, Jul 7, 2010 at 10:11 AM, Ulrich Drepper <drepper@redhat.com> wrote:
> >
> > How does this work with old kernels which didn't initialize f_spare?
> 
> 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.

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.

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Miklos Szeredi]

On Wed, 7 Jul 2010, Christoph Hellwig wrote:
> On Wed, Jul 07, 2010 at 10:31:37AM -0700, Linus Torvalds wrote:
> > On Wed, Jul 7, 2010 at 10:11 AM, Ulrich Drepper <drepper@redhat.com> wrote:
> > >
> > > How does this work with old kernels which didn't initialize f_spare?
> > 
> > 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.
> 
> 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

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Linus Torvalds]

On Wed, Jul 7, 2010 at 10:55 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> 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

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Nick Piggin]

On Wed, Jul 07, 2010 at 11:06:29AM -0700, Linus Torvalds wrote:
> On Wed, Jul 7, 2010 at 10:55 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > 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
> 
> 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.

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Linus Torvalds]

On Wed, Jul 7, 2010 at 11:50 AM, Nick Piggin <npiggin@suse.de> wrote:
>
> 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.

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

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Christoph Hellwig]

On Wed, Jul 07, 2010 at 12:30:50PM -0700, Linus Torvalds wrote:
> On Wed, Jul 7, 2010 at 11:50 AM, Nick Piggin <npiggin@suse.de> wrote:
> >
> > 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.
> 
> 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.

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.

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Linus Torvalds]

On Sat, Jul 17, 2010 at 11:41 PM, Christoph Hellwig <hch@lst.de> wrote:
>
>  (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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Nick Piggin]

On Wed, Jul 07, 2010 at 07:33:11PM +0200, Christoph Hellwig wrote:
> On Wed, Jul 07, 2010 at 10:31:37AM -0700, Linus Torvalds wrote:
> > On Wed, Jul 7, 2010 at 10:11 AM, Ulrich Drepper <drepper@redhat.com> wrote:
> > >
> > > How does this work with old kernels which didn't initialize f_spare?
> > 
> > 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.
> 
> 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.

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.

[PATCH 2/2] add f_flags to struct statfs(64)

[Christoph Hellwig]

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.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/arch/mips/include/asm/statfs.h
===================================================================
--- linux-2.6.orig/arch/mips/include/asm/statfs.h	2010-06-26 09:26:56.000000000 +0200
+++ linux-2.6/arch/mips/include/asm/statfs.h	2010-06-26 10:00:33.864004297 +0200
@@ -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
===================================================================
--- linux-2.6.orig/arch/s390/include/asm/statfs.h	2010-06-26 09:26:56.000000000 +0200
+++ linux-2.6/arch/s390/include/asm/statfs.h	2010-06-26 10:00:33.864004297 +0200
@@ -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
===================================================================
--- linux-2.6.orig/include/asm-generic/statfs.h	2010-06-26 09:26:56.000000000 +0200
+++ linux-2.6/include/asm-generic/statfs.h	2010-06-26 10:00:33.868047809 +0200
@@ -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,6 +79,7 @@ struct compat_statfs64 {
 	__kernel_fsid_t f_fsid;
 	__u32 f_namelen;
 	__u32 f_frsize;
+	__u32 f_flags[5];
 	__u32 f_spare[5];
 } ARCH_PACK_COMPAT_STATFS64;
 
Index: linux-2.6/include/linux/statfs.h
===================================================================
--- linux-2.6.orig/include/linux/statfs.h	2010-06-26 09:26:56.000000000 +0200
+++ linux-2.6/include/linux/statfs.h	2010-06-26 10:19:53.609016448 +0200
@@ -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
===================================================================
--- linux-2.6.orig/fs/statfs.c	2010-06-26 09:40:08.000000000 +0200
+++ linux-2.6/fs/statfs.c	2010-06-26 10:24:27.674272979 +0200
@@ -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;

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Nick Piggin]

On Sat, Jun 26, 2010 at 11:35:07AM +0200, Christoph Hellwig wrote:
> 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.

Thanks.

Acked-by: Nick Piggin <npiggin@suse.de>

Is f_favail important enough to add as well?

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Christoph Hellwig]

On Sat, Jun 26, 2010 at 07:56:53PM +1000, Nick Piggin wrote:
> Is f_favail important enough to add as well?

As Uli if he needs it.  Adding real support for it won't be quite
as easy as adding f_flags, as the individual filesystems would need
to support it.  And those not derived from FFS generally don't
have any different accounting of inodes allocates vs inodes allocated to
non-root user.  In fact faking up any kind of free inodes values is
already difficult enough for modern filesystems.

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Nick Piggin]

On Sat, Jun 26, 2010 at 03:09:28PM +0200, Christoph Hellwig wrote:
> On Sat, Jun 26, 2010 at 07:56:53PM +1000, Nick Piggin wrote:
> > Is f_favail important enough to add as well?
> 
> As Uli if he needs it.  Adding real support for it won't be quite
> as easy as adding f_flags, as the individual filesystems would need
> to support it.
>
>  And those not derived from FFS generally don't
> have any different accounting of inodes allocates vs inodes allocated to
> non-root user.  In fact faking up any kind of free inodes values is
> already difficult enough for modern filesystems.

Yeah, quite likely it is not worth using up another field for this,
and instead the statvfs Linux man page should be updated instead.

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Neil Brown]

On Sat, 26 Jun 2010 11:35:07 +0200
Christoph Hellwig <hch@lst.de> wrote:

> 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.

Hi Christoph,
 while we are adding flags to statfs, I wonder if it might be possible to do
 something to make f_fsid more useful - particularly for nfs-utils.

 Currently some filesystems (e.g. VFAT) set f_fsid based on the device that
 the filesystem is mounted from, while others (e.g. ext3) set f_fsid based on
 the filesystem itself (i.e. from the UUID stored in the superblock).

 This makes f_fsid not generally usable to make the filesystem part of a
 filehandle, as we really don't want to based it in the device if at all
 possible.   This is particularly frustrating as for btrfs, f_fsid is the
 only way to differentiate between subvolumes.

 If we could have a flag "ST_FSID_STABLE" (or similar) which indicates that
 the f_fsid is known to be stable across device renames, then I could remove
 some special casing that was recently added to nfs-utils for btrfs (or at
 least deprecate it).

 And if it was acceptable to use a couple more of the 'spares' to provide a
 full 128bit fsid rather than just a 64bit one, that would be even better.
 But I'm not sure it is worth it (64bits is still quite a lot).

Thanks,
NeilBrown

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: linux-2.6/arch/mips/include/asm/statfs.h
> ===================================================================
> --- linux-2.6.orig/arch/mips/include/asm/statfs.h	2010-06-26 09:26:56.000000000 +0200
> +++ linux-2.6/arch/mips/include/asm/statfs.h	2010-06-26 10:00:33.864004297 +0200
> @@ -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
> ===================================================================
> --- linux-2.6.orig/arch/s390/include/asm/statfs.h	2010-06-26 09:26:56.000000000 +0200
> +++ linux-2.6/arch/s390/include/asm/statfs.h	2010-06-26 10:00:33.864004297 +0200
> @@ -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
> ===================================================================
> --- linux-2.6.orig/include/asm-generic/statfs.h	2010-06-26 09:26:56.000000000 +0200
> +++ linux-2.6/include/asm-generic/statfs.h	2010-06-26 10:00:33.868047809 +0200
> @@ -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,6 +79,7 @@ struct compat_statfs64 {
>  	__kernel_fsid_t f_fsid;
>  	__u32 f_namelen;
>  	__u32 f_frsize;
> +	__u32 f_flags[5];
>  	__u32 f_spare[5];
>  } ARCH_PACK_COMPAT_STATFS64;
>  
> Index: linux-2.6/include/linux/statfs.h
> ===================================================================
> --- linux-2.6.orig/include/linux/statfs.h	2010-06-26 09:26:56.000000000 +0200
> +++ linux-2.6/include/linux/statfs.h	2010-06-26 10:19:53.609016448 +0200
> @@ -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
> ===================================================================
> --- linux-2.6.orig/fs/statfs.c	2010-06-26 09:40:08.000000000 +0200
> +++ linux-2.6/fs/statfs.c	2010-06-26 10:24:27.674272979 +0200
> @@ -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;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Christoph Hellwig]

On Sat, Jun 26, 2010 at 10:55:47PM +1000, Neil Brown wrote:
>  while we are adding flags to statfs, I wonder if it might be possible to do
>  something to make f_fsid more useful - particularly for nfs-utils.

f_fsid really isn't suitable for that.  Even filesystems that had UUIDs
since the dawn of days don't always store it there, e.g. XFS.  I wish
people had left it with that, but the recent rush to stupidly overload
it with an uuid light and using it in nfs-tools didn't help.

Having an API to expose a UUID to userspace would be useful, but f_fsid
isn't it.  If we'll have to add real statvfs syscalls we can add an
f_uuid field, but I'd rather not use up the reamining space in the
statfs structure for that.

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Tao Ma]

Hi Christoph,

On 06/26/2010 05:35 PM, Christoph Hellwig wrote:
> 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.
>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
>
<snip>
> Index: linux-2.6/include/asm-generic/statfs.h
> ===================================================================
> --- linux-2.6.orig/include/asm-generic/statfs.h	2010-06-26 09:26:56.000000000 +0200
> +++ linux-2.6/include/asm-generic/statfs.h	2010-06-26 10:00:33.868047809 +0200
> @@ -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,6 +79,7 @@ struct compat_statfs64 {
>   	__kernel_fsid_t f_fsid;
>   	__u32 f_namelen;
>   	__u32 f_frsize;
> +	__u32 f_flags[5];
>   	__u32 f_spare[5];
>   } ARCH_PACK_COMPAT_STATFS64;
This is a typo?

Regards,
Tao

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Christoph Hellwig]

On Sun, Jun 27, 2010 at 03:55:24PM +0800, Tao Ma wrote:
> >+	__u32 f_flags[5];
> >  	__u32 f_spare[5];
> >  } ARCH_PACK_COMPAT_STATFS64;
> This is a typo?

Yes, it is.

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Andreas Dilger]

On 2010-06-26, at 03:35, Christoph Hellwig wrote:
> We copy the flag values from glibc, and add a new ST_VALID flag to
> denote that f_flags is implemented.

Adding ST_VALID is only of marginal use, IMHO.  If we are worried about this field being filled with garbage, then a single bit still has a 50% chance of being set.

When there was a transition to binary flags in the mount(2) API, we had a full 16-bit magic value (0xC0ED0000, IIRC) in the high bits to verify the validity of the low 16-bit flags.  Then, a couple years later, when all of the tools and kernels were handling the binary flags properly the 16-bit magic was removed and we could use the full 32-bit value.

I would advocate at least an 8-bit magic value, if there is a concern about this field not always being 0-filled on older kernels, or not using any ST_VALID flag at all, if it was always zero-filled in the past.

Cheers, Andreas

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Ulrich Drepper]

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

On 06/28/2010 11:43 AM, Andreas Dilger wrote:
> I would advocate at least an 8-bit magic value, if there is a concern
> about this field not always being 0-filled on older kernels, or not
> using any ST_VALID flag at all, if it was always zero-filled in the
> past.

Did anyone go back and check which kernel versions didn't clear the
spare fields (if any)?

Instead of using a magic number which doesn't provide 100% security in
that case and steals a number of bits which might be needed in future I
rather see a new syscall.  The new {,f}statfs would be an alias for the
old one, exactly the same implementation.  With this it is much safer to
check for the new kernel behavior.  In future this isn't necessary since
from point on it is guaranteed that the spare bits are zeroed.

- -- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)

iEYEARECAAYFAkwo/ZgACgkQ2ijCOnn/RHSdVQCgzgHQj5A3/MLSYMbxX2xotoye
aPsAnRzGEK0pz7LYQ9mIYUHa/upoFuoS
=3oan
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Andreas Dilger]

On 2010-06-28, at 13:52, Ulrich Drepper wrote:
> Did anyone go back and check which kernel versions didn't clear the
> spare fields (if any)?

I haven't had a chance to do so (out of town for the weekend), so before we talk of a new system call I'd prefer to check that it is actually needed.

Cheers, Andreas

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Nick Piggin]

On Mon, Jun 28, 2010 at 12:52:56PM -0700, Ulrich Drepper wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 06/28/2010 11:43 AM, Andreas Dilger wrote:
> > I would advocate at least an 8-bit magic value, if there is a concern
> > about this field not always being 0-filled on older kernels, or not
> > using any ST_VALID flag at all, if it was always zero-filled in the
> > past.

ST_VALID is required to differentiate between no flags, and
flags field unsupported of course.

 
> Did anyone go back and check which kernel versions didn't clear the
> spare fields (if any)?

At least from 2.4.0 does memset it. 2.2.26 does not. Can you
live with that?

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Christoph Hellwig]

On Tue, Jun 29, 2010 at 06:57:59PM +1000, Nick Piggin wrote:
> On Mon, Jun 28, 2010 at 12:52:56PM -0700, Ulrich Drepper wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> > 
> > On 06/28/2010 11:43 AM, Andreas Dilger wrote:
> > > I would advocate at least an 8-bit magic value, if there is a concern
> > > about this field not always being 0-filled on older kernels, or not
> > > using any ST_VALID flag at all, if it was always zero-filled in the
> > > past.
> 
> ST_VALID is required to differentiate between no flags, and
> flags field unsupported of course.

Yes, that was my intention for it.

> 
> > spare fields (if any)?
> 
> At least from 2.4.0 does memset it. 2.2.26 does not. Can you
> live with that?

The spare fields in struct statfs have been zeroed since 2.3.99pre3-8,
before that they were set to a pattern of 0xff.

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Ulrich Drepper]

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

On 06/29/2010 01:57 AM, Nick Piggin wrote:
> ST_VALID is required to differentiate between no flags, and
> flags field unsupported of course.

Not needed if a new syscall would be used.


> At least from 2.4.0 does memset it. 2.2.26 does not. Can you
> live with that?

We still support building glibc for everything from linux 2.0 on, at
least for x86.  I really have no idea whether such old kernels are still
in use and if yes, whether userland gets updated.  I wouldn't have a
problem with bumping the minimum required kernel version.

- -- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)

iEYEARECAAYFAkwpw/kACgkQ2ijCOnn/RHT02gCcCOGJj/fr/WCHJpuJ8CYVMKRu
KhwAoKXX7L5sDXTk8g6CZdIHL0y7S+i4
=tlZM
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Nick Piggin]

On Tue, Jun 29, 2010 at 02:59:21AM -0700, Ulrich Drepper wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 06/29/2010 01:57 AM, Nick Piggin wrote:
> > ST_VALID is required to differentiate between no flags, and
> > flags field unsupported of course.
> 
> Not needed if a new syscall would be used.

True, but we have the spare fields.


> > At least from 2.4.0 does memset it. 2.2.26 does not. Can you
> > live with that?
> 
> We still support building glibc for everything from linux 2.0 on, at
> least for x86.  I really have no idea whether such old kernels are still
> in use and if yes, whether userland gets updated.  I wouldn't have a
> problem with bumping the minimum required kernel version.

I guess most would assume a 2.6 kernel nowadays? With either the
new syscall or Christoph's patch, you need to keep the fallback
around too.

Re: [PATCH 2/2] add f_flags to struct statfs(64)

[Christoph Hellwig]

On Tue, Jun 29, 2010 at 02:59:21AM -0700, Ulrich Drepper wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 06/29/2010 01:57 AM, Nick Piggin wrote:
> > ST_VALID is required to differentiate between no flags, and
> > flags field unsupported of course.
> 
> Not needed if a new syscall would be used.

True.  But if we add a new syscall anyway I'd rather implement the
glibc statvfs64 structure directly.  The gives filesystem which
really want to a chance to implement f_favail, and we could also
store the full uuid in the f_spare fields now that we have more space.