Bug#639897: Please don't check /proc/mounts
Hi,
On Wed, Sep 07, 2011 at 11:30:17AM +0200, Sandro Tosi wrote:
> Hi Aurelien,
> thanks for your reply and sorry for mine being so late
>
> On Fri, Sep 2, 2011 at 21:56, Aurelien Jarno <aurel32@debian.org> wrote:
> > As always, when the libc doesn't have the information from a kernel
> > syscall, it needs to get it another way, and it often ends up using
> > /proc or /sys as the source of information. In some cases it can't take
> > huge amount of time, but the problem is not that the libc doesn't parse
> > the pseudo file inefficiently, just that the kernel has to generate
> > insane amount of useless data.
>
> Thanks for the explanation!
>
> > In your case of statvfs(), the problem has been solved a bit more than
> > one year ago, by exporting the missing data directly through the
> > syscall. For that you need a libc >= 2.13 and a kernel >= 2.6.36. I
> > guess in your case you are using Squeeze, which doesn't meet these
> > requirements.
>
> Yep, I'm on squeeze.
>
> > The libc patch is trivial to backport, having it in squeeze is mostly
> > about convincing the release managers that it's useful and safe. On the
>
> Can you point me to that relevant patch? I can try to talk to RM but I
> guess we won't get that far: given it won't solve the problem at hand
> just by itself, because it requires an updated kernel, it would
> probably get rejected - but worth a try.
Please find attached the two patches, one for the libc and one for the
architectures in ports. Even if they might look big at the first place,
they are actually quite trivail.
> > kernel side, it seems a bit more complicated, as the change has been
> > followed by a lot of small bug fixes.
>
> I see. What would you suggest as a better approach to solve our
> problem? is backporting libc a feasable solution? bpo already have
> 2.6.38 (or now even later) kernel version, so we can probably use
> that.
>
I don't think having a libc in backports.org is really a good idea,
especially with all the current multiarch changes, it would also mean
that plenty of other packages would have to be backported (most of the
toolchain for example). Given that backports is now an official Debian
service, it looks like the best is to get the needed changes in
eglibc/stable and use a kernel from backports.org.
Cheers,
Aurelien
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
commit 3cdaa6adb113a088fdfb87aa6d7747557eccc58d
Author: Ulrich Drepper <drepper@redhat.com>
Date: Wed Aug 11 14:07:28 2010 -0700
f_flags in Linux statfs implementation.
The 2.6.36 kernel provides an additional field in the statfs results.
Use this value in the statvfs emulation to avoid filling in f_flag
the hard way.
diff --git a/ChangeLog b/ChangeLog
index 1d12009..250eaf9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
2010-08-11 Ulrich Drepper <drepper@redhat.com>
+ * sysdeps/unix/sysv/linux/bits/statfs.h (struct statfs): Add f_flags
+ field.
+ (struct statfs64): Likewise.
+ (_STATFS_F_FLAGS): Define.
+ * sysdeps/unix/sysv/linux/s390/bits/statfs.h: Likewise.
+ * sysdeps/unix/sysv/linux/internal_statvfs.c (__statvfs_getflags):
+ Don't define if __ASSUME_STATFS_F_FLAGS is defined.
+ (ST_VALID): Define locally.
+ (INTERNAL_STATVFS): If f_flags has ST_VALID set don't call
+ __statvfs_getflags, use the provided value.
+ * sysdeps/unix/sysv/linux/kernel-features.h: Define
+ __ASSUME_STATFS_F_FLAGS.
+
* sysdeps/unix/sysv/linux/sys/inotify.h (IN_EXCL_UNLINK): Define.
* sysdeps/unix/sysv/linux/Makefile [subdir=misc] (sysdep_headers):
diff --git a/sysdeps/unix/sysv/linux/bits/statfs.h b/sysdeps/unix/sysv/linux/bits/statfs.h
index 0e27865..7bd90d0 100644
--- a/sysdeps/unix/sysv/linux/bits/statfs.h
+++ b/sysdeps/unix/sysv/linux/bits/statfs.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 1997, 1998, 2000, 2002, 2003 Free Software Foundation, Inc.
+/* Copyright (C) 1997,1998,2000,2002,2003,2010 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -42,7 +42,8 @@ struct statfs
__fsid_t f_fsid;
__SWORD_TYPE f_namelen;
__SWORD_TYPE f_frsize;
- __SWORD_TYPE f_spare[5];
+ __SWORD_TYPE f_flags;
+ __SWORD_TYPE f_spare[4];
};
#ifdef __USE_LARGEFILE64
@@ -58,10 +59,12 @@ struct statfs64
__fsid_t f_fsid;
__SWORD_TYPE f_namelen;
__SWORD_TYPE f_frsize;
- __SWORD_TYPE f_spare[5];
+ __SWORD_TYPE f_flags;
+ __SWORD_TYPE f_spare[4];
};
#endif
/* Tell code we have these members. */
#define _STATFS_F_NAMELEN
#define _STATFS_F_FRSIZE
+#define _STATFS_F_FLAGS
diff --git a/sysdeps/unix/sysv/linux/internal_statvfs.c b/sysdeps/unix/sysv/linux/internal_statvfs.c
index 59b173e..0169ae3 100644
--- a/sysdeps/unix/sysv/linux/internal_statvfs.c
+++ b/sysdeps/unix/sysv/linux/internal_statvfs.c
@@ -29,6 +29,11 @@
#include <sys/statfs.h>
#include <sys/statvfs.h>
#include "linux_fsinfo.h"
+#include "kernel-features.h"
+
+
+/* Special internal-only bit value. */
+#define ST_VALID 0x0020
#ifndef STATFS
@@ -37,6 +42,7 @@
# define INTERNAL_STATVFS __internal_statvfs
+# ifndef __ASSUME_STATFS_F_FLAGS
int
__statvfs_getflags (const char *name, int fstype, struct stat64 *st)
{
@@ -200,6 +206,7 @@ __statvfs_getflags (const char *name, int fstype, struct stat64 *st)
return result;
}
+# endif
#else
extern int __statvfs_getflags (const char *name, int fstype,
struct stat64 *st);
@@ -240,9 +247,14 @@ INTERNAL_STATVFS (const char *name, struct STATVFS *buf,
/* XXX I have no idea how to compute f_favail. Any idea??? */
buf->f_favail = buf->f_ffree;
- /* Determining the flags is tricky. We have to read /proc/mounts or
- the /etc/mtab file and search for the entry which matches the given
- file. The way we can test for matching filesystem is using the
- device number. */
- buf->f_flag = __statvfs_getflags (name, fsbuf->f_type, st);
+#ifndef __ASSUME_STATFS_F_FLAGS
+ if ((fsbuf->f_flags & ST_VALID) == 0)
+ /* Determining the flags is tricky. We have to read /proc/mounts or
+ the /etc/mtab file and search for the entry which matches the given
+ file. The way we can test for matching filesystem is using the
+ device number. */
+ buf->f_flag = __statvfs_getflags (name, fsbuf->f_type, st);
+ else
+#endif
+ buf->f_flag = fsbuf->f_flags ^ ST_VALID;
}
diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h
index b3f2456..838b310 100644
--- a/sysdeps/unix/sysv/linux/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/kernel-features.h
@@ -530,3 +530,8 @@
#if __LINUX_KERNEL_VERSION >= 0x020621
# define __ASSUME_RECVMMSG 1
#endif
+
+/* statfs fills in f_flags since 2.6.36. */
+#if __LINUX_KERNEL_VERSION >= 0x020624
+# define __ASSUME_STATFS_F_FLAGS 1
+#endif
diff --git a/sysdeps/unix/sysv/linux/s390/bits/statfs.h b/sysdeps/unix/sysv/linux/s390/bits/statfs.h
index d838e6b..956150b 100644
--- a/sysdeps/unix/sysv/linux/s390/bits/statfs.h
+++ b/sysdeps/unix/sysv/linux/s390/bits/statfs.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 1997, 1998, 2000, 2003 Free Software Foundation, Inc.
+/* Copyright (C) 1997, 1998, 2000, 2003, 2010 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -42,7 +42,8 @@ struct statfs
__fsid_t f_fsid;
int f_namelen;
int f_frsize;
- int f_spare[5];
+ int f_flags;
+ int f_spare[4];
};
#ifdef __USE_LARGEFILE64
@@ -58,10 +59,12 @@ struct statfs64
__fsid_t f_fsid;
int f_namelen;
int f_frsize;
- int f_spare[5];
+ int f_flags;
+ int f_spare[4];
};
#endif
/* Tell code we have this member. */
#define _STATFS_F_NAMELEN
#define _STATFS_F_FRSIZE
+#define _STATFS_F_FLAGS
commit 09551806132f732bfe966b516c54f1f006b17d18
Author: Joseph Myers <joseph@codesourcery.com>
Date: Thu Aug 12 22:18:00 2010 +0000
Add f_flags to struct statfs for MIPS.
diff --git a/ChangeLog.mips b/ChangeLog.mips
index 5623642..2dbed6f 100644
--- a/ChangeLog.mips
+++ b/ChangeLog.mips
@@ -1,5 +1,10 @@
2010-08-12 Joseph Myers <joseph@codesourcery.com>
+ * sysdeps/unix/sysv/linux/mips/bits/statfs.h (struct statfs,
+ struct statfs64): Add f_flags field.
+
+2010-08-12 Joseph Myers <joseph@codesourcery.com>
+
* sysdeps/unix/sysv/linux/mips/sys/inotify.h (IN_EXCL_UNLINK):
Define.
diff --git a/sysdeps/unix/sysv/linux/mips/bits/statfs.h b/sysdeps/unix/sysv/linux/mips/bits/statfs.h
index 2f9bd54..22a9dde 100644
--- a/sysdeps/unix/sysv/linux/mips/bits/statfs.h
+++ b/sysdeps/unix/sysv/linux/mips/bits/statfs.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 1997, 2000 Free Software Foundation, Inc.
+/* Copyright (C) 1997, 2000, 2010 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -45,7 +45,8 @@ struct statfs
/* Linux specials */
__fsid_t f_fsid;
long int f_namelen;
- long int f_spare[6];
+ long int f_flags;
+ long int f_spare[5];
};
#ifdef __USE_LARGEFILE64
@@ -64,7 +65,8 @@ struct statfs64
/* Linux specials */
__fsid_t f_fsid;
long int f_namelen;
- long int f_spare[6];
+ long int f_flags;
+ long int f_spare[5];
};
#endif
Reply to: