Re: Where do we stand with the Xen patches?

[Jeremy Fitzhardinge <jeremy@goop.org>]

Ingo Molnar wrote:
> These look fine but i still need to go over them one last time 
> before pulling them.
>
>   
> Yes. Here too i still need to go over them once more before pulling 
> them.
>   

I've been posting these patches in fundamentally the same form for about 
6 months now.  I don't think you'll find anything surprising.

>> for-ingo/xen/dom0/mtrr
>>
>>    You queried the value of "extending" this interface, given that its
>>    considered to be deprecated.  These changes in no way extend the
>>    interface, but just make the existing interface functional under
>>    Xen.  And while we don't have PAT support, there's no other way of
>>    setting cachability attributes on memory, so not supporting it has a
>>    fairly severe performance impact on things like X.
>>     
>
> Latest Xorg doesnt need /proc/mtrr. By the time this kernel (.31 or 
> later) hits any distribution, /proc/mtrr using Xorg will be a 
> distant memory. So i see no reason why to apply those bits at all, 
> and i see a lot of reasons to not apply them.
>   

In general we don't break usermode ABIs, even when using new kernels 
with old distros, so I don't see why this case is any different.

Besides, these changes are not only for /proc/mtrr, but also to 
implement the internal mtrr_add() APIs that DRM uses to set the MTRR 
inside the kernel, so failing to implement them will cause performance 
regressions on new X servers.

Given that we're talking about 120 lines of code with no runtime impact 
and tiny kernel size impact (when configured), I'm at a loss for what 
your "lot of reasons" might be.  Perhaps you could be more specific.

> As in the past, my main worry is performance overhead of paravirt in 
> general.
>
> The patches that dont affect any native kernel fast path are 
> probably OK (but still pending final review).
>   

These changes don't have anything much to do with paravirt_ops, per se, 
and are all specifically about Xen dom0 support.  Aside from that, none 
of the changes are on performance-critical paths.

> Regarding patches that do change the fastpath i'll do a round of 
> measurements of CONFIG_PARAVIRT against !CONFIG_PARAVIRT kernels, 
> and make up my mind based on that.
>
> You could accelerate this by sending some "perf stat" hard numbers 
> to give us an idea about where we stand today.

I posted them the other day; those perf stat measurements pointing out 
the pv-spinlock problem also showed that paravirt_ops in general has a 
sub-1% performance hit (and possible performance benefit) when running 
mmap-perf.  You added them into the commit log for the patch, so I 
presume you read it.

    J