On Fri, 2009-08-07 at 13:20 +0200, Fabio M. Di Nitto wrote:
> logsys_thread_priority_set expects that wthread is already running
> but doesn't perform any check if it is.
> 
> if logsys_thread_priority_set is invoked before forking the thread,
> a segfault is guaranteed.
> 
> this changes to logsys_thread_priority_set ensure that a request
> for a new scheduler priority is queued if the wthread is not
> created yet and the new scheduler parameters are applied as early
> as possible after the thread is created.
> 
> At the same time:
> 
> - add a few comments on clean up that needs to be done
> - remove a redundant check for policy != SCHED_OTHER as that's
>   corosync need specific. logsys should simply accept the request
>   and apply it.
> 
comment on this inline

> Signed-off-by: Fabio M. Di Nitto <[email protected]>
> ---
> :100644 100644 faa1bd2... a54385f... M        exec/logsys.c
>  exec/logsys.c |   40 +++++++++++++++++++++++++++++++++-------
>  1 files changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/exec/logsys.c b/exec/logsys.c
> index faa1bd2..a54385f 100644
> --- a/exec/logsys.c
> +++ b/exec/logsys.c
> @@ -131,6 +131,10 @@ unsigned int flt_data_size;
>  
>  static int logsys_system_needs_init = LOGSYS_LOGGER_NEEDS_INIT;
>  
> +static int logsys_sched_param_queued = 0;
> +static int logsys_sched_policy;
> +static struct sched_param logsys_sched_param;
> +
>  static int logsys_after_log_ops_yield = 10;
>  
>  /*
> @@ -783,14 +787,31 @@ static void wthread_create (void)
>       pthread_mutex_init (&logsys_cond_mutex, NULL);
>       pthread_cond_init (&logsys_cond, NULL);
>       pthread_mutex_lock (&logsys_cond_mutex);
> -     res = pthread_create (&logsys_thread_id, NULL,
> -             logsys_worker_thread, NULL);
> -
>  
>       /*
> -      * Wait for thread to be started
> +      * TODO: propagate pthread_create errors back to the caller
>        */
> -     wthread_wait_locked ();
> +     res = pthread_create (&logsys_thread_id, NULL,
> +             logsys_worker_thread, NULL);
> +
> +     if (res == 0) {
> +             /*
> +              * Wait for thread to be started
> +              */
> +             wthread_wait_locked ();
> +             if (logsys_sched_param_queued == 1) {
> +                     /*
> +                      * TODO: propagate logsys_thread_priority_set errors 
> back to
> +                      * the caller
> +                      */
> +                     res = logsys_thread_priority_set (logsys_sched_policy,
> +                                                 &logsys_sched_param,
> +                                                 logsys_after_log_ops_yield);
> +                     logsys_sched_param_queued = 0;
> +             }
> +     } else {
> +             wthread_active = 0;
> +     }
>  }
>  
>  static int _logsys_config_subsys_get_unlocked (const char *subsys)
> @@ -1610,15 +1631,20 @@ int logsys_thread_priority_set (
>       int res = 0;
>  
>  #if defined(HAVE_PTHREAD_SETSCHEDPARAM) && 
> defined(HAVE_SCHED_GET_PRIORITY_MAX)
> -     if (policy != SCHED_OTHER) {
I believe the reason this check is here, is that if SCHED_OTHER is
specified, sched param should be null in the call to
pthread_setschedparam.  The default for a new thread is SCHED_OTHER so
there is no need to set the parameter.

Before committing, I'd like you to verify that specifying SCHED_OTHER
works properly.

> +     if (wthread_active == 0) {
> +             logsys_sched_policy = policy;
> +             memcpy(&logsys_sched_param, &param, sizeof(struct sched_param));
> +             logsys_sched_param_queued = 1;
> +     } else {
>               res = pthread_setschedparam (logsys_thread_id, policy, param);
>       }
>  #endif
> +
>       if (after_log_ops_yield > 0) {
>               logsys_after_log_ops_yield = after_log_ops_yield;
>       }
> -     return (res);
>  
> +     return (res);
>  }
>  
>  int logsys_log_rec_store (const char *filename)

_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais

Reply via email to