On 02.09.19 11:07, Jan Beulich wrote:
On 09.08.2019 16:57, Juergen Gross wrote:
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -87,13 +87,13 @@ sched_idle_switch_sched(struct scheduler *new_ops, unsigned
int cpu,
}
static int
-sched_idle_cpu_pick(const struct scheduler *ops, struct vcpu *v)
+sched_idle_cpu_pick(const struct scheduler *ops, struct sched_unit *unit)
{
- return v->processor;
+ return unit->vcpu_list->processor;
}
Neither this nor any of the cpu_pick functions in the sched_*.c files
actually mean to modify "*unit", so unless later changes need this be
non-const I think it should get "const" added.
I think const is fine.
@@ -308,9 +308,17 @@ static void sched_spin_unlock_double(spinlock_t *lock1,
spinlock_t *lock2,
int sched_init_vcpu(struct vcpu *v, unsigned int processor)
{
struct domain *d = v->domain;
+ struct sched_unit *unit;
v->processor = processor;
+ if ( (unit = xzalloc(struct sched_unit)) == NULL )
+ return 1;
+ v->sched_unit = unit;
+ unit->vcpu_list = v;
+ unit->unit_id = v->vcpu_id;
+ unit->domain = d;
I guess it doesn't matter much since this will change quite a bit with
subsequent patches, but generally I'd consider it better to initialize
relevant structure fields first, before hooking it up the structure
itself.
Will change.
@@ -452,13 +465,17 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
void sched_destroy_vcpu(struct vcpu *v)
{
+ struct sched_unit *unit = v->sched_unit;
+
kill_timer(&v->periodic_timer);
kill_timer(&v->singleshot_timer);
kill_timer(&v->poll_timer);
if ( test_and_clear_bool(v->is_urgent) )
atomic_dec(&per_cpu(schedule_data, v->processor).urgent_count);
- sched_remove_vcpu(vcpu_scheduler(v), v);
+ sched_remove_unit(vcpu_scheduler(v), unit);
sched_free_vdata(vcpu_scheduler(v), v->sched_priv);
+ xfree(unit);
+ v->sched_unit = NULL;
Along the lines of the above, storing NULL would generally better be
done prior to actually freeing the pointed at structure.
Yes.
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -140,6 +140,7 @@ void evtchn_destroy(struct domain *d); /* from domain_kill
*/
void evtchn_destroy_final(struct domain *d); /* from complete_domain_destroy
*/
struct waitqueue_vcpu;
+struct sched_unit;
In C I don't think this is needed with ...
@@ -160,6 +161,7 @@ struct vcpu
struct timer poll_timer; /* timeout for SCHEDOP_poll */
+ struct sched_unit *sched_unit;
... this being ahead of any function prototypes using the struct.
Right.
@@ -272,6 +274,12 @@ struct vcpu
struct arch_vcpu arch;
};
+struct sched_unit {
+ struct domain *domain;
+ struct vcpu *vcpu_list;
+ int unit_id;
Any reason for this being plain int (rather than unsigned int)?
I just copied the vcpu_id type. Will change to unsigned int.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
