Boost subshell performance by avoiding expensive pwd related syscalls

src/cmd/ksh93/bltins/cd_pwd.c

@@ -46,14 +46,32 @@ static void rehash(Namval_t *np,void *data)
 		nv_rehash(np,data);
 }
 
+/*
+ * Obtain a file handle to the directory "path" relative to directory "dir"
+ */
+int sh_diropenat(int dir, const char *path)
+{
+	int fd,shfd;
+	if((fd = openat(dir, path, O_DIRECTORY|O_NONBLOCK|O_cloexec)) < 0)
+#if O_SEARCH
+		if(errno != EACCES || (fd = openat(dir, path, O_SEARCH|O_DIRECTORY|O_NONBLOCK|O_cloexec)) < 0)
+#endif
+			return fd;
+	/* Move fd to a number > 10 and register the fd number with the shell */
+	shfd = sh_fcntl(fd, F_dupfd_cloexec, 10);
+	close(fd);
+	return shfd;
+}
+
 int	b_cd(int argc, char *argv[],Shbltin_t *context)
 {
 	char *dir;
 	Pathcomp_t *cdpath = 0;
 	const char *dp;
 	int saverrno=0;
 	int rval,pflag=0,eflag=0,ret=1;
-	char *oldpwd;
+	int newdirfd;
+	char *oldpwd, *cp;
 	Namval_t *opwdnod, *pwdnod;
 	NOT_USED(context);
 	while((rval = optget(argv,sh_optcd))) switch(rval)
@@ -118,13 +136,8 @@ int	b_cd(int argc, char *argv[],Shbltin_t *context)
 	 * If sh_subshell() in subshell.c cannot use fchdir(2) to restore the PWD using a saved file descriptor,
 	 * we must fork any virtual subshell now to avoid the possibility of ending up in the wrong PWD on exit.
 	 */
-	if(sh.subshell && !sh.subshare)
-	{
-#if _lib_fchdir
-		if(!test_inode(sh.pwd,e_dot))
-#endif
-			sh_subfork();
-	}
+	if(sh.subshell && !sh.subshare && (!sh_validate_subpwdfd() || !test_inode(sh.pwd,e_dot)))
+		sh_subfork();
 	/*
 	 * Do $CDPATH processing, except if the path is absolute or the first component is '.' or '..'
 	 */
@@ -181,21 +194,49 @@ int	b_cd(int argc, char *argv[],Shbltin_t *context)
 		}
 		if(!pflag)
 		{
-			char *cp;
 			stkseek(sh.stk,PATH_MAX+PATH_OFFSET);
 			if(*(cp=stkptr(sh.stk,PATH_OFFSET))=='/')
 				if(!pathcanon(cp,PATH_DOTDOT))
 					continue;
 		}
-		if((rval=chdir(path_relative(stkptr(sh.stk,PATH_OFFSET)))) >= 0)
-			goto success;
-		if(errno!=ENOENT && saverrno==0)
+		cp = path_relative(stkptr(sh.stk,PATH_OFFSET));
+		rval = newdirfd = sh_diropenat((sh.pwdfd>0)?sh.pwdfd:AT_FDCWD,cp);
+		if(newdirfd>0)
+		{
+			/* chdir for directories on HSM/tapeworms may take minutes */
+			if((rval=fchdir(newdirfd)) >= 0)
+			{
+				sh_pwdupdate(newdirfd);
+				goto success;
+			}
+			sh_close(newdirfd);
+		}
+#if !O_SEARCH
+		else if((rval=chdir(cp)) >= 0)
+			sh_pwdupdate(sh_diropenat(AT_FDCWD,cp));
+#endif
+		if(saverrno==0)
 			saverrno=errno;
 	}
 	while(cdpath);
 	if(rval<0 && *dir=='/' && *(path_relative(stkptr(sh.stk,PATH_OFFSET)))!='/')
