All of lore.kernel.org
 help / color / mirror / Atom feed
* [Xenomai-help] rt_printf with daemonized task
@ 2009-10-06 15:10 oliver.schlenker
  2009-10-06 15:16 ` Gilles Chanteperdrix
  0 siblings, 1 reply; 21+ messages in thread
From: oliver.schlenker @ 2009-10-06 15:10 UTC (permalink / raw)
  To: xenomai




Hi,

we are using a xenomai system with kernel 2.6.22 and xenomai 2.4.7.

There is some strange behaviour when using rt_printf in one of our
application. The application is setup as a server process, which itself
is setting up several xenomai tasks. 
All rt_printf outputs are displayed correctly as long as the server process is not daemonized. 

If the server process is daemonized with code like

if( (pid=fork()) < 0 )
{
        return(-1);
}else
{
        if( pid != 0 )
        {
            exit(0);
        }
}

all rt_printf-output is printed only when the server process exits
and not during the time the server process is running.


Any idea ?



Thanks,


Oliver




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] rt_printf with daemonized task
  2009-10-06 15:10 [Xenomai-help] rt_printf with daemonized task oliver.schlenker
@ 2009-10-06 15:16 ` Gilles Chanteperdrix
  0 siblings, 0 replies; 21+ messages in thread
From: Gilles Chanteperdrix @ 2009-10-06 15:16 UTC (permalink / raw)
  To: oliver.schlenker; +Cc: xenomai

oliver.schlenker@domain.hid wrote:
> 
> 
> Hi,
> 
> we are using a xenomai system with kernel 2.6.22 and xenomai 2.4.7.
> 
> There is some strange behaviour when using rt_printf in one of our
> application. The application is setup as a server process, which itself
> is setting up several xenomai tasks. 
> All rt_printf outputs are displayed correctly as long as the server process is not daemonized. 
> 
> If the server process is daemonized with code like
> 
> if( (pid=fork()) < 0 )
> {
>         return(-1);
> }else
> {
>         if( pid != 0 )
>         {
>             exit(0);
>         }
> }

This is over-simplified. You process is not really daemonized. You
should really call the glibc daemon() function.

> 
> all rt_printf-output is printed only when the server process exits
> and not during the time the server process is running.
> 
> 
> Any idea ?

I guess you do not need rt_printf output in the parent. So, you should
try and initialize the rt_printf library only in the child.

-- 
                                          Gilles



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
  2009-10-20 10:03 oliver.schlenker
  2009-10-20 16:01 ` Gilles Chanteperdrix
@ 2009-10-20 16:11 ` Jan Kiszka
  1 sibling, 0 replies; 21+ messages in thread
From: Jan Kiszka @ 2009-10-20 16:11 UTC (permalink / raw)
  To: oliver.schlenker; +Cc: xenomai

oliver.schlenker@domain.hid wrote:
> 
> 
> 
> 
> -------- Original Message --------
> Subject: Re: :: rt_printf with daemonized task (20-Okt-2009 11:33)
> From:    Jan Kiszka <jan.kiszka@domain.hid>
> To:      oliver.schlenker@domain.hid
> 
>> See http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/6571 for
>> an example of a patch that may (but need not) be generated via git. What
>> counts is the format: proper title, a few lines of description, signed-off.
>>
>> HTH,
>> Jan
>>
> 
> ok, here is my next trial for a proper patch
> 

That format looks good.

Thanks,
Jan

> 
> Oliver
> 
> 
> ----------------------------------------------------------------------------------------------
> 
> Subject : Fork-save use of rt_print
> 
> Fork save initialisation of rt_print library, necessary child initialisation are
> done via pthread_atfork() when child process is started.
> 
> Signed-off-by: Oliver Schlenker <oliver.schlenker <at> sm motion control.de>
> ---
> 
> --- rt_print.c.original	2008-09-10 10:36:27.000000000 +0200
> +++ rt_print.c.forkonly	2009-10-16 16:19:25.949289800 +0200
>                     
>  	}
>  }
>  
> -void __rt_print_init(void)
> +static void spawn_printer_thread(void)
>  {
>  	pthread_attr_t thattr;
> +
> +	pthread_attr_init(&thattr);
> +	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
> +	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
> +}
> +
> +static void forked_child_init(void)
> +{
> +	struct print_buffer *my_buffer = pthread_getspecific(__buffer_key);
> +	struct print_buffer **pbuffer = &__first_buffer;
> +
> +	/* re-init to avoid finding it locked by some parent thread */
> +	pthread_mutex_init(&__buffer_lock, NULL);
> +
> +	while (*pbuffer) {
> +		if (*pbuffer == my_buffer)
> +			pbuffer = &(*pbuffer)->next;
> +		else
> +			cleanup_buffer(*pbuffer);
> +	}
> +
> +	spawn_printer_thread();
> +}
> +
> +void __rt_print_init(void)
> +{
>  	const char *value_str;
>  	unsigned long long period;
>  
>                    
>  	pthread_mutex_init(&__buffer_lock, NULL);
>  	pthread_key_create(&__buffer_key, (void (*)(void*))cleanup_buffer);
>  
> -	pthread_attr_init(&thattr);
> -	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
> -	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
> +	spawn_printer_thread();
> +	pthread_atfork(NULL, NULL, forked_child_init);
>  } 
> 
> 
> 
> 
> To: jan.kiszka@domain.hid
> Cc: gilles.chanteperdrix@xenomai.org
>     xenomai@xenomai.org


-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
  2009-10-20 16:01 ` Gilles Chanteperdrix
@ 2009-10-20 16:07   ` Jan Kiszka
  0 siblings, 0 replies; 21+ messages in thread
From: Jan Kiszka @ 2009-10-20 16:07 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai, oliver.schlenker

Gilles Chanteperdrix wrote:
> oliver.schlenker@domain.hid wrote:
>> +	while (*pbuffer) {
>> +		if (*pbuffer == my_buffer)
>> +			pbuffer = &(*pbuffer)->next;
>> +		else
>> +			cleanup_buffer(*pbuffer);
>> +	}
> 
> Looks like an infinite loop if *pbuffer != my_buffer, no?
> 

No, cleanup_buffer is shrinking the list until it's NULL (or only
my_buffer).

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
  2009-10-20 10:03 oliver.schlenker
@ 2009-10-20 16:01 ` Gilles Chanteperdrix
  2009-10-20 16:07   ` Jan Kiszka
  2009-10-20 16:11 ` Jan Kiszka
  1 sibling, 1 reply; 21+ messages in thread
From: Gilles Chanteperdrix @ 2009-10-20 16:01 UTC (permalink / raw)
  To: oliver.schlenker; +Cc: jan.kiszka, xenomai

oliver.schlenker@domain.hid wrote:
> +	while (*pbuffer) {
> +		if (*pbuffer == my_buffer)
> +			pbuffer = &(*pbuffer)->next;
> +		else
> +			cleanup_buffer(*pbuffer);
> +	}

Looks like an infinite loop if *pbuffer != my_buffer, no?

-- 
					    Gilles.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
@ 2009-10-20 10:03 oliver.schlenker
  2009-10-20 16:01 ` Gilles Chanteperdrix
  2009-10-20 16:11 ` Jan Kiszka
  0 siblings, 2 replies; 21+ messages in thread
From: oliver.schlenker @ 2009-10-20 10:03 UTC (permalink / raw)
  To: jan.kiszka; +Cc: xenomai






-------- Original Message --------
Subject: Re: :: rt_printf with daemonized task (20-Okt-2009 11:33)
From:    Jan Kiszka <jan.kiszka@siemens.com>
To:      oliver.schlenker@smmotioncontrol.de

> 
> See http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/6571 for
> an example of a patch that may (but need not) be generated via git. What
> counts is the format: proper title, a few lines of description, signed-off.
> 
> HTH,
> Jan
> 

ok, here is my next trial for a proper patch


Oliver


----------------------------------------------------------------------------------------------

Subject : Fork-save use of rt_print

Fork save initialisation of rt_print library, necessary child initialisation are
done via pthread_atfork() when child process is started.

Signed-off-by: Oliver Schlenker <oliver.schlenker <at> sm motion control.de>
---

--- rt_print.c.original	2008-09-10 10:36:27.000000000 +0200
+++ rt_print.c.forkonly	2009-10-16 16:19:25.949289800 +0200
                    
 	}
 }
 
-void __rt_print_init(void)
+static void spawn_printer_thread(void)
 {
 	pthread_attr_t thattr;
+
+	pthread_attr_init(&thattr);
+	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
+	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
+}
+
+static void forked_child_init(void)
+{
+	struct print_buffer *my_buffer = pthread_getspecific(__buffer_key);
+	struct print_buffer **pbuffer = &__first_buffer;
+
+	/* re-init to avoid finding it locked by some parent thread */
+	pthread_mutex_init(&__buffer_lock, NULL);
+
+	while (*pbuffer) {
+		if (*pbuffer == my_buffer)
+			pbuffer = &(*pbuffer)->next;
+		else
+			cleanup_buffer(*pbuffer);
+	}
+
+	spawn_printer_thread();
+}
+
+void __rt_print_init(void)
+{
 	const char *value_str;
 	unsigned long long period;
 
                   
 	pthread_mutex_init(&__buffer_lock, NULL);
 	pthread_key_create(&__buffer_key, (void (*)(void*))cleanup_buffer);
 
-	pthread_attr_init(&thattr);
-	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
-	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
+	spawn_printer_thread();
+	pthread_atfork(NULL, NULL, forked_child_init);
 } 




To: jan.kiszka@siemens.com
Cc: gilles.chanteperdrix@xenomai.org
    xenomai-help@gna.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
  2009-10-20  9:26 oliver.schlenker
@ 2009-10-20  9:33 ` Jan Kiszka
  0 siblings, 0 replies; 21+ messages in thread
From: Jan Kiszka @ 2009-10-20  9:33 UTC (permalink / raw)
  To: oliver.schlenker; +Cc: xenomai

