On 19/01/10 17:15, Jan Friesse wrote:
> Attached is another IPC patch (apply to current trunk). It need some
> cleanups, variable names modification (comments welcomed), little more
> testing (looks very stable) and split to 2-3 patches, but finally should
> solve all issues and cyclic requirements.
>
Some comments inline
> 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,
Please don't add API calls in the middle of the structure. It breaks the
ABI and causes packaging regressions. Any new calls MUST go at the end,
regardless of how messy you think it might look.
> 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 b2b84d2..0944b00 100644
> --- a/trunk/exec/main.c
> +++ b/trunk/exec/main.c
> @@ -156,8 +156,7 @@ void corosync_state_dump (void)
>
> static void unlink_all_completed (void)
> {
> - poll_stop (0);
> - coroipcs_ipc_exit ();
> + poll_stop (corosync_poll_handle);
> 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/mainconfig.c b/trunk/exec/mainconfig.c
> index c35ba72..99dc952 100644
> --- a/trunk/exec/mainconfig.c
> +++ b/trunk/exec/mainconfig.c
> @@ -277,6 +277,8 @@ static int corosync_main_config_log_destination_set (
> objdb_key, replacement);
> }
>
> + mode = logsys_config_mode_get (subsys);
> +
> if (strcmp (value, "yes") == 0) {
> mode |= mode_mask;
> if (logsys_config_mode_set(subsys, mode) < 0) {
> diff --git a/trunk/exec/schedwrk.c b/trunk/exec/schedwrk.c
> index 0c330e9..4cfa38f 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_create_lock (
> hdb_handle_t *handle,
> int (schedwrk_fn) (const void *),
> - const void *context)
> + const void *context,
> + int lock)
I think having a function called 'schedwrk_create_lock' where it's an
option whether it locks or not is confusing. I think that _lock should
be removed from the name.
> {
> 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_create_lock (handle, schedwrk_fn, context, 1);
> +}
> +
> +int schedwrk_create_nolock (
> + hdb_handle_t *handle,
> + int (schedwrk_fn) (const void *),
> + const void *context)
> +{
> + return schedwrk_create_lock (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/exec/service.c b/trunk/exec/service.c
> index d99e7bf..12b4034 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 {
> @@ -99,8 +101,11 @@ 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 hdb_handle_t unlink_service_handle;
> +static int unlink_service_engine;
> +static hdb_handle_t coropoll_handle;
>
> static unsigned int default_services_requested (struct
corosync_api_v1 *corosync_api)
> {
> @@ -272,7 +277,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 +325,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 +332,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_destroy
> (object_service_handle);
> - break;
> + corosync_api->object_find_destroy
> (object_find_handle);
> +
> + /*
> + * Call this again
> + */
> + return (1);
> }
> }
>
> corosync_api->object_find_destroy (object_find_handle);
> }
> }
> - return 0;
> + /*
> + * We finish unlink of all services
> + */
> + return (0);
> }
>
> static unsigned int service_unlink_and_exit (
> @@ -431,6 +439,11 @@ static unsigned int service_unlink_and_exit (
> }
> }
>
> + /*
> + * Exit all ipc connections dependand on this service
> + */
> + coroipcs_ipc_service_exit (*service_id,
> coropoll_handle);
> +
> log_printf(LOGSYS_LEVEL_NOTICE,
> "Service engine unloaded: %s\n",
> ais_service[*service_id]->name);
> @@ -538,12 +551,39 @@ unsigned int
corosync_service_defaults_link_and_init (struct corosync_api_v1 *co
> return (0);
> }
>
> -static int unlink_all_schedwrk_handler (const void *data) {
> +static int service_exit_schedwrk_handler (const void *data);
> +
> +static int service_unlink_schedwrk_handler (const void *data) {
> + struct corosync_api_v1 *api = (struct corosync_api_v1 *)data;
> +
> + /*
> + * Exit all ipc connections dependand on this service
> + */
> + coroipcs_ipc_service_exit (unlink_service_engine, coropoll_handle);
> +
> + log_printf(LOGSYS_LEVEL_NOTICE,
> + "Service engine unloaded: %s\n",
> + ais_service[unlink_service_engine]->name);
> +
> + ais_service[unlink_service_engine] = NULL;
> +
> + lcr_ifact_release (unlink_service_handle);
> +
> + api->schedwrk_create (
> + &swrk_service_exit_handle,
> + &service_exit_schedwrk_handler,
> + api);
> +
> + 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;
> + hdb_handle_t service_handle;
>
> if (called == 0) {
> log_printf(LOGSYS_LEVEL_NOTICE,
> @@ -556,16 +596,31 @@ 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) {
> + unlink_service_engine = current_service_engine;
> + unlink_service_handle = service_handle;
> +
> + api->schedwrk_create_nolock (
> + &swrk_service_unlink_handle,
> + &service_unlink_schedwrk_handler,
> + api);
> + 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;
>
> @@ -579,10 +634,12 @@ void corosync_service_unlink_all (
> if (called == 0) {
> called = 1;
> }
> -
> +
> + coropoll_handle = poll_handle;
> +
> api->schedwrk_create (
> - &unlink_all_handle,
> - &unlink_all_schedwrk_handler,
> + &swrk_service_exit_handle,
> + &service_exit_schedwrk_handler,
> api);
> }
>
> diff --git a/trunk/exec/service.h b/trunk/exec/service.h
> index efb3e44..1228bd0 100644
> --- a/trunk/exec/service.h
> +++ b/trunk/exec/service.h
> @@ -59,7 +59,8 @@ extern unsigned int corosync_service_unlink_and_exit (
> */
> extern 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);
>
> /*
> * 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,
> 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) )
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
