Re: [PATCH v2 03/14] x86/pv: use populate_perdomain_mapping() to map the Xen GDT

["Jürgen Groß" <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 4300 bytes --]

On 04.09.26 08:00, Jan Beulich wrote:
> On 04.09.2026 00:35, George Dunlap wrote:
>> On Thu, Sep 3, 2026 at 5:11 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>
>>> On 02.09.2026 11:43, George Dunlap wrote:
>>>> From: Roger Pau Monné <roger.pau@citrix.com>
>>>>
>>>> Currently, update_xen_slot_in_full_gdt() uses the stashed direct-map
>>>> pointer in d->arch.pv.gdt_ldt_l1tab to update the incoming vcpu's
>>>> page tables with Xen's GDT, by writing a stashed per-cpu copy of a
>>>> pre-baked L1 entry (either 64-bit or compat version).
>>>>
>>>> Switch this to using populate_perdomain_mapping(), which doesn't rely
>>>> on the stashed address of the l1 page in the direct map.  Rather than
>>>> also stashing a pre-baked value for the payload, compute the mfn from
>>>> the per-cpu GDT pointer at use: the conversion is a handful of cycles
>>>> on a path costing thousands, and computing at use removes the
>>>> parallel {,compat_}gdt_l1e bookkeeping along with its boot-ordering
>>>> constraint (the cached value could only be generated after Xen's
>>>> physical relocation, and had to be in place before the first context
>>>> switch; a use-time lookup is correct by construction).  The flags on
>>>> the final mapping are identical.
>>>>
>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>> Assisted-by: Claude Code:claude-fable-5, Claude Code:claude-opus-4-8
>>>> Signed-off-by: George Dunlap <gwd@xenproject.org>
>>>> ---
>>>> Changes in v2:
>>>> - Drop the {,compat_}gdt_mfn caching entirely (suggested by Andrew
>>>>     Cooper): compute virt_to_mfn() from the per-cpu GDT pointer at use.
>>>>     The PDX lookup behind it measures ~5-10 cycles warm against a
>>>>     ~1,500-cycle context switch, and this removes the double
>>>>     bookkeeping and the after-relocation caching constraint.  The
>>>>     cached-MFN assertion goes with the cache: a use-time computation
>>>>     from a live pointer needs no staleness check.
>>>
>>> This looks to contradict what 564d261687c0 ("x86/ctxt-switch: Document
>>> and improve GDT handling") used as justification to put in place the
>>> caching. Also Cc-ing Jürgen, who also was involved there, for possible
>>> further insight.
>>>
>>> Functionally the change looks okay to me, but the above will need
>>> sorting, at the very least by specifically discussing why effectively
>>> undoing that earlier change is okay.
>>
>> So looking back at the thread, Jürgen measured a 14% improvement for
>> something that might be described as a microbenchmark before and after
>> the patch (a benchmark purposely trying to set up an unusual scenario
>> to maximize the effect of context switch overhead, not one to
>> represent a typical workflow).  But are the numbers really plausible?
>> Even at an implausible 100k switches/s across the box, saving 100
>> cycles per switch is about 0.04% of eight 3 GHz cores.
>>
>> At any rate, we're already adding several map/unmap operations, and
>> about to add several more.  Keeping the PTE caching would require
>> adding a separate path that can write just PTEs, which then will
>> potentially further complication future paths where we need to make
>> sure we handle both domain-wide perdomain areas and per-vcpu areas.
>> If it were easy I would already have been keeping it.
>>
>> I'd be inclined to say: Since we're going to be adding more
>> populate_perdomain_mapping() calls anyway, let's do it the simple
>> correct way first; and then explore the idea of stashing mfns of
>> frequently-mapped L1s (rather than having to walk L3 -> L2 -> L1); and
>> at that time look into stashing baked l1es to avoid conversions.
> 
> Perhaps; I'd like to have Jürgen's and/or Andrew's input here, though.

At that time I implemented core scheduling in Xen. I noticed that very
subtle changes in the context switch path could result in unexpected large
performance differences. As I had the performance test for my purpose
already set up, I used it for Andrew's patch (which was a result of my
context switch path performance findings) and really did measure the
impressive effect of it.

Note that you can't only count instructions, often cache effects and
branch predictions are dominating the performance.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]