oliver.schlenker@domain.hid wrote:
> -------- Original Message --------
> Subject: Re: :: rt_printf with daemonized task (19-Okt-2009 16:00)
> From:    Jan Kiszka <jan.kiszka@domain.hid>
> To:      oliver.schlenker@domain.hid
> 
>> Thanks for the patches, but I still have remarks. :)
>>
>> First of all, both patches need proper titles, a few lines of
>> description (see git history to get a feeling), and a signed-off-by to
>> express that you are the copyright holder and are allowed to contribute
>> this to Xenomai.
>>
> 
> I'm not totally sure what to do now, because I have never used git before ...
> 
> In the header part of the xenomai source files there are not description
> of any patches mentioned, and also no signed-off-by of the authors
> of patches.

See http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/6571 for
an example of a patch that may (but need not) be generated via git. What
counts is the format: proper title, a few lines of description, signed-off.

HTH,
Jan

> 
> Maybe it's the best to change the patches according to your last two mails
> and post it again in the list. So the patches are available to anybody
> who would like to use them.
> 
> 
> 
> Oliver
> 
> To: jan.kiszka@domain.hid
> Cc: gilles.chanteperdrix@xenomai.org
>     xenomai@xenomai.org

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
@ 2009-10-20  9:26 oliver.schlenker
  2009-10-20  9:33 ` Jan Kiszka
  0 siblings, 1 reply; 21+ messages in thread
From: oliver.schlenker @ 2009-10-20  9:26 UTC (permalink / raw)
  To: jan.kiszka; +Cc: xenomai






-------- Original Message --------
Subject: Re: :: rt_printf with daemonized task (19-Okt-2009 16:00)
From:    Jan Kiszka <jan.kiszka@siemens.com>
To:      oliver.schlenker@smmotioncontrol.de

> 
> Thanks for the patches, but I still have remarks. :)
> 
> First of all, both patches need proper titles, a few lines of
> description (see git history to get a feeling), and a signed-off-by to
> express that you are the copyright holder and are allowed to contribute
> this to Xenomai.
> 

I'm not totally sure what to do now, because I have never used git before ...

In the header part of the xenomai source files there are not description
of any patches mentioned, and also no signed-off-by of the authors
of patches.

Maybe it's the best to change the patches according to your last two mails
and post it again in the list. So the patches are available to anybody
who would like to use them.



Oliver

To: jan.kiszka@siemens.com
Cc: gilles.chanteperdrix@xenomai.org
    xenomai-help@gna.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
  2009-10-19 13:12 oliver.schlenker
@ 2009-10-19 14:00 ` Jan Kiszka
  0 siblings, 0 replies; 21+ messages in thread
From: Jan Kiszka @ 2009-10-19 14:00 UTC (permalink / raw)
  To: oliver.schlenker; +Cc: xenomai

oliver.schlenker@domain.hid wrote:
> -------- Original Message --------
> Subject: Re: :: rt_printf with daemonized task (09-Okt-2009 16:59)
> From:    Jan Kiszka <jan.kiszka@domain.hid>
> To:      oliver.schlenker@domain.hid
> 
>> Yes (to the former), avoiding code duplication was the motivation for
>> this functions.
>>
>> Another wish: Please split up your changes into two patches: first the
>> atfork fix, on top of it the syslog extension. That allows us, e.g., to
>> apply the fix also to 2.4.
> 
> Here we go, first patch for a fork save rt_print .....
> 

Thanks for the patches, but I still have remarks. :)

First of all, both patches need proper titles, a few lines of
description (see git history to get a feeling), and a signed-off-by to
express that you are the copyright holder and are allowed to contribute
this to Xenomai.

> 
>> Jan
>>
>> -- 
>> Siemens AG, Corporate Technology, CT SE 2
>> Corporate Competence Center Embedded Linux
> 
> 
> Oliver
> 
> 
> 
> --- rt_print.c.original	2008-09-10 10:36:27.000000000 +0200
> +++ rt_print.c.forkonly	2009-10-16 16:19:25.949289800 +0200
>                     
>  	}
>  }
>  
> -void __rt_print_init(void)
> +static void spawn_printer_thread(void)
>  {
>  	pthread_attr_t thattr;
> +
> +	pthread_attr_init(&thattr);
> +	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
> +	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
> +}
> +
> +static void forked_child_init(void)
> +{
> +	struct print_buffer *my_buffer = pthread_getspecific(__buffer_key);
> +	struct print_buffer **pbuffer = &__first_buffer;
> +
> +	/* re-init to avoid finding it locked by some parent thread */
> +	pthread_mutex_init(&__buffer_lock, NULL);
> +
> +	while (*pbuffer) {
> +		if (*pbuffer == my_buffer)
> +			pbuffer = &(*pbuffer)->next;
> +		else
> +			cleanup_buffer(*pbuffer);
> +	}
> +
> +	spawn_printer_thread();
> +}
> +
> +void __rt_print_init(void)
> +{
>  	const char *value_str;
>  	unsigned long long period;
>  
>                    
>  	pthread_mutex_init(&__buffer_lock, NULL);
>  	pthread_key_create(&__buffer_key, (void (*)(void*))cleanup_buffer);
>  
> -	pthread_attr_init(&thattr);
> -	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
> -	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
> +	spawn_printer_thread();
> +	pthread_atfork(NULL,NULL,forked_child_init);

Please add a space after each ','.

>  } 
> 

When the formal things are resolved:

Acked-by: Jan Kiszka <jan.kiszka@domain.hid>

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
  2009-10-19 13:14 oliver.schlenker
