xen/common/domain.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- xen/include/xen/sched.h | 1 + 2 files changed, 47 insertions(+), 2 deletions(-)
Similarly to c/s 98d4d6d8a6 "xen/domain: Introduce domain_teardown()",
introduce a common mechanism for restartable per-vcpu teardown logic.
Extend the PROGRESS() mechanism to support saving and restoring the vcpu loop
variable across hypercalls.
This will eventually supersede domain_reliquish_resources(), and reduce the
quantity of redundant logic performed.
No functional change (yet).
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
This is a prerequisite for the IPT series, to avoid introducing a latent
memory leak bug on ARM.
---
xen/common/domain.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
xen/include/xen/sched.h | 1 +
2 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 164c9d14e9..7be3a7cf36 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -130,6 +130,22 @@ static void vcpu_info_reset(struct vcpu *v)
v->vcpu_info_mfn = INVALID_MFN;
}
+/*
+ * Release resources held by a vcpu. There may or may not be live references
+ * to the vcpu, and it may or may not be fully constructed.
+ *
+ * If d->is_dying is DOMDYING_dead, this must not return non-zero.
+ */
+static int vcpu_teardown(struct vcpu *v)
+{
+ return 0;
+}
+
+/*
+ * Destoy a vcpu once all references to it have been dropped. Used either
+ * from domain_destroy()'s RCU path, or from the vcpu_create() error path
+ * before the vcpu is placed on the domain's vcpu list.
+ */
static void vcpu_destroy(struct vcpu *v)
{
free_vcpu_struct(v);
@@ -206,6 +222,11 @@ struct vcpu *vcpu_create(struct domain *d, unsigned int vcpu_id)
sched_destroy_vcpu(v);
fail_wq:
destroy_waitqueue_vcpu(v);
+
+ /* Must not hit a continuation in this context. */
+ if ( vcpu_teardown(v) )
+ ASSERT_UNREACHABLE();
+
vcpu_destroy(v);
return NULL;
@@ -284,6 +305,9 @@ custom_param("extra_guest_irqs", parse_extra_guest_irqs);
*/
static int domain_teardown(struct domain *d)
{
+ struct vcpu *v;
+ int rc;
+
BUG_ON(!d->is_dying);
/*
@@ -298,7 +322,9 @@ static int domain_teardown(struct domain *d)
* will logically restart work from this point.
*
* PROGRESS() markers must not be in the middle of loops. The loop
- * variable isn't preserved across a continuation.
+ * variable isn't preserved across a continuation. PROGRESS_VCPU()
+ * markers may be used in the middle of for_each_vcpu() loops, which
+ * preserve v but no other loop variables.
*
* To avoid redundant work, there should be a marker before each
* function which may return -ERESTART.
@@ -308,14 +334,32 @@ static int domain_teardown(struct domain *d)
/* Fallthrough */ \
case PROG_ ## x
+#define PROGRESS_VCPU(x) \
+ d->teardown.val = PROG_vcpu_ ## x; \
+ d->teardown.ptr = v; \
+ /* Fallthrough */ \
+ case PROG_vcpu_ ## x: \
+ v = d->teardown.ptr
+
enum {
- PROG_done = 1,
+ PROG_vcpu_teardown = 1,
+ PROG_done,
};
case 0:
+ for_each_vcpu ( d, v )
+ {
+ PROGRESS_VCPU(teardown);
+
+ rc = vcpu_teardown(v);
+ if ( rc )
+ return rc;
+ }
+
PROGRESS(done):
break;
+#undef PROGRESS_VCPU
#undef PROGRESS
default:
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 3e46384a3c..846a77c0bb 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -532,6 +532,7 @@ struct domain
*/
struct {
unsigned int val;
+ struct vcpu *ptr;
} teardown;
};
--
2.11.0
On 19.01.2021 01:32, Andrew Cooper wrote:
> Similarly to c/s 98d4d6d8a6 "xen/domain: Introduce domain_teardown()",
> introduce a common mechanism for restartable per-vcpu teardown logic.
>
> Extend the PROGRESS() mechanism to support saving and restoring the vcpu loop
> variable across hypercalls.
>
> This will eventually supersede domain_reliquish_resources(), and reduce the
> quantity of redundant logic performed.
>
> No functional change (yet).
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
albeit perhaps with a name or type change:
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -532,6 +532,7 @@ struct domain
> */
> struct {
> unsigned int val;
> + struct vcpu *ptr;
> } teardown;
I think the field either wants to be generic (and then of type void *)
or specific (and then be named "vcpu"). Which one is better certainly
depends on possibly anticipated future usage.
Jan
On 19/01/2021 09:14, Jan Beulich wrote:
> On 19.01.2021 01:32, Andrew Cooper wrote:
>> Similarly to c/s 98d4d6d8a6 "xen/domain: Introduce domain_teardown()",
>> introduce a common mechanism for restartable per-vcpu teardown logic.
>>
>> Extend the PROGRESS() mechanism to support saving and restoring the vcpu loop
>> variable across hypercalls.
>>
>> This will eventually supersede domain_reliquish_resources(), and reduce the
>> quantity of redundant logic performed.
>>
>> No functional change (yet).
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Thanks.
> albeit perhaps with a name or type change:
>
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -532,6 +532,7 @@ struct domain
>> */
>> struct {
>> unsigned int val;
>> + struct vcpu *ptr;
>> } teardown;
> I think the field either wants to be generic (and then of type void *)
> or specific (and then be named "vcpu"). Which one is better certainly
> depends on possibly anticipated future usage.
I think I'll rename to vcpu for now. I don't anticipate this being
usable for anything else safely.
~Andrew
© 2016 - 2024 Red Hat, Inc.