Dmitry Adamushko wrote:
> Hi Jan,
> 
>> some news from the testing front: It works fairly well - and it doesn't
>> crash =:).
> 
> Good news indeed :)
> 
>> We set up a quite demanding test scenario which consists of
>> two Sick Laser scanners feeding two UART ports at 500 Kbit/s. The UARTs
>> are on a special PC104 card, sharing the same edge-triggered IRQ line.
>> We were able to get data from both devices running at the same time. But
>> we noticed some overruns of MAX_EDGEIRQ_COUNTER (a few per second). The
>> next step on Monday will be to generate a back-trace with the
>> ipipe-tracer to see if the system is "just" overloaded or if we are
>> still facing problems with the driver and/or IRQ layer. Will be very
>> interesting to see on that radar what's happing.
> 
> Yes.
> 
> And as an additional option,
> it could be interesting to print out to the log if not all "counter" values
> then min,max,average (the same like for the latency :) per second or per
> 1000 interrupts; so to see whether tweaking the MAX_EDGEIRQ_COUNTER may make
> the things better.

Yes, maybe it's too small. But this also depends on the absolute time
required for so many loops, something we should see in the traces then.
I'm afraid that we will finally have to move the UART's read-from-fifo
to task context to reduce the time spent in IRQ context.

> 
> 
>> I attached two patches. One enables xeno_16550A to use the new features,
>> and the other improves the /proc output of your patch slightly.
>>
>> Furthermore, we noticed that virtual IRQs (namely the printk forwarder)
>> get displayed under /proc/xenomai/irq too. Is this useful? We wondered
>> what IRQ 34 might be until code analysis of Xenomai and Ipipe revealed
>> it (__ipipe_printk_virq). If it is considered useful, we should at least
>> mark those irqs virtual in the output or even give them names as well.
> 
> It's a virq but it can not be __ipipe_printk_virq :)
> The latter one is registered in the linux domain while we are
> printing out the irqs of the primary one in /proc/xenomai/irq.
> 
> It should be xnarch_escalation_virq.
> 
> Probably, BASE_VIRQ == 32 (if NR_IRQS == 16).
> 
> So,
> 
> 32 -    __ipipe_printk_virq (Linux domain)
> 33 -    rthal_apc_virq (Linux domain)
> 34 -    xnarch_escalation_virq (Xeno domain)

Ok, I stopped digging deeper after noticing the first VIRQ being
registered to the dumped IRQ list.

> 
> They are ordered (that's why they have such irq numbers) as
> ipipe_alloc_virq()
> is called for them.
> 
> That can be confirmed by :
> 
> # cat /proc/ipipe/Xenomai
> 
> ...
> irq 32-33: passed, virtual
> irq 34: grabbed, virtual    <--- 1 virq is in the primary domain
> ...
> 
> # cat /proc/ipipe/Linux
> 
> irq 32-33: grabbed, virtual    <--- 2 virqs are in the linux domain
> irq 34: passed, virtual

Oh, yes. The information was so close and yet I didn't see it. :)

> 
> 
> Yep, the output is confusing. At least, we may :
> 
> 1) add "virq" or "virtual" quilification, smth like 34 (virtual);
> 
> 2) not print them out at all.
> 
> Maybe we'd better go for 1, just in case somebody would like to know
> the actual number of virq occuried. Although, I can imagine that only
> for debugging reasons and of no avail for the normal users, so maybe 2 :)

I have no final opinion yet. Having some names also behind the virtual
IRQs would be really nice for debugging, but it would also break another
bulk of API calls. Just marking those IRQs virtual would render the
related output of /proc/xenomai/irq not very useful in my eye - as long
as you do not know what's behind it. Just dropping this information is
then likely better.

> 
> 
>> Then I stumbled over the xnintr structure. Why do you keep a copy of the
>> device name?
>> A "const char *" should be enough, we just have to demand
>> that it will remain valid as long as the xnintr structure itself (i.e.
>> during the IRQ being attached). Saves a few bytes. :)
> 
> Could be if all the users of it are initially kernel-mode entities.
> 
> But e.g. there is a version of rt_intr_create() that may be called by
> user-space
> apps and I extended it to support the "name" parameter too :
> 
> int rt_intr_create(RT_INTR *intr, const char *name, unsigned irq, int mode);
> 
> In such a case, the kernel-side rt_intr_create() is called with the "name"
> allocated on the stack in skins/native/syscall.c : __rt_intr_create())
> so the initial copy of the name can not remain valid.

But you could create buffer space for a copy of the name in the same
block already allocated for RT_INTR. Should be straightforward.

Moreover, but that's a different topic, I would vote for

config XENO_OPT_NATIVE_INTR
        default n

That feature is not needed when following the (preferred) RTDM path, and
is therefore only ballast to xeno_native on most systems. Guess I have
to prepare a patch...

Jan

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to