@ 2009-10-19 14:00 ` Jan Kiszka
  0 siblings, 0 replies; 21+ messages in thread
From: Jan Kiszka @ 2009-10-19 14:00 UTC (permalink / raw)
  To: oliver.schlenker; +Cc: xenomai

oliver.schlenker@domain.hid wrote:
> 
> 
> 
> 
> -------- Original Message --------
> Subject: Re: :: rt_printf with daemonized task (09-Okt-2009 16:59)
> From:    Jan Kiszka <jan.kiszka@domain.hid>
> To:      oliver.schlenker@domain.hid
> 
>> Yes (to the former), avoiding code duplication was the motivation for
>> this functions.
>>
>> Another wish: Please split up your changes into two patches: first the
>> atfork fix, on top of it the syslog extension. That allows us, e.g., to
>> apply the fix also to 2.4.
> 
> ... and the next patch for a syslog enabled rt_print.
> 
> 
>> Jan
>>
>> -- 
>> Siemens AG, Corporate Technology, CT SE 2
>> Corporate Competence Center Embedded Linux
> 
> 
> Oliver
> 
> 
> 
> 
> 
> --- rtdk.h.original	2007-12-09 11:46:36.000000000 +0100
> +++ rtdk.h	2009-10-09 11:46:52.767271100 +0200
>                  
>  int rt_vprintf(const char *format, va_list args);
>  int rt_fprintf(FILE *stream, const char *format, ...);
>  int rt_printf(const char *format, ...);
> +void rt_syslog( int priority, char *format, ...);

Stray space.

> +void rt_vsyslog(int priority, char *format, va_list args);
> +
>  
>  int rt_print_init(size_t buffer_size, const char *name);
>  void rt_print_cleanup(void); 
> 
> 
> 
> --- rt_print.c.forkonly	2009-10-16 16:19:25.949289800 +0200
> +++ rt_print.c.wsyslog	2009-10-16 16:31:01.952464300 +0200
>                  
>  #include <stdlib.h>
>  #include <string.h>
>  #include <unistd.h>
> +#include <syslog.h>
>  
>  #include <rtdk.h>
>  #include <asm/xenomai/system.h>
>                   
>  
>  #define RT_PRINT_LINE_BREAK		256
>  
> +#define RT_PRINT_SYSLOG_STREAM	NULL
> +
>  struct entry_head {
>  	FILE *dest;
>  	uint32_t seq_no;
> +	int priority;
>  	char text[1];
>  } __attribute__((packed));
>  
>                     
>  static pthread_key_t __buffer_key;
>  static pthread_t __printer_thread;
>  
> +static int print_to_buffer(FILE *stream, int priority,
> +                           const char *format,va_list args);

If you move the definition of print_to_buffer right below this prototype
block, you would avoid having to add this prototype and you would have
likely generated a more reviewable patch. The large code movement below
is very hard to check regarding correctness as I do not see the actually
modified lines.

>  static void cleanup_buffer(struct print_buffer *buffer);
>  static void print_buffers(void);
> +static void forked_child_init(void);

Stray prototype.

>  
>  /* *** rt_print API *** */
>  
>  int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  {
> -	struct print_buffer *buffer = pthread_getspecific(__buffer_key);
> -	off_t write_pos, read_pos;
> -	struct entry_head *head;
> -	int len;
> -	int res;
> -
> -	if (!buffer) {
> -		res = 0;
> -		if (__auto_init)
> -			res = rt_print_init(0, NULL);
> -		else
> -			res = EIO;
> -
> -		if (res) {
> -			errno = res;
> -			return -1;
> -		}
> -		buffer = pthread_getspecific(__buffer_key);
> -	}
> -
> -	/* Take a snapshot of the ring buffer state */
> -	write_pos = buffer->write_pos;
> -	read_pos = buffer->read_pos;
> -	xnarch_read_memory_barrier();
> -
> -	/* Is our write limit the end of the ring buffer? */
> -	if (write_pos >= read_pos) {
> -		/* Keep a savety margin to the end for at least an empty entry */
> -		len = buffer->size - write_pos - sizeof(struct entry_head);
> -
> -		/* Special case: We were stuck at the end of the ring buffer
> -		   with space left there only for one empty entry. Now
> -		   read_pos was moved forward and we can wrap around. */
> -		if (len == 0 && read_pos > sizeof(struct entry_head)) {
> -			/* Write out empty entry */
> -			head = buffer->ring + write_pos;
> -			head->seq_no = __seq_no;
> -			head->text[0] = 0;
> -
> -			/* Forward to the ring buffer start */
> -			write_pos = 0;
> -			len = read_pos - 1;
> -		}
> -	} else {
> -		/* Our limit is the read_pos ahead of our write_pos. One byte
> -		   margin is required to detect a full ring. */
> -		len = read_pos - write_pos - 1;
> -	}
> -
> -	/* Account for head length */
> -	len -= sizeof(struct entry_head);
> -	if (len < 0)
> -		len = 0;
> -
> -	head = buffer->ring + write_pos;
> -
> -	res = vsnprintf(head->text, len, format, args);
> -
> -	if (res < len) {
> -		/* Text was written completely, res contains its length */
> -		len = res;
> -	} else {
> -		/* Text was truncated, remove closing \0 that entry_head
> -		   already includes */
> -		len--;
> -	}
> -
> -	/* If we were able to write some text, finalise the entry */
> -	if (len > 0) {
> -		head->seq_no = ++__seq_no;
> -		head->dest = stream;
> -
> -		/* Move forward by text and head length */
> -		write_pos += len + sizeof(struct entry_head);
> -	}
> -
> -	/* Wrap around early if there is more space on the other side */
> -	if (write_pos >= buffer->size - RT_PRINT_LINE_BREAK &&
> -	    read_pos <= write_pos && read_pos > buffer->size - write_pos) {
> -		/* An empty entry marks the wrap-around */
> -		head = buffer->ring + write_pos;
> -		head->seq_no = __seq_no;
> -		head->text[0] = 0;
> -
> -		write_pos = 0;
> -	}
> -
> -	/* All entry data must be written before we can update write_pos */
> -	xnarch_write_memory_barrier();
> -
> -	buffer->write_pos = write_pos;
> -
> -	return res;
> +	return print_to_buffer(stream, 0, format, args);
>  }
>  
>  int rt_vprintf(const char *format, va_list args)
>                     
>  	return n;
>  }
>  
> +void rt_syslog(int priority, char *format, ...)
> +{
> +	va_list args;
> +
> +	va_start(args, format);
> +	print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
> +	va_end(args);
> +
> +	return;

Unneeded return.

> +}
> +
> +void rt_vsyslog(int priority, char *format, va_list args )
> +{
> +	print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
> +
> +	return;

Here too.

> +}
> +
>  static void set_buffer_name(struct print_buffer *buffer, const char *name)
>  {
>  	int n;
>                      
>  
>  /* *** Deferred Output Management *** */
>  
> +static int print_to_buffer(FILE *stream, int priority,
> +							const char *format,va_list args)

When moving this function, please also indent this properly and take
care for a space after the ','.

> +{
> +	struct print_buffer *buffer = pthread_getspecific(__buffer_key);
> +	off_t write_pos, read_pos;
> +	struct entry_head *head;
> +	int len;
> +	int res;
> +
> +	if (!buffer) {
> +		res = 0;
> +		if (__auto_init)
> +			res = rt_print_init(0, NULL);
> +		else
> +			res = EIO;
> +
> +		if (res) {
> +			errno = res;
> +			return -1;
> +		}
> +		buffer = pthread_getspecific(__buffer_key);
> +	}
> +
> +	/* Take a snapshot of the ring buffer state */
> +	write_pos = buffer->write_pos;
> +	read_pos = buffer->read_pos;
> +	xnarch_read_memory_barrier();
> +
> +	/* Is our write limit the end of the ring buffer? */
> +	if (write_pos >= read_pos) {
> +		/* Keep a savety margin to the end for at least an empty entry */
> +		len = buffer->size - write_pos - sizeof(struct entry_head);
> +
> +		/* Special case: We were stuck at the end of the ring buffer
> +		   with space left there only for one empty entry. Now
> +		   read_pos was moved forward and we can wrap around. */
> +		if (len == 0 && read_pos > sizeof(struct entry_head)) {
> +			/* Write out empty entry */
> +			head = buffer->ring + write_pos;
> +			head->seq_no = __seq_no;
> +			head->priority = 0;
> +			head->text[0] = 0;
> +
> +			/* Forward to the ring buffer start */
> +			write_pos = 0;
> +			len = read_pos - 1;
> +		}
> +	} else {
> +		/* Our limit is the read_pos ahead of our write_pos. One byte
> +		   margin is required to detect a full ring. */
> +		len = read_pos - write_pos - 1;
> +	}
> +
> +	/* Account for head length */
> +	len -= sizeof(struct entry_head);
> +	if (len < 0)
> +		len = 0;
> +
> +	head = buffer->ring + write_pos;
> +
> +	res = vsnprintf(head->text, len, format, args);
> +
> +	if (res < len) {
> +		/* Text was written completely, res contains its length */
> +		len = res;
> +	} else {
> +		/* Text was truncated, remove closing \0 that entry_head
> +		   already includes */
> +		len--;
> +	}
> +
> +	/* If we were able to write some text, finalise the entry */
> +	if (len > 0) {
> +		head->seq_no = ++__seq_no;
> +		head->priority = priority;
> +		head->dest = stream;
> +
> +		/* Move forward by text and head length */
> +		write_pos += len + sizeof(struct entry_head);
> +	}
> +
> +	/* Wrap around early if there is more space on the other side */
> +	if (write_pos >= buffer->size - RT_PRINT_LINE_BREAK &&
> +	    read_pos <= write_pos && read_pos > buffer->size - write_pos) {
> +		/* An empty entry marks the wrap-around */
> +		head = buffer->ring + write_pos;
> +		head->seq_no = __seq_no;
> +		head->priority = priority;
> +		head->text[0] = 0;
> +
> +		write_pos = 0;
> +	}
> +
> +	/* All entry data must be written before we can update write_pos */
> +	xnarch_write_memory_barrier();
> +
> +	buffer->write_pos = write_pos;
> +
> +	return res;
> +}
> +
>  static void cleanup_buffer(struct print_buffer *buffer)
>  {
>  	struct print_buffer *prev, *next;
>                     
>  
>  		if (len) {
>  			/* Print out non-empty entry and proceed */
> -			fprintf(head->dest, "%s", head->text);
> +			/* Check if output goes to syslog */
> +			if (head->dest == RT_PRINT_SYSLOG_STREAM) {
> +			   	syslog( head->priority, "%s", head->text );
> +			} else {
> +				/* Output goes to specified stream */
> +				fprintf(head->dest, "%s", head->text);
> +			}
> +
>  			read_pos += sizeof(*head) + len;
>  		} else {
>  			/* Emptry entries mark the wrap-around */ 
> 
> 
> 

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
@ 2009-10-19 13:14 oliver.schlenker
  2009-10-19 14:00 ` Jan Kiszka
  0 siblings, 1 reply; 21+ messages in thread
From: oliver.schlenker @ 2009-10-19 13:14 UTC (permalink / raw)
  To: jan.kiszka; +Cc: xenomai






-------- Original Message --------
Subject: Re: :: rt_printf with daemonized task (09-Okt-2009 16:59)
From:    Jan Kiszka <jan.kiszka@siemens.com>
To:      oliver.schlenker@smmotioncontrol.de

> 
> Yes (to the former), avoiding code duplication was the motivation for
> this functions.
> 
> Another wish: Please split up your changes into two patches: first the
> atfork fix, on top of it the syslog extension. That allows us, e.g., to
> apply the fix also to 2.4.

... and the next patch for a syslog enabled rt_print.


> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT SE 2
> Corporate Competence Center Embedded Linux


Oliver





--- rtdk.h.original	2007-12-09 11:46:36.000000000 +0100
+++ rtdk.h	2009-10-09 11:46:52.767271100 +0200
                 
 int rt_vprintf(const char *format, va_list args);
 int rt_fprintf(FILE *stream, const char *format, ...);
 int rt_printf(const char *format, ...);
+void rt_syslog( int priority, char *format, ...);
+void rt_vsyslog(int priority, char *format, va_list args);
+
 
 int rt_print_init(size_t buffer_size, const char *name);
 void rt_print_cleanup(void); 



--- rt_print.c.forkonly	2009-10-16 16:19:25.949289800 +0200
+++ rt_print.c.wsyslog	2009-10-16 16:31:01.952464300 +0200
                 
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <syslog.h>
 
 #include <rtdk.h>
 #include <asm/xenomai/system.h>
                  
 
 #define RT_PRINT_LINE_BREAK		256
 
+#define RT_PRINT_SYSLOG_STREAM	NULL
+
 struct entry_head {
 	FILE *dest;
 	uint32_t seq_no;
+	int priority;
 	char text[1];
 } __attribute__((packed));
 
                    
 static pthread_key_t __buffer_key;
 static pthread_t __printer_thread;
 
+static int print_to_buffer(FILE *stream, int priority,
+                           const char *format,va_list args);
 static void cleanup_buffer(struct print_buffer *buffer);
 static void print_buffers(void);
+static void forked_child_init(void);
 
 /* *** rt_print API *** */
 
 int rt_vfprintf(FILE *stream, const char *format, va_list args)
 {
-	struct print_buffer *buffer = pthread_getspecific(__buffer_key);
-	off_t write_pos, read_pos;
-	struct entry_head *head;
-	int len;
-	int res;
-
-	if (!buffer) {
-		res = 0;
-		if (__auto_init)
-			res = rt_print_init(0, NULL);
-		else
-			res = EIO;
-
-		if (res) {
-			errno = res;
-			return -1;
-		}
-		buffer = pthread_getspecific(__buffer_key);
-	}
-
-	/* Take a snapshot of the ring buffer state */
-	write_pos = buffer->write_pos;
-	read_pos = buffer->read_pos;
-	xnarch_read_memory_barrier();
-
-	/* Is our write limit the end of the ring buffer? */
-	if (write_pos >= read_pos) {
-		/* Keep a savety margin to the end for at least an empty entry */
-		len = buffer->size - write_pos - sizeof(struct entry_head);
-
-		/* Special case: We were stuck at the end of the ring buffer
-		   with space left there only for one empty entry. Now
-		   read_pos was moved forward and we can wrap around. */
-		if (len == 0 && read_pos > sizeof(struct entry_head)) {
-			/* Write out empty entry */
-			head = buffer->ring + write_pos;
-			head->seq_no = __seq_no;
-			head->text[0] = 0;
-
-			/* Forward to the ring buffer start */
-			write_pos = 0;
-			len = read_pos - 1;
-		}
-	} else {
-		/* Our limit is the read_pos ahead of our write_pos. One byte
-		   margin is required to detect a full ring. */
-		len = read_pos - write_pos - 1;
-	}
-
-	/* Account for head length */
-	len -= sizeof(struct entry_head);
-	if (len < 0)
-		len = 0;
-
-	head = buffer->ring + write_pos;
-
-	res = vsnprintf(head->text, len, format, args);
-
-	if (res < len) {
-		/* Text was written completely, res contains its length */
-		len = res;
-	} else {
-		/* Text was truncated, remove closing \0 that entry_head
-		   already includes */
-		len--;
-	}
-
-	/* If we were able to write some text, finalise the entry */
-	if (len > 0) {
-		head->seq_no = ++__seq_no;
-		head->dest = stream;
-
-		/* Move forward by text and head length */
-		write_pos += len + sizeof(struct entry_head);
-	}
-
-	/* Wrap around early if there is more space on the other side */
-	if (write_pos >= buffer->size - RT_PRINT_LINE_BREAK &&
-	    read_pos <= write_pos && read_pos > buffer->size - write_pos) {
-		/* An empty entry marks the wrap-around */
-		head = buffer->ring + write_pos;
-		head->seq_no = __seq_no;
-		head->text[0] = 0;
-
-		write_pos = 0;
-	}
-
-	/* All entry data must be written before we can update write_pos */
-	xnarch_write_memory_barrier();
-
-	buffer->write_pos = write_pos;
-
-	return res;
+	return print_to_buffer(stream, 0, format, args);
 }
 
 int rt_vprintf(const char *format, va_list args)
                    
 	return n;
 }
 
