diff options
author | Zdenek Kabelac <zkabelac@redhat.com> | 2015-10-13 09:43:09 +0200 |
---|---|---|
committer | Zdenek Kabelac <zkabelac@redhat.com> | 2015-10-13 15:55:05 +0200 |
commit | 76ea01dd207ce185891418c3f8ffdcff14bf2672 (patch) | |
tree | 53c15ab55fd062da281e9b2617f4671aa1f2d172 | |
parent | 362558cd66bc08f8a46ababef66f2df21e8bd6fc (diff) | |
download | lvm2-76ea01dd207ce185891418c3f8ffdcff14bf2672.tar.gz |
dmeventd: new initialization of plugin threads
Rework thread creation code to better use resources.
New code will not leak 'timeout' registered thread on error path.
Also if the thread already exist - avoid creation of thread
object and it's later destruction.
If the race is noticed during adding new monitoring thread,
such thread is put on cleanup list and -EEXIST is reported.
-rw-r--r-- | WHATS_NEW_DM | 1 | ||||
-rw-r--r-- | daemons/dmeventd/dmeventd.c | 170 |
2 files changed, 86 insertions, 85 deletions
diff --git a/WHATS_NEW_DM b/WHATS_NEW_DM index efe8b371a..f041ed134 100644 --- a/WHATS_NEW_DM +++ b/WHATS_NEW_DM @@ -1,5 +1,6 @@ Version 1.02.110 - ====================================== + Reworked thread initialization for dmeventd plugins. Dmeventd handles snapshot overflow for now equally as invalid. Convert dmeventd to use common logging macro system from libdm. Return -ENOMEM when device registration fails instead of 0 (=success). diff --git a/daemons/dmeventd/dmeventd.c b/daemons/dmeventd/dmeventd.c index 3e54f797f..ba7d01c83 100644 --- a/daemons/dmeventd/dmeventd.c +++ b/daemons/dmeventd/dmeventd.c @@ -990,91 +990,6 @@ static int _active(struct message_data *message_data) } /* - * Register for an event. - * - * Only one caller at a time here, because we use - * a FIFO and lock it against multiple accesses. - */ -static int _register_for_event(struct message_data *message_data) -{ - int ret = 0; - struct thread_status *thread, *thread_new = NULL; - struct dso_data *dso_data; - - if (!(dso_data = _lookup_dso(message_data)) && - !(dso_data = _load_dso(message_data))) { - stack; -#ifdef ELIBACC - ret = -ELIBACC; -#else - ret = -ENODEV; -#endif - goto out; - } - - /* Preallocate thread status struct to avoid deadlock. */ - if (!(thread_new = _alloc_thread_status(message_data, dso_data))) { - stack; - ret = -ENOMEM; - goto out; - } - - if (!_fill_device_data(thread_new)) { - stack; - ret = -ENODEV; - goto out; - } - - /* If creation of timeout thread fails (as it may), we fail - here completely. The client is responsible for either - retrying later or trying to register without timeout - events. However, if timeout thread cannot be started, it - usually means we are so starved on resources that we are - almost as good as dead already... */ - if ((thread_new->events & DM_EVENT_TIMEOUT) && - (ret = -_register_for_timeout(thread_new))) - goto out; - - _lock_mutex(); - if (!(thread = _lookup_thread_status(message_data))) { - _unlock_mutex(); - - if (!_do_register_device(thread_new)) { - ret = -ENOMEM; - goto_out; - } - - thread = thread_new; - thread_new = NULL; - - /* Try to create the monitoring thread for this device. */ - _lock_mutex(); - if ((ret = -_create_thread(thread))) { - _unlock_mutex(); - _do_unregister_device(thread); - _free_thread_status(thread); - goto out; - } - - LINK_THREAD(thread); - } - - /* Or event # into events bitfield. */ - thread->events |= message_data->events_field; - _unlock_mutex(); - - out: - /* - * Deallocate thread status after releasing - * the lock in case we haven't used it. - */ - if (thread_new) - _free_thread_status(thread_new); - - return ret; -} - -/* * Unregister for an event. * * Only one caller at a time here as with register_for_event(). @@ -1128,6 +1043,91 @@ static int _unregister_for_event(struct message_data *message_data) } /* + * Register for an event. + * + * Only one caller at a time here, because we use + * a FIFO and lock it against multiple accesses. + */ +static int _register_for_event(struct message_data *message_data) +{ + int ret = 0; + struct thread_status *thread; + struct dso_data *dso_data; + + if (!(dso_data = _lookup_dso(message_data)) && + !(dso_data = _load_dso(message_data))) { + stack; +#ifdef ELIBACC + ret = -ELIBACC; +#else + ret = -ENODEV; +#endif + return ret; + } + + _lock_mutex(); + + if ((thread = _lookup_thread_status(message_data))) + /* Or event # into events bitfield. */ + thread->events |= message_data->events_field; + + _unlock_mutex(); + + if (!thread) { + if (!(thread = _alloc_thread_status(message_data, dso_data))) { + stack; + return -ENOMEM; + } + + if (!_fill_device_data(thread)) { + ret = -ENODEV; + goto_out; + } + + if (!_do_register_device(thread)) { + ret = -ENOMEM; + goto_out; + } + + if ((ret = -_create_thread(thread))) { + _do_unregister_device(thread); + goto_out; + } + + _lock_mutex(); + if (_lookup_thread_status(message_data)) { + DEBUGLOG("Race, uuid already registered, marking Thr %x unused.", + (int)thread->thread); + thread->status = DM_THREAD_SHUTDOWN; + thread->events = 0; + LINK(thread, &_thread_registry_unused); + _unlock_mutex(); + ret = -EEXIST; /* race ? */ + goto_out; + } + + LINK_THREAD(thread); + _unlock_mutex(); + } + + /* If creation of timeout thread fails (as it may), we fail + here completely. The client is responsible for either + retrying later or trying to register without timeout + events. However, if timeout thread cannot be started, it + usually means we are so starved on resources that we are + almost as good as dead already... */ + if ((thread->events & DM_EVENT_TIMEOUT) && + (ret = -_register_for_timeout(thread))) + _unregister_for_event(message_data); + + return ret; +out: + _free_thread_status(thread); + + return ret; +} + +/* * Get registered device. * * Only one caller at a time here as with register_for_event(). |