Actually,
this was first question I've asked myself ;) So I've made test app:

#include <stdio.h>
#include <sys/types.h>
#include <unistd.h>

int main(void)
{
        int i;
        int c = 0;

        for (i = 0; i < 100000000;i++) {
//      c = c + getpid();
//      c = c + rand();
        }
        printf("%u\n", c);
        return 0;
}

And if I uncomment c = c + getpid line, runtime is (VM on ~3 years old
quite low-end Xeon, compiled with -O2) ->

real    0m0.515s
user    0m0.511s
sys     0m0.004s

For c = c + rand(), it's:
real    0m1.436s
user    0m1.432s
sys     0m0.004s

In other words, it's for sure not 100% free, but 100000000 in 0.5s (->
one getpid takes ~ .00000000515000000746 sec) is probably ok.

Honza

Fabio M. Di Nitto napsal(a):
> The patch looks good, but I have one simple question... isn't expensive
> to call getpid() on each log_rec?
> 
> Cheers
> Fabio
> 
> On 03/25/2013 04:13 PM, Jan Friesse wrote:
>> When serivce plugin calls fork and child uses logsys, it may lead to
>> corrupted fdata (idx is updated but it's not shared between child and
>> parent, but fdata is mmaped file and this is shared).
>>
>> Solutions is to:
>> - Store corosync pid
>> - On log, check if current pid == corosync pid
>> - If so -> store in fdata, if not -> don't store
>>
>> Signed-off-by: Jan Friesse <[email protected]>
>> ---
>>  exec/logsys.c |   28 ++++++++++++++++++----------
>>  1 files changed, 18 insertions(+), 10 deletions(-)
>>
>> diff --git a/exec/logsys.c b/exec/logsys.c
>> index c4611bc..cd6a311 100644
>> --- a/exec/logsys.c
>> +++ b/exec/logsys.c
>> @@ -205,6 +205,8 @@ static int logsys_dropped_messages = 0;
>>  
>>  void *logsys_rec_end;
>>  
>> +static pid_t startup_pid = 0;
>> +
>>  static DECLARE_LIST_INIT(logsys_print_finished_records);
>>  
>>  #define FDMAX_ARGS  64
>> @@ -1242,16 +1244,20 @@ void _logsys_log_vprintf (
>>              short_file_name++; /* move past the "/" */
>>  #endif /* BUILDING_IN_PLACE */
>>  
>> -    /*
>> -     * Create a log record
>> -     */
>> -    _logsys_log_rec (
>> -            rec_ident,
>> -            function_name,
>> -            short_file_name,
>> -            file_line,
>> -            logsys_print_buffer, len + 1,
>> -            LOGSYS_REC_END);
>> +    if (startup_pid == 0 || startup_pid == getpid()) {
>> +            /*
>> +             * Create a log record if we are really true corosync
>> +             * process (not fork of some service) or if we didn't finished
>> +             * initialization yet.
>> +             */
>> +            _logsys_log_rec (
>> +                    rec_ident,
>> +                    function_name,
>> +                    short_file_name,
>> +                    file_line,
>> +                    logsys_print_buffer, len + 1,
>> +                    LOGSYS_REC_END);
>> +    }
>>  
>>      /*
>>       * If logsys is not going to print a message to a log target don't
>> @@ -1324,6 +1330,8 @@ int _logsys_config_subsys_get (const char *subsys)
>>  void logsys_fork_completed (void)
>>  {
>>      logsys_loggers[LOGSYS_MAX_SUBSYS_COUNT].mode &= ~LOGSYS_MODE_FORK;
>> +    startup_pid = getpid();
>> +
>>      (void)_logsys_wthread_create ();
>>  }
>>  
>>
> 
> _______________________________________________
> discuss mailing list
> [email protected]
> http://lists.corosync.org/mailman/listinfo/discuss

_______________________________________________
discuss mailing list
[email protected]
http://lists.corosync.org/mailman/listinfo/discuss

Reply via email to