+void rt_syslog(int priority, char *format, ...)
+{
+	va_list args;
+
+	va_start(args, format);
+	print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
+	va_end(args);
+
+	return;
+}
+
+void rt_vsyslog(int priority, char *format, va_list args )
+{
+	print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
+
+	return;
+}
+
 static void set_buffer_name(struct print_buffer *buffer, const char *name)
 {
 	int n;
                     
 
 /* *** Deferred Output Management *** */
 
+static int print_to_buffer(FILE *stream, int priority,
+							const char *format,va_list args)
+{
+	struct print_buffer *buffer = pthread_getspecific(__buffer_key);
+	off_t write_pos, read_pos;
+	struct entry_head *head;
+	int len;
+	int res;
+
+	if (!buffer) {
+		res = 0;
+		if (__auto_init)
+			res = rt_print_init(0, NULL);
+		else
+			res = EIO;
+
+		if (res) {
+			errno = res;
+			return -1;
+		}
+		buffer = pthread_getspecific(__buffer_key);
+	}
+
+	/* Take a snapshot of the ring buffer state */
+	write_pos = buffer->write_pos;
+	read_pos = buffer->read_pos;
+	xnarch_read_memory_barrier();
+
+	/* Is our write limit the end of the ring buffer? */
+	if (write_pos >= read_pos) {
+		/* Keep a savety margin to the end for at least an empty entry */
+		len = buffer->size - write_pos - sizeof(struct entry_head);
+
+		/* Special case: We were stuck at the end of the ring buffer
+		   with space left there only for one empty entry. Now
+		   read_pos was moved forward and we can wrap around. */
+		if (len == 0 && read_pos > sizeof(struct entry_head)) {
+			/* Write out empty entry */
+			head = buffer->ring + write_pos;
+			head->seq_no = __seq_no;
+			head->priority = 0;
+			head->text[0] = 0;
+
+			/* Forward to the ring buffer start */
+			write_pos = 0;
+			len = read_pos - 1;
+		}
+	} else {
+		/* Our limit is the read_pos ahead of our write_pos. One byte
+		   margin is required to detect a full ring. */
+		len = read_pos - write_pos - 1;
+	}
+
+	/* Account for head length */
+	len -= sizeof(struct entry_head);
+	if (len < 0)
+		len = 0;
+
+	head = buffer->ring + write_pos;
+
+	res = vsnprintf(head->text, len, format, args);
+
+	if (res < len) {
+		/* Text was written completely, res contains its length */
+		len = res;
+	} else {
+		/* Text was truncated, remove closing \0 that entry_head
+		   already includes */
+		len--;
+	}
+
+	/* If we were able to write some text, finalise the entry */
+	if (len > 0) {
+		head->seq_no = ++__seq_no;
+		head->priority = priority;
+		head->dest = stream;
+
+		/* Move forward by text and head length */
+		write_pos += len + sizeof(struct entry_head);
+	}
+
+	/* Wrap around early if there is more space on the other side */
+	if (write_pos >= buffer->size - RT_PRINT_LINE_BREAK &&
+	    read_pos <= write_pos && read_pos > buffer->size - write_pos) {
+		/* An empty entry marks the wrap-around */
+		head = buffer->ring + write_pos;
+		head->seq_no = __seq_no;
+		head->priority = priority;
+		head->text[0] = 0;
+
+		write_pos = 0;
+	}
+
+	/* All entry data must be written before we can update write_pos */
+	xnarch_write_memory_barrier();
+
+	buffer->write_pos = write_pos;
+
+	return res;
+}
+
 static void cleanup_buffer(struct print_buffer *buffer)
 {
 	struct print_buffer *prev, *next;
                    
 
 		if (len) {
 			/* Print out non-empty entry and proceed */
-			fprintf(head->dest, "%s", head->text);
+			/* Check if output goes to syslog */
+			if (head->dest == RT_PRINT_SYSLOG_STREAM) {
+			   	syslog( head->priority, "%s", head->text );
+			} else {
+				/* Output goes to specified stream */
+				fprintf(head->dest, "%s", head->text);
+			}
+
 			read_pos += sizeof(*head) + len;
 		} else {
 			/* Emptry entries mark the wrap-around */ 










To: jan.kiszka@siemens.com
Cc: gilles.chanteperdrix@xenomai.org
    xenomai-help@gna.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
@ 2009-10-19 13:12 oliver.schlenker
  2009-10-19 14:00 ` Jan Kiszka
  0 siblings, 1 reply; 21+ messages in thread
From: oliver.schlenker @ 2009-10-19 13:12 UTC (permalink / raw)
  To: jan.kiszka; +Cc: xenomai






-------- Original Message --------
Subject: Re: :: rt_printf with daemonized task (09-Okt-2009 16:59)
From:    Jan Kiszka <jan.kiszka@siemens.com>
To:      oliver.schlenker@smmotioncontrol.de

> 
> Yes (to the former), avoiding code duplication was the motivation for
> this functions.
> 
> Another wish: Please split up your changes into two patches: first the
> atfork fix, on top of it the syslog extension. That allows us, e.g., to
> apply the fix also to 2.4.

Here we go, first patch for a fork save rt_print .....


> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT SE 2
> Corporate Competence Center Embedded Linux


Oliver



--- rt_print.c.original	2008-09-10 10:36:27.000000000 +0200
+++ rt_print.c.forkonly	2009-10-16 16:19:25.949289800 +0200
                    
 	}
 }
 
-void __rt_print_init(void)
+static void spawn_printer_thread(void)
 {
 	pthread_attr_t thattr;
+
+	pthread_attr_init(&thattr);
+	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
+	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
+}
+
+static void forked_child_init(void)
+{
+	struct print_buffer *my_buffer = pthread_getspecific(__buffer_key);
+	struct print_buffer **pbuffer = &__first_buffer;
+
+	/* re-init to avoid finding it locked by some parent thread */
+	pthread_mutex_init(&__buffer_lock, NULL);
+
+	while (*pbuffer) {
+		if (*pbuffer == my_buffer)
+			pbuffer = &(*pbuffer)->next;
+		else
+			cleanup_buffer(*pbuffer);
+	}
+
+	spawn_printer_thread();
+}
+
+void __rt_print_init(void)
+{
 	const char *value_str;
 	unsigned long long period;
 
                   
 	pthread_mutex_init(&__buffer_lock, NULL);
 	pthread_key_create(&__buffer_key, (void (*)(void*))cleanup_buffer);
 
-	pthread_attr_init(&thattr);
-	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
-	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
+	spawn_printer_thread();
+	pthread_atfork(NULL,NULL,forked_child_init);
 } 




To: jan.kiszka@siemens.com
Cc: gilles.chanteperdrix@xenomai.org
    xenomai-help@gna.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
  2009-10-09 14:48 oliver.schlenker
@ 2009-10-09 14:59 ` Jan Kiszka
  0 siblings, 0 replies; 21+ messages in thread
From: Jan Kiszka @ 2009-10-09 14:59 UTC (permalink / raw)
  To: oliver.schlenker; +Cc: xenomai

oliver.schlenker@domain.hid wrote:
> Hi Jan,
> 
> thank you for the hints and advices. I'm going to integrate the
> changes, as described below,  test it and send new patch.
> 
> -------- Original Message --------
> Subject: Re: :: rt_printf with daemonized task (09-Okt-2009 15:58)
> From:    Jan Kiszka <jan.kiszka@domain.hid>
> To:      oliver.schlenker@domain.hid
> 
>> Hi Oliver,
>>
>>> +++ rtdk.h	2009-10-09 11:46:52.767271100 +0200
>>>                  
>>>  int rt_vprintf(const char *format, va_list args);
>>>  int rt_fprintf(FILE *stream, const char *format, ...);
>>>  int rt_printf(const char *format, ...);
>>> +void rt_syslog( int priority, char *format, ...);
>>                  ^^^
>> Stray whitespace.
> 
> ok, removed.
> 
>>>  
>>>  #define RT_PRINT_LINE_BREAK		256
>>>  
>>> +#define RT_PRINT_SYSLOG_STREAM	((FILE *)-9999)
>> Better use NULL here.
> 
> ok, changed to NULL.
> 
>>> format, va_list args)
>> Line with >80 characters.
> 
> Is there any reason why a line should not exceed 80 chars ?

Xenomai follows Linux kernel coding style. And Linux (like many other
projects) argue that limited line length helps readability and enforces
the developer to think about different code structuring before
introducing the 10th indention level.

> I did change it anyway.
> 
> 
>>>  
>>> +int rt_vfprintf(FILE *stream, const char *format, va_list args)
>>> +{
>>> +	return rt_print_to_buffer( stream, 0, format, args);
>> Stray whitespace.
> 
> ok, removed.
> 
>>> +void rt_vsyslog(int priority, char *format, va_list args )
>> Whitespace.
> 
> also removed.
> 
>>> +{
>>> +
>>> +	rt_print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
>>> +
>>> +	return;
>>> +}
>>> +
>>>  static void set_buffer_name(struct print_buffer *buffer, const char *name)
>>>  {
>>>  	int n;
>>>                     
>>>  
>>>  		if (len) {
>>>  			/* Print out non-empty entry and proceed */
>>> -			fprintf(head->dest, "%s", head->text);
>>> +			/* Check if output goes to syslog */
>>> +			if (head->dest == RT_PRINT_SYSLOG_STREAM) {
>>> +			   	syslog( head->priority, "%s", head->text );
>> Stray whitespaces. And one /may/ argue that we do not need braces here,
>> but that's a grey area.
> 
> whitespace removed, braces are left (I feel more convienent with braces).
> 
>> Here is my version:
>>
>> static void spawn_printer_thread(void)
>> {
>> 	pthread_attr_t thattr;
>>
>> 	pthread_attr_init(&thattr);
>> 	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
>> 	pthread_create(&printer_thread, &thattr, printer_loop, NULL);
>> }
>>
>> static void forked_child_init(void)
>> {
>> 	struct print_buffer *my_buffer = pthread_getspecific(buffer_key);
>> 	struct print_buffer **pbuffer = &first_buffer;
>>
>> 	/* re-init to avoid finding it locked by some parent thread */
>> 	pthread_mutex_init(&buffer_lock, NULL);
>>
>> 	while (*pbuffer) {
>> 		if (*pbuffer == my_buffer)
>> 			pbuffer = &(*pbuffer)->next;
>> 		else
>> 			cleanup_buffer(*pbuffer);
>> 	}
>>
>> 	spawn_printer_thread();
>> }
>>
>> The major differences are:
>>  - my code is totally untested :)
>>  - we have to play safe and re-init the mutex
>>  - we should not purge a potentially existing buffer of the main thread
>>  - I don't think we have to re-install the fork hook for the child (the
>>    parent's installation should survive the fork)
>>
>> and...
> 
> ok, I got the points. I'm going to integrate your version.
> Should I use spawn_printer_thread() also in __rt_print_init() or leave
> the 4 lines of code ?
> 

Yes (to the former), avoiding code duplication was the motivation for
this functions.

Another wish: Please split up your changes into two patches: first the
atfork fix, on top of it the syslog extension. That allows us, e.g., to
apply the fix also to 2.4.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
@ 2009-10-09 14:48 oliver.schlenker
  2009-10-09 14:59 ` Jan Kiszka
  0 siblings, 1 reply; 21+ messages in thread
From: oliver.schlenker @ 2009-10-09 14:48 UTC (permalink / raw)
  To: jan.kiszka; +Cc: xenomai




Hi Jan,

thank you for the hints and advices. I'm going to integrate the
changes, as described below,  test it and send new patch.

-------- Original Message --------
Subject: Re: :: rt_printf with daemonized task (09-Okt-2009 15:58)
From:    Jan Kiszka <jan.kiszka@siemens.com>
To:      oliver.schlenker@smmotioncontrol.de

> Hi Oliver,
> 
> > +++ rtdk.h	2009-10-09 11:46:52.767271100 +0200
> >                  
> >  int rt_vprintf(const char *format, va_list args);
> >  int rt_fprintf(FILE *stream, const char *format, ...);
> >  int rt_printf(const char *format, ...);
> > +void rt_syslog( int priority, char *format, ...);
>                  ^^^
> Stray whitespace.

ok, removed.

> >  
> >  #define RT_PRINT_LINE_BREAK		256
> >  
> > +#define RT_PRINT_SYSLOG_STREAM	((FILE *)-9999)
> 
> Better use NULL here.

ok, changed to NULL.

> > format, va_list args)
> 
> Line with >80 characters.

Is there any reason why a line should not exceed 80 chars ?
I did change it anyway.


> >  
> > +int rt_vfprintf(FILE *stream, const char *format, va_list args)
> > +{
> > +	return rt_print_to_buffer( stream, 0, format, args);
> 
> Stray whitespace.

ok, removed.

> > +void rt_vsyslog(int priority, char *format, va_list args )
> 
> Whitespace.

also removed.

> 
> > +{
> > +
> > +	rt_print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
> > +
> > +	return;
> > +}
> > +
> >  static void set_buffer_name(struct print_buffer *buffer, const char *name)
> >  {
> >  	int n;
> >                     
> >  
> >  		if (len) {
> >  			/* Print out non-empty entry and proceed */
> > -			fprintf(head->dest, "%s", head->text);
> > +			/* Check if output goes to syslog */
> > +			if (head->dest == RT_PRINT_SYSLOG_STREAM) {
> > +			   	syslog( head->priority, "%s", head->text );
> 
> Stray whitespaces. And one /may/ argue that we do not need braces here,
> but that's a grey area.

whitespace removed, braces are left (I feel more convienent with braces).

> 
> Here is my version:
> 
> static void spawn_printer_thread(void)
> {
> 	pthread_attr_t thattr;
> 
> 	pthread_attr_init(&thattr);
> 	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
> 	pthread_create(&printer_thread, &thattr, printer_loop, NULL);
> }
> 
> static void forked_child_init(void)
> {
> 	struct print_buffer *my_buffer = pthread_getspecific(buffer_key);
> 	struct print_buffer **pbuffer = &first_buffer;
> 
> 	/* re-init to avoid finding it locked by some parent thread */
> 	pthread_mutex_init(&buffer_lock, NULL);
> 
> 	while (*pbuffer) {
> 		if (*pbuffer == my_buffer)
> 			pbuffer = &(*pbuffer)->next;
> 		else
> 			cleanup_buffer(*pbuffer);
> 	}
> 
> 	spawn_printer_thread();
> }
> 
> The major differences are:
>  - my code is totally untested :)
>  - we have to play safe and re-init the mutex
>  - we should not purge a potentially existing buffer of the main thread
>  - I don't think we have to re-install the fork hook for the child (the
>    parent's installation should survive the fork)
> 
> and...

ok, I got the points. I'm going to integrate your version.
Should I use spawn_printer_thread() also in __rt_print_init() or leave
the 4 lines of code ?


> 
> > +
> > +static void rt_print_reinit_atfork_prepare(void)
> > +{
> > +	struct print_buffer *buffer,*next_b;
> > +
> > +	/* Clean parent process buffer list */
> > +	buffer = __first_buffer;
> > +	while (buffer) {
> > +		next_b = buffer->next;
> > +		cleanup_buffer(buffer);
> > +		buffer = next_b;
> > +	}
> > +}
> > +
> 
>  - we must not destroy the father's environment

ok, rt_print_reinit_atfork_prepare() is removed.



Oliver


To: jan.kiszka@siemens.com
Cc: gilles.chanteperdrix@xenomai.org
    xenomai-help@gna.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
  2009-10-09 13:34 oliver.schlenker
  2009-10-09 13:49 ` Gilles Chanteperdrix
@ 2009-10-09 13:58 ` Jan Kiszka
  1 sibling, 0 replies; 21+ messages in thread
From: Jan Kiszka @ 2009-10-09 13:58 UTC (permalink / raw)
  To: oliver.schlenker; +Cc: xenomai

Hi Oliver,

oliver.schlenker@domain.hid wrote:
> I have done several changes in rt_print.c and rtdk.h to extend
> the functionality of rt-print lib as follows :
> - Fork save initialisation of the library by using pthread_atfork()
>   There are now two hooks integrated :
>     1. A prepare hook which is called to prepare the fork. This
>         hook clears all open print buffers
>     2. A child hook which is called after the fork on the child side.
>        This hook clears also all open print buffers (just ot be on the save side)
>        and is spawning another printer thread.
> 
> - Adding two addition functions
>     1. rt_syslog( int priority, char *format, ...)
>         which reports all messages to the syslog, using the print buffers of rtprint
>     2. rt_vsyslog( int priority, char *format, va_list args )
>         same as above for va_list arguments.
> 
> Both changes do run in my application as expected. It's now possible to use
> rt_print-lib in a daemonized process and rt_print output can be directed
> to syslog via rt_syslog/rt_vsyslog.

Thanks for the patch. I just have a few comments, see below.

> 
> 
> Oliver
> 
> 
> 
> --- rtdk.h.original	2007-12-09 11:46:36.000000000 +0100
> +++ rtdk.h	2009-10-09 11:46:52.767271100 +0200
>                  
>  int rt_vprintf(const char *format, va_list args);
>  int rt_fprintf(FILE *stream, const char *format, ...);
>  int rt_printf(const char *format, ...);
> +void rt_syslog( int priority, char *format, ...);
                 ^^^
Stray whitespace.

> +void rt_vsyslog(int priority, char *format, va_list args);
> +
>  
>  int rt_print_init(size_t buffer_size, const char *name);
>  void rt_print_cleanup(void); 
> 
> 
> --- rt_print.c.original	2008-09-10 10:36:27.000000000 +0200
> +++ rt_print.c	2009-10-09 11:47:16.014443900 +0200
>                  
>  #include <stdlib.h>
>  #include <string.h>
>  #include <unistd.h>
> +#include <syslog.h>
>  
>  #include <rtdk.h>
>  #include <asm/xenomai/system.h>
>                   
>  
>  #define RT_PRINT_LINE_BREAK		256
>  
> +#define RT_PRINT_SYSLOG_STREAM	((FILE *)-9999)

Better use NULL here.

> +
>  struct entry_head {
>  	FILE *dest;
>  	uint32_t seq_no;
> +	int priority;
>  	char text[1];
>  } __attribute__((packed));
>  
>                    
>  
>  static void cleanup_buffer(struct print_buffer *buffer);
>  static void print_buffers(void);
> +static void rt_print_reinit_atfork_prepare(void);
> +static void rt_print_reinit_atfork_child(void);
>  
>  /* *** rt_print API *** */
>  
> -int rt_vfprintf(FILE *stream, const char *format, va_list args)
> +static int rt_print_to_buffer(FILE *stream, int priority, const char *format, va_list args)

Line with >80 characters.

>  {
>  	struct print_buffer *buffer = pthread_getspecific(__buffer_key);
>  	off_t write_pos, read_pos;
>                    
>  			/* Write out empty entry */
>  			head = buffer->ring + write_pos;
>  			head->seq_no = __seq_no;
> +			head->priority = 0;
>  			head->text[0] = 0;
>  
>  			/* Forward to the ring buffer start */
>                    
>  	/* If we were able to write some text, finalise the entry */
>  	if (len > 0) {
>  		head->seq_no = ++__seq_no;
> +		head->priority = priority;
>  		head->dest = stream;
>  
>  		/* Move forward by text and head length */
>                    
>  		/* An empty entry marks the wrap-around */
>  		head = buffer->ring + write_pos;
>  		head->seq_no = __seq_no;
> +		head->priority = priority;
>  		head->text[0] = 0;
>  
>  		write_pos = 0;
>                     
>  	return res;
>  }
>  
> +int rt_vfprintf(FILE *stream, const char *format, va_list args)
> +{
> +	return rt_print_to_buffer( stream, 0, format, args);

Stray whitespace.

> +}
> +
>  int rt_vprintf(const char *format, va_list args)
>  {
>  	return rt_vfprintf(stdout, format, args);
>                     
>  	return n;
>  }
>  
> +void rt_syslog(int priority, char *format, ...)
> +{
> +	va_list args;
> +
> +	va_start(args, format);
> +	rt_print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
> +	va_end(args);
> +
> +	return;
> +}
> +
> +void rt_vsyslog(int priority, char *format, va_list args )

Whitespace.

> +{
> +
> +	rt_print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
> +
> +	return;
> +}
> +
>  static void set_buffer_name(struct print_buffer *buffer, const char *name)
>  {
>  	int n;
>                     
>  
>  		if (len) {
>  			/* Print out non-empty entry and proceed */
> -			fprintf(head->dest, "%s", head->text);
> +			/* Check if output goes to syslog */
> +			if (head->dest == RT_PRINT_SYSLOG_STREAM) {
> +			   	syslog( head->priority, "%s", head->text );

Stray whitespaces. And one /may/ argue that we do not need braces here,
but that's a grey area.

> +			} else {
> +				/* Output goes to specified stream */
> +				fprintf(head->dest, "%s", head->text);
> +			}
> +
>  			read_pos += sizeof(*head) + len;
>  		} else {
>  			/* Emptry entries mark the wrap-around */
>                     
>  	}
>  }
>  
> +static void rt_print_reinit_atfork_child(void)
> +{
> +	pthread_attr_t thattr;
> +	struct print_buffer *buffer,*next_b;
> +
> +	/* Clean parent process buffer list -
> +	   Shouldn't be necessary because also done at atfork_parent
> +	*/
> +	buffer = __first_buffer;
> +	while (buffer) {
> +		next_b = buffer->next;
> +		cleanup_buffer(buffer);
> +		buffer = next_b;
> +	}
> +
> +	/* Throw new thread with printer loop in child process */
> +	pthread_attr_init(&thattr);
> +	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
> +	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
> +	pthread_atfork(rt_print_reinit_atfork_prepare,NULL,rt_print_reinit_atfork_child);
> +}

Here is my version:

static void spawn_printer_thread(void)
{
	pthread_attr_t thattr;

	pthread_attr_init(&thattr);
	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
	pthread_create(&printer_thread, &thattr, printer_loop, NULL);
}

static void forked_child_init(void)
{
	struct print_buffer *my_buffer = pthread_getspecific(buffer_key);
	struct print_buffer **pbuffer = &first_buffer;

	/* re-init to avoid finding it locked by some parent thread */
	pthread_mutex_init(&buffer_lock, NULL);

	while (*pbuffer) {
		if (*pbuffer == my_buffer)
			pbuffer = &(*pbuffer)->next;
		else
			cleanup_buffer(*pbuffer);
	}

	spawn_printer_thread();
}

The major differences are:
 - my code is totally untested :)
 - we have to play safe and re-init the mutex
 - we should not purge a potentially existing buffer of the main thread
 - I don't think we have to re-install the fork hook for the child (the
   parent's installation should survive the fork)

and...

> +
> +static void rt_print_reinit_atfork_prepare(void)
> +{
> +	struct print_buffer *buffer,*next_b;
> +
> +	/* Clean parent process buffer list */
> +	buffer = __first_buffer;
> +	while (buffer) {
> +		next_b = buffer->next;
> +		cleanup_buffer(buffer);
> +		buffer = next_b;
> +	}
> +}
> +

 - we must not destroy the father's environment

>  void __rt_print_init(void)
>  {
>  	pthread_attr_t thattr;
>                    
>  	pthread_attr_init(&thattr);
>  	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
>  	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
> +	pthread_atfork(rt_print_reinit_atfork_prepare,NULL,rt_print_reinit_atfork_child);
>  } 
> 

Besides that, nice patch!

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
@ 2009-10-09 13:54 oliver.schlenker
  0 siblings, 0 replies; 21+ messages in thread
From: oliver.schlenker @ 2009-10-09 13:54 UTC (permalink / raw)
  To: gilles.chanteperdrix; +Cc: jan.kiszka, xenomai






-------- Original Message --------
Subject: Re: :: rt_printf with daemonized task (09-Okt-2009 15:49)
From:    Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>
To:      oliver.schlenker@smmotioncontrol.de

> oliver.schlenker@smmotioncontrol.de wrote:
> > +#define RT_PRINT_SYSLOG_STREAM	((FILE *)-9999)
> 
> -9999 may be a valid pointer. Only values between -4096 and 4095 can not
> be valid pointers.
> 
> -- 
>                                           Gilles
> 

ok, did change it to -99.



Oliver



--- rt_print.c.original	2008-09-10 10:36:27.000000000 +0200
+++ rt_print.c	2009-10-09 15:52:07.404908000 +0200
                 
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <syslog.h>
 
 #include <rtdk.h>
 #include <asm/xenomai/system.h>
                  
 
 #define RT_PRINT_LINE_BREAK		256
 
+#define RT_PRINT_SYSLOG_STREAM	((FILE *)-99)
+
 struct entry_head {
 	FILE *dest;
 	uint32_t seq_no;
+	int priority;
 	char text[1];
 } __attribute__((packed));
 
                   
 
 static void cleanup_buffer(struct print_buffer *buffer);
 static void print_buffers(void);
+static void rt_print_reinit_atfork_prepare(void);
+static void rt_print_reinit_atfork_child(void);
 
 /* *** rt_print API *** */
 
-int rt_vfprintf(FILE *stream, const char *format, va_list args)
+static int rt_print_to_buffer(FILE *stream, int priority, const char *format, va_list args)
 {
 	struct print_buffer *buffer = pthread_getspecific(__buffer_key);
 	off_t write_pos, read_pos;
                   
 			/* Write out empty entry */
 			head = buffer->ring + write_pos;
 			head->seq_no = __seq_no;
+			head->priority = 0;
 			head->text[0] = 0;
 
 			/* Forward to the ring buffer start */
                   
 	/* If we were able to write some text, finalise the entry */
 	if (len > 0) {
 		head->seq_no = ++__seq_no;
+		head->priority = priority;
 		head->dest = stream;
 
 		/* Move forward by text and head length */
                   
 		/* An empty entry marks the wrap-around */
 		head = buffer->ring + write_pos;
 		head->seq_no = __seq_no;
+		head->priority = priority;
 		head->text[0] = 0;
 
 		write_pos = 0;
                    
 	return res;
 }
 
+int rt_vfprintf(FILE *stream, const char *format, va_list args)
+{
+	return rt_print_to_buffer( stream, 0, format, args);
+}
+
 int rt_vprintf(const char *format, va_list args)
 {
 	return rt_vfprintf(stdout, format, args);
                    
 	return n;
 }
 
+void rt_syslog(int priority, char *format, ...)
+{
+	va_list args;
+
+	va_start(args, format);
+	rt_print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
+	va_end(args);
+
+	return;
+}
+
+void rt_vsyslog(int priority, char *format, va_list args )
+{
+
+	rt_print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
+
+	return;
+}
+
 static void set_buffer_name(struct print_buffer *buffer, const char *name)
 {
 	int n;
                    
 
 		if (len) {
 			/* Print out non-empty entry and proceed */
-			fprintf(head->dest, "%s", head->text);
+			/* Check if output goes to syslog */
+			if (head->dest == RT_PRINT_SYSLOG_STREAM) {
+			   	syslog( head->priority, "%s", head->text );
+			} else {
+				/* Output goes to specified stream */
+				fprintf(head->dest, "%s", head->text);
+			}
+
 			read_pos += sizeof(*head) + len;
 		} else {
 			/* Emptry entries mark the wrap-around */
                    
 	}
 }
 
+static void rt_print_reinit_atfork_child(void)
+{
+	pthread_attr_t thattr;
+	struct print_buffer *buffer,*next_b;
+
+	/* Clean parent process buffer list -
+	   Shouldn't be necessary because also done at atfork_parent
+	*/
+	buffer = __first_buffer;
+	while (buffer) {
+		next_b = buffer->next;
+		cleanup_buffer(buffer);
+		buffer = next_b;
+	}
+
+	/* Throw new thread with printer loop in child process */
+	pthread_attr_init(&thattr);
+	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
+	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
+	pthread_atfork(rt_print_reinit_atfork_prepare,NULL,rt_print_reinit_atfork_child);
+}
+
+static void rt_print_reinit_atfork_prepare(void)
+{
+	struct print_buffer *buffer,*next_b;
+
+	/* Clean parent process buffer list */
+	buffer = __first_buffer;
+	while (buffer) {
+		next_b = buffer->next;
+		cleanup_buffer(buffer);
+		buffer = next_b;
+	}
+}
+
 void __rt_print_init(void)
 {
 	pthread_attr_t thattr;
                   
 	pthread_attr_init(&thattr);
 	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
 	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
+	pthread_atfork(rt_print_reinit_atfork_prepare,NULL,rt_print_reinit_atfork_child);
 } 

To: gilles.chanteperdrix@xenomai.org
Cc: jan.kiszka@siemens.com
    xenomai-help@gna.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
  2009-10-09 13:34 oliver.schlenker
@ 2009-10-09 13:49 ` Gilles Chanteperdrix
  2009-10-09 13:58 ` Jan Kiszka
  1 sibling, 0 replies; 21+ messages in thread
From: Gilles Chanteperdrix @ 2009-10-09 13:49 UTC (permalink / raw)
  To: oliver.schlenker; +Cc: jan.kiszka, xenomai

oliver.schlenker@domain.hid wrote:
> +#define RT_PRINT_SYSLOG_STREAM	((FILE *)-9999)

-9999 may be a valid pointer. Only values between -4096 and 4095 can not
be valid pointers.

-- 
                                          Gilles



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
@ 2009-10-09 13:34 oliver.schlenker
  2009-10-09 13:49 ` Gilles Chanteperdrix
  2009-10-09 13:58 ` Jan Kiszka
  0 siblings, 2 replies; 21+ messages in thread
From: oliver.schlenker @ 2009-10-09 13:34 UTC (permalink / raw)
  To: jan.kiszka; +Cc: xenomai




-------- Original Message --------
Subject: Re: :: rt_printf with daemonized task (08-Okt-2009 15:17)
From:    Jan Kiszka <jan.kiszka@siemens.com>
To:      oliver.schlenker@smmotioncontrol.de

> oliver.schlenker@smmotioncontrol.de wrote:
> > 
> > 
> > 
> > 
> > -------- Original Message --------
> > Subject: Re: Re : rt_printf with daemonized task (08-Okt-2009 12:16)
> > From:    Jan Kiszka <jan.kiszka@siemens.com>
> > To:      oliver.schlenker@smmotioncontrol.de
> > 
> >> Gilles Chanteperdrix wrote:
> >>> Jan Kiszka wrote:
> >>>> Gilles Chanteperdrix wrote:
> >>>>> oliver.schlenker@smmotioncontrol.de wrote:
> >>>>>>>> int main( int arc, char *argv[] ) { int i;
> >>>>>>>>
> >>>>>>>> rt_print_auto_init(1);
> >>>>>>>>
> >>>>>>>> rt_printf("--------------- TEST RT-PRINTF  1 ------------\n");
> >>>>>>>>
> >>>>>>>> sleep(1);
> >>>>>>>>
> >>>>>>>> daemon(0,0);
> >>>>>>>>
> >>>>>>>> rt_print_auto_init(1);
> >>>>>> Ok, understood, at least for the scenario where the rt_print feature
> >>>>>> is initialised befor the fork/daemon call. What I don't understand
> >>>>>> is, why it does not work if the rt_print feature is initialised 
> after
> >>>>>> the fork / daemon.
> >>>>> From the way I understand your code, you never tried to initialize 
> the
> >>>>> rt_print feature only after the fork, your code initializes it both
> >>>>> before and after.
> >>>>>
> >>>> There are two initializations: The base init done via __rt_print_init 
> on
> >>>> library loading and the one to be done per-thread via rt_print_init (
> or
> >>>> on first rt_printf). That printer thread is initialized via the former
> >>>> one. On fork, we do not need to re-run the full __rt_print_init
> >>>> (variables and resources are cloned on fork), we just need to spawn
> >>>> another printer thread.
> >>> Unless I am wrong, rtdk also maintains a list of the thread buffers
> >>> which need to be polled. After the fork, this list will be intact, but
> >>> the threads to which belong the buffers will no longer exist. So, IMO,
> >>> the fork handler should also free all these buffers and reset the list
> >>> to the empty state.
> >> Famous last words: That should work without tweaking. A print_buffer
> >> only contains data references, nothing that points to some uncloned 
> thread.
> >>
> >> Jan
> >>
> >> -- 
> >> Siemens AG, Corporate Technology, CT SE 2
> >> Corporate Competence Center Embedded Linux
> > 
> > To have a fork save rtprint library it's necessary to
> > 
> > 1. Reset rtdk thread buffer list
> >    
> >     I have no idea where to look at for the rtdk thread buffer list. Inside 
> > rt_print.c ?
> 
> It's all there, it's called first_buffer. Just walk that list and call
> cleanup_buffers on the entries.
> 
> >    
> > 2. Spawning another printer thread
> >     with code like :
> > 	pthread_attr_init(&thattr);
> > 	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
> > 	pthread_create(&__printer_thread, &thattr, printer_loop, NULL); 
> > 
> > 
> > Is there anything else to do ?
> > 
> 
> Probably not, but I was already wrong before. :)
> 
> > 
> > I'm also planning to add a function to the lib like :
> > 
> > rt_print_to_syslog( int priority )
> 
> Better provide some rt_syslog(int priority, char *format, ...),
> corresponding to the standard service.
> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT SE 2
> Corporate Competence Center Embedded Linux


I have done several changes in rt_print.c and rtdk.h to extend
the functionality of rt-print lib as follows :
- Fork save initialisation of the library by using pthread_atfork()
  There are now two hooks integrated :
    1. A prepare hook which is called to prepare the fork. This
        hook clears all open print buffers
    2. A child hook which is called after the fork on the child side.
       This hook clears also all open print buffers (just ot be on the save side)
       and is spawning another printer thread.

- Adding two addition functions
    1. rt_syslog( int priority, char *format, ...)
        which reports all messages to the syslog, using the print buffers of rtprint
    2. rt_vsyslog( int priority, char *format, va_list args )
        same as above for va_list arguments.

Both changes do run in my application as expected. It's now possible to use
rt_print-lib in a daemonized process and rt_print output can be directed
to syslog via rt_syslog/rt_vsyslog.


Oliver



--- rtdk.h.original	2007-12-09 11:46:36.000000000 +0100
+++ rtdk.h	2009-10-09 11:46:52.767271100 +0200
                 
 int rt_vprintf(const char *format, va_list args);
 int rt_fprintf(FILE *stream, const char *format, ...);
 int rt_printf(const char *format, ...);
+void rt_syslog( int priority, char *format, ...);
+void rt_vsyslog(int priority, char *format, va_list args);
+
 
 int rt_print_init(size_t buffer_size, const char *name);
 void rt_print_cleanup(void); 


--- rt_print.c.original	2008-09-10 10:36:27.000000000 +0200
+++ rt_print.c	2009-10-09 11:47:16.014443900 +0200
                 
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <syslog.h>
 
 #include <rtdk.h>
 #include <asm/xenomai/system.h>
                  
 
 #define RT_PRINT_LINE_BREAK		256
 
+#define RT_PRINT_SYSLOG_STREAM	((FILE *)-9999)
+
 struct entry_head {
 	FILE *dest;
 	uint32_t seq_no;
+	int priority;
 	char text[1];
 } __attribute__((packed));
 
                   
 
 static void cleanup_buffer(struct print_buffer *buffer);
 static void print_buffers(void);
+static void rt_print_reinit_atfork_prepare(void);
+static void rt_print_reinit_atfork_child(void);
 
 /* *** rt_print API *** */
 
-int rt_vfprintf(FILE *stream, const char *format, va_list args)
+static int rt_print_to_buffer(FILE *stream, int priority, const char *format, va_list args)
 {
 	struct print_buffer *buffer = pthread_getspecific(__buffer_key);
 	off_t write_pos, read_pos;
                   
 			/* Write out empty entry */
 			head = buffer->ring + write_pos;
 			head->seq_no = __seq_no;
+			head->priority = 0;
 			head->text[0] = 0;
 
 			/* Forward to the ring buffer start */
                   
 	/* If we were able to write some text, finalise the entry */
 	if (len > 0) {
 		head->seq_no = ++__seq_no;
+		head->priority = priority;
 		head->dest = stream;
 
 		/* Move forward by text and head length */
                   
 		/* An empty entry marks the wrap-around */
 		head = buffer->ring + write_pos;
 		head->seq_no = __seq_no;
+		head->priority = priority;
 		head->text[0] = 0;
 
 		write_pos = 0;
                    
 	return res;
 }
 
+int rt_vfprintf(FILE *stream, const char *format, va_list args)
+{
+	return rt_print_to_buffer( stream, 0, format, args);
+}
+
 int rt_vprintf(const char *format, va_list args)
 {
 	return rt_vfprintf(stdout, format, args);
                    
 	return n;
 }
 
+void rt_syslog(int priority, char *format, ...)
+{
+	va_list args;
+
+	va_start(args, format);
+	rt_print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
+	va_end(args);
+
+	return;
+}
+
+void rt_vsyslog(int priority, char *format, va_list args )
+{
+
+	rt_print_to_buffer(RT_PRINT_SYSLOG_STREAM, priority, format, args);
+
+	return;
+}
+
 static void set_buffer_name(struct print_buffer *buffer, const char *name)
 {
 	int n;
                    
 
 		if (len) {
 			/* Print out non-empty entry and proceed */
-			fprintf(head->dest, "%s", head->text);
+			/* Check if output goes to syslog */
+			if (head->dest == RT_PRINT_SYSLOG_STREAM) {
+			   	syslog( head->priority, "%s", head->text );
+			} else {
+				/* Output goes to specified stream */
+				fprintf(head->dest, "%s", head->text);
+			}
+
 			read_pos += sizeof(*head) + len;
 		} else {
 			/* Emptry entries mark the wrap-around */
                    
 	}
 }
 
+static void rt_print_reinit_atfork_child(void)
+{
+	pthread_attr_t thattr;
+	struct print_buffer *buffer,*next_b;
+
+	/* Clean parent process buffer list -
+	   Shouldn't be necessary because also done at atfork_parent
+	*/
+	buffer = __first_buffer;
+	while (buffer) {
+		next_b = buffer->next;
+		cleanup_buffer(buffer);
+		buffer = next_b;
+	}
+
+	/* Throw new thread with printer loop in child process */
+	pthread_attr_init(&thattr);
+	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
+	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
+	pthread_atfork(rt_print_reinit_atfork_prepare,NULL,rt_print_reinit_atfork_child);
+}
+
+static void rt_print_reinit_atfork_prepare(void)
+{
+	struct print_buffer *buffer,*next_b;
+
+	/* Clean parent process buffer list */
+	buffer = __first_buffer;
+	while (buffer) {
+		next_b = buffer->next;
+		cleanup_buffer(buffer);
+		buffer = next_b;
+	}
+}
+
 void __rt_print_init(void)
 {
 	pthread_attr_t thattr;
                   
 	pthread_attr_init(&thattr);
 	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
 	pthread_create(&__printer_thread, &thattr, printer_loop, NULL);
+	pthread_atfork(rt_print_reinit_atfork_prepare,NULL,rt_print_reinit_atfork_child);
 } 




To: jan.kiszka@siemens.com
Cc: gilles.chanteperdrix@xenomai.org
    xenomai-help@gna.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
  2009-10-08 12:01 [Xenomai-help] :: " oliver.schlenker
@ 2009-10-08 13:17 ` Jan Kiszka
  0 siblings, 0 replies; 21+ messages in thread
From: Jan Kiszka @ 2009-10-08 13:17 UTC (permalink / raw)
  To: oliver.schlenker; +Cc: xenomai

oliver.schlenker@domain.hid wrote:
> 
> 
> 
> 
> -------- Original Message --------
> Subject: Re: Re : rt_printf with daemonized task (08-Okt-2009 12:16)
> From:    Jan Kiszka <jan.kiszka@domain.hid>
> To:      oliver.schlenker@domain.hid
> 
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> oliver.schlenker@domain.hid wrote:
>>>>>>>> int main( int arc, char *argv[] ) { int i;
>>>>>>>>
>>>>>>>> rt_print_auto_init(1);
>>>>>>>>
>>>>>>>> rt_printf("--------------- TEST RT-PRINTF  1 ------------\n");
>>>>>>>>
>>>>>>>> sleep(1);
>>>>>>>>
>>>>>>>> daemon(0,0);
>>>>>>>>
>>>>>>>> rt_print_auto_init(1);
>>>>>> Ok, understood, at least for the scenario where the rt_print feature
>>>>>> is initialised befor the fork/daemon call. What I don't understand
>>>>>> is, why it does not work if the rt_print feature is initialised after
>>>>>> the fork / daemon.
>>>>> From the way I understand your code, you never tried to initialize the
>>>>> rt_print feature only after the fork, your code initializes it both
>>>>> before and after.
>>>>>
>>>> There are two initializations: The base init done via __rt_print_init on
>>>> library loading and the one to be done per-thread via rt_print_init (or
>>>> on first rt_printf). That printer thread is initialized via the former
>>>> one. On fork, we do not need to re-run the full __rt_print_init
>>>> (variables and resources are cloned on fork), we just need to spawn
>>>> another printer thread.
>>> Unless I am wrong, rtdk also maintains a list of the thread buffers
>>> which need to be polled. After the fork, this list will be intact, but
>>> the threads to which belong the buffers will no longer exist. So, IMO,
>>> the fork handler should also free all these buffers and reset the list
>>> to the empty state.
>> Famous last words: That should work without tweaking. A print_buffer
>> only contains data references, nothing that points to some uncloned thread.
>>
>> Jan
>>
>> -- 
>> Siemens AG, Corporate Technology, CT SE 2
>> Corporate Competence Center Embedded Linux
> 
> To have a fork save rtprint library it's necessary to
> 
> 1. Reset rtdk thread buffer list
>    
>     I have no idea where to look at for the rtdk thread buffer list. Inside rt_print.c ?

It's all there, it's called first_buffer. Just walk that list and call
cleanup_buffers on the entries.

>    
> 2. Spawning another printer thread
>     with code like :
> 	pthread_attr_init(&thattr);
> 	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
> 	pthread_create(&__printer_thread, &thattr, printer_loop, NULL); 
> 
> 
> Is there anything else to do ?
> 

Probably not, but I was already wrong before. :)

> 
> I'm also planning to add a function to the lib like :
> 
> rt_print_to_syslog( int priority )

Better provide some rt_syslog(int priority, char *format, ...),
corresponding to the standard service.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] :: rt_printf with daemonized task
@ 2009-10-08 12:01 oliver.schlenker
  2009-10-08 13:17 ` Jan Kiszka
  0 siblings, 1 reply; 21+ messages in thread
From: oliver.schlenker @ 2009-10-08 12:01 UTC (permalink / raw)
  To: jan.kiszka, gilles.chanteperdrix; +Cc: xenomai






-------- Original Message --------
Subject: Re: Re : rt_printf with daemonized task (08-Okt-2009 12:16)
From:    Jan Kiszka <jan.kiszka@siemens.com>
To:      oliver.schlenker@smmotioncontrol.de

> Gilles Chanteperdrix wrote:
> > Jan Kiszka wrote:
> >> Gilles Chanteperdrix wrote:
> >>> oliver.schlenker@smmotioncontrol.de wrote:
> >>>>>> int main( int arc, char *argv[] ) { int i;
> >>>>>>
> >>>>>> rt_print_auto_init(1);
> >>>>>>
> >>>>>> rt_printf("--------------- TEST RT-PRINTF  1 ------------\n");
> >>>>>>
> >>>>>> sleep(1);
> >>>>>>
> >>>>>> daemon(0,0);
> >>>>>>
> >>>>>> rt_print_auto_init(1);
> >>>> Ok, understood, at least for the scenario where the rt_print feature
> >>>> is initialised befor the fork/daemon call. What I don't understand
> >>>> is, why it does not work if the rt_print feature is initialised after
> >>>> the fork / daemon.
> >>> From the way I understand your code, you never tried to initialize the
> >>> rt_print feature only after the fork, your code initializes it both
> >>> before and after.
> >>>
> >> There are two initializations: The base init done via __rt_print_init on
> >> library loading and the one to be done per-thread via rt_print_init (or
> >> on first rt_printf). That printer thread is initialized via the former
> >> one. On fork, we do not need to re-run the full __rt_print_init
> >> (variables and resources are cloned on fork), we just need to spawn
> >> another printer thread.
> > 
> > Unless I am wrong, rtdk also maintains a list of the thread buffers
> > which need to be polled. After the fork, this list will be intact, but
> > the threads to which belong the buffers will no longer exist. So, IMO,
> > the fork handler should also free all these buffers and reset the list
> > to the empty state.
> 
> Famous last words: That should work without tweaking. A print_buffer
> only contains data references, nothing that points to some uncloned thread.
> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT SE 2
> Corporate Competence Center Embedded Linux

To have a fork save rtprint library it's necessary to

1. Reset rtdk thread buffer list
   
    I have no idea where to look at for the rtdk thread buffer list. Inside rt_print.c ?
   
2. Spawning another printer thread
    with code like :
	pthread_attr_init(&thattr);
	pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
	pthread_create(&__printer_thread, &thattr, printer_loop, NULL); 


Is there anything else to do ?


I'm also planning to add a function to the lib like :

rt_print_to_syslog( int priority )
If priority is zero, all messages are going to fprintf() output.
If priority is non-zero, all messages are going to syslog( priority, <message> ) output.

Is it ok to add therefor a new variable to struct print_buffer ?


If everything is coming to work, I would like to send it as a patch.



Oliver








To: jan.kiszka@siemens.com
    gilles.chanteperdrix@xenomai.org
Cc: xenomai-help@gna.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Xenomai-help] rt_printf with daemonized task
@ 2009-10-08  7:15 oliver.schlenker
  0 siblings, 0 replies; 21+ messages in thread
From: oliver.schlenker @ 2009-10-08  7:15 UTC (permalink / raw)
  To: gilles.chanteperdrix; +Cc: xenomai






-------- Original Message --------
Subject: Re: Re-8: [Xenomai-help] rt_printf with daemonized task (07-Okt-2009 17:24)
From:    Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>
To:      oliver.schlenker@smmotioncontrol.de

> oliver.schlenker@smmotioncontrol.de wrote:
> > 
> > 
> > As you expected, changing the syslog call from
> >     syslog( LOG_ERR | LOG_LOCAL4, head->text ); 
> > to
> >     syslog( LOG_ERR | LOG_LOCAL4, "%s", head->text ); 
> > 
> > did not change anything.
> > 
> > To track down the problem further, I have made a small test application :
> > 
> > #include <stdlib.h>
> > #include <stdio.h>
> > 
> > #include <native/task.h>
> > #include <native/timer.h>
> > #include <rtdk.h>
> > 
> > 
> > int main( int arc, char *argv[] )
> > {
> > int i;
> > 
> > rt_print_auto_init(1);
> > 
> > rt_printf("--------------- TEST RT-PRINTF  1 ------------\n");
> > 
> > 	sleep(1);
> > 
> > daemon(0,0);
> > 
> > 	rt_print_auto_init(1);
> > 
> > 	rt_printf("--------------- TEST RT-PRINTF  2 ------------\n");
> > 
> > 	sleep(10);
> > 
> > return(0);
> > }
> > 
> > 
> > 
> > This application gives the following results :
> > - If the daemon() function is not called, all rt_printf() do work well
> >   and output is shown in syslog
> > - If daemon() function is called, and first call to sleep(1) is omitted,
> >   there is no output in the syslog whatsoever.
> > - If daemon() function is called, and first call to sleep(1) is executed,
> >   there is output of the first rt_printf in the syslog but not of the 
> > second
> >   one.
> > 
> > I do not have any idea why daemonizing of the process is not giving
> > any output to the syslog.
> 
> Could you try daemon(0, 1)?

No change.

> 

First of all, I would like to apologize for the inconvenience I put on you. I wasn't
aware of this situation.

> Could you also try to send normal replies to our replies, not mails
> which do not appear as replies and start new threads?

Hopefully with this mail it did change. If not please let me know what to do.

> 
> Could you also remove the annoying feature that ask me to confirm that I
> want you to be notified that I read your mail?

Feature should now be turned off, please let me also know, if not.

> 
> -- 
>                                           Gilles
> 


Oliver


To: gilles.chanteperdrix@xenomai.org
Cc: xenomai-help@gna.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2009-10-20 16:11 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-06 15:10 [Xenomai-help] rt_printf with daemonized task oliver.schlenker
2009-10-06 15:16 ` Gilles Chanteperdrix
2009-10-08  7:15 oliver.schlenker
2009-10-08 12:01 [Xenomai-help] :: " oliver.schlenker
2009-10-08 13:17 ` Jan Kiszka
2009-10-09 13:34 oliver.schlenker
2009-10-09 13:49 ` Gilles Chanteperdrix
2009-10-09 13:58 ` Jan Kiszka
2009-10-09 13:54 oliver.schlenker
2009-10-09 14:48 oliver.schlenker
2009-10-09 14:59 ` Jan Kiszka
2009-10-19 13:12 oliver.schlenker
2009-10-19 14:00 ` Jan Kiszka
2009-10-19 13:14 oliver.schlenker
2009-10-19 14:00 ` Jan Kiszka
2009-10-20  9:26 oliver.schlenker
2009-10-20  9:33 ` Jan Kiszka
2009-10-20 10:03 oliver.schlenker
2009-10-20 16:01 ` Gilles Chanteperdrix
2009-10-20 16:07   ` Jan Kiszka
2009-10-20 16:11 ` Jan Kiszka

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.