autofs-5.0.3 - make handle_mounts startup condition distinct From: Ian Kent When starting a number of mounts we can get contention for the startup condition used to synchronize the handle_mounts thread completion. This patch makes the condition used distinct for each thread creation. --- CHANGELOG | 1 + daemon/automount.c | 62 ++++++++++++++++++++++++++++++++++++++++++++---- include/automount.h | 4 +++ lib/master.c | 29 +++++++++++----------- modules/mount_autofs.c | 35 +++++++++++++-------------- 5 files changed, 93 insertions(+), 38 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 3921552..9da7be3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -25,6 +25,7 @@ - fix couple of memory leaks. - add command line option to override check for daemon already running. - don't use proc file system when checking if the daemon is running. +- make handle_mounts startup condition distinct. 14/01/2008 autofs-5.0.3 ----------------------- diff --git a/daemon/automount.c b/daemon/automount.c index dbf267c..086affb 100644 --- a/daemon/automount.c +++ b/daemon/automount.c @@ -1461,6 +1461,55 @@ static void mutex_operation_wait(pthread_mutex_t *mutex) return; } +int handle_mounts_startup_cond_init(struct startup_cond *suc) +{ + int status; + + status = pthread_mutex_init(&suc->mutex, NULL); + if (status) + return status; + + status = pthread_cond_init(&suc->cond, NULL); + if (status) { + status = pthread_mutex_destroy(&suc->mutex); + if (status) + fatal(status); + return status; + } + + status = pthread_mutex_lock(&suc->mutex); + if (status) { + status = pthread_mutex_destroy(&suc->mutex); + if (status) + fatal(status); + status = pthread_cond_destroy(&suc->cond); + if (status) + fatal(status); + } + + return 0; +} + +void handle_mounts_startup_cond_destroy(void *arg) +{ + struct startup_cond *suc = (struct startup_cond *) arg; + int status; + + status = pthread_mutex_unlock(&suc->mutex); + if (status) + fatal(status); + + status = pthread_mutex_destroy(&suc->mutex); + if (status) + fatal(status); + + status = pthread_cond_destroy(&suc->cond); + if (status) + fatal(status); + + return; +} + static void handle_mounts_cleanup(void *arg) { struct autofs_point *ap; @@ -1512,17 +1561,20 @@ static void handle_mounts_cleanup(void *arg) void *handle_mounts(void *arg) { + struct startup_cond *suc; struct autofs_point *ap; int cancel_state, status = 0; - ap = (struct autofs_point *) arg; + suc = (struct startup_cond *) arg; + + ap = suc->ap; - pthread_cleanup_push(return_start_status, &suc); + pthread_cleanup_push(return_start_status, suc); pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cancel_state); state_mutex_lock(ap); - status = pthread_mutex_lock(&suc.mutex); + status = pthread_mutex_lock(&suc->mutex); if (status) { logerr("failed to lock startup condition mutex!"); fatal(status); @@ -1530,7 +1582,7 @@ void *handle_mounts(void *arg) if (mount_autofs(ap) < 0) { crit(ap->logopt, "mount of %s failed!", ap->path); - suc.status = 1; + suc->status = 1; state_mutex_unlock(ap); umount_autofs(ap, 1); pthread_setcancelstate(cancel_state, NULL); @@ -1540,7 +1592,7 @@ void *handle_mounts(void *arg) if (ap->ghost && ap->type != LKP_DIRECT) info(ap->logopt, "ghosting enabled"); - suc.status = 0; + suc->status = 0; pthread_cleanup_pop(1); /* We often start several automounters at the same time. Add some diff --git a/include/automount.h b/include/automount.h index da1bf8f..1a20cd9 100644 --- a/include/automount.h +++ b/include/automount.h @@ -331,10 +331,14 @@ int ncat_path(char *buf, size_t len, struct startup_cond { pthread_mutex_t mutex; pthread_cond_t cond; + struct autofs_point *ap; unsigned int done; unsigned int status; }; +int handle_mounts_startup_cond_init(struct startup_cond *suc); +void handle_mounts_startup_cond_destroy(void *arg); + struct master_readmap_cond { pthread_mutex_t mutex; pthread_cond_t cond; diff --git a/lib/master.c b/lib/master.c index 4a34dd4..edd3bdc 100644 --- a/lib/master.c +++ b/lib/master.c @@ -997,28 +997,31 @@ next: static int master_do_mount(struct master_mapent *entry) { + struct startup_cond suc; struct autofs_point *ap; pthread_t thid; int status; - status = pthread_mutex_lock(&suc.mutex); - if (status) - fatal(status); + ap = entry->ap; + + if (handle_mounts_startup_cond_init(&suc)) { + crit(ap->logopt, + "failed to init startup cond for mount %s", entry->path); + return 0; + } + suc.ap = ap; suc.done = 0; suc.status = 0; - ap = entry->ap; - debug(ap->logopt, "mounting %s", entry->path); - if (pthread_create(&thid, &thread_attr, handle_mounts, ap)) { + status = pthread_create(&thid, &thread_attr, handle_mounts, &suc); + if (status) { crit(ap->logopt, "failed to create mount handler thread for %s", entry->path); - status = pthread_mutex_unlock(&suc.mutex); - if (status) - fatal(status); + handle_mounts_startup_cond_destroy(&suc); return 0; } entry->thid = thid; @@ -1031,15 +1034,11 @@ static int master_do_mount(struct master_mapent *entry) if (suc.status) { error(ap->logopt, "failed to startup mount"); - status = pthread_mutex_unlock(&suc.mutex); - if (status) - fatal(status); + handle_mounts_startup_cond_destroy(&suc); return 0; } - status = pthread_mutex_unlock(&suc.mutex); - if (status) - fatal(status); + handle_mounts_startup_cond_destroy(&suc); return 1; } diff --git a/modules/mount_autofs.c b/modules/mount_autofs.c index 356fb14..6f66564 100644 --- a/modules/mount_autofs.c +++ b/modules/mount_autofs.c @@ -46,6 +46,7 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int name_len, const char *what, const char *fstype, const char *c_options, void *context) { + struct startup_cond suc; pthread_t thid; char *fullpath; const char **argv; @@ -210,34 +211,34 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, source->mc = cache_init(entry->ap, source); if (!source->mc) { error(ap->logopt, MODPREFIX "failed to init source cache"); + master_free_map_source(source, 0); master_free_mapent(entry); return 1; } mounts_mutex_lock(ap); - status = pthread_mutex_lock(&suc.mutex); - if (status) { - crit(ap->logopt, - MODPREFIX "failed to lock startup condition mutex!"); - cache_release(source); + if (handle_mounts_startup_cond_init(&suc)) { + crit(ap->logopt, MODPREFIX + "failed to init startup cond for mount %s", entry->path); + mounts_mutex_unlock(ap); + master_free_map_source(source, 1); master_free_mapent(entry); return 1; } + suc.ap = nap; suc.done = 0; suc.status = 0; - if (pthread_create(&thid, NULL, handle_mounts, nap)) { + if (pthread_create(&thid, &thread_attr, handle_mounts, &suc)) { crit(ap->logopt, MODPREFIX "failed to create mount handler thread for %s", fullpath); + handle_mounts_startup_cond_destroy(&suc); mounts_mutex_unlock(ap); - status = pthread_mutex_unlock(&suc.mutex); - if (status) - fatal(status); - cache_release(source); + master_free_map_source(source, 1); master_free_mapent(entry); return 1; } @@ -246,8 +247,10 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, while (!suc.done) { status = pthread_cond_wait(&suc.cond, &suc.mutex); if (status) { + handle_mounts_startup_cond_destroy(&suc); mounts_mutex_unlock(ap); - pthread_mutex_unlock(&suc.mutex); + master_free_map_source(source, 1); + master_free_mapent(entry); fatal(status); } } @@ -255,10 +258,9 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, if (suc.status) { crit(ap->logopt, MODPREFIX "failed to create submount for %s", fullpath); + handle_mounts_startup_cond_destroy(&suc); mounts_mutex_unlock(ap); - status = pthread_mutex_unlock(&suc.mutex); - if (status) - fatal(status); + master_free_map_source(source, 1); master_free_mapent(entry); return 1; } @@ -266,12 +268,9 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, ap->submnt_count++; list_add(&nap->mounts, &ap->submounts); + handle_mounts_startup_cond_destroy(&suc); mounts_mutex_unlock(ap); - status = pthread_mutex_unlock(&suc.mutex); - if (status) - fatal(status); - return 0; }