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

[Keir Fraser <keir.xen@gmail.com>]

On 02/05/2011 13:29, "Jan Beulich" <JBeulich@novell.com> wrote:

>>>> 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
> 
> I'm not sure - the patch doesn't introduce the locking (i.e. the
> translation arrays used without the patch also get updated under
> lock). I'm also not certain about slot recycling aspects (i.e. what
> would the result be if freeing slots got deferred via RCU, but the
> same slot is then needed to be used again before the grace period
> expires). Quite possibly this consideration is mute, just resulting
> from my only half-baked understanding of RCU...

The most straightforward way to convert to RCU with the most similar
synchronising semantics would be to add a 'live' boolean flag to each
pirq-related struct that is stored in a radix tree. Then:
 * insertions into radix tree would be moved before acquisition of the
irq_desc lock, then set 'live' under the lock
 * deletions would clear 'live' under the lock, then do the actual radix
deletion would happen after irq_desc lock release;
 * lookups would happen as usual under the irq_desc lock, but with an extra
test of the 'live' flag.

The main complexity of this approach would probably be in breaking up the
insertions/deletions across the irq_desc-lock critical section. Basically
the 'live' flag update would happen wherever the insertion/deletion happens
right now, but the physical insertion/deletion would be moved respectively
earlier/later.

We'd probably also need an extra lock to protect against concurrent
radix-tree update operations (should be pretty straightforward to add
however, needing to protect *only* the radix-tree update calls).

This is a pretty nice way to go imo.

 -- Keir

> Jan
> 
>> 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.
>> 
>>  -- Keir
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel