please see comments below...

>
> Another approach is about dropping the non-atomic update sequence
> that hurts, tolerating
> null runs of the virq when the seldom preemption case is seen, but
> without requiring hw
> interrupt masking to protect the shared stuff. Livelocking Linux
> inside the virq handler
> would still be possible whenever the RT side spams the kernel log,
> but this would not be
> an issue for us, since there is no such thing as a fair real-time
> system anyway.
>
> --- kernel/printk.c   2 Nov 2005 16:29:34 -0000   1.2
> +++ kernel/printk.c   15 Nov 2005 09:11:33 -0000
> @@ -511,24 +511,23 @@
>
>   static ipipe_spinlock_t __ipipe_printk_lock = IPIPE_SPIN_LOCK_UNLOCKED;
>
> -static int __ipipe_printk_fill;
> +static atomic_t __ipipe_printk_fill;
>
>   static char __ipipe_printk_buf[__LOG_BUF_LEN];
>
>   void __ipipe_flush_printk (unsigned virq)
>   {
>      char *p = __ipipe_printk_buf;
> -   int out = 0, len;
> +   int len;
>
> -   clear_bit(IPIPE_PPRINTK_FLAG,&ipipe_root_domain->flags);
> -
> -   while (out < __ipipe_printk_fill) {
> -      len = strlen(p) + 1;
> -      printk("%s",p);
> -      p += len;
> -      out += len;
> +   while (test_and_clear_bit(IPIPE_PPRINTK_FLAG,&ipipe_root_domain->flags)) {
> +      while (atomic_read(&__ipipe_printk_fill) > 0) {
> +         len = strlen(p) + 1;
> +         printk("%s",p);
> +         p += len;

> +         atomic_sub(len,&__ipipe_printk_fill);

But that's wrong. __ipipe_printk_fill is used as an index in printk() and here you use it as a number of bytes still need to be printed.

let's suppose:

p = __ipipe_printk_buf = [ 5 bytes \0] [10 bytes \0]
__ipipe_printk_fill == 17;

1) first iteration of __ipipe_flush_printk

5-byte string is printed out

p = __ipipe_printk_buf + 6 ---> [10 bytes \0]
__ipipe_printk_fill == 11;

...

In the mean time, on another CPU:

2) printk() does

> +      r = vscnprintf(__ipipe_printk_buf + atomic_read(&__ipipe_printk_fill),
>                   fbytes, fmt, args) + 1; /* account for the null byte */

__ipipe_printk_buf + atomic_read(&__ipipe_printk_fill) == __ipipe_printk_buf + 11

and that points right in the middle of the still non-printed 10-bytes long string [5 bytes \0] [10 bytes (*** this will be broken ***) \0]

So we are loosing data again.

is something wrong? anyway, there must be a small correction somewhere around :)

> +      }
>      }
> -   __ipipe_printk_fill = 0;
>   }
>
>   asmlinkage int printk(const char *fmt, ...)
> @@ -548,12 +547,12 @@
>
>      spin_lock_irqsave_hw(&__ipipe_printk_lock,flags);
>
> -   fbytes = __LOG_BUF_LEN - __ipipe_printk_fill;
> +   fbytes = __LOG_BUF_LEN - atomic_read(&__ipipe_printk_fill);
>
>      if (fbytes > 1)   {
> -      r = vscnprintf(__ipipe_printk_buf + __ipipe_printk_fill,
> +      r = vscnprintf(__ipipe_printk_buf + atomic_read(&__ipipe_printk_fill),
>                   fbytes, fmt, args) + 1; /* account for the null byte */
> -      __ipipe_printk_fill += r;
> +      atomic_add(r,&__ipipe_printk_fill);
>      } else
>         r = 0;
>

> --
>
> Philippe.

---

Dmitry

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

Reply via email to