Patches 1/4 and 2/4 are committed as 2647 and 2648. Attached are fixed patches.
- extern comment - I was just duplicating style in that file (sadly all functions was without extern). So attached patch makes all functions in schedwrk.h extern. - coropoll handle should be fixed now (one thing I really don't understand is, why coroipcs_init_state_v2 have .poll_dispatch_destroy (function with such name doesn't exist in coropoll.c) and no poll_dispatch_delete? Compatibility? Anyway, corosync_poll_dispatch_destroy just calls poll_dispatch_delete. - seus is Service Exit and Unlink. Better name proposals are highly welcomed. Regards, Honza
>From b4a99eb2675363497d70a0908761389fdd057059 Mon Sep 17 00:00:00 2001 From: Jan Friesse <[email protected]> Date: Thu, 21 Jan 2010 13:55:27 +0100 Subject: [PATCH 1/2] 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 | 11 ++++++++--- trunk/include/corosync/engine/coroapi.h | 5 +++++ 4 files changed, 41 insertions(+), 7 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..68ce60d 100644 --- a/trunk/exec/schedwrk.h +++ b/trunk/exec/schedwrk.h @@ -34,15 +34,20 @@ #ifndef SCHEDWRK_H_DEFINED #define SCHEDWRK_H_DEFINED -void schedwrk_init ( +extern void schedwrk_init ( void (*serialize_lock_fn) (void), void (*serialize_unlock_fn) (void)); -int schedwrk_create ( +extern int schedwrk_create ( hdb_handle_t *handle, int (schedwrk_fn) (const void *), const void *context); -void schedwrk_destroy (hdb_handle_t handle); +extern int schedwrk_create_nolock ( + hdb_handle_t *handle, + int (schedwrk_fn) (const void *), + const void *context); + +extern 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 601eca29083d84e4d10759dc67177eca3edff5f5 Mon Sep 17 00:00:00 2001 From: Jan Friesse <[email protected]> Date: Thu, 21 Jan 2010 14:17:21 +0100 Subject: [PATCH 2/2] Fix segfaults of corosync on exit --- trunk/exec/coroipcs.c | 34 +++--------- trunk/exec/main.c | 8 +++- trunk/exec/service.c | 103 ++++++++++++++++++++++++++++++------- trunk/exec/service.h | 4 +- trunk/include/corosync/coroipcs.h | 2 +- 5 files changed, 103 insertions(+), 48 deletions(-) diff --git a/trunk/exec/coroipcs.c b/trunk/exec/coroipcs.c index a0ef133..2ab083b 100644 --- a/trunk/exec/coroipcs.c +++ b/trunk/exec/coroipcs.c @@ -1043,41 +1043,25 @@ 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) { - struct list_head *list; + struct list_head *list, *list_next; struct conn_info *conn_info; - 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); + api->poll_dispatch_destroy (conn_info->fd, NULL); + while (conn_info_destroy (conn_info) != -1) + ; } } diff --git a/trunk/exec/main.c b/trunk/exec/main.c index 0aab772..4a96dff 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); @@ -949,6 +948,12 @@ static void corosync_poll_dispatch_modify ( corosync_poll_handler_dispatch); } +static void corosync_poll_dispatch_destroy ( + int fd, + void *context) +{ + poll_dispatch_delete (corosync_poll_handle, fd); +} static hdb_handle_t corosync_stats_create_connection (const char* name, const pid_t pid, const int fd) @@ -1086,6 +1091,7 @@ static struct coroipcs_init_state_v2 ipc_init_state_v2 = { .poll_accept_add = corosync_poll_accept_add, .poll_dispatch_add = corosync_poll_dispatch_add, .poll_dispatch_modify = corosync_poll_dispatch_modify, + .poll_dispatch_destroy = corosync_poll_dispatch_destroy, .init_fn_get = corosync_init_fn_get, .exit_fn_get = corosync_exit_fn_get, .handler_fn_get = corosync_handler_fn_get, diff --git a/trunk/exec/service.c b/trunk/exec/service.c index d99e7bf..b05af15 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,15 @@ static struct default_service default_services[] = { } }; +/* + * service exit and unlink schedwrk handler data structure + */ +struct seus_handler_data { + hdb_handle_t service_handle; + int service_engine; + 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 +110,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 +283,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 +331,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 +338,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 +552,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 dependent on this service + */ + coroipcs_ipc_service_exit (cb_data->service_engine); + + 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,10 +603,25 @@ 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); } @@ -568,6 +630,7 @@ void corosync_service_unlink_all ( void (*unlink_all_complete) (void)) { static int called = 0; + static struct seus_handler_data cb_data; assert (api); @@ -579,11 +642,13 @@ void corosync_service_unlink_all ( if (called == 0) { called = 1; } - + + cb_data.api = api; + 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..2f8b9a0 100644 --- a/trunk/exec/service.h +++ b/trunk/exec/service.h @@ -58,8 +58,8 @@ 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)); /* * Load all of the default services diff --git a/trunk/include/corosync/coroipcs.h b/trunk/include/corosync/coroipcs.h index 3838af8..76fd674 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); extern int coroipcs_handler_accept ( int fd, -- 1.6.2.5
_______________________________________________ Openais mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/openais
