Re: [Xenomai-help] kernel oopses when killing realtime task

[Philippe Gerum <rpm@xenomai.org>]

On Wed, 2010-10-13 at 11:26 +0200, Pavel Machek wrote:
> Hi!
> 
> > > Thanks!
> > > 
> > > I hope I applied the patch correctly, it was slightly
> > > tricky... Unfortunately, it causes hang at first attempt to kill
> > > realtime task :-(.
> > 
> > Ah. Weird. The IRQ drain code is supposed to be called only when an
> > interrupt is unregistered from the Xenomai layer, which is not directly
> > related to killing a task. Maybe some of the task cleanup code involves
> > unregistering IRQs though, so this might explain.
> 
> Task cleanup definitely unregisters IRQ, yes.
> 
> > > > +void ipipe_drain_interrupt(struct ipipe_domain *ipd, unsigned int irq)
> > > > +{
> > > > +	struct ipipe_percpu_domain_data *p;
> > > > +	unsigned long flags;
> > > > +	int cpu;
> > > > +
> > > > +	flags = ipipe_critical_enter(NULL);
> > > > +	clear_bit(IPIPE_HANDLE_FLAG, &ipd->irqs[irq].control);
> > > > +	clear_bit(IPIPE_STICKY_FLAG, &ipd->irqs[irq].control);
> > > > +	set_bit(IPIPE_PASS_FLAG, &ipd->irqs[irq].control);
> > > > +	ipipe_critical_exit(flags);
> > > > +
> > > > +	for_each_online_cpu(cpu) {
> > > > +		local_irq_save_hw(flags);
> > > > +		p = ipipe_percpudom_ptr(ipd, cpu);
> > > > +		do {
> > > > +			local_irq_restore_hw(flags);
> > > > +			cpu_relax();
> > > > +			local_irq_save_hw(flags);
> > > > +		} while (test_bit(irq, p->irqpend_lomask) ||
> > > > +			 test_bit(IPIPE_STALL_FLAG, &p->status));
> > > > +		local_irq_restore_hw(flags);
> > > > +	}
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(ipipe_drain_interrupt);
> > > > +
> > > 
> > > This is the interesting place, right?
> > 
> > Yes. I messed up. We need the test on the STALL bit to make 100% sure
> > that we are not about clearing the function pointer under the feet of
> > the wired IRQ dispatcher, but at the same time, we enter the drain
> > routine with the STALL bit set for the target domain. So this code is
> > plain silly (patent not granted, way too much prior art).
> 
> :-).
> 
> > As a quick check, you could try removing the second test_bit from the
> > end loop condition.
> 
> Yes, with IPIPE_STALL_FLAG test removed it "works", but I assume that
> at that point it is placebo patch ;-).

Basically, yes, since we have problems with wired IRQs, so...

> 
> Should we test IPIPE_STALL_FLAG on all but current CPUs?

That would solve this particular issue, but we should drain the pipeline
out of any Xenomai critical section. The way it is done now may induce a
deadlock (e.g. CPU0 waiting for CPU1 to acknowledge critical entry in
ipipe_enter_critical when getting some IPI, and CPU1 waiting hw IRQs off
for CPU0 to release the Xenomai lock that annoys us right now).

I'll come up with something hopefully better and tested in the next
days.

-- 
Philippe.