autofs-5.1.7 - fix offset entries order

From: Ian Kent <raven@themaw.net>

While it's rare it's possible that a mapent entry might not have

it's offsets in shortest to longest path order.

If this happens adding an entry to the mapent tree can result in

an incorrect tree topology that doesn't work. That's because adding

tree entries ensures that nodes in a sub-tree are placed below the

containing node so the containing node must be present for that to

work. This topology is critical to the performance of map entries

that have a very large number of offsets such as an NFS server with

many exports.

There's no other choice but make a traversal after the offset entries

have all been added to create the mapent tree.

Signed-off-by: Ian Kent <raven@themaw.net>

---

 CHANGELOG           |    1 

 include/automount.h |    1 

 lib/cache.c         |    1 

 modules/parse_sun.c |   74 +++++++++++++++++++++++++++++++++++++++++-----------

 4 files changed, 62 insertions(+), 15 deletions(-)

--- autofs-5.1.4.orig/CHANGELOG

+++ autofs-5.1.4/CHANGELOG

@@ -68,6 +68,7 @@

 - add ext_mount_hash_mutex lock helpers.

 - fix amd section mounts map reload.

 - fix amd hosts mount expire.

+- fix offset entries order.

 xx/xx/2018 autofs-5.1.5

 - fix flag file permission.

--- autofs-5.1.4.orig/include/automount.h

+++ autofs-5.1.4/include/automount.h

@@ -169,6 +169,7 @@ struct mapent {

 	/* Parent nesting point within multi-mount */

 	struct tree_node *mm_parent;

 	struct tree_node node;

+	struct list_head work;

 	char *key;

 	size_t len;

 	char *mapent;

--- autofs-5.1.4.orig/lib/cache.c

+++ autofs-5.1.4/lib/cache.c

@@ -559,6 +559,7 @@ int cache_add(struct mapent_cache *mc, s

 	me->mm_parent = NULL;

 	INIT_TREE_NODE(&me->node);

 	INIT_LIST_HEAD(&me->ino_index);

+	INIT_LIST_HEAD(&me->work);

 	me->ioctlfd = -1;

 	me->dev = (dev_t) -1;

 	me->ino = (ino_t) -1;

--- autofs-5.1.4.orig/modules/parse_sun.c

+++ autofs-5.1.4/modules/parse_sun.c

@@ -791,14 +791,15 @@ static int check_is_multi(const char *ma

 static int

 update_offset_entry(struct autofs_point *ap,

-		    struct mapent_cache *mc, const char *name,

-		    const char *m_root, int m_root_len,

+		    struct mapent_cache *mc, struct list_head *offsets,

+		    const char *name, const char *m_root, int m_root_len,

 		    const char *m_offset, const char *myoptions,

 		    const char *loc, time_t age)

 {

 	char m_key[PATH_MAX + 1];

 	char m_mapent[MAPENT_MAX_LEN + 1];

 	int o_len, m_key_len, m_options_len, m_mapent_len;

+	struct mapent *me;

 	int ret;

 	memset(m_mapent, 0, MAPENT_MAX_LEN + 1);

@@ -864,8 +865,29 @@ update_offset_entry(struct autofs_point

 	cache_writelock(mc);

 	ret = cache_update_offset(mc, name, m_key, m_mapent, age);

-	if (!tree_mapent_add_node(mc, name, m_key))

-		error(ap->logopt, "failed to add offset %s to tree", m_key);

+	me = cache_lookup_distinct(mc, m_key);

+	if (me && list_empty(&me->work)) {

+		struct list_head *last;

+

+		/* Offset entries really need to be in shortest to

+		 * longest path order. If not and the list of offsets

+		 * is large there will be a performace hit.

+		 */

+		list_for_each_prev(last, offsets) {

+			struct mapent *this;

+

+			this = list_entry(last, struct mapent, work);

+			if (me->len >= this->len) {

+				if (last->next == offsets)

+					list_add_tail(&me->work, offsets);

+				else

+					list_add_tail(&me->work, last);

+				break;

+			}

+		}

+		if (list_empty(&me->work))

+			list_add(&me->work, offsets);

+	}

 	cache_unlock(mc);

 	if (ret == CHE_DUPLICATE) {

@@ -1211,6 +1233,25 @@ static char *do_expandsunent(const char

 	return mapent;

 }

+static void cleanup_offset_entries(struct autofs_point *ap,

+				   struct mapent_cache *mc,

+				   struct list_head *offsets)

+{

+	struct mapent *me, *tmp;

+	int ret;

+

+	if (list_empty(offsets))

+		return;

+	cache_writelock(mc);

+	list_for_each_entry_safe(me, tmp, offsets, work) {

+		list_del(&me->work);

+		ret = cache_delete(mc, me->key);

+		if (ret != CHE_OK)

+			crit(ap->logopt, "failed to delete offset %s", me->key);

+	}

+	cache_unlock(mc);

+}

+

 /*

  * syntax is:

  *	[-options] location [location] ...

@@ -1230,7 +1271,8 @@ int parse_mount(struct autofs_point *ap,

 	char buf[MAX_ERR_BUF];

 	struct map_source *source;

 	struct mapent_cache *mc;

-	struct mapent *me;

+	struct mapent *me, *oe, *tmp;

+	LIST_HEAD(offsets);

 	char *pmapent, *options;

 	const char *p;

 	int mapent_len, rv = 0;

@@ -1446,9 +1488,7 @@ dont_expand:

 			if (!m_offset) {

 				warn(ap->logopt, MODPREFIX "null path or out of memory");

-				cache_writelock(mc);

-				tree_mapent_delete_offsets(mc, name);

-				cache_unlock(mc);

+				cleanup_offset_entries(ap, mc, &offsets);

 				free(options);

 				free(pmapent);

 				pthread_setcancelstate(cur_state, NULL);

@@ -1463,9 +1503,7 @@ dont_expand:

 			l = parse_mapent(p, options, &myoptions, &loc, ap->logopt);

 			if (!l) {

-				cache_writelock(mc);

-				tree_mapent_delete_offsets(mc, name);

-				cache_unlock(mc);

+				cleanup_offset_entries(ap, mc, &offsets);

 				free(m_offset);

 				free(options);

 				free(pmapent);

@@ -1476,15 +1514,13 @@ dont_expand:

 			p += l;

 			p = skipspace(p);

-			status = update_offset_entry(ap, mc,

+			status = update_offset_entry(ap, mc, &offsets,

 						     name, m_root, m_root_len,

 						     m_offset, myoptions, loc, age);

 			if (status != CHE_OK) {

 				warn(ap->logopt, MODPREFIX "error adding multi-mount");

-				cache_writelock(mc);

-				tree_mapent_delete_offsets(mc, name);

-				cache_unlock(mc);

+				cleanup_offset_entries(ap, mc, &offsets);

 				free(m_offset);

 				free(options);

 				free(pmapent);

@@ -1501,6 +1537,14 @@ dont_expand:

 			free(myoptions);

 		} while (*p == '/' || (*p == '"' && *(p + 1) == '/'));

+		cache_writelock(mc);

+		list_for_each_entry_safe(oe, tmp, &offsets, work) {

+			if (!tree_mapent_add_node(mc, name, oe->key))

+				error(ap->logopt, "failed to add offset %s to tree", oe->key);

+			list_del_init(&oe->work);

+		}

+		cache_unlock(mc);

+

 		rv = mount_subtree(ap, mc, name, NULL, options, ctxt);

 		free(options);