Attached is previous patch, split to 4 parts and better tested. Regarding Chrissie comments: - about ABI compatibility. I'm pretty sure, that in initialization, is OK to have item anywhere. Definition is of course augmented on the end. - schedwrk_create_lock - This was really bad name. I hope schedwrk_internal_create is better.
Regards, Honza
>From e9cb38c27919ed6c8d02286bfa4cca9becf290e4 Mon Sep 17 00:00:00 2001 From: Jan Friesse <[email protected]> Date: Wed, 20 Jan 2010 14:53:54 +0100 Subject: [PATCH 1/4] poll_dispatch_delete - ability to return 0 Patch fixes poll_dispatch_delete, so it is able to return 0 (success), when requested FD was found. --- trunk/exec/coropoll.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/trunk/exec/coropoll.c b/trunk/exec/coropoll.c index 3c8df8d..8f18f0f 100644 --- a/trunk/exec/coropoll.c +++ b/trunk/exec/coropoll.c @@ -260,6 +260,8 @@ int poll_dispatch_delete ( poll_instance->ufds[i].fd = -1; poll_instance->poll_entries[i].ufd.fd = -1; poll_instance->poll_entries[i].ufd.revents = 0; + + res = 0; break; } } -- 1.6.2.5
>From 9237249db7a01f773821675ff65c4fe645d0dcd4 Mon Sep 17 00:00:00 2001 From: Jan Friesse <[email protected]> Date: Wed, 20 Jan 2010 15:02:26 +0100 Subject: [PATCH 2/4] Pass correct poll handle to poll_stop on exit --- trunk/exec/main.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/trunk/exec/main.c b/trunk/exec/main.c index b2b84d2..0aab772 100644 --- a/trunk/exec/main.c +++ b/trunk/exec/main.c @@ -156,7 +156,7 @@ void corosync_state_dump (void) static void unlink_all_completed (void) { - poll_stop (0); + poll_stop (corosync_poll_handle); coroipcs_ipc_exit (); totempg_finalize (); -- 1.6.2.5
>From 5f375059417f1c5eae47b2ebf37e510dcda17fa1 Mon Sep 17 00:00:00 2001 From: Jan Friesse <[email protected]> Date: Wed, 20 Jan 2010 15:18:00 +0100 Subject: [PATCH 3/4] Add schedwrk_create_nolock function This patch adds schedwrk_create_nolock, which will not call serialize_lock before execution of callback. --- trunk/exec/apidef.c | 1 + trunk/exec/schedwrk.c | 31 +++++++++++++++++++++++++++---- trunk/exec/schedwrk.h | 5 +++++ trunk/include/corosync/engine/coroapi.h | 5 +++++ 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/trunk/exec/apidef.c b/trunk/exec/apidef.c index 5e1f178..adb2c88 100644 --- a/trunk/exec/apidef.c +++ b/trunk/exec/apidef.c @@ -129,6 +129,7 @@ static struct corosync_api_v1 apidef_corosync_api_v1 = { .tpg_groups_reserve = NULL, .tpg_groups_release = NULL, .schedwrk_create = schedwrk_create, + .schedwrk_create_nolock = schedwrk_create_nolock, .schedwrk_destroy = schedwrk_destroy, .sync_request = NULL, //sync_request, .quorum_is_quorate = corosync_quorum_is_quorate, diff --git a/trunk/exec/schedwrk.c b/trunk/exec/schedwrk.c index 0c330e9..0591063 100644 --- a/trunk/exec/schedwrk.c +++ b/trunk/exec/schedwrk.c @@ -46,6 +46,7 @@ struct schedwrk_instance { int (*schedwrk_fn) (const void *); const void *context; void *callback_handle; + int lock; }; union u { @@ -70,9 +71,13 @@ static int schedwrk_do (enum totem_callback_token_type type, const void *context goto error_exit; } - serialize_lock (); + if (instance->lock) + serialize_lock (); + res = instance->schedwrk_fn (instance->context); - serialize_unlock (); + + if (instance->lock) + serialize_unlock (); if (res == 0) { hdb_handle_destroy (&schedwrk_instance_database, hdb_nocheck_convert (handle)); @@ -93,10 +98,11 @@ void schedwrk_init ( serialize_unlock = serialize_unlock_fn; } -int schedwrk_create ( +static int schedwrk_internal_create ( hdb_handle_t *handle, int (schedwrk_fn) (const void *), - const void *context) + const void *context, + int lock) { struct schedwrk_instance *instance; int res; @@ -121,6 +127,7 @@ int schedwrk_create ( instance->schedwrk_fn = schedwrk_fn; instance->context = context; + instance->lock = lock; hdb_handle_put (&schedwrk_instance_database, *handle); @@ -133,6 +140,22 @@ error_exit: return (-1); } +int schedwrk_create ( + hdb_handle_t *handle, + int (schedwrk_fn) (const void *), + const void *context) +{ + return schedwrk_internal_create (handle, schedwrk_fn, context, 1); +} + +int schedwrk_create_nolock ( + hdb_handle_t *handle, + int (schedwrk_fn) (const void *), + const void *context) +{ + return schedwrk_internal_create (handle, schedwrk_fn, context, 0); +} + void schedwrk_destroy (hdb_handle_t handle) { hdb_handle_destroy (&schedwrk_instance_database, handle); diff --git a/trunk/exec/schedwrk.h b/trunk/exec/schedwrk.h index e67215b..29d6409 100644 --- a/trunk/exec/schedwrk.h +++ b/trunk/exec/schedwrk.h @@ -43,6 +43,11 @@ int schedwrk_create ( int (schedwrk_fn) (const void *), const void *context); +int schedwrk_create_nolock ( + hdb_handle_t *handle, + int (schedwrk_fn) (const void *), + const void *context); + void schedwrk_destroy (hdb_handle_t handle); #endif /* SCHEDWRK_H_DEFINED */ diff --git a/trunk/include/corosync/engine/coroapi.h b/trunk/include/corosync/engine/coroapi.h index 0c487ca..556a568 100644 --- a/trunk/include/corosync/engine/coroapi.h +++ b/trunk/include/corosync/engine/coroapi.h @@ -627,6 +627,11 @@ struct corosync_api_v1 { objdb_value_types_t *type); void *(*totem_get_stats)(void); + + int (*schedwrk_create_nolock) ( + hdb_handle_t *handle, + int (schedwrk_fn) (const void *), + const void *context); }; #define SERVICE_ID_MAKE(a,b) ( ((a)<<16) | (b) ) -- 1.6.2.5
>From 08ddd5d5aa5dd447491347b4493d6d237ad9b230 Mon Sep 17 00:00:00 2001 From: Jan Friesse <[email protected]> Date: Wed, 20 Jan 2010 17:46:35 +0100 Subject: [PATCH 4/4] Fix segfaults of corosync on exit --- trunk/exec/coroipcs.c | 36 ++++--------- trunk/exec/main.c | 3 +- trunk/exec/service.c | 108 ++++++++++++++++++++++++++++++------- trunk/exec/service.h | 5 +- trunk/include/corosync/coroipcs.h | 2 +- 5 files changed, 104 insertions(+), 50 deletions(-) diff --git a/trunk/exec/coroipcs.c b/trunk/exec/coroipcs.c index a0ef133..9f63c69 100644 --- a/trunk/exec/coroipcs.c +++ b/trunk/exec/coroipcs.c @@ -75,6 +75,7 @@ #include <corosync/hdb.h> #include <corosync/coroipcs.h> #include <corosync/coroipc_ipc.h> +#include <corosync/totem/coropoll.h> #define LOGSYS_UTILS_ONLY 1 #include <corosync/engine/logsys.h> @@ -1043,41 +1044,26 @@ static void _corosync_ipc_init(void) api->poll_accept_add (server_fd); } -void coroipcs_ipc_exit (void) +void coroipcs_ipc_service_exit (unsigned int service, hdb_handle_t poll_handle) { - struct list_head *list; + struct list_head *list, *list_next; struct conn_info *conn_info; - unsigned int res; + //unsigned int res; for (list = conn_info_list_head.next; list != &conn_info_list_head; - list = list->next) { + list = list_next) { + + list_next = list->next; conn_info = list_entry (list, struct conn_info, list); - if (conn_info->state != CONN_STATE_THREAD_ACTIVE) + if (conn_info->service != service || conn_info->state != CONN_STATE_THREAD_ACTIVE) continue; ipc_disconnect (conn_info); - -#if _POSIX_THREAD_PROCESS_SHARED > 0 - sem_destroy (&conn_info->control_buffer->sem0); - sem_destroy (&conn_info->control_buffer->sem1); - sem_destroy (&conn_info->control_buffer->sem2); -#else - semctl (conn_info->semid, 0, IPC_RMID); -#endif - - /* - * Unmap memory segments - */ - res = munmap ((void *)conn_info->control_buffer, - conn_info->control_size); - res = munmap ((void *)conn_info->request_buffer, - conn_info->request_size); - res = munmap ((void *)conn_info->response_buffer, - conn_info->response_size); - res = circular_memory_unmap (conn_info->dispatch_buffer, - conn_info->dispatch_size); + poll_dispatch_delete (poll_handle, conn_info->fd); + while (conn_info_destroy (conn_info) != -1) + ; } } diff --git a/trunk/exec/main.c b/trunk/exec/main.c index 0aab772..0944b00 100644 --- a/trunk/exec/main.c +++ b/trunk/exec/main.c @@ -157,7 +157,6 @@ void corosync_state_dump (void) static void unlink_all_completed (void) { poll_stop (corosync_poll_handle); - coroipcs_ipc_exit (); totempg_finalize (); corosync_exit_error (AIS_DONE_EXIT); @@ -180,7 +179,7 @@ static void *corosync_exit_thread_handler (void *arg) { sem_wait (&corosync_exit_sem); - corosync_service_unlink_all (api, unlink_all_completed); + corosync_service_unlink_all (api, unlink_all_completed, corosync_poll_handle); return arg; } diff --git a/trunk/exec/service.c b/trunk/exec/service.c index d99e7bf..eb352da 100644 --- a/trunk/exec/service.c +++ b/trunk/exec/service.c @@ -55,6 +55,8 @@ #include <corosync/engine/coroapi.h> #include "service.h" +#include <corosync/coroipcs.h> + LOGSYS_DECLARE_SUBSYS ("SERV"); struct default_service { @@ -89,6 +91,16 @@ static struct default_service default_services[] = { } }; +/* + * service exit and unlink handler data structure + */ +struct seus_handler_data { + hdb_handle_t service_handle; + int service_engine; + hdb_handle_t coropoll_handle; + struct corosync_api_v1 *api; +}; + struct corosync_service_engine *ais_service[SERVICE_HANDLER_MAXIMUM_COUNT]; hdb_handle_t service_stats_handle[SERVICE_HANDLER_MAXIMUM_COUNT][64]; @@ -99,8 +111,8 @@ static hdb_handle_t object_stats_services_handle; static void (*service_unlink_all_complete) (void) = NULL; -static hdb_handle_t unlink_all_handle; - +static hdb_handle_t swrk_service_exit_handle; +static hdb_handle_t swrk_service_unlink_handle; static unsigned int default_services_requested (struct corosync_api_v1 *corosync_api) { @@ -272,7 +284,8 @@ corosync_service_unlink_priority ( struct corosync_api_v1 *corosync_api, int lowest_priority, int *current_priority, - int *current_service_engine) + int *current_service_engine, + hdb_handle_t *current_service_handle) { unsigned short *service_id; hdb_handle_t object_service_handle; @@ -319,11 +332,6 @@ corosync_service_unlink_priority ( return (-1); } } - log_printf(LOGSYS_LEVEL_NOTICE, - "Service engine unloaded: %s\n", - ais_service[*current_service_engine]->name); - - ais_service[*current_service_engine] = NULL; res = corosync_api->object_key_get ( object_service_handle, @@ -331,17 +339,24 @@ corosync_service_unlink_priority ( (void *)&found_service_handle, NULL); - lcr_ifact_release (*found_service_handle); + *current_service_handle = *found_service_handle; + + corosync_api->object_find_destroy (object_find_handle); - corosync_api->object_destroy (object_service_handle); - break; + /* + * Call should call this function again + */ + return (1); } } corosync_api->object_find_destroy (object_find_handle); } } - return 0; + /* + * We finish unlink of all services -> no need to call this function again + */ + return (0); } static unsigned int service_unlink_and_exit ( @@ -538,12 +553,45 @@ unsigned int corosync_service_defaults_link_and_init (struct corosync_api_v1 *co return (0); } -static int unlink_all_schedwrk_handler (const void *data) { +/* + * Declaration of exit_schedwrk_handler, because of cycle + * (service_exit_schedwrk_handler calls service_unlink_schedwrk_handler, and vice-versa) + */ +static int service_exit_schedwrk_handler (const void *data); + +static int service_unlink_schedwrk_handler (const void *data) { + struct seus_handler_data *cb_data = (struct seus_handler_data *)data; + struct corosync_api_v1 *api = (struct corosync_api_v1 *)cb_data->api; + + /* + * Exit all ipc connections dependand on this service + */ + coroipcs_ipc_service_exit (cb_data->service_engine, cb_data->coropoll_handle); + + log_printf(LOGSYS_LEVEL_NOTICE, + "Service engine unloaded: %s\n", + ais_service[cb_data->service_engine]->name); + + ais_service[cb_data->service_engine] = NULL; + + lcr_ifact_release (cb_data->service_handle); + + api->schedwrk_create ( + &swrk_service_exit_handle, + &service_exit_schedwrk_handler, + data); + + return 0; +} + +static int service_exit_schedwrk_handler (const void *data) { int res; static int current_priority = 0; static int current_service_engine = 0; static int called = 0; - struct corosync_api_v1 *api = (struct corosync_api_v1 *)data; + struct seus_handler_data *cb_data = (struct seus_handler_data *)data; + struct corosync_api_v1 *api = (struct corosync_api_v1 *)cb_data->api; + hdb_handle_t service_handle; if (called == 0) { log_printf(LOGSYS_LEVEL_NOTICE, @@ -556,18 +604,35 @@ static int unlink_all_schedwrk_handler (const void *data) { api, 0, ¤t_priority, - ¤t_service_engine); + ¤t_service_engine, + &service_handle); if (res == 0) { service_unlink_all_complete(); + return (res); + } + + if (res == 1) { + cb_data->service_engine = current_service_engine; + cb_data->service_handle = service_handle; + + api->schedwrk_create_nolock ( + &swrk_service_unlink_handle, + &service_unlink_schedwrk_handler, + data); + + return (0); } + return (res); } void corosync_service_unlink_all ( struct corosync_api_v1 *api, - void (*unlink_all_complete) (void)) + void (*unlink_all_complete) (void), + hdb_handle_t poll_handle) { static int called = 0; + static struct seus_handler_data cb_data; assert (api); @@ -579,11 +644,14 @@ void corosync_service_unlink_all ( if (called == 0) { called = 1; } - + + cb_data.api = api; + cb_data.coropoll_handle = poll_handle; + api->schedwrk_create ( - &unlink_all_handle, - &unlink_all_schedwrk_handler, - api); + &swrk_service_exit_handle, + &service_exit_schedwrk_handler, + &cb_data); } struct service_unlink_and_exit_data { diff --git a/trunk/exec/service.h b/trunk/exec/service.h index efb3e44..f94889c 100644 --- a/trunk/exec/service.h +++ b/trunk/exec/service.h @@ -58,8 +58,9 @@ extern unsigned int corosync_service_unlink_and_exit ( * Unlink and exit all corosync services */ extern void corosync_service_unlink_all ( - struct corosync_api_v1 *api, - void (*unlink_all_complete) (void)); + struct corosync_api_v1 *api, + void (*unlink_all_complete) (void), + hdb_handle_t poll_handle); /* * Load all of the default services diff --git a/trunk/include/corosync/coroipcs.h b/trunk/include/corosync/coroipcs.h index 3838af8..614037e 100644 --- a/trunk/include/corosync/coroipcs.h +++ b/trunk/include/corosync/coroipcs.h @@ -150,7 +150,7 @@ extern void coroipcs_refcount_inc (void *conn); extern void coroipcs_refcount_dec (void *conn); -extern void coroipcs_ipc_exit (void); +extern void coroipcs_ipc_service_exit (unsigned int service, hdb_handle_t poll_handle); extern int coroipcs_handler_accept ( int fd, -- 1.6.2.5
_______________________________________________ Openais mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/openais
