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

Re: [Xen-devel] [PATCH 4/5] x86/pv: Drop support for paging out the LDT



>>> On 18.01.18 at 12:00, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 18/01/18 10:57, Andrew Cooper wrote:
>> On 18/01/18 10:38, George Dunlap wrote:
>>> On Thu, Jan 18, 2018 at 7:59 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>> On 12.01.18 at 19:37, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> --- a/xen/arch/x86/domain.c
>>>>> +++ b/xen/arch/x86/domain.c
>>>>> @@ -1942,11 +1942,8 @@ int domain_relinquish_resources(struct domain *d)
>>>>>          {
>>>>>              for_each_vcpu ( d, v )
>>>>>              {
>>>>> -                /*
>>>>> -                 * Relinquish GDT mappings. No need for explicit 
>>>>> unmapping of
>>>>> -                 * the LDT as it automatically gets squashed with the 
>>>>> guest
>>>>> -                 * mappings.
>>>>> -                 */
>>>>> +                /* Relinquish GDT/LDT mappings. */
>>>>> +                pv_destroy_ldt(v);
>>>>>                  pv_destroy_gdt(v);
>>>> The domain is dead at this point, so the order doesn't matter much,
>>>> but strictly speaking you should destroy the GDT before destroying
>>>> the LDT (just like LDT _loads_ always need to come _after_ GDT
>>>> adjustments).
>> By that logic, I've got it the correct way round (which is they way I
>> intended).  Allocation and freeing of the LDT is nested within the scope
>> of the GDT.

Ah, I see how I didn't properly express what I mean. The idea
behind the remark was that the GDT might still have a reference
to the LDT, which would become stale with the LDT gone. The
better thing to compare with would be construction of an LDT
versus insertion of the respective descriptor into the GDT.

>>>> Everything else here looks fine, but the initial comment may need
>>>> further discussion. For example we may want to consider a
>>>> two-stage phasing out of the feature, with a couple of years in
>>>> between: Make the functionality dependent upon a default-off
>>>> command line option for the time being, and issue a bright warning
>>>> when someone actually enables it (telling them to tell us).
>>> One of the problems we have is that people seem to wait for 2-3 years
>>> after a release has been made to start updating to it.  So we'd have
>>> to leave such a warning for probably 5 years minimum.
>> In almost any other case, I'd agree, and would be the first to suggest this.
>>
>> However, This patch is strictly necessary for a more complete XPTI,
>> which is how I stumbled onto the issue in my KAISER series to begin
>> with.  It is the only codepath where we ever poke at a remote critical
>> data structure, which is why
> 
> Sorry - sent too early.
> 
> ... which is why isolating the LDT in a per-cpu doesn't work in
> combination with this algorithm.

Right, in the context of that series I can see it likely becoming
unavoidable to remove this functionality (aiui it could be kept,
but would become more complicated). Not having heard back
on the incompleteness discussion on stage 1 yet, I can't really
conclude at this point whether we will actually _need_ (not
just want) this full series making things per-CPU (taken together
with there so far not being any promise of it to help recover
performance).

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®.