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

Re: [Xen-devel] [PATCH] x86/PV: fix unintended dependency of m2p-strict mode on migration-v2



>>> On 01.02.16 at 18:31, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 01/02/16 16:51, Jan Beulich wrote:
>>>>> On 01.02.16 at 17:34, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 01/02/16 16:28, Jan Beulich wrote:
>>>>>>> On 01.02.16 at 15:07, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> On 01/02/16 13:20, Jan Beulich wrote:
>>>>>> Ping? (I'd really like to get this resolved, so we don't need to
>>>>>> indefinitely run with non-upstream behavior in our distros.)
>>>>> My remaining issue is whether this loop gets executed by default.
>>>>>
>>>>> I realise that there is a difference between legacy and v2 migration,
>>>>> and that v2 migration by default worked.  If that means we managed to
>>>>> skip this loop in its entirety for v2, then I am far less concerned
>>>>> about the overhead.
>>>> But had been there before: Of course we could skip the loop if
>>>> the bit in d->vm_assist doesn't change. But as expressed before,
>>>> with you having already indicated that perhaps it would be better
>>>> to have v2 migration do the relevant operations in the other (v1)
>>>> order, the moment that was actually done the benefit of avoiding
>>>> the loop would be gone.
>>>>
>>>> To be clear - if rendering the code dead (which is what you ask
>>>> for) until v2 migration happens to get changed is the only way to
>>>> get this code in, I will submit a v2 with that extra conditional.
>>> Migration v2 currently loads vcpu context before pinning the pagetables,
>>> which means that the vm_assist should get set up properly, before L4
>>> tables are processed.
>>>
>>> It was my understanding that this is the correct way around, and
>>> m2p-strict mode only broke when you backported it to migration v1 systems?
>> Yes, but when we discussed this you said "in hindsight, it would have
>> been more sensible to swap pin and vCPU context, but I guess that
>> would break m2p-strict in the same way", and whether one is more
>> "correct" than the other is pretty questionable.
> 
> Right, but as it currently stands, migration v2 gets things the correct
> way around?
> 
> Does that mean that, at the moment, this loop ends up getting skipped?
> 
> If it does, then the patch is fine.

Actually while adding the extra logic it occurred to me that the
loop indeed would already get skipped, due to
d->arch.pv_domain.nr_l4_pages being zero if page table
pinning didn't happen yet. Except that due to preemption later
in the function the loop would have got entered once the first
L4 page appeared, and then it would perhaps execute multiple
times. I.e. the extra check is necessary regardless of your
desire to avoid the loop entirely.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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