-		rval = chdir(dir);
-	/* use absolute chdir() if relative chdir() fails */
+	{
+		rval = newdirfd = sh_diropenat((sh.pwdfd>0)?sh.pwdfd:AT_FDCWD,dir);
+		if(newdirfd>0)
+		{
+			/* chdir for directories on HSM/tapeworms may take minutes */
+			if((rval=fchdir(newdirfd)) >= 0)
+			{
+				sh_pwdupdate(newdirfd);
+				goto success;
+			}
+			sh_close(newdirfd);
+		}
+#if !O_SEARCH
+		else if((rval=chdir(dir)) >= 0)
+			sh_pwdupdate(sh_diropenat(AT_FDCWD,dir));
+#endif
+	}
 	if(rval<0)
 	{
 		if(saverrno)
@@ -221,7 +262,7 @@ int	b_cd(int argc, char *argv[],Shbltin_t *context)
 	if(*dp && (*dp!='.'||dp[1]) && strchr(dir,'/'))
 		sfputr(sfstdout,dir,'\n');
 	nv_putval(opwdnod,oldpwd,NV_RDONLY);
-	free((void*)sh.pwd);
+	free(sh.pwd);
 	if(*dir == '/')
 	{
 		size_t len = strlen(dir);

src/cmd/ksh93/features/externs

@@ -1,6 +1,6 @@
 hdr	nc
 mem	exception.name,_exception.name math.h
-lib	setreuid,setregid,nice,fork,fchdir
+lib	setreuid,setregid,nice,fork
 lib	pathnative,pathposix
 lib	memcntl sys/mman.h
 

src/cmd/ksh93/include/defs.h

@@ -114,6 +114,7 @@ extern char		*sh_checkid(char*,char*);
 extern void		sh_chktrap(void);
 extern void		sh_deparse(Sfio_t*,const Shnode_t*,int,int);
 extern int		sh_debug(const char*,const char*,const char*,char *const[],int);
+extern int		sh_diropenat(int,const char *);
 extern char 		**sh_envgen(void);
 extern Sfdouble_t	sh_arith(const char*);
 extern void		*sh_arithcomp(char*);
@@ -141,6 +142,8 @@ extern Dt_t		*sh_subfuntree(int);
 extern void		sh_subjobcheck(pid_t);
 extern int		sh_subsavefd(int);
 extern void		sh_subtmpfile(void);
+extern void		sh_pwdupdate(int);
+extern int		sh_validate_subpwdfd(void);
 extern char 		*sh_substitute(const char*,const char*,char*);
 extern void		sh_timetraps(void);
 extern const char	*_sh_translate(const char*);

src/cmd/ksh93/include/shell.h

@@ -371,7 +371,8 @@ struct Shell_s
 	int		offsets[10];
 	Sfio_t		**sftable;
 	unsigned char	*fdstatus;
-	const char	*pwd;
+	char		*pwd;
+	int		pwdfd;		/* file descriptor for pwd */
 	void		*jmpbuffer;
 	void		*mktype;
 	Sfio_t		*strbuf;

src/cmd/ksh93/sh/fault.c

@@ -687,6 +687,8 @@ noreturn void sh_done(int sig)
 	if(sh_isoption(SH_NOEXEC))
 		kiaclose((Lex_t*)sh.lex_context);
 #endif /* SHOPT_KIA */
+	if(sh.pwdfd > 0)
+		close(sh.pwdfd);
 	/* Exit with portable 8-bit status (128 + signum) if last child process exits due to signal */
 	if(sh.chldexitsig)
 		savxit = savxit & ~SH_EXITSIG | 0200;

src/cmd/ksh93/sh/init.c

@@ -1906,11 +1906,7 @@ static void env_init(void)
 	}
 	if(save_env_n)
 		sh.save_env_n = save_env_n;
-	if(nv_isnull(PWDNOD) || nv_isattr(PWDNOD,NV_TAGGED))
-	{
-		nv_offattr(PWDNOD,NV_TAGGED);
-		path_pwd();
-	}
+	path_pwd();
 	if((cp = nv_getval(SHELLNOD)) && (sh_type(cp)&SH_TYPE_RESTRICTED))
 		sh_onoption(SH_RESTRICTED); /* restricted shell */
 }

src/cmd/ksh93/sh/name.c

