[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86: use alternatives for FS/GS base accesses



>>> On 25.09.18 at 18:52, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 29/08/18 17:03, Jan Beulich wrote:
>> Eliminates a couple of branches in particular from the context switch
>> path.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> I've already expressed a dis-inclination to this patch, because it looks
> like a micro-optimisation which won't actually affect measureable
> performance.  (And as said before, I could be wrong, but I don't think I
> am...)

Iirc you had indicated you first of all don't like the mix of some constructs
using alternatives and some not. Eliminating conditional branches is
always a Good Thing (tm), it seems to me. And that's not just for
performance (inside Xen we can't assume at all that any code path,
even the context switch one, is hot enough to have any BTB entries
allocated), but also for ease of looking at the assembly, should there
be a need to do so. Overall I think we ought to make much heavier use
of alternatives patching, so I view this only as a first step towards this.

Otherwise, btw, why did you not object to e.g. clac() / stac() using
alternatives patching? As with so  many other things, I very much think
we should settle on a fundamental approach, and then write all code
consistently. If we followed what you say, we'd have to limit patching
to cases where conditionals can't (reasonably) express what we want.

> Have you done some perf analysis since you last posted it?

I don't view this as a worthwhile use of my time, to be honest. Even
a non-measurable improvement is an improvement. I'd understand
your objection if there was a fair reason to be afraid of worse
performance as a result of this change.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.