Patch 1/2 looks good Patch 2/2 looks good please merge 1/2
before merging 2/2, have andrew give a quick sanity check to 2/2. Regards -steve On Thu, 2010-01-21 at 17:13 +0100, Jan Friesse wrote: > 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. > ya > - 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. just how it ended up :) > - seus is Service Exit and Unlink. Better name proposals are highly > welcomed. > > Regards, > Honza > plain text document attachment > (0001-Add-schedwrk_create_nolock-function.patch) > 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) ) > plain text document attachment > (0002-Fix-segfaults-of-corosync-on-exit.patch) > 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, _______________________________________________ Openais mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/openais
