Re: [xen-unstable test] 6947: regressions - trouble: broken/fail/pass

["Jan Beulich" <JBeulich@novell.com>]

>>> On 02.05.11 at 14:19, Keir Fraser <keir.xen@gmail.com> wrote:
> On 02/05/2011 13:00, "Jan Beulich" <JBeulich@novell.com> wrote:
> 
>>> (3) Restructure the interrupt code to do less work in IRQ context. For
>>> example tasklet-per-irq, and schedule on the local cpu. Protect a bunch of
>>> the PIRQ structures with a non-IRQ lock. Would increase interrupt latency if
>>> the local CPU is interrupted in hypervisor context. I'm not sure about this
>>> one -- I'm not that happy about the amount of work now done in hardirq
>>> context, but I'm not sure on the performance impact of deferring the work.
>> 
>> I'm not inclined to make changes in this area for the purpose at hand
>> either (again, Linux gets away without this - would have to check how
>> e.g. KVM gets the TLB flushing done, or whether they don't defer
>> flushes like we do).
> 
> Oh, another way would be to make lookup_slot invocations from IRQ context be
> RCU-safe. Then the radix tree updates would not have to synchronise on the
> irq_desc lock? And I believe Linux has examples of RCU-safe usage of radix
> trees -- certainly Linux's radix-tree.h mentions RCU.
> 
> I must say this would be far more attractive to me than hacking the xmalloc
> subsystem. That's pretty nasty.

I think that I can actually get away with two stage insertion/removal
without needing RCU, based on the fact that prior to these changes
we have the translation arrays also hold zero values that mean "does
not have a valid translation". Hence I can do tree insertion (removal)
with just d->event_lock held, but data not yet (no longer) populated,
and valid <-> invalid transitions only happening with the IRQ's
descriptor lock held (and interrupts disabled). All this requires is that
readers properly deal with the non-populated state, which they
already had to in the first version of the patch anyway.

Jan