@@ -1507,11 +1507,7 @@ Namval_t *nv_open(const char *name, Dt_t *root, int flags)
 	{
 		cp++;
 		if(sh_isstate(SH_INIT))
-		{
 			nv_putval(np, cp, NV_RDONLY);
-			if(np==PWDNOD)
-				nv_onattr(np,NV_TAGGED);
-		}
 		else
 		{
 			char *sub=0, *prefix= sh.prefix;

src/cmd/ksh93/sh/path.c

@@ -213,8 +213,8 @@ char *path_pwd(void)
 	if(sh.pwd)
 	{
 		if(*sh.pwd=='/')
-			return (char*)sh.pwd;
-		free((void*)sh.pwd);
+			return sh.pwd;
+		free(sh.pwd);
 	}
 	/* First see if PWD variable is correct */
 	pwdnod = sh_scoped(PWDNOD);
@@ -249,7 +249,9 @@ char *path_pwd(void)
 	if(!tofree)
 		cp = sh_strdup(cp);
 	sh.pwd = cp;
-	return (char*)sh.pwd;
+	/* Set sh.pwdfd */
+	sh_pwdupdate(sh_diropenat(AT_FDCWD,cp));
+	return sh.pwd;
 }
 
 /*

src/cmd/ksh93/sh/subshell.c

@@ -39,14 +39,6 @@
 #   define PIPE_BUF	512
 #endif
 
-#ifndef O_SEARCH
-#   ifdef O_PATH
-#	define O_SEARCH	O_PATH
-#   else
-#	define O_SEARCH	O_RDONLY
-#   endif
-#endif
-
 /*
  * Note that the following structure must be the same
  * size as the Dtlink_t structure
@@ -90,12 +82,9 @@ static struct subshell
 	char		comsub;
 	unsigned int	rand_seed;          /* parent shell $RANDOM seed */
 	int		rand_last;          /* last random number from $RANDOM in parent shell */
-	int		rand_state;         /* 0 means sp->rand_seed hasn't been set, 1 is the opposite */
+	char		rand_state;         /* 0 means sp->rand_seed hasn't been set, 1 is the opposite */
 	uint32_t	srand_upper_bound;  /* parent shell's upper bound for $SRANDOM */
-#if _lib_fchdir
-	int		pwdfd;	/* file descriptor for PWD */
-	char		pwdclose;
-#endif /* _lib_fchdir */
+	int		pwdfd;              /* parent shell's file descriptor for PWD */
 } *subshell_data;
 
 static unsigned int subenv;
@@ -457,6 +446,28 @@ void sh_subjobcheck(pid_t pid)
 	}
 }
 
+/*
+ * Set the file descriptor for the current shell's PWD without wiping
+ * out the stored file descriptor for the parent shell's PWD.
+ */
+void sh_pwdupdate(int fd)
+{
+	struct subshell *sp = subshell_data;
+	if(!(sh.subshell && !sh.subshare && sh.pwdfd == sp->pwdfd) && sh.pwdfd > 0)
+		sh_close(sh.pwdfd);
+	sh.pwdfd = fd;
+}
+
+/*
+ * Check if the parent shell has a valid PWD fd to return to.
+ * Only for use by cd inside of virtual subshells.
+ */
+int sh_validate_subpwdfd(void)
+{
+	struct subshell *sp = subshell_data;
+	return sp->pwdfd > 0;
+}
+
 /*
  * Run command tree <t> in a virtual subshell
  * If comsub is not null, then output will be placed in temp file (or buffer)
@@ -505,8 +516,6 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub)
 		path_get(e_dot);
 		sh.pathinit = 0;
 	}
-	if(!sh.pwd)
-		path_pwd();
 	sp->bckpid = sh.bckpid;
 	if(comsub)
 		sh_stats(STAT_COMSUB);
@@ -521,38 +530,9 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub)
 		sh.comsub = comsub;
 	if(!sh.subshare)
 	{
-		struct subshell *xp;
 		char *save_debugtrap = 0;
-#if _lib_fchdir
-		sp->pwdfd = -1;
-		for(xp=sp->prev; xp; xp=xp->prev)
-		{
-			if(xp->pwdfd>0 && xp->pwd && strcmp(xp->pwd,sh.pwd)==0)
-			{
-				sp->pwdfd = xp->pwdfd;
-				break;
-			}
-		}
-		if(sp->pwdfd<0)
-		{
-			int n = open(e_dot,O_SEARCH);
-			if(n>=0)
-			{
-				sp->pwdfd = n;
-				if(n<10)
-				{
-					sp->pwdfd = sh_fcntl(n,F_DUPFD,10);
-					close(n);
-				}
-				if(sp->pwdfd>0)
-				{
-					fcntl(sp->pwdfd,F_SETFD,FD_CLOEXEC);
-					sp->pwdclose = 1;
-				}
-			}
-		}
-#endif /* _lib_fchdir */
-		sp->pwd = (sh.pwd?sh_strdup(sh.pwd):0);
+		sp->pwd = sh_strdup(sh.pwd);
+		sp->pwdfd = sh.pwdfd;
 		sp->mask = sh.mask;
 		sh_stats(STAT_SUBSHELL);
 		/* save trap table */
