|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 3/6] xen/sched: Improvements to the {alloc, free}_domdata() interfaces
The main purpose of this change is for the subsequent cleanup it enables, but
it stands on its own merits.
In principle, these hooks are optional, but the SCHED_OP() default aliases a
memory allocation failure, which causes arinc653 to play the dangerous game of
passing its priv pointer back, and remembering not to actually free it.
Redefine alloc_domdata to use ERR_PTR() for errors, NULL for nothing, and
non-NULL for a real allocation, which allows the hook to become properly
optional. Redefine free_domdata to be idempotent.
For arinc653, this means the dummy hooks can be dropped entirely. For the
other schedulers, this means returning ERR_PTR(-ENOMEM) instead of NULL for
memory allocation failures, and modifying the free hooks to cope with a NULL
pointer. While making the alterations, drop some spurious casts to void *.
Introduce and use proper wrappers for sched_{alloc,free}_domdata(). These are
strictly better than SCHED_OP(), as the source code is visible to
grep/cscope/tags, the generated code is better, and there can be proper
per-hook defaults and checks.
Callers of the alloc hooks are switched to using IS_ERR(), rather than
checking for NULL.
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
CC: Dario Faggioli <dfaggioli@xxxxxxxx>
CC: Meng Xu <mengxu@xxxxxxxxxxxxx>
CC: Josh Whitehead <josh.whitehead@xxxxxxxxxxxxxxx>
CC: Robert VanVossen <robert.vanvossen@xxxxxxxxxxxxxxx>
---
xen/common/sched_arinc653.c | 31 -------------------------------
xen/common/sched_credit.c | 8 ++++----
xen/common/sched_credit2.c | 24 +++++++++++++-----------
xen/common/sched_null.c | 22 +++++++++++++---------
xen/common/sched_rt.c | 21 +++++++++++++--------
xen/common/schedule.c | 12 ++++++------
xen/include/xen/sched-if.h | 27 ++++++++++++++++++++++++++-
7 files changed, 75 insertions(+), 70 deletions(-)
diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index 982845b..17e765d 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -455,34 +455,6 @@ a653sched_free_vdata(const struct scheduler *ops, void
*priv)
}
/**
- * This function allocates scheduler-specific data for a domain
- *
- * We do not actually make use of any per-domain data but the hypervisor
- * expects a non-NULL return value
- *
- * @param ops Pointer to this instance of the scheduler structure
- *
- * @return Pointer to the allocated data
- */
-static void *
-a653sched_alloc_domdata(const struct scheduler *ops, struct domain *dom)
-{
- /* return a non-NULL value to keep schedule.c happy */
- return SCHED_PRIV(ops);
-}
-
-/**
- * This function frees scheduler-specific data for a domain
- *
- * @param ops Pointer to this instance of the scheduler structure
- */
-static void
-a653sched_free_domdata(const struct scheduler *ops, void *data)
-{
- /* nop */
-}
-
-/**
* Xen scheduler callback function to sleep a VCPU
*
* @param ops Pointer to this instance of the scheduler structure
@@ -740,9 +712,6 @@ static const struct scheduler sched_arinc653_def = {
.free_vdata = a653sched_free_vdata,
.alloc_vdata = a653sched_alloc_vdata,
- .free_domdata = a653sched_free_domdata,
- .alloc_domdata = a653sched_alloc_domdata,
-
.init_domain = NULL,
.destroy_domain = NULL,
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index f8212db..e2133df 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -1282,7 +1282,7 @@ csched_alloc_domdata(const struct scheduler *ops, struct
domain *dom)
sdom = xzalloc(struct csched_dom);
if ( sdom == NULL )
- return NULL;
+ return ERR_PTR(-ENOMEM);
/* Initialize credit and weight */
INIT_LIST_HEAD(&sdom->active_vcpu);
@@ -1290,7 +1290,7 @@ csched_alloc_domdata(const struct scheduler *ops, struct
domain *dom)
sdom->dom = dom;
sdom->weight = CSCHED_DEFAULT_WEIGHT;
- return (void *)sdom;
+ return sdom;
}
static int
@@ -1302,8 +1302,8 @@ csched_dom_init(const struct scheduler *ops, struct
domain *dom)
return 0;
sdom = csched_alloc_domdata(ops, dom);
- if ( sdom == NULL )
- return -ENOMEM;
+ if ( IS_ERR(sdom) )
+ return PTR_ERR(sdom);
dom->sched_priv = sdom;
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index b094b3c..29a24d6 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -3012,7 +3012,7 @@ csched2_alloc_domdata(const struct scheduler *ops, struct
domain *dom)
sdom = xzalloc(struct csched2_dom);
if ( sdom == NULL )
- return NULL;
+ return ERR_PTR(-ENOMEM);
/* Initialize credit, cap and weight */
INIT_LIST_HEAD(&sdom->sdom_elem);
@@ -3032,7 +3032,7 @@ csched2_alloc_domdata(const struct scheduler *ops, struct
domain *dom)
write_unlock_irqrestore(&prv->lock, flags);
- return (void *)sdom;
+ return sdom;
}
static int
@@ -3044,8 +3044,8 @@ csched2_dom_init(const struct scheduler *ops, struct
domain *dom)
return 0;
sdom = csched2_alloc_domdata(ops, dom);
- if ( sdom == NULL )
- return -ENOMEM;
+ if ( IS_ERR(sdom) )
+ return PTR_ERR(sdom);
dom->sched_priv = sdom;
@@ -3055,19 +3055,21 @@ csched2_dom_init(const struct scheduler *ops, struct
domain *dom)
static void
csched2_free_domdata(const struct scheduler *ops, void *data)
{
- unsigned long flags;
struct csched2_dom *sdom = data;
struct csched2_private *prv = csched2_priv(ops);
- kill_timer(&sdom->repl_timer);
-
- write_lock_irqsave(&prv->lock, flags);
+ if ( sdom )
+ {
+ unsigned long flags;
- list_del_init(&sdom->sdom_elem);
+ kill_timer(&sdom->repl_timer);
- write_unlock_irqrestore(&prv->lock, flags);
+ write_lock_irqsave(&prv->lock, flags);
+ list_del_init(&sdom->sdom_elem);
+ write_unlock_irqrestore(&prv->lock, flags);
- xfree(data);
+ xfree(sdom);
+ }
}
static void
diff --git a/xen/common/sched_null.c b/xen/common/sched_null.c
index b4a24ba..4dd405b 100644
--- a/xen/common/sched_null.c
+++ b/xen/common/sched_null.c
@@ -231,7 +231,7 @@ static void * null_alloc_domdata(const struct scheduler
*ops,
ndom = xzalloc(struct null_dom);
if ( ndom == NULL )
- return NULL;
+ return ERR_PTR(-ENOMEM);
ndom->dom = d;
@@ -239,20 +239,24 @@ static void * null_alloc_domdata(const struct scheduler
*ops,
list_add_tail(&ndom->ndom_elem, &null_priv(ops)->ndom);
spin_unlock_irqrestore(&prv->lock, flags);
- return (void*)ndom;
+ return ndom;
}
static void null_free_domdata(const struct scheduler *ops, void *data)
{
- unsigned long flags;
struct null_dom *ndom = data;
struct null_private *prv = null_priv(ops);
- spin_lock_irqsave(&prv->lock, flags);
- list_del_init(&ndom->ndom_elem);
- spin_unlock_irqrestore(&prv->lock, flags);
+ if ( ndom )
+ {
+ unsigned long flags;
+
+ spin_lock_irqsave(&prv->lock, flags);
+ list_del_init(&ndom->ndom_elem);
+ spin_unlock_irqrestore(&prv->lock, flags);
- xfree(data);
+ xfree(ndom);
+ }
}
static int null_dom_init(const struct scheduler *ops, struct domain *d)
@@ -263,8 +267,8 @@ static int null_dom_init(const struct scheduler *ops,
struct domain *d)
return 0;
ndom = null_alloc_domdata(ops, d);
- if ( ndom == NULL )
- return -ENOMEM;
+ if ( IS_ERR(ndom) )
+ return PTR_ERR(ndom);
d->sched_priv = ndom;
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index a202802..e4ff5c1 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -820,7 +820,7 @@ rt_alloc_domdata(const struct scheduler *ops, struct domain
*dom)
sdom = xzalloc(struct rt_dom);
if ( sdom == NULL )
- return NULL;
+ return ERR_PTR(-ENOMEM);
INIT_LIST_HEAD(&sdom->sdom_elem);
sdom->dom = dom;
@@ -836,14 +836,19 @@ rt_alloc_domdata(const struct scheduler *ops, struct
domain *dom)
static void
rt_free_domdata(const struct scheduler *ops, void *data)
{
- unsigned long flags;
struct rt_dom *sdom = data;
struct rt_private *prv = rt_priv(ops);
- spin_lock_irqsave(&prv->lock, flags);
- list_del_init(&sdom->sdom_elem);
- spin_unlock_irqrestore(&prv->lock, flags);
- xfree(data);
+ if ( sdom )
+ {
+ unsigned long flags;
+
+ spin_lock_irqsave(&prv->lock, flags);
+ list_del_init(&sdom->sdom_elem);
+ spin_unlock_irqrestore(&prv->lock, flags);
+
+ xfree(sdom);
+ }
}
static int
@@ -856,8 +861,8 @@ rt_dom_init(const struct scheduler *ops, struct domain *dom)
return 0;
sdom = rt_alloc_domdata(ops, dom);
- if ( sdom == NULL )
- return -ENOMEM;
+ if ( IS_ERR(sdom) )
+ return PTR_ERR(sdom);
dom->sched_priv = sdom;
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index b788426..08a31b6 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -318,14 +318,14 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
return -EBUSY;
}
- domdata = SCHED_OP(c->sched, alloc_domdata, d);
- if ( domdata == NULL )
- return -ENOMEM;
+ domdata = sched_alloc_domdata(c->sched, d);
+ if ( IS_ERR(domdata) )
+ return PTR_ERR(domdata);
vcpu_priv = xzalloc_array(void *, d->max_vcpus);
if ( vcpu_priv == NULL )
{
- SCHED_OP(c->sched, free_domdata, domdata);
+ sched_free_domdata(c->sched, domdata);
return -ENOMEM;
}
@@ -337,7 +337,7 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
for_each_vcpu ( d, v )
xfree(vcpu_priv[v->vcpu_id]);
xfree(vcpu_priv);
- SCHED_OP(c->sched, free_domdata, domdata);
+ sched_free_domdata(c->sched, domdata);
return -ENOMEM;
}
}
@@ -393,7 +393,7 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
domain_unpause(d);
- SCHED_OP(old_ops, free_domdata, old_domdata);
+ sched_free_domdata(old_ops, old_domdata);
xfree(vcpu_priv);
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index c4a4935..56e7d0c 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -146,8 +146,11 @@ struct scheduler {
void * (*alloc_pdata) (const struct scheduler *, int);
void (*init_pdata) (const struct scheduler *, void *, int);
void (*deinit_pdata) (const struct scheduler *, void *, int);
- void (*free_domdata) (const struct scheduler *, void *);
+
+ /* Returns ERR_PTR(-err) for error, NULL for 'nothing needed'. */
void * (*alloc_domdata) (const struct scheduler *, struct domain *);
+ /* Idempotent. */
+ void (*free_domdata) (const struct scheduler *, void *);
void (*switch_sched) (struct scheduler *, unsigned int,
void *, void *);
@@ -181,6 +184,28 @@ struct scheduler {
void (*tick_resume) (const struct scheduler *, unsigned int);
};
+static inline void *sched_alloc_domdata(const struct scheduler *s,
+ struct domain *d)
+{
+ if ( s->alloc_domdata )
+ return s->alloc_domdata(s, d);
+ else
+ return NULL;
+}
+
+static inline void sched_free_domdata(const struct scheduler *s,
+ void *data)
+{
+ if ( s->free_domdata )
+ s->free_domdata(s, data);
+ else
+ /*
+ * Check that if there isn't a free_domdata hook, we haven't got any
+ * data we're expected to deal with.
+ */
+ ASSERT(!data);
+}
+
#define REGISTER_SCHEDULER(x) static const struct scheduler *x##_entry \
__used_section(".data.schedulers") = &x;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |