On 20/01/15 02:01, Pádraig Brady wrote: > On 30/12/14 22:06, Bernhard Voelker wrote: >> I just pulled the recent gnulib update, and now the above, expensive >> test fails: >> >> + ulimit -v 40000 >> + du -sh d >> du: fts_read failed: d: Cannot allocate memory >> + fail=1 >> >> I guess this due to the inclusion of libmount? > > Yes I get the same issue in that test: > http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=blob;f=tests/rm/many-dir-entries-vs-OOM.sh > > Indeed there is significant overhead in using libmount as shown below. > This is a crazy amount of overhead just to read /proc/self/mountinfo, > and is the sort of creeping dependencies I hate. The proposed solution > in the attached gnulib patch, is to require ./configure --with-libmount > to enable this feature. I.E. it's disabled by default. > > cheers, > Pádraig > > ======= without ========= > $ (ulimit -v 5380; du -s .) > > $ ldd /usr/bin/du > linux-vdso.so.1 => (0x00007fff0d7fe000) > libc.so.6 => /lib64/libc.so.6 (0x00007fd414a32000) > /lib64/ld-linux-x86-64.so.2 (0x00007fd414e0b000) > > $ time du -s src/du.c >/dev/null > real 0m0.003s > user 0m0.000s > sys 0m0.003s > > ======= with ========= > $ (ulimit -v 23250; src/du -s .) > > $ ldd src/du > linux-vdso.so.1 => (0x00007fff76ca8000) > libc.so.6 => /lib64/libc.so.6 (0x00007f2a1f742000) > /lib64/ld-linux-x86-64.so.2 (0x00007f2a1fd61000) > libmount.so.1 => /lib64/libmount.so.1 (0x00007f2a1faff000) > libblkid.so.1 => /lib64/libblkid.so.1 (0x00007f2a1f501000) > libuuid.so.1 => /lib64/libuuid.so.1 (0x00007f2a1f2fc000) > libselinux.so.1 => /lib64/libselinux.so.1 (0x00007f2a1f0d7000) > libpcre.so.1 => /lib64/libpcre.so.1 (0x00007f2a1ee69000) > liblzma.so.5 => /lib64/liblzma.so.5 (0x00007f2a1ec44000) > libdl.so.2 => /lib64/libdl.so.2 (0x00007f2a1ea40000) > libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f2a1e823000) > > $ time src/du -s src/du.c >/dev/null > real 0m0.006s > user 0m0.001s > sys 0m0.005s
Attached is a gnulib patch to avoid libmount entirely, for this fairly trivial parsing task. thanks, Pádraig.
From 0f17946b8c90ab26b38350185ec179aa6ce6b55f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Thu, 2 Apr 2015 04:18:02 +0100 Subject: [PATCH] mountlist: remove dependency on libmount * lib/mountlist.c (read_file_system_list): Parse /proc/self/mountinfo directly, rather than depending on libmount, which has many dependencies as detailed at: http://lists.gnu.org/archive/html/bug-gnulib/2015-01/msg00063.html Note we restrict this to __linux__ as that's probably where this interface will remain. If ever porting, it would be best to first pull the makedev() wrapper from coreutils to a gnulib module. Note also we don't add a getline dependency to the mountlist module, as all Linux versions are sufficient. --- ChangeLog | 13 ++++++ DEPENDENCIES | 7 ---- lib/mountlist.c | 119 ++++++++++++++++++++++++++++++++++++++++++------------- m4/ls-mntd-fs.m4 | 23 +---------- 4 files changed, 106 insertions(+), 56 deletions(-) diff --git a/ChangeLog b/ChangeLog index 41dd857..1b0fcbb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2015-04-02 Pádraig Brady <[email protected]> + + mountlist: remove dependency on libmount + * lib/mountlist.c (read_file_system_list): Parse /proc/self/mountinfo + directly, rather than depending on libmount, which has many + dependencies as detailed at: + http://lists.gnu.org/archive/html/bug-gnulib/2015-01/msg00063.html + Note we restrict this to __linux__ as that's probably where this + interface will remain. If ever porting, it would be best + to first pull the makedev() wrapper from coreutils to a gnulib module. + Note also we don't add a getline dependency to the mountlist module, + as all Linux versions are sufficient. + 2015-03-24 Pádraig Brady <[email protected]> quotearg-simple-tests: add missing gl_FUNC_MMAP_ANON dependency diff --git a/DEPENDENCIES b/DEPENDENCIES index 44f7ca0..e19a37e 100644 --- a/DEPENDENCIES +++ b/DEPENDENCIES @@ -162,10 +162,3 @@ at any time. + Download: http://ftp.gnu.org/gnu/libtool/ ftp://ftp.gnu.org/gnu/libtool/ - -* util-linux - + Optional. - Needed if you want to support /proc/self/mountinfo available on Linux. - This will give an ability to propagate device ID of a mounted file system. - + Download: - http://www.kernel.org/pub/linux/utils/util-linux/ diff --git a/lib/mountlist.c b/lib/mountlist.c index dce4ce6..38f4cbe 100644 --- a/lib/mountlist.c +++ b/lib/mountlist.c @@ -58,6 +58,7 @@ #ifdef MOUNTED_GETMNTENT1 /* 4.3BSD, SunOS, HP-UX, Dynix, Irix. */ # include <mntent.h> +# include <sys/types.h> # if !defined MOUNTED # if defined _PATH_MOUNTED /* GNU libc */ # define MOUNTED _PATH_MOUNTED @@ -128,12 +129,6 @@ # include <sys/mntent.h> #endif -#ifdef MOUNTED_PROC_MOUNTINFO -/* Use /proc/self/mountinfo instead of /proc/self/mounts (/etc/mtab) - * on Linux, if available */ -# include <libmount/libmount.h> -#endif - #ifndef HAVE_HASMNTOPT # define hasmntopt(mnt, opt) ((char *) 0) #endif @@ -392,6 +387,34 @@ dev_from_mount_options (char const *mount_options) #endif +#if defined MOUNTED_GETMNTENT1 && defined __linux__ + +/* Unescape the paths in mount tables. + STR is updated in place. */ + +static void +unescape_tab (char *str) +{ + size_t i, j = 0; + size_t len = strlen (str) + 1; + for (i = 0; i < len; i++) + { + if (str[i] == '\\' && (i + 4 < len) + && str[i + 1] >= '0' && str[i + 1] <= '3' + && str[i + 2] >= '0' && str[i + 2] <= '7' + && str[i + 3] >= '0' && str[i + 3] <= '7') + { + str[j++] = (str[i + 1] - '0') * 64 + + (str[i + 2] - '0') * 8 + + (str[i + 3] - '0'); + i += 3; + } + else + str[j++] = str[i]; + } +} +#endif + /* Return a list of the currently mounted file systems, or NULL on error. Add each entry to the tail of the list so that they stay in order. If NEED_FS_TYPE is true, ensure that the file system type fields in @@ -438,30 +461,66 @@ read_file_system_list (bool need_fs_type) #ifdef MOUNTED_GETMNTENT1 /* GNU/Linux, 4.3BSD, SunOS, HP-UX, Dynix, Irix. */ { -#ifdef MOUNTED_PROC_MOUNTINFO - struct libmnt_table *fstable = NULL; - - fstable = mnt_new_table_from_file ("/proc/self/mountinfo"); + FILE *fp; - if (fstable != NULL) +#ifdef __linux__ + /* Try parsing mountinfo first, as that has device IDs. */ + char const *mountinfo = "/proc/self/mountinfo"; + fp = fopen (mountinfo, "r"); + if (fp != NULL) { - struct libmnt_fs *fs; - struct libmnt_iter *iter; - - iter = mnt_new_iter (MNT_ITER_FORWARD); + char *line = NULL; + size_t buf_size = 0; - while (iter && mnt_table_next_fs (fstable, iter, &fs) == 0) + while (getline (&line, &buf_size, fp) != -1) { + unsigned int devmaj, devmin; + int target_s, target_e, type_s, type_e, source_s, source_e; + char test; + int rc; + + rc = sscanf(line, "%*u " /* id - discarded */ + "%*u " /* parent - discarded */ + "%u:%u " /* dev major:minor */ + "%*s " /* mountroot - discarded */ + "%n%*s%n" /* target, start and end */ + "%c", /* more data... */ + &devmaj, &devmin, + &target_s, &target_e, + &test); + if (rc != 3 && rc != 5) /* 5 if %n included in count. */ + continue; + + /* skip optional fields, terminated by " - " */ + char *dash = strstr (line + target_e, " - "); + if (! dash) + continue; + + rc = sscanf(dash, " - " + "%n%*s%n " /* FS type, start and end */ + "%n%*s%n " /* source, start and end */ + "%c", /* more data... */ + &type_s, &type_e, + &source_s, &source_e, + &test); + if (rc != 1 && rc != 5) /* 5 if %n included in count. */ + continue; + + /* manipulate the target and source strings in place. */ + line[target_e] = '\0'; + dash[type_e] = '\0'; + dash[source_e] = '\0'; + unescape_tab (line + target_s); + unescape_tab (dash + source_s); + me = xmalloc (sizeof *me); - me->me_devname = xstrdup (mnt_fs_get_source (fs)); - me->me_mountdir = xstrdup (mnt_fs_get_target (fs)); - me->me_type = xstrdup (mnt_fs_get_fstype (fs)); + me->me_devname = xstrdup (dash + source_s); + me->me_mountdir = xstrdup (line + target_s); + me->me_type = xstrdup (dash + type_s); me->me_type_malloced = 1; - me->me_dev = mnt_fs_get_devno (fs); - /* Note we don't use mnt_fs_is_pseudofs() or mnt_fs_is_netfs() here - as libmount's classification is non-compatible currently. - Also we pass "false" for the "Bind" option as that's only + me->me_dev = makedev (devmaj, devmin); + /* we pass "false" for the "Bind" option as that's only significant when the Fs_type is "none" which will not be the case when parsing "/proc/self/mountinfo", and only applies for static /etc/mtab files. */ @@ -471,15 +530,19 @@ read_file_system_list (bool need_fs_type) /* Add to the linked list. */ *mtail = me; mtail = &me->me_next; + } - mnt_free_iter (iter); - mnt_free_table (fstable); + free (line); + + if (ferror (fp)) + goto free_then_fail; + if (fclose (fp) == EOF) + goto free_then_fail; } - else /* fallback to /proc/self/mounts (/etc/mtab) if anything failed */ -#endif /* MOUNTED_PROC_MOUNTINFO */ + else /* fallback to /proc/self/mounts (/etc/mtab). */ +#endif /* __linux __ */ { - FILE * fp; struct mntent *mnt; char const *table = MOUNTED; diff --git a/m4/ls-mntd-fs.m4 b/m4/ls-mntd-fs.m4 index 59a951e..7e4aa19 100644 --- a/m4/ls-mntd-fs.m4 +++ b/m4/ls-mntd-fs.m4 @@ -1,4 +1,4 @@ -# serial 31 +# serial 32 # How to list mounted file systems. # Copyright (C) 1998-2004, 2006, 2009-2015 Free Software Foundation, Inc. @@ -120,7 +120,7 @@ if test $ac_cv_func_getmntent = yes; then # Determine whether it's the one-argument variant or the two-argument one. if test -z "$ac_list_mounted_fs"; then - # 4.3BSD, SunOS, HP-UX, Dynix, Irix + # GNU/Linux, 4.3BSD, SunOS, HP-UX, Dynix, Irix AC_MSG_CHECKING([for one-argument getmntent function]) AC_CACHE_VAL([fu_cv_sys_mounted_getmntent1], [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ @@ -152,25 +152,6 @@ if test $ac_cv_func_getmntent = yes; then of mounted file systems, and that function takes a single argument. (4.3BSD, SunOS, HP-UX, Dynix, Irix)]) AC_CHECK_FUNCS([hasmntopt]) - - AC_ARG_WITH([libmount], - [AS_HELP_STRING([--with-libmount], - [use libmount if available to parse the system mount list.])], - [], - dnl libmount has the advantage of propagating accurate device IDs for - dnl each entry, but the disadvantage of many dependent shared libs - dnl with associated runtime startup overhead and virt mem usage. - [with_libmount=no]) - - # Check for libmount to support /proc/self/mountinfo on Linux - if test "x$with_libmount" != xno; then - AC_CHECK_LIB([mount], [mnt_new_table_from_file], - [AC_DEFINE([MOUNTED_PROC_MOUNTINFO], [1], - [Define if want to use /proc/self/mountinfo on Linux.]) - LIBS="-lmount $LIBS"], - [test -f /proc/self/mountinfo && - AC_MSG_WARN([/proc/self/mountinfo present but libmount missing.])]) - fi fi fi -- 2.1.0
