[PATCH 0/4] Current state of Vals patches and the file map lookup patches

[PATCH 0/4] Current state of Vals patches and the file map lookup patches

[Ian Kent]

I've pulled in the patches that Val posted and made a couple of
minor changes. I also have what I think is just about what we
need to reduce the overhead of file map scanning.

Check'em out please folks.

I'm a little concerned about the "fix kernel includes" patch because
there should be more to it and I've altered it in line with that. I'm
not sure if we should post a kernel patch first, pull in the updated
kernel header files to the tar and finally make the other couple of
changes then. But then I may have it wrong so check it out.

Ian

---

Ian Kent (1):
      autofs-5.0.4 - always read file maps

Valerie Aurora Henson (3):
      autofs-5.0.4 - fix kernel includes
      autofs-5.0.4 - easy alloca replacements
      autofs-5.0.4 - make MAX_ERR_BUF and PARSE_MAX_BUF use easier to audit


 daemon/automount.c             |   29 ++++----
 daemon/direct.c                |   12 +--
 daemon/flag.c                  |   13 ++--
 daemon/indirect.c              |   12 +--
 daemon/lookup.c                |    9 ++-
 daemon/module.c                |   45 ++++---------
 include/automount.h            |    2 -
 include/linux/auto_dev-ioctl.h |   10 +++
 include/linux/auto_fs.h        |    5 +
 lib/cache.c                    |   31 +++------
 lib/cat_path.c                 |    1 
 modules/lookup_file.c          |  141 +++++++++++++---------------------------
 modules/lookup_ldap.c          |  142 +++++++++++++++++++++++-----------------
 modules/lookup_nisplus.c       |   71 ++++++++++++--------
 modules/mount_autofs.c         |    1 
 modules/mount_bind.c           |    7 +-
 modules/mount_changer.c        |    5 -
 modules/mount_ext2.c           |    5 -
 modules/mount_generic.c        |    5 -
 19 files changed, 248 insertions(+), 298 deletions(-)

-- 
Signature

[PATCH 1/4] autofs-5.0.4 - always read file maps

[Ian Kent]

autofs tries to not load an entire map into the internal cache unless it
has to. For maps that do get loaded into the cache it relies on checks to
work out if a map is up to date in order to trigger a map read. This is
fine for maps that can do direct key lookups but file maps need to do a
linear search through the file when locating an entry for a key. For large
maps this can be a huge overhead. This patch make autofs always load file
based maps at start and makes use of the map file mtime to discover if the
cache needs to be refreshed.
---

 daemon/lookup.c       |    9 +++++--
 modules/lookup_file.c |   59 +++++++++++++++++--------------------------------
 2 files changed, 27 insertions(+), 41 deletions(-)

