[PATCH] Optimisation for readdir using fstatat.

Jeremy Allison jra at samba.org
Fri May 24 14:37:35 MDT 2013


On Fri, May 24, 2013 at 01:31:17PM -0700, Jeremy Allison wrote:
> On Fri, May 24, 2013 at 01:23:20PM -0700, Jeremy Allison wrote:
> > On Fri, May 24, 2013 at 04:19:59PM -0400, Ira Cooper wrote:
> > > A concern raised by Richard, which I haven't had the chance to look at... Is
> > > the use of "stat" here premature?
> > 
> > No.
> > 
> > > (For every readdir, are we guaranteed to stat, and not lstat etc...)
> > 
> > If we pass in a SMB_STRUCT_STAT *sbuf then yes, we are
> > guaranteed to stat(). If it's a POSIX path then we will
> > ignore the reply from readdir() here and re-stat using
> > lstat() on top (see the comment in the patch). I checked
> > out all codepaths before adding this.
> 
> If you're really worried about this I can add a:
> 
> int flags = (lp_posix_pathnames() ? AT_SYMLINK_NOFOLLOW : 0);
> 
> arg to the fstatat() call just to make sure we can't
> mess up here.

Updated for taste :-). Bless whichever version you're
happy with :-).

Jeremy.
-------------- next part --------------
From a1044b693603e24c86efb5384172abb7727abd3a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 24 May 2013 10:33:16 -0700
Subject: [PATCH 1/2] Add HAVE_DIRFD to waf/autoconf. Detect fstatat.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/configure.in | 6 ++++++
 source3/wscript      | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/source3/configure.in b/source3/configure.in
index 0e2f126..b7a9620 100644
--- a/source3/configure.in
+++ b/source3/configure.in
@@ -834,9 +834,15 @@ fi
 
 AC_CHECK_FUNCS(dirfd)
 if test x"$ac_cv_func_dirfd" = x"yes"; then
+	AC_DEFINE(HAVE_DIRFD,1,[Whether we have dirfd])
 	default_shared_modules="$default_shared_modules vfs_syncops vfs_dirsort"
 fi
 
+AC_CHECK_FUNCS(fstatat)
+if test x"$ac_cv_func_fstatat" = x"yes"; then
+	AC_DEFINE(HAVE_FSTATAT,1,[Whether we have fstatat])
+fi
+
 AC_CACHE_CHECK([for struct sigevent type],samba_cv_struct_sigevent, [
     AC_TRY_COMPILE([
 #include <sys/types.h>
diff --git a/source3/wscript b/source3/wscript
index dba6cdc..92207ef 100644
--- a/source3/wscript
+++ b/source3/wscript
@@ -452,6 +452,10 @@ return acl_get_perm_np(permset_d, perm);
 
     if conf.CHECK_FUNCS('dirfd'):
         conf.DEFINE('HAVE_DIRFD_DECL', 1)
+        conf.DEFINE('HAVE_DIRFD', 1)
+
+    if conf.CHECK_FUNCS('fstatat'):
+        conf.DEFINE('HAVE_fstatat', 1)
 
     conf.CHECK_CODE('struct statfs fsd; fsid_t fsid = fsd.f_fsid; return statfs(".", &fsd);',
                     'HAVE_STATFS_F_FSID',
-- 
1.8.2.1


From fb7da6aa1456b47c8b80861dc5d11ebd9d9bbb6f Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 24 May 2013 10:33:38 -0700
Subject: [PATCH 2/2] Optimization on POSIX platforms that have fstatat.

Tests show significant speedup in directory listings
by using fstatat instead of a full pathname walk.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/vfs_default.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 8804e62..82d059c 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -376,11 +376,30 @@ static struct dirent *vfswrap_readdir(vfs_handle_struct *handle,
 
 	START_PROFILE(syscall_readdir);
 	result = readdir(dirp);
-	/* Default Posix readdir() does not give us stat info.
-	 * Set to invalid to indicate we didn't return this info. */
-	if (sbuf)
-		SET_STAT_INVALID(*sbuf);
 	END_PROFILE(syscall_readdir);
+	if (sbuf) {
+		/* Default Posix readdir() does not give us stat info.
+		 * Set to invalid to indicate we didn't return this info. */
+		SET_STAT_INVALID(*sbuf);
+#if defined(HAVE_DIRFD) && defined(HAVE_FSTATAT)
+		if (result != NULL) {
+			/* See if we can efficiently return this. */
+			struct stat st;
+			int flags = (lp_posix_pathnames() ?
+				AT_SYMLINK_NOFOLLOW : 0);
+			int ret = fstatat(dirfd(dirp),
+					result->d_name,
+					&st,
+					flags);
+			if (ret == 0) {
+				init_stat_ex_from_stat(sbuf,
+					&st,
+					lp_fake_dir_create_times(
+						SNUM(handle->conn)));
+			}
+		}
+#endif
+	}
 	return result;
 }
 
-- 
1.8.2.1



More information about the samba-technical mailing list