From patchwork Tue May 27 06:08:14 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Wong X-Patchwork-Id: 1164 Received: (qmail 27625 invoked by alias); 27 May 2014 06:08:20 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 27442 invoked by uid 89); 27 May 2014 06:08:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: dcvr.yhbt.net Date: Tue, 27 May 2014 06:08:14 +0000 From: Eric Wong To: =?utf-8?B?T25kxZllaiBCw61sa2E=?= Cc: Siddhesh Poyarekar , libc-alpha@sourceware.org Subject: Re: [RESEND][BZ #15132][PATCH] avoid stat/fstat in statvfs/fstatvfs Message-ID: <20140527060814.GA28198@dcvr.yhbt.net> References: <20140125220000.GA19098@dcvr.yhbt.net> <20140127050402.GC2149@spoyarek.pnq.redhat.com> <20140204181702.GA25215@dcvr.yhbt.net> <20140221114609.GA7772@domone.podge> <20140222063909.GA6861@dcvr.yhbt.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140222063909.GA6861@dcvr.yhbt.net> Eric Wong wrote: > Ondřej Bílka wrote: > > > Sorry for the delay. I've only tested on 3.12+ so far on x86_64. > > > I'll try to get a VM setup on <2.6.36 soon > > > > Did you test also with these kernels? > > Unfortunately the machine running 2.6.32.x CentOS 6.x kernel > already had Linux commit 365b18189789bfa1acd9939e6312b8a4b4577b28 > (add f_flags to struct statfs(64)) backported to it :< > > Probably just be easier for me to test with the following kernel patch > than to get an older kernel running: > > --- a/fs/statfs.c > +++ b/fs/statfs.c > @@ -42,7 +42,7 @@ static int flags_by_sb(int s_flags) > > static int calculate_f_flags(struct vfsmount *mnt) > { > - return ST_VALID | flags_by_mnt(mnt->mnt_flags) | > + return flags_by_mnt(mnt->mnt_flags) | > flags_by_sb(mnt->mnt_sb->s_flags); > } > I tested with the above patch applied 3.14 kernel and verified it correctly falls back to using stat/fstat if the ST_VALID flag is missing. On an unpatched 3.14 kernel, the same executables/binaries avoid stat/fstat when statvfs/fstatvfs are called. I straced the following program to verify the presence/lack-of extraneous stat/fstat syscalls after statfs/fstatfs: strace /path/to/lib/ld-linux-x86-64.so.2 /tmp/statvfs -------------------------- /tmp/statvfs.c ---------------------------- #include #include #include #include int main(int argc, char *argv[]) { struct statvfs buf; const char *path = argc == 2 ? argv[1] : "/"; int fd; statvfs(path, &buf); fd = open(path, O_DIRECTORY, O_RDONLY); fstatvfs(fd, &buf); return 0; } Original patch with updated commit message: -------------------------------- 8< --------------------------------- From: Eric Wong Subject: [PATCH] avoid stat/fstat in statvfs/fstatvfs Delay the use of stat/fstat until stat data is required. When the kernel returns ST_VALID, stat data is not used by __internal_statvfs. 2014-05-27 Eric Wong [BZ #15132] * sysdeps/unix/sysv/linux/internal_statvfs.c (__statvfs_getflags): Call fstat64 or stat64 internally, depending on arguments passed. Replace stat buffer argument with file descriptor argument. (INTERNAL_STATVFS): Update arguments to match __statvfs_getflags. * sysdeps/unix/sysv/linux/fstatvfs.c (fstatvfs): Pass fd to __internal_statvfs instead of calling fstat64. * sysdeps/unix/sysv/linux/fstatvfs64.c (__fstatvfs64): Pass fd to __internal_statvfs64 instead of calling fstat64. * sysdeps/unix/sysv/linux/statvfs.c (statvfs): Pass -1 to __internal_statvfs instead of calling stat64. * sysdeps/unix/sysv/linux/statvfs64.c (__statvfs64): Pass -1 to __internal_statvfs64 instead of calling stat64. --- sysdeps/unix/sysv/linux/fstatvfs.c | 4 ++-- sysdeps/unix/sysv/linux/fstatvfs64.c | 10 +++------- sysdeps/unix/sysv/linux/internal_statvfs.c | 15 ++++++++------- sysdeps/unix/sysv/linux/statvfs.c | 6 ++---- sysdeps/unix/sysv/linux/statvfs64.c | 10 +++------- 5 files changed, 18 insertions(+), 27 deletions(-) diff --git a/sysdeps/unix/sysv/linux/fstatvfs.c b/sysdeps/unix/sysv/linux/fstatvfs.c index da1f27e..689ab7a 100644 --- a/sysdeps/unix/sysv/linux/fstatvfs.c +++ b/sysdeps/unix/sysv/linux/fstatvfs.c @@ -22,7 +22,7 @@ #include extern void __internal_statvfs (const char *name, struct statvfs *buf, - struct statfs *fsbuf, struct stat64 *st); + struct statfs *fsbuf, int fd); int @@ -36,7 +36,7 @@ fstatvfs (int fd, struct statvfs *buf) return -1; /* Convert the result. */ - __internal_statvfs (NULL, buf, &fsbuf, fstat64 (fd, &st) == -1 ? NULL : &st); + __internal_statvfs (NULL, buf, &fsbuf, fd); /* We signal success if the statfs call succeeded. */ return 0; diff --git a/sysdeps/unix/sysv/linux/fstatvfs64.c b/sysdeps/unix/sysv/linux/fstatvfs64.c index dd10c1f..006b059 100644 --- a/sysdeps/unix/sysv/linux/fstatvfs64.c +++ b/sysdeps/unix/sysv/linux/fstatvfs64.c @@ -25,7 +25,7 @@ extern void __internal_statvfs64 (const char *name, struct statvfs64 *buf, - struct statfs64 *fsbuf, struct stat64 *st); + struct statfs64 *fsbuf, int fd); /* Return information about the filesystem on which FD resides. */ @@ -60,12 +60,8 @@ __fstatvfs64 (int fd, struct statvfs64 *buf) #endif if (res == 0) - { - /* Convert the result. */ - struct stat64 st; - __internal_statvfs64 (NULL, buf, &fsbuf, - fstat64 (fd, &st) == -1 ? NULL : &st); - } + /* Convert the result. */ + __internal_statvfs64 (NULL, buf, &fsbuf, fd); return res; } diff --git a/sysdeps/unix/sysv/linux/internal_statvfs.c b/sysdeps/unix/sysv/linux/internal_statvfs.c index 2424c13..d71ed0c 100644 --- a/sysdeps/unix/sysv/linux/internal_statvfs.c +++ b/sysdeps/unix/sysv/linux/internal_statvfs.c @@ -43,9 +43,11 @@ # ifndef __ASSUME_STATFS_F_FLAGS int -__statvfs_getflags (const char *name, int fstype, struct stat64 *st) +__statvfs_getflags (const char *name, int fstype, int fd) { - if (st == NULL) + struct stat64 st; + + if ((fd < 0 ? stat64 (name, &st) : fstat64 (fd, &st)) < 0) return 0; const char *fsname = NULL; @@ -159,7 +161,7 @@ __statvfs_getflags (const char *name, int fstype, struct stat64 *st) /* Find out about the device the current entry is for. */ struct stat64 fsst; if (stat64 (mntbuf.mnt_dir, &fsst) >= 0 - && st->st_dev == fsst.st_dev) + && st.st_dev == fsst.st_dev) { /* Bingo, we found the entry for the device FD is on. Now interpret the option string. */ @@ -222,14 +224,13 @@ __statvfs_getflags (const char *name, int fstype, struct stat64 *st) } # endif #else -extern int __statvfs_getflags (const char *name, int fstype, - struct stat64 *st); +extern int __statvfs_getflags (const char *name, int fstype, int fd); #endif void INTERNAL_STATVFS (const char *name, struct STATVFS *buf, - struct STATFS *fsbuf, struct stat64 *st) + struct STATFS *fsbuf, int fd) { /* Now fill in the fields we have information for. */ buf->f_bsize = fsbuf->f_bsize; @@ -272,7 +273,7 @@ INTERNAL_STATVFS (const char *name, struct STATVFS *buf, 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); + buf->f_flag = __statvfs_getflags (name, fsbuf->f_type, fd); else #endif buf->f_flag = fsbuf->f_flags ^ ST_VALID; diff --git a/sysdeps/unix/sysv/linux/statvfs.c b/sysdeps/unix/sysv/linux/statvfs.c index 8a3df97..1427961 100644 --- a/sysdeps/unix/sysv/linux/statvfs.c +++ b/sysdeps/unix/sysv/linux/statvfs.c @@ -22,22 +22,20 @@ #include extern void __internal_statvfs (const char *name, struct statvfs *buf, - struct statfs *fsbuf, struct stat64 *st); + struct statfs *fsbuf, int fd); int statvfs (const char *file, struct statvfs *buf) { struct statfs fsbuf; - struct stat64 st; /* Get as much information as possible from the system. */ if (__statfs (file, &fsbuf) < 0) return -1; /* Convert the result. */ - __internal_statvfs (file, buf, &fsbuf, - stat64 (file, &st) == -1 ? NULL : &st); + __internal_statvfs (file, buf, &fsbuf, -1); /* We signal success if the statfs call succeeded. */ return 0; diff --git a/sysdeps/unix/sysv/linux/statvfs64.c b/sysdeps/unix/sysv/linux/statvfs64.c index e33e923..0ec8524 100644 --- a/sysdeps/unix/sysv/linux/statvfs64.c +++ b/sysdeps/unix/sysv/linux/statvfs64.c @@ -26,7 +26,7 @@ extern void __internal_statvfs64 (const char *name, struct statvfs64 *buf, - struct statfs64 *fsbuf, struct stat64 *st); + struct statfs64 *fsbuf, int fd); /* Return information about the filesystem on which FILE resides. */ @@ -61,12 +61,8 @@ __statvfs64 (const char *file, struct statvfs64 *buf) #endif if (res == 0) - { - /* Convert the result. */ - struct stat64 st; - __internal_statvfs64 (file, buf, &fsbuf, - stat64 (file, &st) == -1 ? NULL : &st); - } + /* Convert the result. */ + __internal_statvfs64 (file, buf, &fsbuf, -1); return res; }