diff --git a/daemon/lookup.c b/daemon/lookup.c
index 741d846..e034348 100644
--- a/daemon/lookup.c
+++ b/daemon/lookup.c
@@ -283,10 +283,13 @@ static int do_read_map(struct autofs_point *ap, struct map_source *map, time_t a
 	 * for the fail cases to function correctly and to cache the
 	 * lookup handle.
 	 *
-	 * We always need to whole map for direct mounts in order to
-	 * mount the triggers.
+	 * We always need to read the whole map for direct mounts in
+	 * order to mount the triggers. We also want to read the whole
+	 * map if it's a file map to avoid potentially lengthy linear
+	 * file scanning.
 	 */
-	if (!(ap->flags & MOUNT_FLAG_GHOST) && ap->type != LKP_DIRECT)
+	if (strcmp(map->type, "file") &&
+	    !(ap->flags & MOUNT_FLAG_GHOST) && ap->type != LKP_DIRECT)
 		return NSS_STATUS_SUCCESS;
 
 	if (!map->stale)
diff --git a/modules/lookup_file.c b/modules/lookup_file.c
index 95b9f6f..7672725 100644
--- a/modules/lookup_file.c
+++ b/modules/lookup_file.c
@@ -44,7 +44,6 @@ typedef enum { esc_none, esc_char, esc_val, esc_all } ESCAPES;
 
 struct lookup_context {
 	const char *mapname;
-	time_t mtime;
 	struct parse_mod *parse;
 };
 
@@ -54,7 +53,6 @@ int lookup_init(const char *mapfmt, int argc, const char *const *argv, void **co
 {
 	struct lookup_context *ctxt;
 	char buf[MAX_ERR_BUF];
-	struct stat st;
 
 	*context = NULL;
 
@@ -87,15 +85,6 @@ int lookup_init(const char *mapfmt, int argc, const char *const *argv, void **co
 		return 1;
 	}
 
-	if (stat(ctxt->mapname, &st)) {
-		free(ctxt);
-		logmsg(MODPREFIX "file map %s, could not stat",
-		     argv[0]);
-		return 1;
-	}
-		
-	ctxt->mtime = st.st_mtime;
-
 	if (!mapfmt)
 		mapfmt = MAPFMT_DEFAULT;
 
@@ -391,7 +380,6 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 	int blen;
 	char *path;
 	char *ent;
-	struct stat st;
 	FILE *f;
 	int fd;
 	unsigned int path_len, ent_len;
@@ -504,13 +492,6 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 			break;
 	}
 
-	if (fstat(fd, &st)) {
-		crit(logopt, MODPREFIX "file map %s, could not stat",
-		       ctxt->mapname);
-		return NSS_STATUS_UNAVAIL;
-	}
-	ctxt->mtime = st.st_mtime;
-
 	fclose(f);
 
 	return NSS_STATUS_SUCCESS;
@@ -642,7 +623,6 @@ int lookup_read_map(struct autofs_point *ap, time_t age, void *context)
 	struct mapent_cache *mc;
 	char *key;
 	char *mapent;
-	struct stat st;
 	FILE *f;
 	int fd;
 	unsigned int k_len, m_len;
@@ -748,13 +728,6 @@ int lookup_read_map(struct autofs_point *ap, time_t age, void *context)
 			break;
 	}
 
-	if (fstat(fd, &st)) {
-		crit(ap->logopt,
-		     MODPREFIX "file map %s, could not stat",
-		     ctxt->mapname);
-		return NSS_STATUS_UNAVAIL;
-	}
-	ctxt->mtime = st.st_mtime;
 	source->age = age;
 
 	fclose(f);
@@ -951,9 +924,6 @@ static int check_map_indirect(struct autofs_point *ap,
 	if (ret == CHE_FAIL)
 		return NSS_STATUS_NOTFOUND;
 
-	if (ret & CHE_UPDATED)
-		source->stale = 1;
-
 	pthread_cleanup_push(cache_lock_cleanup, mc);
 	cache_writelock(mc);
 	exists = cache_lookup_distinct(mc, key);
@@ -963,7 +933,6 @@ static int check_map_indirect(struct autofs_point *ap,
 			free(exists->mapent);
 			exists->mapent = NULL;
 			exists->status = 0;
-			source->stale = 1;
 		}
 	}
 	pthread_cleanup_pop(1);
@@ -985,14 +954,8 @@ static int check_map_indirect(struct autofs_point *ap,
 		we = cache_lookup_distinct(mc, "*");
 		if (we) {
 			/* Wildcard entry existed and is now gone */
-			if (we->source == source && (wild & CHE_MISSING)) {
+			if (we->source == source && (wild & CHE_MISSING))
 				cache_delete(mc, "*");
-				source->stale = 1;
-			}
-		} else {
-			/* Wildcard not in map but now is */
-			if (wild & (CHE_OK | CHE_UPDATED))
-				source->stale = 1;
 		}
 		pthread_cleanup_pop(1);
 
@@ -1062,9 +1025,28 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	 * we never know about it.
 	 */
 	if (ap->type == LKP_INDIRECT && *key != '/') {
+		struct stat st;
 		char *lkp_key;
 
+		/*
+		 * We can skip the map lookup and cache update altogether
+		 * if we know the map hasn't been modified since it was
+		 * last read. If it has then we can mark the map stale
+		 * so a re-read is triggered following the lookup.
+		 */
+		if (stat(ctxt->mapname, &st)) {
+			error(ap->logopt, MODPREFIX
+			      "file map %s, could not stat", ctxt->mapname);
+			return NSS_STATUS_UNAVAIL;
+		}
+
 		cache_readlock(mc);
+		me = cache_lookup_first(mc);
+		if (me && st.st_mtime <= me->age)
+			goto do_cache_lookup;
+		else
+			source->stale = 1;
+
 		me = cache_lookup_distinct(mc, key);
 		if (me && me->multi)
 			lkp_key = strdup(me->multi->key);
@@ -1088,6 +1070,7 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	}
 
 	cache_readlock(mc);
+do_cache_lookup:
 	me = cache_lookup(mc, key);
 	/* Stale mapent => check for entry in alternate source or wildcard */
 	if (me && !me->mapent) {

[PATCH 2/4] autofs-5.0.4 - make MAX_ERR_BUF and PARSE_MAX_BUF use easier to audit

[Ian Kent]

From: Valerie Aurora Henson <vaurora@redhat.com>

Non-critical changes to make auditing buffer lengths easier.

* Some buffers were the wrong (too big) size, some were used twice for
  different purposes.
* Use sizeof(buf) instead of repeating the *MAX* define in functions
  that need to know the size of a statically allocated buffer.
* Fix a compiler warning about discarding the const on a string.
---

 modules/lookup_ldap.c |   49 ++++++++++++++++++++++---------------------------
 1 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
index 6ba80eb..c7aecf0 100644
--- a/modules/lookup_ldap.c
+++ b/modules/lookup_ldap.c
@@ -274,7 +274,7 @@ LDAP *init_ldap_connection(unsigned logopt, const char *uri, struct lookup_conte
 
 static int get_query_dn(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt, const char *class, const char *key)
 {
-	char buf[PARSE_MAX_BUF];
+	char buf[MAX_ERR_BUF];
 	char *query, *dn, *qdn;
 	LDAPMessage *result, *e;
 	struct ldap_searchdn *sdns = NULL;
@@ -298,7 +298,7 @@ static int get_query_dn(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt
 
 	query = alloca(l);
 	if (query == NULL) {
-		char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
+		char *estr = strerror_r(errno, buf, sizeof(buf));
 		crit(logopt, MODPREFIX "alloca: %s", estr);
 		return NSS_STATUS_UNAVAIL;
 	}
@@ -1073,7 +1073,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c
 			}
 			if (!tmp) {
 				char *estr;
-				estr = strerror_r(errno, buf, MAX_ERR_BUF);
+				estr = strerror_r(errno, buf, sizeof(buf));
 				logerr(MODPREFIX "malloc: %s", estr);
 				return 0;
 			}
@@ -1095,7 +1095,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c
 			tmp = malloc(l + 1);
 			if (!tmp) {
 				char *estr;
-				estr = strerror_r(errno, buf, MAX_ERR_BUF);
+				estr = strerror_r(errno, buf, sizeof(buf));
 				crit(logopt, MODPREFIX "malloc: %s", estr);
 				return 0;
 			}
@@ -1130,7 +1130,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c
 		/* Isolate the server's name. */
 		if (!tmp) {
 			char *estr;
-			estr = strerror_r(errno, buf, MAX_ERR_BUF);
+			estr = strerror_r(errno, buf, sizeof(buf));
 			logerr(MODPREFIX "malloc: %s", estr);
 			return 0;
 		}
@@ -1171,7 +1171,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c
 				ctxt->mapname = map;
 			else {
 				char *estr;
-				estr = strerror_r(errno, buf, MAX_ERR_BUF);
+				estr = strerror_r(errno, buf, sizeof(buf));
 				logerr(MODPREFIX "malloc: %s", estr);
 				if (ctxt->server)
 					free(ctxt->server);
@@ -1182,7 +1182,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c
 			base = malloc(l + 1);
 			if (!base) {
 				char *estr;
-				estr = strerror_r(errno, buf, MAX_ERR_BUF);
+				estr = strerror_r(errno, buf, sizeof(buf));
 				logerr(MODPREFIX "malloc: %s", estr);
 				if (ctxt->server)
 					free(ctxt->server);
@@ -1196,7 +1196,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c
 		char *map = malloc(l + 1);
 		if (!map) {
 			char *estr;
-			estr = strerror_r(errno, buf, MAX_ERR_BUF);
+			estr = strerror_r(errno, buf, sizeof(buf));
 			logerr(MODPREFIX "malloc: %s", estr);
 			if (ctxt->server)
 				free(ctxt->server);
@@ -1309,7 +1309,7 @@ int lookup_init(const char *mapfmt, int argc, const char *const *argv, void **co
 	/* If we can't build a context, bail. */
 	ctxt = malloc(sizeof(struct lookup_context));
 	if (!ctxt) {
-		char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
+		char *estr = strerror_r(errno, buf, sizeof(buf));
 		logerr(MODPREFIX "malloc: %s", estr);
 		return 1;
 	}
@@ -1410,8 +1410,9 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 	unsigned int timeout = master->default_timeout;
 	unsigned int logging = master->default_logging;
 	unsigned int logopt = master->logopt;
-	int rv, l, count, blen;
-	char buf[PARSE_MAX_BUF];
+	int rv, l, count;
+	char buf[MAX_ERR_BUF];
+	char parse_buf[PARSE_MAX_BUF];
 	char *query;
 	LDAPMessage *result, *e;
 	char *class, *info, *entry;
@@ -1433,7 +1434,7 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 
 	query = alloca(l);
 	if (query == NULL) {
-		char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
+		char *estr = strerror_r(errno, buf, sizeof(buf));
 		logerr(MODPREFIX "alloca: %s", estr);
 		return NSS_STATUS_UNAVAIL;
 	}
@@ -1523,18 +1524,12 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 			goto next;
 		}
 
-		blen = strlen(*keyValue) + 1 + strlen(*values) + 2;
-		if (blen > PARSE_MAX_BUF) {
+		if (snprintf(parse_buf, sizeof(parse_buf), "%s %s",
+			     *keyValue, *values) >= sizeof(parse_buf)) {
 			error(logopt, MODPREFIX "map entry too long");
 			ldap_value_free(values);
 			goto next;
 		}
-		memset(buf, 0, PARSE_MAX_BUF);
-
-		strcpy(buf, *keyValue);
-		strcat(buf, " ");
-		strcat(buf, *values);
-
 		master_parse_entry(buf, timeout, logging, age);
 next:
 		ldap_value_free(keyValue);
@@ -1552,7 +1547,7 @@ static int get_percent_decoded_len(const char *name)
 {
 	int escapes = 0;
 	int escaped = 0;
-	char *tmp = name;
+	const char *tmp = name;
 	int look_for_close = 0;
 
 	while (*tmp) {
@@ -2051,7 +2046,7 @@ static int do_get_entries(struct ldap_search_params *sp, struct map_source *sour
 				mapent = malloc(v_len + 1);
 				if (!mapent) {
 					char *estr;
-					estr = strerror_r(errno, buf, MAX_ERR_BUF);
+					estr = strerror_r(errno, buf, sizeof(buf));
 					logerr(MODPREFIX "malloc: %s", estr);
 					ldap_value_free_len(bvValues);
 					goto next;
@@ -2071,7 +2066,7 @@ static int do_get_entries(struct ldap_search_params *sp, struct map_source *sour
 					mapent_len = new_size;
 				} else {
 					char *estr;
-					estr = strerror_r(errno, buf, MAX_ERR_BUF);
+					estr = strerror_r(errno, buf, sizeof(buf));
 					logerr(MODPREFIX "realloc: %s", estr);
 				}
 			}
@@ -2172,7 +2167,7 @@ static int read_one_map(struct autofs_point *ap,
 
 	sp.query = alloca(l);
 	if (sp.query == NULL) {
-		char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
+		char *estr = strerror_r(errno, buf, sizeof(buf));
 		logerr(MODPREFIX "malloc: %s", estr);
 		return NSS_STATUS_UNAVAIL;
 	}
@@ -2326,7 +2321,7 @@ static int lookup_one(struct autofs_point *ap,
 
 	query = alloca(l);
 	if (query == NULL) {
-		char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
+		char *estr = strerror_r(errno, buf, sizeof(buf));
 		crit(ap->logopt, MODPREFIX "malloc: %s", estr);
 		if (enc_len1) {
 			free(enc_key1);
@@ -2498,7 +2493,7 @@ static int lookup_one(struct autofs_point *ap,
 				mapent = malloc(v_len + 1);
 				if (!mapent) {
 					char *estr;
-					estr = strerror_r(errno, buf, MAX_ERR_BUF);
+					estr = strerror_r(errno, buf, sizeof(buf));
 					logerr(MODPREFIX "malloc: %s", estr);
 					ldap_value_free_len(bvValues);
 					goto next;
@@ -2518,7 +2513,7 @@ static int lookup_one(struct autofs_point *ap,
 					mapent_len = new_size;
 				} else {
 					char *estr;
-					estr = strerror_r(errno, buf, MAX_ERR_BUF);
+					estr = strerror_r(errno, buf, sizeof(buf));
 					logerr(MODPREFIX "realloc: %s", estr);
 				}
 			}

[PATCH 3/4] autofs-5.0.4 - easy alloca replacements

[Ian Kent]

From: Valerie Aurora Henson <vaurora@redhat.com>

alloca() is compiler-dependent, non-standard, and has undefined
behavior when it fails (IOW, the program crashes).  Replace with
normal C stack variables where possible and malloc() where not.
---

 daemon/automount.c       |   29 ++++++--------
 daemon/direct.c          |   12 ++----
 daemon/flag.c            |   13 +++---
 daemon/indirect.c        |   12 ++----
 daemon/module.c          |   45 +++++++---------------
 lib/cache.c              |   31 +++++----------
 lib/cat_path.c           |    1 
 modules/lookup_file.c    |   82 +++++++++++++----------------------------
 modules/lookup_ldap.c    |   93 +++++++++++++++++++++++++++++-----------------
 modules/lookup_nisplus.c |   71 ++++++++++++++++++++---------------
 modules/mount_autofs.c   |    1 
 modules/mount_bind.c     |    7 +--
 modules/mount_changer.c  |    5 --
 modules/mount_ext2.c     |    5 --
 modules/mount_generic.c  |    5 --
 15 files changed, 184 insertions(+), 228 deletions(-)

diff --git a/daemon/automount.c b/daemon/automount.c
index e120f50..c167da0 100644
--- a/daemon/automount.c
+++ b/daemon/automount.c
@@ -125,8 +125,8 @@ static int do_mkdir(const char *parent, const char *path, mode_t mode)
 
 int mkdir_path(const char *path, mode_t mode)
 {
-	char *buf = alloca(strlen(path) + 1);
-	char *parent = alloca(strlen(path) + 1);
+	char buf[PATH_MAX];
+	char parent[PATH_MAX];
 	const char *cp = path, *lcp = path;
 	char *bp = buf, *pp = parent;
 
@@ -161,7 +161,7 @@ int mkdir_path(const char *path, mode_t mode)
 int rmdir_path(struct autofs_point *ap, const char *path, dev_t dev)
 {
 	int len = strlen(path);
-	char *buf = alloca(len + 1);
+	char buf[PATH_MAX];
 	char *cp;
 	int first = 1;
 	struct stat st;
@@ -466,20 +466,17 @@ static int umount_subtree_mounts(struct autofs_point *ap, const char *path, unsi
 	pthread_cleanup_push(cache_lock_cleanup, mc);
 
 	if (me->multi) {
-		char *root, *base;
-		size_t ap_len;
+		char root[PATH_MAX];
+		char *base;
 		int cur_state;
 
-		ap_len = strlen(ap->path);
-
-		if (!strchr(me->multi->key, '/')) {
+		if (!strchr(me->multi->key, '/'))
 			/* Indirect multi-mount root */
-			root = alloca(ap_len + strlen(me->multi->key) + 2);
-			strcpy(root, ap->path);
-			strcat(root, "/");
-			strcat(root, me->multi->key);
-		} else
-			root = me->multi->key;
+			/* sprintf okay - if it's mounted, it's
+			 * PATH_MAX or less bytes */
+			sprintf(root, "%s/%s", ap->path, me->multi->key);
+		else
+			strcpy(root, me->multi->key);
 
 		if (is_mm_root)
 			base = NULL;
@@ -927,14 +924,14 @@ static int get_pkt(struct autofs_point *ap, union autofs_v5_packet_union *pkt)
 
 int do_expire(struct autofs_point *ap, const char *name, int namelen)
 {
-	char buf[PATH_MAX + 1];
+	char buf[PATH_MAX];
 	int len, ret;
 
 	if (*name != '/') {
 		len = ncat_path(buf, sizeof(buf), ap->path, name, namelen);
 	} else {
 		len = snprintf(buf, PATH_MAX, "%s", name);
-		if (len > PATH_MAX)
+		if (len >= PATH_MAX)
 			len = 0;
 	}
 
diff --git a/daemon/direct.c b/daemon/direct.c
index c0243c4..4c81d14 100644
--- a/daemon/direct.c
+++ b/daemon/direct.c
@@ -636,7 +636,9 @@ int mount_autofs_offset(struct autofs_point *ap, struct mapent *me, const char *
 	time_t timeout = ap->exp_timeout;
 	struct stat st;
 	int ioctlfd, status, ret;
-	const char *type, *map_name = NULL;
+	const char *hosts_map_name = "-hosts";
+	const char *map_name = hosts_map_name;
+	const char *type;
 	char mountpoint[PATH_MAX];
 
 	if (ops->version && ap->flags & MOUNT_FLAG_REMOUNT) {
@@ -739,13 +741,7 @@ int mount_autofs_offset(struct autofs_point *ap, struct mapent *me, const char *
 	      mp->options, mountpoint);
 
 	type = ap->entry->maps->type;
-	if (type && !strcmp(ap->entry->maps->type, "hosts")) {
-		char *tmp = alloca(7);
-		if (tmp) {
-			strcpy(tmp, "-hosts");
-			map_name = (const char *) tmp;
-		}
-	} else
+	if (!type || strcmp(ap->entry->maps->type, "hosts"))
 		map_name = me->mc->map->argv[0];
 
 	ret = mount(map_name, mountpoint, "autofs", MS_MGC_VAL, mp->options);
diff --git a/daemon/flag.c b/daemon/flag.c
index e43cece..f8fe163 100644
--- a/daemon/flag.c
+++ b/daemon/flag.c
@@ -23,10 +23,10 @@
 #include <sys/stat.h>
 #include <time.h>
 #include <string.h>
-#include <alloca.h>
 #include <stdio.h>
 #include <signal.h>
 #include <errno.h>
+#include <limits.h>
 
 #include "automount.h"
 
@@ -113,12 +113,13 @@ void release_flag_file(void)
 /* * Try to create flag file */
 int aquire_flag_file(void)
 {
-	char *linkf;
-	int len;
+	char linkf[PATH_MAX];
+	size_t len;
 
-	len = strlen(FLAG_FILE) + MAX_PIDSIZE;
-	linkf = alloca(len + 1);
-	snprintf(linkf, len, "%s.%d", FLAG_FILE, getpid());
+	len = snprintf(linkf, sizeof(linkf), "%s.%d", FLAG_FILE, getpid());
+	if (len >= sizeof(linkf))
+		/* Didn't acquire it */
+		return 0;
 
 	/*
 	 * Repeat until it was us who made the link or we find the
diff --git a/daemon/indirect.c b/daemon/indirect.c
index 9d3745c..4b7124c 100644
--- a/daemon/indirect.c
+++ b/daemon/indirect.c
@@ -89,7 +89,9 @@ static int do_mount_autofs_indirect(struct autofs_point *ap, const char *root)
 	struct ioctl_ops *ops = get_ioctl_ops();
 	time_t timeout = ap->exp_timeout;
 	char *options = NULL;
-	const char *type, *map_name = NULL;
+	const char *hosts_map_name = "-hosts";
+	const char *map_name = hosts_map_name;
+	const char *type;
 	struct stat st;
 	struct mnt_list *mnts;
 	int ret;
@@ -141,13 +143,7 @@ static int do_mount_autofs_indirect(struct autofs_point *ap, const char *root)
 	}
 
 	type = ap->entry->maps->type;
-	if (type && !strcmp(ap->entry->maps->type, "hosts")) {
-		char *tmp = alloca(7);
-		if (tmp) {
-			strcpy(tmp, "-hosts");
-			map_name = (const char *) tmp;
-		}
-	} else
+	if (!type || strcmp(ap->entry->maps->type, "hosts"))
 		map_name = ap->entry->maps->argv[0];
 
 	ret = mount(map_name, root, "autofs", MS_MGC_VAL, options);
diff --git a/daemon/module.c b/daemon/module.c
index e593d75..466d8d7 100644
--- a/daemon/module.c
+++ b/daemon/module.c
@@ -58,15 +58,11 @@ struct lookup_mod *open_lookup(const char *name, const char *err_prefix,
 {
 	struct lookup_mod *mod;
 	char buf[MAX_ERR_BUF];
-	char *fnbuf;
-	size_t size_name;
-	size_t size_fnbuf;
+	char fnbuf[PATH_MAX];
+	size_t size;
 	void *dh;
 	int *ver;
 
-	size_name = _strlen(name, PATH_MAX + 1);
-	if (!size_name)
-		return NULL;
 
 	mod = malloc(sizeof(struct lookup_mod));
 	if (!mod) {
@@ -77,9 +73,9 @@ struct lookup_mod *open_lookup(const char *name, const char *err_prefix,
 		return NULL;
 	}
 
-	size_fnbuf = size_name + strlen(AUTOFS_LIB_DIR) + 13;
-	fnbuf = alloca(size_fnbuf);
-	if (!fnbuf) {
+	size = snprintf(fnbuf, sizeof(fnbuf),
+			"%s/lookup_%s.so", AUTOFS_LIB_DIR, name);
+	if (size >= sizeof(fnbuf)) {
 		free(mod);
 		if (err_prefix) {
 			char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
@@ -87,7 +83,6 @@ struct lookup_mod *open_lookup(const char *name, const char *err_prefix,
 		}
 		return NULL;
 	}
-	snprintf(fnbuf, size_fnbuf, "%s/lookup_%s.so", AUTOFS_LIB_DIR, name);
 
 	if (!(dh = dlopen(fnbuf, RTLD_NOW))) {
 		if (err_prefix)
@@ -141,15 +136,11 @@ struct parse_mod *open_parse(const char *name, const char *err_prefix,
 {
 	struct parse_mod *mod;
 	char buf[MAX_ERR_BUF];
-	char *fnbuf;
-	size_t size_name;
-	size_t size_fnbuf;
+	char fnbuf[PATH_MAX];
+	size_t size;
 	void *dh;
 	int *ver;
 
-	size_name = _strlen(name, PATH_MAX + 1);
-	if (!size_name)
-		return NULL;
 
 	mod = malloc(sizeof(struct parse_mod));
 	if (!mod) {
@@ -160,9 +151,9 @@ struct parse_mod *open_parse(const char *name, const char *err_prefix,
 		return NULL;
 	}
 
-	size_fnbuf = size_name + strlen(AUTOFS_LIB_DIR) + 13;
-	fnbuf = alloca(size_fnbuf);
-	if (!fnbuf) {
+	size = snprintf(fnbuf, sizeof(fnbuf),
+			"%s/parse_%s.so", AUTOFS_LIB_DIR, name);
+	if (size >= sizeof(fnbuf)) {
 		free(mod);
 		if (err_prefix) {
 			char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
@@ -170,7 +161,6 @@ struct parse_mod *open_parse(const char *name, const char *err_prefix,
 		}
 		return NULL;
 	}
-	snprintf(fnbuf, size_fnbuf, "%s/parse_%s.so", AUTOFS_LIB_DIR, name);
 
 	if (!(dh = dlopen(fnbuf, RTLD_NOW))) {
 		if (err_prefix)
@@ -222,15 +212,11 @@ struct mount_mod *open_mount(const char *name, const char *err_prefix)
 {
 	struct mount_mod *mod;
 	char buf[MAX_ERR_BUF];
-	char *fnbuf;
-	size_t size_name;
-	size_t size_fnbuf;
+	char fnbuf[PATH_MAX];
+	size_t size;
 	void *dh;
 	int *ver;
 
-	size_name = _strlen(name, PATH_MAX + 1);
-	if (!size_name)
-		return NULL;
 
 	mod = malloc(sizeof(struct mount_mod));
 	if (!mod) {
@@ -241,9 +227,9 @@ struct mount_mod *open_mount(const char *name, const char *err_prefix)
 		return NULL;
 	}
 
-	size_fnbuf = size_name + strlen(AUTOFS_LIB_DIR) + 13;
-	fnbuf = alloca(size_fnbuf);
-	if (!fnbuf) {
+	size = snprintf(fnbuf, sizeof(fnbuf),
+			"%s/mount_%s.so", AUTOFS_LIB_DIR, name);
+	if (size >= sizeof(fnbuf)) {
 		free(mod);
 		if (err_prefix) {
 			char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
@@ -251,7 +237,6 @@ struct mount_mod *open_mount(const char *name, const char *err_prefix)
 		}
 		return NULL;
 	}
-	snprintf(fnbuf, size_fnbuf, "%s/mount_%s.so", AUTOFS_LIB_DIR, name);
 
 	if (!(dh = dlopen(fnbuf, RTLD_NOW))) {
 		if (err_prefix)
diff --git a/lib/cache.c b/lib/cache.c
index edb3192..1dfc563 100644
--- a/lib/cache.c
+++ b/lib/cache.c
@@ -482,27 +482,23 @@ struct mapent *cache_lookup_offset(const char *prefix, const char *offset, int s
 {
 	struct list_head *p;
 	struct mapent *this;
-	int plen = strlen(prefix);
-	char *o_key;
+	/* Keys for direct maps may be as long as a path name */
+	char o_key[PATH_MAX];
+	/* Avoid "//" at the beginning of paths */
+	const char *path_prefix = strlen(prefix) > 1 ? prefix : "";
+	size_t size;
 
 	/* root offset duplicates "/" */
-	if (plen > 1) {
-		o_key = alloca(plen + strlen(offset) + 1);
-		strcpy(o_key, prefix);
-		strcat(o_key, offset);
-	} else {
-		o_key = alloca(strlen(offset) + 1);
-		strcpy(o_key, offset);
-	}
+	size = snprintf(o_key, sizeof(o_key), "%s%s", path_prefix, offset);
+	if (size >= sizeof(o_key))
+		return NULL;
 
 	list_for_each(p, head) {
 		this = list_entry(p, struct mapent, multi_list);
 		if (!strcmp(&this->key[start], o_key))
-			goto done;
+			return this;
 	}
-	this = NULL;
-done:
-	return this;
+	return NULL;
 }
 
 /* cache must be read locked by caller */
@@ -759,13 +755,8 @@ int cache_delete(struct mapent_cache *mc, const char *key)
 	struct mapent *me = NULL, *pred;
 	u_int32_t hashval = hash(key, mc->size);
 	int status, ret = CHE_OK;
-	char *this;
+	char this[PATH_MAX];
 
-	this = alloca(strlen(key) + 1);
-	if (!this) {
-		ret = CHE_FAIL;
-		goto done;
-	}
 	strcpy(this, key);
 
 	me = mc->hash[hashval];
diff --git a/lib/cat_path.c b/lib/cat_path.c
index 576b424..60669db 100644
--- a/lib/cat_path.c
+++ b/lib/cat_path.c
@@ -12,7 +12,6 @@
  *
  * ----------------------------------------------------------------------- */
 
-#include <alloca.h>
 #include <string.h>
 #include <limits.h>
 #include <ctype.h>
diff --git a/modules/lookup_file.c b/modules/lookup_file.c
index 7672725..6f95e09 100644
--- a/modules/lookup_file.c
+++ b/modules/lookup_file.c
@@ -378,8 +378,8 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 	unsigned int logopt = master->logopt;
 	char *buffer;
 	int blen;
-	char *path;
-	char *ent;
+	char path[KEY_MAX_LEN + 1];
+	char ent[MAPENT_MAX_LEN + 1];
 	FILE *f;
 	int fd;
 	unsigned int path_len, ent_len;
@@ -394,20 +394,6 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 		return NSS_STATUS_UNAVAIL;
 	}
 
-	path = alloca(KEY_MAX_LEN + 1);
-	if (!path) {
-		error(logopt,
-		      MODPREFIX "could not malloc storage for path");
-		return NSS_STATUS_UNAVAIL;
-	}
-
-	ent = alloca(MAPENT_MAX_LEN + 1);
-	if (!ent) {
-		error(logopt,
-		      MODPREFIX "could not malloc storage for mapent");
-		return NSS_STATUS_UNAVAIL;
-	}
-
 	f = open_fopen_r(ctxt->mapname);
 	if (!f) {
 		error(logopt,
@@ -621,8 +607,8 @@ int lookup_read_map(struct autofs_point *ap, time_t age, void *context)
 	struct lookup_context *ctxt = (struct lookup_context *) context;
 	struct map_source *source;
 	struct mapent_cache *mc;
-	char *key;
-	char *mapent;
+	char key[KEY_MAX_LEN + 1];
+	char mapent[MAPENT_MAX_LEN + 1];
 	FILE *f;
 	int fd;
 	unsigned int k_len, m_len;
@@ -643,20 +629,6 @@ int lookup_read_map(struct autofs_point *ap, time_t age, void *context)
 		return NSS_STATUS_UNAVAIL;
 	}
 
-	key = alloca(KEY_MAX_LEN + 1);
-	if (!key) {
-		error(ap->logopt,
-		      MODPREFIX "could not malloc storage for key");
-		return NSS_STATUS_UNAVAIL;
-	}
-
-	mapent = alloca(MAPENT_MAX_LEN + 1);
-	if (!mapent) {
-		error(ap->logopt,
-		      MODPREFIX "could not malloc storage for mapent");
-		return NSS_STATUS_UNAVAIL;
-	}
-
 	f = open_fopen_r(ctxt->mapname);
 	if (!f) {
 		error(ap->logopt,
@@ -978,7 +950,7 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	char key[KEY_MAX_LEN + 1];
 	int key_len;
 	char *mapent = NULL;
-	int mapent_len;
+	char mapent_buf[MAPENT_MAX_LEN + 1];
 	int status = 0;
 	int ret = 1;
 
@@ -1082,38 +1054,36 @@ do_cache_lookup:
 	}
 	if (me && (me->source == source || *me->key == '/')) {
 		pthread_cleanup_push(cache_lock_cleanup, mc);
-		mapent_len = strlen(me->mapent);
-		mapent = alloca(mapent_len + 1);
-		strcpy(mapent, me->mapent);
+		strcpy(mapent_buf, me->mapent);
+		mapent = mapent_buf;
 		pthread_cleanup_pop(0);
 	}
 	cache_unlock(mc);
 
-	if (mapent) {
-		master_source_current_wait(ap->entry);
-		ap->entry->current = source;
+	if (!mapent)
+		return NSS_STATUS_TRYAGAIN;
 
-		debug(ap->logopt, MODPREFIX "%s -> %s", key, mapent);
-		ret = ctxt->parse->parse_mount(ap, key, key_len,
-					mapent, ctxt->parse->context);
-		if (ret) {
-			time_t now = time(NULL);
-			int rv = CHE_OK;
+	master_source_current_wait(ap->entry);
+	ap->entry->current = source;
 
-			cache_writelock(mc);
+	debug(ap->logopt, MODPREFIX "%s -> %s", key, mapent);
+	ret = ctxt->parse->parse_mount(ap, key, key_len,
+				       mapent, ctxt->parse->context);
+	if (ret) {
+		time_t now = time(NULL);
+		int rv = CHE_OK;
+
+		cache_writelock(mc);
+		me = cache_lookup_distinct(mc, key);
+		if (!me)
+			rv = cache_update(mc, source, key, NULL, now);
+		if (rv != CHE_FAIL) {
 			me = cache_lookup_distinct(mc, key);
-			if (!me)
-				rv = cache_update(mc, source, key, NULL, now);
-			if (rv != CHE_FAIL) {
-				me = cache_lookup_distinct(mc, key);
-				me->status = now + ap->negative_timeout;
-			}
-			cache_unlock(mc);
+			me->status = now + ap->negative_timeout;
 		}
-	}
-
-	if (ret)
+		cache_unlock(mc);
 		return NSS_STATUS_TRYAGAIN;
+	}
 
 	return NSS_STATUS_SUCCESS;
 }
diff --git a/modules/lookup_ldap.c b/modules/lookup_ldap.c
index c7aecf0..da575bd 100644
--- a/modules/lookup_ldap.c
+++ b/modules/lookup_ldap.c
@@ -296,10 +296,10 @@ static int get_query_dn(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt
 	if (ctxt->mapname)
 		l += strlen(key) + strlen(ctxt->mapname) + strlen("(&(=))");
 
-	query = alloca(l);
+	query = malloc(l);
 	if (query == NULL) {
 		char *estr = strerror_r(errno, buf, sizeof(buf));
-		crit(logopt, MODPREFIX "alloca: %s", estr);
+		crit(logopt, MODPREFIX "malloc: %s", estr);
 		return NSS_STATUS_UNAVAIL;
 	}
 
@@ -312,6 +312,7 @@ static int get_query_dn(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt
 		     key, (int) strlen(ctxt->mapname), ctxt->mapname) >= l) {
 			debug(logopt,
 			      MODPREFIX "error forming query string");
+			free(query);
 			return 0;
 		}
 		scope = LDAP_SCOPE_SUBTREE;
@@ -319,6 +320,7 @@ static int get_query_dn(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt
 		if (sprintf(query, "(objectclass=%s)", class) >= l) {
 			debug(logopt,
 			      MODPREFIX "error forming query string");
+			free(query);
 			return 0;
 		}
 		scope = LDAP_SCOPE_SUBTREE;
@@ -342,6 +344,7 @@ static int get_query_dn(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt
 			error(logopt,
 			      MODPREFIX "query failed for %s: %s",
 			      query, ldap_err2string(rv));
+			free(query);
 			return 0;
 		}
 
@@ -355,6 +358,7 @@ static int get_query_dn(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt
 			      MODPREFIX "query succeeded, no matches for %s",
 			      query);
 			ldap_msgfree(result);
+			free(query);
 			return 0;
 		}
 	} else {
@@ -397,10 +401,12 @@ static int get_query_dn(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt
 			ldap_msgfree(result);
 			error(logopt,
 			      MODPREFIX "failed to find query dn under search base dns");
+			free(query);
 			return 0;
 		}
 	}
 
+	free(query);
 	qdn = strdup(dn);
 	ldap_memfree(dn);
 	ldap_msgfree(result);
@@ -1172,7 +1178,7 @@ static int parse_server_string(unsigned logopt, const char *url, struct lookup_c
 			else {
 				char *estr;
 				estr = strerror_r(errno, buf, sizeof(buf));
-				logerr(MODPREFIX "malloc: %s", estr);
+				logerr(MODPREFIX "strdup: %s", estr);
 				if (ctxt->server)
 					free(ctxt->server);
 				return 0;
@@ -1432,23 +1438,26 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 
 	l = strlen("(objectclass=)") + strlen(class) + 1;
 
-	query = alloca(l);
+	query = malloc(l);
 	if (query == NULL) {
 		char *estr = strerror_r(errno, buf, sizeof(buf));
-		logerr(MODPREFIX "alloca: %s", estr);
+		logerr(MODPREFIX "malloc: %s", estr);
 		return NSS_STATUS_UNAVAIL;
 	}
 
 	if (sprintf(query, "(objectclass=%s)", class) >= l) {
 		error(logopt, MODPREFIX "error forming query string");
+		free(query);
 		return NSS_STATUS_UNAVAIL;
 	}
 	query[l] = '\0';
 
 	/* Initialize the LDAP context. */
 	ldap = do_reconnect(logopt, ctxt);
-	if (!ldap)
+	if (!ldap) {
+		free(query);
 		return NSS_STATUS_UNAVAIL;
+	}
 
 	/* Look around. */
 	debug(logopt,
@@ -1460,6 +1469,7 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 		error(logopt, MODPREFIX "query failed for %s: %s",
 		      query, ldap_err2string(rv));
 		unbind_ldap_connection(logging, ldap, ctxt);
+		free(query);
 		return NSS_STATUS_NOTFOUND;
 	}
 
@@ -1470,6 +1480,7 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 		      query);
 		ldap_msgfree(result);
 		unbind_ldap_connection(logging, ldap, ctxt);
+		free(query);
 		return NSS_STATUS_NOTFOUND;
 	} else
 		debug(logopt, MODPREFIX "examining entries");
@@ -1539,6 +1550,7 @@ next:
 	/* Clean up. */
 	ldap_msgfree(result);
 	unbind_ldap_connection(logopt, ldap, ctxt);
+	free(query);
 
 	return NSS_STATUS_SUCCESS;
 }
@@ -2165,7 +2177,7 @@ static int read_one_map(struct autofs_point *ap,
 	/* Build a query string. */
 	l = strlen("(objectclass=)") + strlen(class) + 1;
 
-	sp.query = alloca(l);
+	sp.query = malloc(l);
 	if (sp.query == NULL) {
 		char *estr = strerror_r(errno, buf, sizeof(buf));
 		logerr(MODPREFIX "malloc: %s", estr);
@@ -2174,14 +2186,17 @@ static int read_one_map(struct autofs_point *ap,
 
 	if (sprintf(sp.query, "(objectclass=%s)", class) >= l) {
 		error(ap->logopt, MODPREFIX "error forming query string");
+		free(sp.query);
 		return NSS_STATUS_UNAVAIL;
 	}
 	sp.query[l] = '\0';
 
 	/* Initialize the LDAP context. */
 	sp.ldap = do_reconnect(ap->logopt, ctxt);
-	if (!sp.ldap)
+	if (!sp.ldap) {
+		free(sp.query);
 		return NSS_STATUS_UNAVAIL;
+	}
 
 	/* Look around. */
 	debug(ap->logopt,
@@ -2206,6 +2221,7 @@ static int read_one_map(struct autofs_point *ap,
 		if (rv != LDAP_SUCCESS || !sp.result) {
 			unbind_ldap_connection(ap->logopt, sp.ldap, ctxt);
 			*result_ldap = rv;
+			free(sp.query);
 			return NSS_STATUS_UNAVAIL;
 		}
 
@@ -2214,6 +2230,7 @@ static int read_one_map(struct autofs_point *ap,
 			ldap_msgfree(sp.result);
 			unbind_ldap_connection(ap->logopt, sp.ldap, ctxt);
 			*result_ldap = rv;
+			free(sp.query);
 			return NSS_STATUS_NOTFOUND;
 		}
 		ldap_msgfree(sp.result);
@@ -2224,6 +2241,7 @@ static int read_one_map(struct autofs_point *ap,
 	unbind_ldap_connection(ap->logopt, sp.ldap, ctxt);
 
 	source->age = age;
+	free(sp.query);
 
 	return NSS_STATUS_SUCCESS;
 }
@@ -2319,7 +2337,7 @@ static int lookup_one(struct autofs_point *ap,
 	if (enc_len1)
 		l += 2*strlen(entry) + enc_len1 + enc_len2 + 6;
 
-	query = alloca(l);
+	query = malloc(l);
 	if (query == NULL) {
 		char *estr = strerror_r(errno, buf, sizeof(buf));
 		crit(ap->logopt, MODPREFIX "malloc: %s", estr);
@@ -2327,6 +2345,7 @@ static int lookup_one(struct autofs_point *ap,
 			free(enc_key1);
 			free(enc_key2);
 		}
+		free(query);
 		return CHE_FAIL;
 	}
 
@@ -2358,14 +2377,17 @@ static int lookup_one(struct autofs_point *ap,
 	if (ql >= l) {
 		error(ap->logopt,
 		      MODPREFIX "error forming query string");
+		free(query);
 		return CHE_FAIL;
 	}
 	query[ql] = '\0';
 
 	/* Initialize the LDAP context. */
 	ldap = do_reconnect(ap->logopt, ctxt);
-	if (!ldap)
+	if (!ldap) {
+		free(query);
 		return CHE_UNAVAIL;
+	}
 
 	debug(ap->logopt,
 	      MODPREFIX "searching for \"%s\" under \"%s\"", query, ctxt->qdn);
@@ -2375,6 +2397,7 @@ static int lookup_one(struct autofs_point *ap,
 	if ((rv != LDAP_SUCCESS) || !result) {
 		crit(ap->logopt, MODPREFIX "query failed for %s", query);
 		unbind_ldap_connection(ap->logopt, ldap, ctxt);
+		free(query);
 		return CHE_FAIL;
 	}
 
@@ -2387,6 +2410,7 @@ static int lookup_one(struct autofs_point *ap,
 		     MODPREFIX "got answer, but no entry for %s", query);
 		ldap_msgfree(result);
 		unbind_ldap_connection(ap->logopt, ldap, ctxt);
+		free(query);
 		return CHE_MISSING;
 	}
 
@@ -2601,6 +2625,7 @@ next:
 		}
 	}
 	pthread_cleanup_pop(1);
+	free(query);
 
 	return ret;
 }
@@ -2687,7 +2712,7 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	char key[KEY_MAX_LEN + 1];
 	int key_len;
 	char *mapent = NULL;
-	int mapent_len;
+	char mapent_buf[MAPENT_MAX_LEN + 1];
 	int status = 0;
 	int ret = 1;
 
@@ -2757,38 +2782,36 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			me = cache_lookup_distinct(mc, "*");
 	}
 	if (me && (me->source == source || *me->key == '/')) {
-		mapent_len = strlen(me->mapent);
-		mapent = alloca(mapent_len + 1);
-		strcpy(mapent, me->mapent);
+		strcpy(mapent_buf, me->mapent);
+		mapent = mapent_buf;
 	}
 	cache_unlock(mc);
 
-	if (mapent) {
-		master_source_current_wait(ap->entry);
-		ap->entry->current = source;
+	if (!mapent)
+		return NSS_STATUS_TRYAGAIN;
 
-		debug(ap->logopt, MODPREFIX "%s -> %s", key, mapent);
-		ret = ctxt->parse->parse_mount(ap, key, key_len,
-					 mapent, ctxt->parse->context);
-		if (ret) {
-			time_t now = time(NULL);
-			int rv = CHE_OK;
+	master_source_current_wait(ap->entry);
+	ap->entry->current = source;
 
-			/* Record the the mount fail in the cache */
-			cache_writelock(mc);
+	debug(ap->logopt, MODPREFIX "%s -> %s", key, mapent);
+	ret = ctxt->parse->parse_mount(ap, key, key_len,
+				       mapent, ctxt->parse->context);
+	if (ret) {
+		time_t now = time(NULL);
+		int rv = CHE_OK;
+
+		/* Record the the mount fail in the cache */
+		cache_writelock(mc);
+		me = cache_lookup_distinct(mc, key);
+		if (!me)
+			rv = cache_update(mc, source, key, NULL, now);
+		if (rv != CHE_FAIL) {
 			me = cache_lookup_distinct(mc, key);
-			if (!me)
-				rv = cache_update(mc, source, key, NULL, now);
-			if (rv != CHE_FAIL) {
-				me = cache_lookup_distinct(mc, key);
-				me->status = now + ap->negative_timeout;
-			}
-			cache_unlock(mc);
+			me->status = now + ap->negative_timeout;
 		}
-	}
-
-	if (ret)
+		cache_unlock(mc);
 		return NSS_STATUS_TRYAGAIN;
+	}
 
 	return NSS_STATUS_SUCCESS;
 }
diff --git a/modules/lookup_nisplus.c b/modules/lookup_nisplus.c
index 4c3ce60..0c75905 100644
--- a/modules/lookup_nisplus.c
+++ b/modules/lookup_nisplus.c
@@ -92,10 +92,10 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 	int cur_state, len;
 
 	pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cur_state);
-	tablename = alloca(strlen(ctxt->mapname) + strlen(ctxt->domainname) + 20);
+	tablename = malloc(strlen(ctxt->mapname) + strlen(ctxt->domainname) + 20);
 	if (!tablename) {
 		char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
-		logerr(MODPREFIX "alloca: %s", estr);
+		logerr(MODPREFIX "malloc: %s", estr);
 		pthread_setcancelstate(cur_state, NULL);
 		return NSS_STATUS_UNAVAIL;
 	}
@@ -107,6 +107,7 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 		nis_freeresult(result);
 		crit(logopt,
 		     MODPREFIX "couldn't locate nis+ table %s", ctxt->mapname);
+		free(tablename);
 		pthread_setcancelstate(cur_state, NULL);
 		return NSS_STATUS_NOTFOUND;
 	}
@@ -118,6 +119,7 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 		nis_freeresult(result);
 		crit(logopt,
 		     MODPREFIX "couldn't enumrate nis+ map %s", ctxt->mapname);
+		free(tablename);
 		pthread_setcancelstate(cur_state, NULL);
 		return NSS_STATUS_UNAVAIL;
 	}
@@ -155,6 +157,7 @@ int lookup_read_master(struct master *master, time_t age, void *context)
 	}
 
 	nis_freeresult(result);
+	free(tablename);
 	pthread_setcancelstate(cur_state, NULL);
 
 	return NSS_STATUS_SUCCESS;
@@ -180,10 +183,10 @@ int lookup_read_map(struct autofs_point *ap, time_t age, void *context)
 	mc = source->mc;
 
 	pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cur_state);
-	tablename = alloca(strlen(ctxt->mapname) + strlen(ctxt->domainname) + 20);
+	tablename = malloc(strlen(ctxt->mapname) + strlen(ctxt->domainname) + 20);
 	if (!tablename) {
 		char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
-		logerr(MODPREFIX "alloca: %s", estr);
+		logerr(MODPREFIX "malloc: %s", estr);
 		pthread_setcancelstate(cur_state, NULL);
 		return NSS_STATUS_UNAVAIL;
 	}
@@ -195,6 +198,7 @@ int lookup_read_map(struct autofs_point *ap, time_t age, void *context)
 		nis_freeresult(result);
 		crit(ap->logopt,
 		     MODPREFIX "couldn't locate nis+ table %s", ctxt->mapname);
+		free(tablename);
 		pthread_setcancelstate(cur_state, NULL);
 		return NSS_STATUS_NOTFOUND;
 	}
@@ -206,6 +210,7 @@ int lookup_read_map(struct autofs_point *ap, time_t age, void *context)
 		nis_freeresult(result);
 		crit(ap->logopt,
 		     MODPREFIX "couldn't enumrate nis+ map %s", ctxt->mapname);
+		free(tablename);
 		pthread_setcancelstate(cur_state, NULL);
 		return NSS_STATUS_UNAVAIL;
 	}
@@ -245,6 +250,7 @@ int lookup_read_map(struct autofs_point *ap, time_t age, void *context)
 
 	source->age = age;
 
+	free(tablename);
 	pthread_setcancelstate(cur_state, NULL);
 
 	return NSS_STATUS_SUCCESS;
@@ -271,11 +277,11 @@ static int lookup_one(struct autofs_point *ap,
 	mc = source->mc;
 
 	pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cur_state);
-	tablename = alloca(strlen(key) +
-			strlen(ctxt->mapname) + strlen(ctxt->domainname) + 20);
+	tablename = malloc(strlen(key) + strlen(ctxt->mapname) +
+			   strlen(ctxt->domainname) + 20);
 	if (!tablename) {
 		char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
-		logerr(MODPREFIX "alloca: %s", estr);
+		logerr(MODPREFIX "malloc: %s", estr);
 		pthread_setcancelstate(cur_state, NULL);
 		return -1;
 	}
@@ -286,6 +292,7 @@ static int lookup_one(struct autofs_point *ap,
 	if (result->status != NIS_SUCCESS && result->status != NIS_S_SUCCESS) {
 		nis_error rs = result->status;
 		nis_freeresult(result);
+		free(tablename);
 		pthread_setcancelstate(cur_state, NULL);
 		if (rs == NIS_NOTFOUND ||
 		    rs == NIS_S_NOTFOUND ||
@@ -303,6 +310,7 @@ static int lookup_one(struct autofs_point *ap,
 	cache_unlock(mc);
 
 	nis_freeresult(result);
+	free(tablename);
 	pthread_setcancelstate(cur_state, NULL);
 
 	return ret;
@@ -327,10 +335,10 @@ static int lookup_wild(struct autofs_point *ap, struct lookup_context *ctxt)
 	mc = source->mc;
 
 	pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cur_state);
-	tablename = alloca(strlen(ctxt->mapname) + strlen(ctxt->domainname) + 20);
+	tablename = malloc(strlen(ctxt->mapname) + strlen(ctxt->domainname) + 20);
 	if (!tablename) {
 		char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
-		logerr(MODPREFIX "alloca: %s", estr);
+		logerr(MODPREFIX "malloc: %s", estr);
 		pthread_setcancelstate(cur_state, NULL);
 		return -1;
 	}
@@ -341,6 +349,7 @@ static int lookup_wild(struct autofs_point *ap, struct lookup_context *ctxt)
 	if (result->status != NIS_SUCCESS && result->status != NIS_S_SUCCESS) {
 		nis_error rs = result->status;
 		nis_freeresult(result);
+		free(tablename);
 		pthread_setcancelstate(cur_state, NULL);
 		if (rs == NIS_NOTFOUND ||
 		    rs == NIS_S_NOTFOUND ||
@@ -357,6 +366,7 @@ static int lookup_wild(struct autofs_point *ap, struct lookup_context *ctxt)
 	cache_unlock(mc);
 
 	nis_freeresult(result);
+	free(tablename);
 	pthread_setcancelstate(cur_state, NULL);
 
 	return ret;
@@ -546,36 +556,37 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	}
 	if (me && (me->source == source || *me->key == '/')) {
 		mapent_len = strlen(me->mapent);
-		mapent = alloca(mapent_len + 1);
+		mapent = malloc(mapent_len + 1);
 		strcpy(mapent, me->mapent);
 	}
 	cache_unlock(mc);
 
-	if (mapent) {
-		master_source_current_wait(ap->entry);
-		ap->entry->current = source;
+	if (!mapent)
+		return NSS_STATUS_TRYAGAIN;
 
-		debug(ap->logopt, MODPREFIX "%s -> %s", key, mapent);
-		ret = ctxt->parse->parse_mount(ap, key, key_len,
-					       mapent, ctxt->parse->context);
-		if (ret) {
-			time_t now = time(NULL);
-			int rv = CHE_OK;
+	master_source_current_wait(ap->entry);
+	ap->entry->current = source;
+
+	debug(ap->logopt, MODPREFIX "%s -> %s", key, mapent);
+	ret = ctxt->parse->parse_mount(ap, key, key_len,
+				       mapent, ctxt->parse->context);
+	if (ret) {
+		time_t now = time(NULL);
+		int rv = CHE_OK;
 
-			cache_writelock(mc);
+		cache_writelock(mc);
+		me = cache_lookup_distinct(mc, key);
+		if (!me)
+			rv = cache_update(mc, source, key, NULL, now);
+		if (rv != CHE_FAIL) {
 			me = cache_lookup_distinct(mc, key);
-			if (!me)
-				rv = cache_update(mc, source, key, NULL, now);
-			if (rv != CHE_FAIL) {
-				me = cache_lookup_distinct(mc, key);
-				me->status = time(NULL) + ap->negative_timeout;
-			}
-			cache_unlock(mc);
+			me->status = time(NULL) + ap->negative_timeout;
 		}
-	}
-
-	if (ret)
+		cache_unlock(mc);
+		free(mapent);
 		return NSS_STATUS_TRYAGAIN;
+	}
+	free(mapent);
 
 	return NSS_STATUS_SUCCESS;
 }
diff --git a/modules/mount_autofs.c b/modules/mount_autofs.c
index 82a5ef3..182da4e 100644
--- a/modules/mount_autofs.c
+++ b/modules/mount_autofs.c
@@ -18,7 +18,6 @@
 #include <malloc.h>
 #include <string.h>
 #include <signal.h>
-#include <alloca.h>
 #include <sys/param.h>
 #include <sys/types.h>
 #include <sys/stat.h>
diff --git a/modules/mount_bind.c b/modules/mount_bind.c
index 361f0c2..b8ef581 100644
--- a/modules/mount_bind.c
+++ b/modules/mount_bind.c
@@ -69,7 +69,7 @@ out:
 int mount_mount(struct autofs_point *ap, const char *root, const char *name, int name_len,
 		const char *what, const char *fstype, const char *options, void *context)
 {
-	char *fullpath;
+	char fullpath[PATH_MAX];
 	char buf[MAX_ERR_BUF];
 	int err;
 	int i, len;
@@ -80,14 +80,11 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
 	/* Root offset of multi-mount */
 	len = strlen(root);
 	if (root[len - 1] == '/') {
-		fullpath = alloca(len);
 		len = snprintf(fullpath, len, "%s", root);
 	/* Direct mount name is absolute path so don't use root */
 	} else if (*name == '/') {
-		fullpath = alloca(len + 1);
 		len = sprintf(fullpath, "%s", root);
 	} else {
-		fullpath = alloca(len + name_len + 2);
 		len = sprintf(fullpath, "%s/%s", root, name);
 	}
 	fullpath[len] = '\0';
@@ -141,7 +138,7 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
 		}
 	} else {
 		char *cp;
-		char *basepath = alloca(strlen(fullpath) + 1);
+		char basepath[PATH_MAX];
 		int status;
 		struct stat st;
 
diff --git a/modules/mount_changer.c b/modules/mount_changer.c
index 92bb72b..024d244 100644
--- a/modules/mount_changer.c
+++ b/modules/mount_changer.c
@@ -44,7 +44,7 @@ int mount_init(void **context)
 int mount_mount(struct autofs_point *ap, const char *root, const char *name, int name_len,
 		const char *what, const char *fstype, const char *options, void *context)
 {
-	char *fullpath;
+	char fullpath[PATH_MAX];
 	char buf[MAX_ERR_BUF];
 	int err;
 	int len, status, existed = 1;
@@ -57,14 +57,11 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
 	/* Root offset of multi-mount */
 	len = strlen(root);
 	if (root[len - 1] == '/') {
-		fullpath = alloca(len);
 		len = snprintf(fullpath, len, "%s", root);
 	/* Direct mount name is absolute path so don't use root */
 	} else if (*name == '/') {
-		fullpath = alloca(len + 1);
 		len = sprintf(fullpath, "%s", root);
 	} else {
-		fullpath = alloca(len + name_len + 2);
 		len = sprintf(fullpath, "%s/%s", root, name);
 	}
 	fullpath[len] = '\0';
diff --git a/modules/mount_ext2.c b/modules/mount_ext2.c
index 192ec04..85329ab 100644
--- a/modules/mount_ext2.c
+++ b/modules/mount_ext2.c
@@ -36,7 +36,7 @@ int mount_init(void **context)
 int mount_mount(struct autofs_point *ap, const char *root, const char *name, int name_len,
 		const char *what, const char *fstype, const char *options, void *context)
 {
-	char *fullpath;
+	char fullpath[PATH_MAX];
 	char buf[MAX_ERR_BUF];
 	const char *p, *p1;
 	int err, ro = 0;
@@ -49,14 +49,11 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
 	/* Root offset of multi-mount */
 	len = strlen(root);
 	if (root[len - 1] == '/') {
-		fullpath = alloca(len);
 		len = snprintf(fullpath, len, "%s", root);
 	/* Direct mount name is absolute path so don't use root */
 	} else if (*name == '/') {
-		fullpath = alloca(len + 1);
 		len = sprintf(fullpath, "%s", root);
 	} else {
-		fullpath = alloca(len + name_len + 2);
 		len = sprintf(fullpath, "%s/%s", root, name);
 	}
 	fullpath[len] = '\0';
diff --git a/modules/mount_generic.c b/modules/mount_generic.c
index 6d7b4b3..1dc1bfd 100644
--- a/modules/mount_generic.c
+++ b/modules/mount_generic.c
@@ -37,7 +37,7 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
 		const char *what, const char *fstype, const char *options,
 		void *context)
 {
-	char *fullpath;
+	char fullpath[PATH_MAX];
 	char buf[MAX_ERR_BUF];
 	int err;
 	int len, status, existed = 1;
@@ -48,14 +48,11 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
 	/* Root offset of multi-mount */
 	len = strlen(root);
 	if (root[len - 1] == '/') {
-		fullpath = alloca(len);
 		len = snprintf(fullpath, len, "%s", root);
 	/* Direct mount name is absolute path so don't use root */
 	} else if (*name == '/') {
-		fullpath = alloca(len + 1);
 		len = sprintf(fullpath, "%s", root);
 	} else {
-		fullpath = alloca(len + name_len + 2);
 		len = sprintf(fullpath, "%s/%s", root, name);
 	}
 	fullpath[len] = '\0';

[PATCH 4/4] autofs-5.0.4 - fix kernel includes

[Ian Kent]

From: Valerie Aurora Henson <vaurora@redhat.com>

autofs_dev-ioctl.h is included by both the kernel module and the
userland tools, and it includes two kernel include files.  The compile
worked if you had kernel headers installed but failed otherwise.

imk: there are a couple of other instances were we include kernel-headers
includes. I've tried to fix that up.
---

 include/automount.h            |    2 +-
 include/linux/auto_dev-ioctl.h |   10 ++++++++++
 include/linux/auto_fs.h        |    5 ++++-
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/automount.h b/include/automount.h
index 005d209..8a83fb4 100644
--- a/include/automount.h
+++ b/include/automount.h
@@ -13,7 +13,7 @@
 #include <limits.h>
 #include <time.h>
 #include <syslog.h>
-#include <linux/types.h>
+#include <sys/types.h>
 #include <pthread.h>
 #include <sched.h>
 #include <errno.h>
diff --git a/include/linux/auto_dev-ioctl.h b/include/linux/auto_dev-ioctl.h
index 91a7739..8c5b71a 100644
--- a/include/linux/auto_dev-ioctl.h
+++ b/include/linux/auto_dev-ioctl.h
@@ -10,8 +10,18 @@
 #ifndef _LINUX_AUTO_DEV_IOCTL_H
 #define _LINUX_AUTO_DEV_IOCTL_H
 
+/* This file is shared by both user and kernel space */
+#ifdef __KERNEL__
 #include <linux/string.h>
 #include <linux/types.h>
+#else
+#include <string.h>
+#ifndef _LINUX_AUTO_FS_H
+#include <sys/types.h>
+typedef		u_int32_t	__u32;
+typedef		u_int64_t	__u64;
+#endif
+#endif /* __KERNEL__ */
 
 #define AUTOFS_DEVICE_NAME		"autofs"
 
diff --git a/include/linux/auto_fs.h b/include/linux/auto_fs.h
index bd39f09..5340fc6 100644
--- a/include/linux/auto_fs.h
+++ b/include/linux/auto_fs.h
@@ -18,9 +18,12 @@
 #include <linux/fs.h>
 #include <linux/limits.h>
 #include <asm/types.h>
+#else
+typedef		u_int32_t	__u32;
+typedef		u_int64_t	__u64;
 #endif /* __KERNEL__ */
 
-#include <linux/ioctl.h>
+#include <asm/ioctl.h>
 
 /* This file describes autofs v3 */
 #define AUTOFS_PROTO_VERSION	3

Re: [PATCH 1/4] autofs-5.0.4 - always read file maps

[Paul Wankadia]

[-- Attachment #1.1: Type: text/plain, Size: 7557 bytes --]

On Mon, Jan 19, 2009 at 11:24 PM, Ian Kent <raven@themaw.net> wrote:

autofs tries to not load an entire map into the internal cache unless it
> has to. For maps that do get loaded into the cache it relies on checks to
> work out if a map is up to date in order to trigger a map read. This is
> fine for maps that can do direct key lookups but file maps need to do a
> linear search through the file when locating an entry for a key. For large
> maps this can be a huge overhead. This patch make autofs always load file
> based maps at start and makes use of the map file mtime to discover if the
> cache needs to be refreshed.
> ---
>
>  daemon/lookup.c       |    9 +++++--
>  modules/lookup_file.c |   59
> +++++++++++++++++--------------------------------
>  2 files changed, 27 insertions(+), 41 deletions(-)
>
> diff --git a/daemon/lookup.c b/daemon/lookup.c
> index 741d846..e034348 100644
> --- a/daemon/lookup.c
> +++ b/daemon/lookup.c
> @@ -283,10 +283,13 @@ static int do_read_map(struct autofs_point *ap,
> struct map_source *map, time_t a
>         * for the fail cases to function correctly and to cache the
>         * lookup handle.
>         *
> -        * We always need to whole map for direct mounts in order to
> -        * mount the triggers.
> +        * We always need to read the whole map for direct mounts in
> +        * order to mount the triggers. We also want to read the whole
> +        * map if it's a file map to avoid potentially lengthy linear
> +        * file scanning.
>         */
> -       if (!(ap->flags & MOUNT_FLAG_GHOST) && ap->type != LKP_DIRECT)
> +       if (strcmp(map->type, "file") &&
> +           !(ap->flags & MOUNT_FLAG_GHOST) && ap->type != LKP_DIRECT)
>                return NSS_STATUS_SUCCESS;
>
>        if (!map->stale)
> diff --git a/modules/lookup_file.c b/modules/lookup_file.c
> index 95b9f6f..7672725 100644
> --- a/modules/lookup_file.c
> +++ b/modules/lookup_file.c
> @@ -44,7 +44,6 @@ typedef enum { esc_none, esc_char, esc_val, esc_all }
> ESCAPES;
>
>  struct lookup_context {
>        const char *mapname;
> -       time_t mtime;
>        struct parse_mod *parse;
>  };
>
> @@ -54,7 +53,6 @@ int lookup_init(const char *mapfmt, int argc, const char
> *const *argv, void **co
>  {
>        struct lookup_context *ctxt;
>        char buf[MAX_ERR_BUF];
> -       struct stat st;
>
>        *context = NULL;
>
> @@ -87,15 +85,6 @@ int lookup_init(const char *mapfmt, int argc, const char
> *const *argv, void **co
>                return 1;
>        }
>
> -       if (stat(ctxt->mapname, &st)) {
> -               free(ctxt);
> -               logmsg(MODPREFIX "file map %s, could not stat",
> -                    argv[0]);
> -               return 1;
> -       }
> -
> -       ctxt->mtime = st.st_mtime;
> -
>        if (!mapfmt)
>                mapfmt = MAPFMT_DEFAULT;
>
> @@ -391,7 +380,6 @@ int lookup_read_master(struct master *master, time_t
> age, void *context)
>        int blen;
>        char *path;
>        char *ent;
> -       struct stat st;
>        FILE *f;
>        int fd;


You should be able to get rid of `fd' and the fileno(3) call now.

       unsigned int path_len, ent_len;
> @@ -504,13 +492,6 @@ int lookup_read_master(struct master *master, time_t
> age, void *context)
>                        break;
>        }
>
> -       if (fstat(fd, &st)) {
> -               crit(logopt, MODPREFIX "file map %s, could not stat",
> -                      ctxt->mapname);
> -               return NSS_STATUS_UNAVAIL;
> -       }
> -       ctxt->mtime = st.st_mtime;
> -
>        fclose(f);
>
>        return NSS_STATUS_SUCCESS;
> @@ -642,7 +623,6 @@ int lookup_read_map(struct autofs_point *ap, time_t
> age, void *context)
>        struct mapent_cache *mc;
>        char *key;
>        char *mapent;
> -       struct stat st;
>        FILE *f;
>        int fd;


You should be able to get rid of `fd' and the fileno(3) call now.

       unsigned int k_len, m_len;
> @@ -748,13 +728,6 @@ int lookup_read_map(struct autofs_point *ap, time_t
> age, void *context)
>                        break;
>        }
>
> -       if (fstat(fd, &st)) {
> -               crit(ap->logopt,
> -                    MODPREFIX "file map %s, could not stat",
> -                    ctxt->mapname);
> -               return NSS_STATUS_UNAVAIL;
> -       }
> -       ctxt->mtime = st.st_mtime;
>        source->age = age;
>
>        fclose(f);
> @@ -951,9 +924,6 @@ static int check_map_indirect(struct autofs_point *ap,
>        if (ret == CHE_FAIL)
>                return NSS_STATUS_NOTFOUND;
>
> -       if (ret & CHE_UPDATED)
> -               source->stale = 1;
> -
>        pthread_cleanup_push(cache_lock_cleanup, mc);
>        cache_writelock(mc);
>        exists = cache_lookup_distinct(mc, key);
> @@ -963,7 +933,6 @@ static int check_map_indirect(struct autofs_point *ap,
>                        free(exists->mapent);
>                        exists->mapent = NULL;
>                        exists->status = 0;
> -                       source->stale = 1;
>                }
>        }
>        pthread_cleanup_pop(1);
> @@ -985,14 +954,8 @@ static int check_map_indirect(struct autofs_point *ap,
>                we = cache_lookup_distinct(mc, "*");
>                if (we) {
>                        /* Wildcard entry existed and is now gone */
> -                       if (we->source == source && (wild & CHE_MISSING)) {
> +                       if (we->source == source && (wild & CHE_MISSING))
>                                cache_delete(mc, "*");
> -                               source->stale = 1;
> -                       }
> -               } else {
> -                       /* Wildcard not in map but now is */
> -                       if (wild & (CHE_OK | CHE_UPDATED))
> -                               source->stale = 1;
>                }
>                pthread_cleanup_pop(1);
>
> @@ -1062,9 +1025,28 @@ int lookup_mount(struct autofs_point *ap, const char
> *name, int name_len, void *
>         * we never know about it.
>         */
>        if (ap->type == LKP_INDIRECT && *key != '/') {
> +               struct stat st;
>                char *lkp_key;
>
> +               /*
> +                * We can skip the map lookup and cache update altogether
> +                * if we know the map hasn't been modified since it was
> +                * last read. If it has then we can mark the map stale
> +                * so a re-read is triggered following the lookup.
> +                */
> +               if (stat(ctxt->mapname, &st)) {
> +                       error(ap->logopt, MODPREFIX
> +                             "file map %s, could not stat",
> ctxt->mapname);
> +                       return NSS_STATUS_UNAVAIL;
> +               }
> +
>                cache_readlock(mc);
> +               me = cache_lookup_first(mc);
> +               if (me && st.st_mtime <= me->age)
> +                       goto do_cache_lookup;
> +               else
> +                       source->stale = 1;


Perhaps some output for debugging purposes wouldn't go astray?

+
>                me = cache_lookup_distinct(mc, key);
>                if (me && me->multi)
>                        lkp_key = strdup(me->multi->key);
> @@ -1088,6 +1070,7 @@ int lookup_mount(struct autofs_point *ap, const char
> *name, int name_len, void *
>        }
>
>        cache_readlock(mc);
> +do_cache_lookup:
>        me = cache_lookup(mc, key);
>        /* Stale mapent => check for entry in alternate source or wildcard
> */
>        if (me && !me->mapent) {
>
>

[-- Attachment #1.2: Type: text/html, Size: 13299 bytes --]

[-- Attachment #2: Type: text/plain, Size: 140 bytes --]

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

Re: [PATCH 1/4] autofs-5.0.4 - always read file maps

[Ian Kent]

On Tue, 2009-01-20 at 12:31 +1100, Paul Wankadia wrote:
> On Mon, Jan 19, 2009 at 11:24 PM, Ian Kent <raven@themaw.net> wrote:

snip ...

>         @@ -391,7 +380,6 @@ int lookup_read_master(struct master
>         *master, time_t age, void *context)
>                int blen;
>                char *path;
>                char *ent;
>         -       struct stat st;
>                FILE *f;
>                int fd;
> 
> You should be able to get rid of `fd' and the fileno(3) call now.
> 
> 
>                unsigned int path_len, ent_len;
>         @@ -504,13 +492,6 @@ int lookup_read_master(struct master
>         *master, time_t age, void *context)
>                                break;
>                }
>         
>         -       if (fstat(fd, &st)) {
>         -               crit(logopt, MODPREFIX "file map %s, could not
>         stat",
>         -                      ctxt->mapname);
>         -               return NSS_STATUS_UNAVAIL;
>         -       }
>         -       ctxt->mtime = st.st_mtime;
>         -
>                fclose(f);
>         
>                return NSS_STATUS_SUCCESS;
>         @@ -642,7 +623,6 @@ int lookup_read_map(struct autofs_point
>         *ap, time_t age, void *context)
>                struct mapent_cache *mc;
>                char *key;
>                char *mapent;
>         -       struct stat st;
>                FILE *f;
>                int fd;
> 
> You should be able to get rid of `fd' and the fileno(3) call now.

Yep, quite right.

> 
> 
>                unsigned int k_len, m_len;
>         @@ -748,13 +728,6 @@ int lookup_read_map(struct autofs_point
>         *ap, time_t age, void *context)
>                                break;
>                }
>         
>         -       if (fstat(fd, &st)) {
>         -               crit(ap->logopt,
>         -                    MODPREFIX "file map %s, could not stat",
>         -                    ctxt->mapname);
>         -               return NSS_STATUS_UNAVAIL;
>         -       }
>         -       ctxt->mtime = st.st_mtime;
>                source->age = age;
>         
>                fclose(f);
>         @@ -951,9 +924,6 @@ static int check_map_indirect(struct
>         autofs_point *ap,
>                if (ret == CHE_FAIL)
>                        return NSS_STATUS_NOTFOUND;
>         
>         -       if (ret & CHE_UPDATED)
>         -               source->stale = 1;
>         -
>                pthread_cleanup_push(cache_lock_cleanup, mc);
>                cache_writelock(mc);
>                exists = cache_lookup_distinct(mc, key);
>         @@ -963,7 +933,6 @@ static int check_map_indirect(struct
>         autofs_point *ap,
>                                free(exists->mapent);
>                                exists->mapent = NULL;
>                                exists->status = 0;
>         -                       source->stale = 1;
>                        }
>                }
>                pthread_cleanup_pop(1);
>         @@ -985,14 +954,8 @@ static int check_map_indirect(struct
>         autofs_point *ap,
>                        we = cache_lookup_distinct(mc, "*");
>                        if (we) {
>                                /* Wildcard entry existed and is now
>         gone */
>         -                       if (we->source == source && (wild &
>         CHE_MISSING)) {
>         +                       if (we->source == source && (wild &
>         CHE_MISSING))
>                                        cache_delete(mc, "*");
>         -                               source->stale = 1;
>         -                       }
>         -               } else {
>         -                       /* Wildcard not in map but now is */
>         -                       if (wild & (CHE_OK | CHE_UPDATED))
>         -                               source->stale = 1;
>                        }
>                        pthread_cleanup_pop(1);
>         
>         @@ -1062,9 +1025,28 @@ int lookup_mount(struct autofs_point
>         *ap, const char *name, int name_len, void *
>                 * we never know about it.
>                 */
>                if (ap->type == LKP_INDIRECT && *key != '/') {
>         +               struct stat st;
>                        char *lkp_key;
>         
>         +               /*
>         +                * We can skip the map lookup and cache update
>         altogether
>         +                * if we know the map hasn't been modified
>         since it was
>         +                * last read. If it has then we can mark the
>         map stale
>         +                * so a re-read is triggered following the
>         lookup.
>         +                */
>         +               if (stat(ctxt->mapname, &st)) {
>         +                       error(ap->logopt, MODPREFIX
>         +                             "file map %s, could not stat",
>         ctxt->mapname);
>         +                       return NSS_STATUS_UNAVAIL;
>         +               }
>         +
>                        cache_readlock(mc);
>         +               me = cache_lookup_first(mc);
>         +               if (me && st.st_mtime <= me->age)
>         +                       goto do_cache_lookup;
>         +               else
>         +                       source->stale = 1;
> 
> Perhaps some output for debugging purposes wouldn't go astray?

Mmm .. I'll see what I can do.

> 
> 
>         +
>                        me = cache_lookup_distinct(mc, key);
>                        if (me && me->multi)
>                                lkp_key = strdup(me->multi->key);
>         @@ -1088,6 +1070,7 @@ int lookup_mount(struct autofs_point
>         *ap, const char *name, int name_len, void *
>                }
>         
>                cache_readlock(mc);
>         +do_cache_lookup:
>                me = cache_lookup(mc, key);
>                /* Stale mapent => check for entry in alternate source
>         or wildcard */
>                if (me && !me->mapent) {
>         
> 

Re: [PATCH 0/4] Current state of Vals patches and the file map lookup patches

[Valerie Aurora Henson]

On Mon, Jan 19, 2009 at 09:24:33PM +0900, Ian Kent wrote:
> I've pulled in the patches that Val posted and made a couple of
> minor changes. I also have what I think is just about what we
> need to reduce the overhead of file map scanning.

Great, thank you!

> Check'em out please folks.

I have a crumpled post-it note on my desk saying "valgrind automount."
How hard would this be to do while running the Connectathon suite?  It
would sure make me feel better about these patches.

> I'm a little concerned about the "fix kernel includes" patch because
> there should be more to it and I've altered it in line with that. I'm
> not sure if we should post a kernel patch first, pull in the updated
> kernel header files to the tar and finally make the other couple of
> changes then. But then I may have it wrong so check it out.

Yeah, there's a trade-off there.  I think the approach you've taken is
the most sensible, given that nothing makes me more insane than out of
sync user and kernel code.

-VAL

Re: [PATCH 0/4] Current state of Vals patches and the file map lookup patches

[Ian Kent]

On Mon, 2009-01-19 at 22:26 -0500, Valerie Aurora Henson wrote:
> On Mon, Jan 19, 2009 at 09:24:33PM +0900, Ian Kent wrote:
> > I've pulled in the patches that Val posted and made a couple of
> > minor changes. I also have what I think is just about what we
> > need to reduce the overhead of file map scanning.
> 
> Great, thank you!
> 
> > Check'em out please folks.
> 
> I have a crumpled post-it note on my desk saying "valgrind automount."
> How hard would this be to do while running the Connectathon suite?  It
> would sure make me feel better about these patches.

Oh yeah, I need to get onto cleaning that up and send it over so you can
have a look at it and the license statement. Sorry, I will get to it.

There may be something to be gained from running valgrind against
automount and I do so from time to time. Last time I did got a bunch of
leak records but I've been unable to track them down. You may have more
success.

As far as Connectathon goes that shouldn't be too hard to do but you
might find it's a bit painful.

The way the autofs Connectathon works is you run a setup script after
setting the values you want in the configuration. I found that running
it as root and opening up permissions on the export directory of the two
NFS servers needed and then closing them after works best. After that
the test suite assumes autofs is running so you can run "valgrind
automount -f" (I think that is the only option needed) to run in
foreground. We might need to change the log output code a bit, I'm not
sure.

The real problems start when you hit autofs hard then valgrind becomes
quite challenged. The whole thing gets really slow.

> 
> > I'm a little concerned about the "fix kernel includes" patch because
> > there should be more to it and I've altered it in line with that. I'm
> > not sure if we should post a kernel patch first, pull in the updated
> > kernel header files to the tar and finally make the other couple of
> > changes then. But then I may have it wrong so check it out.
> 
> Yeah, there's a trade-off there.  I think the approach you've taken is
> the most sensible, given that nothing makes me more insane than out of
> sync user and kernel code.

So I think I'll defer that patch for a little while then.

Ian