Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> When exiting a process, rtdk buffers are not flushed. Fix that by
>>>> introducing a destructor for the library which flushes the buffers.
>>>>
>>>> When forking, if the main thread has an rt_printf buffer, it is reused by
>>>> the child, which is a good thing, however, it is not emptied, so could 
>>>> cause
>>>> the same message to be printed twice. Fix that by emptying the main thread
>>>> buffer upon fork.
>>>>
>>>> When spawning the rt_print thread, it uses a stack size of
>>>> PTHREAD_STACK_MIN, which may be too small for printf to work reliably on
>>>> some architectures, set the stack size to the
>>>> empirically-known-to-be-working size of... 32Kb.
>>>> ---
>>>>  src/rtdk/init.c     |    5 +++++
>>>>  src/rtdk/internal.h |    1 +
>>>>  src/rtdk/rt_print.c |   21 ++++++++++++++++++++-
>>>>  3 files changed, 26 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/src/rtdk/init.c b/src/rtdk/init.c
>>>> index 2084c73..b864175 100644
>>>> --- a/src/rtdk/init.c
>>>> +++ b/src/rtdk/init.c
>>>> @@ -22,3 +22,8 @@ static __attribute__ ((constructor)) void 
>>>> __init_rtdk(void)
>>>>  {
>>>>    __rt_print_init();
>>>>  }
>>>> +
>>>> +static __attribute__ ((destructor)) void __exit_rtdk(void)
>>>> +{
>>>> +  __rt_print_exit();
>>>> +}
>>>> diff --git a/src/rtdk/internal.h b/src/rtdk/internal.h
>>>> index bd15b2d..345d4fa 100644
>>>> --- a/src/rtdk/internal.h
>>>> +++ b/src/rtdk/internal.h
>>>> @@ -23,6 +23,7 @@
>>>>  #include <sys/time.h>
>>>>  
>>>>  void __rt_print_init(void);
>>>> +void __rt_print_exit(void);
>>>>  
>>>>  void __real_free(void *ptr);
>>>>  void *__real_malloc(size_t size);
>>>> diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
>>>> index c6b5c55..dae7d7b 100644
>>>> --- a/src/rtdk/rt_print.c
>>>> +++ b/src/rtdk/rt_print.c
>>>> @@ -455,7 +455,7 @@ static void spawn_printer_thread(void)
>>>>    pthread_attr_t thattr;
>>>>  
>>>>    pthread_attr_init(&thattr);
>>>> -  pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
>>>> +  pthread_attr_setstacksize(&thattr, 32768);
>>> Mmh... maybe rather $something +PTHREAD_STACK_MIN?
>> No, PTHREAD_STACK_MIN sucks, it is not appropriate, and have very
>> varying values on different platforms. On some plaforms, with some
>> versions of glibc, it is 4096, way too small, so that printf generates a
>> segfault due to a stack overflow.
>>
>> However, due to the work done when solving the stack issue reported by
>> Stefan, I hid the magic constants in a function called xeno_stacksize
>> and xeno_stacksize(0) gives a reasonnable default value.
> 
> OK, then go for this service (is it already merged)?
> 
>>>>    pthread_create(&printer_thread, &thattr, printer_loop, NULL);
>>>>  }
>>>>  
>>>> @@ -464,6 +464,15 @@ static void forked_child_init(void)
>>>>    struct print_buffer *my_buffer = pthread_getspecific(buffer_key);
>>>>    struct print_buffer **pbuffer = &first_buffer;
>>>>  
>>>> +  if (my_buffer) {
>>>> +          /* Any content of my_buffer should be printed by our parent,
>>>> +             not us. */
>>>> +          memset(my_buffer->ring, 0, my_buffer->size);
>>>> +
>>>> +          my_buffer->read_pos  = 0;
>>>> +          my_buffer->write_pos = 0;
>>>> +  }
>>>> +
>>> Ack.
>>>
>>>>    /* re-init to avoid finding it locked by some parent thread */
>>>>    pthread_mutex_init(&buffer_lock, NULL);
>>>>  
>>>> @@ -518,3 +527,13 @@ void __rt_print_init(void)
>>>>    spawn_printer_thread();
>>>>    pthread_atfork(NULL, NULL, forked_child_init);
>>>>  }
>>>> +
>>>> +void __rt_print_exit(void)
>>>> +{
>>>> +  if (buffers) {
>>>> +          /* Flush the buffers. Do not call print_buffers here
>>>> +           * since we do not know if our stack is big enough. */
>>>> +          nanosleep(&print_period, NULL);
>>>> +          nanosleep(&print_period, NULL);
>>>> +  }
>>>> +}
>>> IIRC, that's what cleanup_buffer is supposed to do, triggered by the
>>> pthread key destruction. Can you explain why it failed in your scenario.
>> pthread_key destruction is called when a thread is cancelled. My
>> testcase was calling exit.
> 
> exit() does not trigger these cleanup hooks? Too bad.
> 
>> Besides, cleanup_buffer flushes the buffers
>> from the context of the calling thread, which is a bad idea, due to the
>> stack sizes situation.
> 
> If a thread requesting rt_printk services is not capable of performing
> this work during it's cleanup, when there is no significant stack load,
> I would say it's misconfigured /wrt stack size.

Ok, but is is printf which causes the significant stack load. It is fine
with me. However, in the case of exit, the main thread is not even
necessarily supposed to be able to call rt_printf, so, I think leaving
the job to the printing thread is better.

-- 
Gilles Chanteperdrix, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core

Reply via email to