* [Xen-devel] [PATCH] xen/sched: remove sched_init_pdata()
@ 2020-02-10 15:39 Juergen Gross
2020-02-11 10:37 ` Dario Faggioli
0 siblings, 1 reply; 3+ messages in thread
From: Juergen Gross @ 2020-02-10 15:39 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, George Dunlap, Meng Xu, Dario Faggioli
sched_init_pdata() is used nowhere, it can be removed. Same applies to
the .init_pdata hook of the per-scheduler interface.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
xen/common/sched/credit.c | 12 ------------
xen/common/sched/credit2.c | 21 ---------------------
xen/common/sched/null.c | 10 ----------
xen/common/sched/private.h | 8 --------
xen/common/sched/rt.c | 31 -------------------------------
5 files changed, 82 deletions(-)
diff --git a/xen/common/sched/credit.c b/xen/common/sched/credit.c
index 05946eea6e..93d89da278 100644
--- a/xen/common/sched/credit.c
+++ b/xen/common/sched/credit.c
@@ -614,17 +614,6 @@ init_pdata(struct csched_private *prv, struct csched_pcpu *spc, int cpu)
spc->nr_runnable = 0;
}
-static void
-csched_init_pdata(const struct scheduler *ops, void *pdata, int cpu)
-{
- unsigned long flags;
- struct csched_private *prv = CSCHED_PRIV(ops);
-
- spin_lock_irqsave(&prv->lock, flags);
- init_pdata(prv, pdata, cpu);
- spin_unlock_irqrestore(&prv->lock, flags);
-}
-
/* Change the scheduler of cpu to us (Credit). */
static spinlock_t *
csched_switch_sched(struct scheduler *new_ops, unsigned int cpu,
@@ -2273,7 +2262,6 @@ static const struct scheduler sched_credit_def = {
.alloc_udata = csched_alloc_udata,
.free_udata = csched_free_udata,
.alloc_pdata = csched_alloc_pdata,
- .init_pdata = csched_init_pdata,
.deinit_pdata = csched_deinit_pdata,
.free_pdata = csched_free_pdata,
.switch_sched = csched_switch_sched,
diff --git a/xen/common/sched/credit2.c b/xen/common/sched/credit2.c
index 231f87d960..b965cd1c7b 100644
--- a/xen/common/sched/credit2.c
+++ b/xen/common/sched/credit2.c
@@ -3818,26 +3818,6 @@ init_pdata(struct csched2_private *prv, struct csched2_pcpu *spc,
return spc->runq_id;
}
-static void
-csched2_init_pdata(const struct scheduler *ops, void *pdata, int cpu)
-{
- struct csched2_private *prv = csched2_priv(ops);
- spinlock_t *old_lock;
- unsigned long flags;
- unsigned rqi;
-
- write_lock_irqsave(&prv->lock, flags);
- old_lock = pcpu_schedule_lock(cpu);
-
- rqi = init_pdata(prv, pdata, cpu);
- /* Move the scheduler lock to the new runq lock. */
- get_sched_res(cpu)->schedule_lock = &prv->rqd[rqi].lock;
-
- /* _Not_ pcpu_schedule_unlock(): schedule_lock may have changed! */
- spin_unlock(old_lock);
- write_unlock_irqrestore(&prv->lock, flags);
-}
-
/* Change the scheduler of cpu to us (Credit2). */
static spinlock_t *
csched2_switch_sched(struct scheduler *new_ops, unsigned int cpu,
@@ -4085,7 +4065,6 @@ static const struct scheduler sched_credit2_def = {
.alloc_udata = csched2_alloc_udata,
.free_udata = csched2_free_udata,
.alloc_pdata = csched2_alloc_pdata,
- .init_pdata = csched2_init_pdata,
.deinit_pdata = csched2_deinit_pdata,
.free_pdata = csched2_free_pdata,
.switch_sched = csched2_switch_sched,
diff --git a/xen/common/sched/null.c b/xen/common/sched/null.c
index 8c3101649d..82d5d1baab 100644
--- a/xen/common/sched/null.c
+++ b/xen/common/sched/null.c
@@ -166,15 +166,6 @@ static void init_pdata(struct null_private *prv, struct null_pcpu *npc,
npc->unit = NULL;
}
-static void null_init_pdata(const struct scheduler *ops, void *pdata, int cpu)
-{
- struct null_private *prv = null_priv(ops);
-
- ASSERT(pdata);
-
- init_pdata(prv, pdata, cpu);
-}
-
static void null_deinit_pdata(const struct scheduler *ops, void *pcpu, int cpu)
{
struct null_private *prv = null_priv(ops);
@@ -1042,7 +1033,6 @@ static const struct scheduler sched_null_def = {
.deinit = null_deinit,
.alloc_pdata = null_alloc_pdata,
.free_pdata = null_free_pdata,
- .init_pdata = null_init_pdata,
.switch_sched = null_switch_sched,
.deinit_pdata = null_deinit_pdata,
diff --git a/xen/common/sched/private.h b/xen/common/sched/private.h
index 2a94179baa..367811a12f 100644
--- a/xen/common/sched/private.h
+++ b/xen/common/sched/private.h
@@ -306,7 +306,6 @@ struct scheduler {
struct sched_unit *, void *);
void (*free_pdata) (const struct scheduler *, void *, int);
void * (*alloc_pdata) (const struct scheduler *, int);
- void (*init_pdata) (const struct scheduler *, void *, int);
void (*deinit_pdata) (const struct scheduler *, void *, int);
/* Returns ERR_PTR(-err) for error, NULL for 'nothing needed'. */
@@ -408,13 +407,6 @@ static inline void sched_free_pdata(const struct scheduler *s, void *data,
s->free_pdata(s, data, cpu);
}
-static inline void sched_init_pdata(const struct scheduler *s, void *data,
- int cpu)
-{
- if ( s->init_pdata )
- s->init_pdata(s, data, cpu);
-}
-
static inline void sched_deinit_pdata(const struct scheduler *s, void *data,
int cpu)
{
diff --git a/xen/common/sched/rt.c b/xen/common/sched/rt.c
index 66585ed50a..c24cd2ac32 100644
--- a/xen/common/sched/rt.c
+++ b/xen/common/sched/rt.c
@@ -713,36 +713,6 @@ rt_deinit(struct scheduler *ops)
xfree(prv);
}
-/*
- * Point per_cpu spinlock to the global system lock;
- * All cpu have same global system lock
- */
-static void
-rt_init_pdata(const struct scheduler *ops, void *pdata, int cpu)
-{
- struct rt_private *prv = rt_priv(ops);
- spinlock_t *old_lock;
- unsigned long flags;
-
- old_lock = pcpu_schedule_lock_irqsave(cpu, &flags);
-
- /*
- * TIMER_STATUS_invalid means we are the first cpu that sees the timer
- * allocated but not initialized, and so it's up to us to initialize it.
- */
- if ( prv->repl_timer.status == TIMER_STATUS_invalid )
- {
- init_timer(&prv->repl_timer, repl_timer_handler, (void *)ops, cpu);
- dprintk(XENLOG_DEBUG, "RTDS: timer initialized on cpu %u\n", cpu);
- }
-
- /* Move the scheduler lock to our global runqueue lock. */
- get_sched_res(cpu)->schedule_lock = &prv->lock;
-
- /* _Not_ pcpu_schedule_unlock(): per_cpu().schedule_lock changed! */
- spin_unlock_irqrestore(old_lock, flags);
-}
-
/* Change the scheduler of cpu to us (RTDS). */
static spinlock_t *
rt_switch_sched(struct scheduler *new_ops, unsigned int cpu,
@@ -1568,7 +1538,6 @@ static const struct scheduler sched_rtds_def = {
.dump_settings = rt_dump,
.init = rt_init,
.deinit = rt_deinit,
- .init_pdata = rt_init_pdata,
.switch_sched = rt_switch_sched,
.deinit_pdata = rt_deinit_pdata,
.alloc_domdata = rt_alloc_domdata,
--
2.16.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Xen-devel] [PATCH] xen/sched: remove sched_init_pdata()
2020-02-10 15:39 [Xen-devel] [PATCH] xen/sched: remove sched_init_pdata() Juergen Gross
@ 2020-02-11 10:37 ` Dario Faggioli
2020-02-11 10:39 ` Jürgen Groß
0 siblings, 1 reply; 3+ messages in thread
From: Dario Faggioli @ 2020-02-11 10:37 UTC (permalink / raw)
To: Juergen Gross, xen-devel; +Cc: George Dunlap, Meng Xu
[-- Attachment #1.1: Type: text/plain, Size: 2086 bytes --]
On Mon, 2020-02-10 at 16:39 +0100, Juergen Gross wrote:
> sched_init_pdata() is used nowhere, it can be removed. Same applies
> to
> the .init_pdata hook of the per-scheduler interface.
>
Right, and that appear to be the case since
f855dd962523b6cb47a92037bdd28b1485141abe ("sched: add minimalistic idle
scheduler for free cpus"), which removed all call sites.
And that is because switching to a scheduler always happens via
switch_sched from the idle scheduler, and it's there that we do all the
initializations, right?
This change is obviously doing the right thing, removing code that is
never called! :-)
Can we, though:
- add a mention to the commit above and a quick explanation of things
in the changelog?
- update the following comments too:
1) in cpu_schedule_callback()
"* This happens by calling the deinit_pdata and free_pdata hooks, in this
* order. If no per-pCPU memory was allocated, there is no need to
* provide an implementation of free_pdata. deinit_pdata may, however,
* be necessary/useful in this case too (e.g., it can undo something done
* on scheduler wide data structure during init_pdata). Both deinit_pdata
* and free_pdata are called during CPU_DEAD."
2) schedule_cpu_add()
"* - a valid instance of per-CPU scheduler specific data, as it is
* allocated by sched_alloc_pdata(). Note that we do not want to
* initialize it yet (i.e., we are not calling sched_init_pdata()).
* That will be done by the target scheduler, in sched_switch_sched(),
* in proper ordering and with locking."
Regards
--
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 157 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Xen-devel] [PATCH] xen/sched: remove sched_init_pdata()
2020-02-11 10:37 ` Dario Faggioli
@ 2020-02-11 10:39 ` Jürgen Groß
0 siblings, 0 replies; 3+ messages in thread
From: Jürgen Groß @ 2020-02-11 10:39 UTC (permalink / raw)
To: Dario Faggioli, xen-devel; +Cc: George Dunlap, Meng Xu
On 11.02.20 11:37, Dario Faggioli wrote:
> On Mon, 2020-02-10 at 16:39 +0100, Juergen Gross wrote:
>> sched_init_pdata() is used nowhere, it can be removed. Same applies
>> to
>> the .init_pdata hook of the per-scheduler interface.
>>
> Right, and that appear to be the case since
> f855dd962523b6cb47a92037bdd28b1485141abe ("sched: add minimalistic idle
> scheduler for free cpus"), which removed all call sites.
>
> And that is because switching to a scheduler always happens via
> switch_sched from the idle scheduler, and it's there that we do all the
> initializations, right?
>
> This change is obviously doing the right thing, removing code that is
> never called! :-)
>
> Can we, though:
> - add a mention to the commit above and a quick explanation of things
> in the changelog?
Okay.
> - update the following comments too:
> 1) in cpu_schedule_callback()
>
> "* This happens by calling the deinit_pdata and free_pdata hooks, in this
> * order. If no per-pCPU memory was allocated, there is no need to
> * provide an implementation of free_pdata. deinit_pdata may, however,
> * be necessary/useful in this case too (e.g., it can undo something done
> * on scheduler wide data structure during init_pdata). Both deinit_pdata
> * and free_pdata are called during CPU_DEAD."
>
> 2) schedule_cpu_add()
>
> "* - a valid instance of per-CPU scheduler specific data, as it is
> * allocated by sched_alloc_pdata(). Note that we do not want to
> * initialize it yet (i.e., we are not calling sched_init_pdata()).
> * That will be done by the target scheduler, in sched_switch_sched(),
> * in proper ordering and with locking."
Oh, I missed those. Will modify the comments.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-02-11 10:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-10 15:39 [Xen-devel] [PATCH] xen/sched: remove sched_init_pdata() Juergen Gross
2020-02-11 10:37 ` Dario Faggioli
2020-02-11 10:39 ` Jürgen Groß
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).