Re: [Xen-devel] [PATCH v3 1/2] x86/mem-sharing: Bulk mem-sharing entire domains

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.05.16 at 18:29, <tamas@xxxxxxxxxxxxx> wrote:
> On Fri, May 13, 2016 at 10:12 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>> On 13.05.16 at 17:31, <tamas@xxxxxxxxxxxxx> wrote:
>>> On Fri, May 13, 2016 at 9:09 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>> On 13.05.16 at 16:50, <tamas@xxxxxxxxxxxxx> wrote:
>>>>> On Fri, May 13, 2016 at 6:00 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>>>> On 12.05.16 at 17:25, <tamas@xxxxxxxxxxxxx> wrote:
>>>>>>> @@ -1468,6 +1505,69 @@ int
>>>>>>> +            if ( rc )
>>>>>>> +            {
>>>>>>> +                rcu_unlock_domain(cd);
>>>>>>> +                goto out;
>>>>>>> +            }
>>>>>>> +
>>>>>>> +            if ( !mem_sharing_enabled(cd) )
>>>>>>> +            {
>>>>>>> +                rcu_unlock_domain(cd);
>>>>>>> +                rc = -EINVAL;
>>>>>>> +                goto out;
>>>>>>> +            }
>>>>>>> +
>>>>>>> +            if ( !atomic_read(&d->pause_count) ||
>>>>>>> +                 !atomic_read(&cd->pause_count) )
>>>>>>> +            {
>>>>>>> +                rcu_unlock_domain(cd);
>>>>>>> +                rc = -EINVAL;
>>>>>>> +                goto out;
>>>>>>> +            }
>>>>>>> +
>>>>>>> +            max_sgfn = domain_get_maximum_gpfn(d);
>>>>>>> +            max_cgfn = domain_get_maximum_gpfn(cd);
>>>>>>> +
>>>>>>> +            if ( max_sgfn != max_cgfn || max_sgfn < mso.u.bulk.start )
>>>>>>
>>>>>> Why would the two domains need to agree in their maximum
>>>>>> GPFN? There's nothing similar in this file so far. Nor does the
>>>>>> right side of the || match anything pre-existing...
>>>>>
>>>>> The use-case for this function is to deduplicate identical VMs, not to
>>>>> blindly share pages across arbitrary domains. So this is a safety
>>>>> check to avoid accidentally running this function on domains that
>>>>> obviously are not identical. The right hand size is a safety check
>>>>> against not properly initialized input structs where the start point
>>>>> is obviously outside the memory of the domain.
>>>>
>>>> Is that use case the only possible one, or merely the one you care
>>>> about? In the former case, I'd be okay as long as a respctive
>>>> comment got added.
>>>
>>> I would say "blind" sharing like this only makes sense for identical
>>> VMs. Replacing the memory of the VM with that of another one not in
>>> the same state will lead to some spectacular crash for sure, so we
>>> should do at least some sanity checking before.
>>
>> Crash? Pages would be shared only if their contents match (and
>> unshared when they're about to diverge), and once that's
>> guaranteed, I don't see how it matters whether the two involved
>> guests are identical. I agree that between dissimilar guests the
>> potential for sharing is likely much lower, but that's about all.
> 
> No, that's not how it works. Xen's mem_sharing doesn't do _any_ checks
> on the contents of the pages before sharing. It's up to the user to
> know what he is doing.

Ok, okay - that explains to me why this feature is rarely used and
not fully supported.

Jan


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