@@ -626,21 +606,6 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub)
 		if(sh.savesig < 0)
 		{
 			sh.savesig = 0;
-#if _lib_fchdir
-			if(sp->pwdfd < 0 && !sh.subshare)	/* if we couldn't get a file descriptor to our PWD ... */
-				sh_subfork();			/* ...we have to fork, as we cannot fchdir back to it. */
-#else
-			if(!sh.subshare)
-			{
-				if(sp->pwd && access(sp->pwd,X_OK)<0)
-				{
-					free(sp->pwd);
-					sp->pwd = NULL;
-				}
-				if(!sp->pwd)
-					sh_subfork();
-			}
-#endif /* _lib_fchdir */
 			/* Virtual subshells are not safe to suspend (^Z, SIGTSTP) in the interactive main shell. */
 			if(sh_isstate(SH_INTERACTIVE))
 			{
@@ -824,25 +789,29 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub)
 			free(savsig);
 		}
 		sh.options = sp->options;
-		/* restore the present working directory */
-#if _lib_fchdir
-		if(sp->pwdfd > 0 && fchdir(sp->pwdfd) < 0)
-#else
-		if(sp->pwd && strcmp(sp->pwd,sh.pwd) && chdir(sp->pwd) < 0)
-#endif /* _lib_fchdir */
+		if(sh.pwdfd != sp->pwdfd)
 		{
-			saveerrno = errno;
-			fatalerror = 2;
+			/*
+			 * Restore the parent shell's present working directory.
+			 * Note: cd will always fork if sp->pwdfd is -1 (after calling sh_validate_subpwdfd()),
+			 * which only occurs when a subshell is started with sh.pwdfd == -1. As such, in this
+			 * if block sp->pwdfd is always > 0 (whilst sh.pwdfd is guaranteed to differ, and
+			 * might not be valid).
+			 */
+			if(fchdir(sp->pwdfd) < 0)
+			{
+				/* Couldn't fchdir back; close the fd and cope with the error */
+				sh_close(sp->pwdfd);
+				saveerrno = errno;
+				fatalerror = 2;
+			}
+			else if(sp->pwd && strcmp(sp->pwd,sh.pwd))
+				path_newdir(sh.pathlist);
+			if(fatalerror != 2)
+				sh_pwdupdate(sp->pwdfd);
 		}
-		else if(sp->pwd && strcmp(sp->pwd,sh.pwd))
-			path_newdir(sh.pathlist);
-		if(sh.pwd)
-			free((void*)sh.pwd);
+		free(sh.pwd);
 		sh.pwd = sp->pwd;
-#if _lib_fchdir
-		if(sp->pwdclose)
-			close(sp->pwdfd);
-#endif /* _lib_fchdir */
 		if(sp->mask!=sh.mask)
 			umask(sh.mask=sp->mask);
 		if(sh.coutpipe!=sp->coutpipe)
@@ -918,8 +887,7 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub)
 				UNREACHABLE();
 			case 2:
 				/* reinit PWD as it will be wrong */
-				if(sh.pwd)
-					free((void*)sh.pwd);
+				free(sh.pwd);
 				sh.pwd = NULL;
 				path_pwd();
 				errno = saveerrno;