autofs-5.1.7 - fix offset entries order From: Ian Kent 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 --- CHANGELOG | 1 + include/automount.h | 1 + lib/cache.c | 1 + modules/parse_sun.c | 74 +++++++++++++++++++++++++++++++++++++++++---------- 4 files changed, 62 insertions(+), 15 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 94eb6a2c..7b360f52 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -71,6 +71,7 @@ - dont use AUTOFS_DEV_IOCTL_CLOSEMOUNT. - fix lookup_prune_one_cache() refactoring change. - fix amd hosts mount expire. +- fix offset entries order. 25/01/2021 autofs-5.1.7 - make bind mounts propagation slave by default. diff --git a/include/automount.h b/include/automount.h index 51a0bf0e..d279744d 100644 --- a/include/automount.h +++ b/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; diff --git a/lib/cache.c b/lib/cache.c index ef761739..66dda5d9 100644 --- a/lib/cache.c +++ b/lib/cache.c @@ -559,6 +559,7 @@ int cache_add(struct mapent_cache *mc, struct map_source *ms, const char *key, c 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; diff --git a/modules/parse_sun.c b/modules/parse_sun.c index 03a63290..b206a326 100644 --- a/modules/parse_sun.c +++ b/modules/parse_sun.c @@ -789,14 +789,15 @@ static int check_is_multi(const char *mapent) 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); @@ -862,8 +863,29 @@ update_offset_entry(struct autofs_point *ap, 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) { @@ -1209,6 +1231,25 @@ static char *do_expandsunent(const char *src, const char *key, 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] ... @@ -1228,7 +1269,8 @@ int parse_mount(struct autofs_point *ap, const char *name, 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; @@ -1444,9 +1486,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); @@ -1461,9 +1501,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); @@ -1474,15 +1512,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); @@ -1499,6 +1535,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);