On 19/02/2019 22:39, Razvan Cojocaru wrote:
> On 2/20/19 12:18 AM, Andrew Cooper wrote:
>> A subsequent change is going to need an x86-specific unmapping step, so take
>> the opportunity to split the current vcpu unmapping out into a dedicated
>> path.
>>
>> No practical change.
>>
>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
>> ---
>> CC: Jan Beulich <jbeul...@suse.com>
>> CC: Wei Liu <wei.l...@citrix.com>
>> CC: Roger Pau Monné <roger....@citrix.com>
>> CC: Razvan Cojocaru <rcojoc...@bitdefender.com>
>> CC: Tamas K Lengyel <ta...@tklengyel.com>
>> CC: Juergen Gross <jgr...@suse.com>
>> ---
>> xen/common/domain.c | 16 +++++++++++++---
>> xen/include/xen/domain.h | 4 ++++
>> 2 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>> index 32bca8d..e66f7ea 100644
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -700,10 +700,21 @@ int rcu_lock_live_remote_domain_by_id(domid_t dom,
>> struct domain **d)
>> return 0;
>> }
>>
>> +static void domain_unmap_resources(struct domain *d)
>> +{
>> + struct vcpu *v;
>> +
>> + for_each_vcpu ( d, v )
>> + {
>> + unmap_vcpu_info(v);
>> +
>> + arch_vcpu_unmap_resources(v);
>> + }
>> +}
>> +
>> int domain_kill(struct domain *d)
>> {
>> int rc = 0;
>> - struct vcpu *v;
>>
>> if ( d == current->domain )
>> return -EINVAL;
>> @@ -732,13 +743,12 @@ int domain_kill(struct domain *d)
>> d->tmem_client = NULL;
>> /* fallthrough */
>> case DOMDYING_dying:
>> + domain_unmap_resources(d);
>> rc = domain_relinquish_resources(d);
>> if ( rc != 0 )
>> break;
>> if ( cpupool_move_domain(d, cpupool0) )
>> return -ERESTART;a
>> - for_each_vcpu ( d, v )
>> - unmap_vcpu_info(v);
> So before this change there was a leak of some sort here? I see that
> before this patch unmap_vcpu_info() was called only if rc == 0, but it
> is now called unconditionally _before_ domain_relinquish_resources().
>
> This does appear to change the behaviour of the code in the error case.
>
> If that's intended:
> Reviewed-by: Razvan Cojocaru <rcojoc...@bitdefender.com>
domain_kill() is a very long running hypercall, and takes about 15
minutes of wallclock time for domains with 1T of RAM (before the idle
scrubbing changes went in).
It will frequently break out with -ERESTART for continuation purposes,
but doesn't fail outright.
However, with hindsight it would probably be better just to put the
altp2m case in domain_relinquish_resources() and do away with this patch
entirely.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
