From: Dario Faggioli <raistlin@linux.it>
To: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
"Keir (Xen.org)" <keir@xen.org>
Subject: [PATCH 1 of 1] sched: rework locking for dump debugkey.
Date: Wed, 18 Jan 2012 17:24:54 +0100 [thread overview]
Message-ID: <1326903894.5856.35.camel@Solace> (raw)
In-Reply-To: <1326903221.5856.33.camel@Solace>
[-- Attachment #1.1.1: Type: text/plain, Size: 8097 bytes --]
As in all other paths, locking should be dealt with in the
specific schedulers, not in schedule.c.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
diff -r 15ab61865ecb xen/common/sched_credit.c
--- a/xen/common/sched_credit.c Tue Jan 17 12:40:52 2012 +0000
+++ b/xen/common/sched_credit.c Wed Jan 18 15:02:30 2012 +0000
@@ -1451,11 +1451,16 @@ static void
csched_dump_pcpu(const struct scheduler *ops, int cpu)
{
struct list_head *runq, *iter;
+ struct csched_private *prv = CSCHED_PRIV(ops);
struct csched_pcpu *spc;
struct csched_vcpu *svc;
+ unsigned long flags;
int loop;
#define cpustr keyhandler_scratch
+ /* Domains' parameters are changed under csched_priv lock */
+ spin_lock_irqsave(&prv->lock, flags);
+
spc = CSCHED_PCPU(cpu);
runq = &spc->runq;
@@ -1464,6 +1469,12 @@ csched_dump_pcpu(const struct scheduler
cpumask_scnprintf(cpustr, sizeof(cpustr), per_cpu(cpu_core_mask, cpu));
printk("core=%s\n", cpustr);
+ /*
+ * We need runq lock as well, and as there's one runq per CPU,
+ * this is the correct one to take for this CPU/runq.
+ */
+ pcpu_schedule_lock(cpu);
+
/* current VCPU */
svc = CSCHED_VCPU(per_cpu(schedule_data, cpu).curr);
if ( svc )
@@ -1482,6 +1493,9 @@ csched_dump_pcpu(const struct scheduler
csched_dump_vcpu(svc);
}
}
+
+ pcpu_schedule_unlock(cpu);
+ spin_unlock_irqrestore(&prv->lock, flags);
#undef cpustr
}
@@ -1493,7 +1507,7 @@ csched_dump(const struct scheduler *ops)
int loop;
unsigned long flags;
- spin_lock_irqsave(&(prv->lock), flags);
+ spin_lock_irqsave(&prv->lock, flags);
#define idlers_buf keyhandler_scratch
@@ -1537,12 +1551,14 @@ csched_dump(const struct scheduler *ops)
svc = list_entry(iter_svc, struct csched_vcpu, active_vcpu_elem);
printk("\t%3d: ", ++loop);
+ vcpu_schedule_lock(svc->vcpu);
csched_dump_vcpu(svc);
+ vcpu_schedule_unlock(svc->vcpu);
}
}
#undef idlers_buf
- spin_unlock_irqrestore(&(prv->lock), flags);
+ spin_unlock_irqrestore(&prv->lock, flags);
}
static int
diff -r 15ab61865ecb xen/common/sched_credit2.c
--- a/xen/common/sched_credit2.c Tue Jan 17 12:40:52 2012 +0000
+++ b/xen/common/sched_credit2.c Wed Jan 18 15:02:30 2012 +0000
@@ -53,8 +53,6 @@
* credit2 wiki page:
* http://wiki.xen.org/wiki/Credit2_Scheduler_Development
* TODO:
- * + Immediate bug-fixes
- * - Do per-runqueue, grab proper lock for dump debugkey
* + Multiple sockets
* - Detect cpu layout and make runqueue map, one per L2 (make_runq_map())
* - Simple load balancer / runqueue assignment
@@ -1759,12 +1757,16 @@ csched_dump_vcpu(struct csched_vcpu *svc
static void
csched_dump_pcpu(const struct scheduler *ops, int cpu)
{
+ struct csched_private *prv = CSCHED_PRIV(ops);
struct list_head *runq, *iter;
struct csched_vcpu *svc;
+ unsigned long flags;
+ spinlock_t *lock;
int loop;
char cpustr[100];
- /* FIXME: Do locking properly for access to runqueue structures */
+ /* Domains' parameters are changed under csched_priv lock */
+ spin_lock_irqsave(&prv->lock, flags);
runq = &RQD(ops, cpu)->runq;
@@ -1773,6 +1775,13 @@ csched_dump_pcpu(const struct scheduler
cpumask_scnprintf(cpustr, sizeof(cpustr), per_cpu(cpu_core_mask, cpu));
printk("core=%s\n", cpustr);
+ /*
+ * We need runq lock as well, and here's how we get to it
+ * for the requested cpu.
+ */
+ lock = per_cpu(schedule_data, cpu).schedule_lock;
+ spin_lock(lock);
+
/* current VCPU */
svc = CSCHED_VCPU(per_cpu(schedule_data, cpu).curr);
if ( svc )
@@ -1791,6 +1800,9 @@ csched_dump_pcpu(const struct scheduler
csched_dump_vcpu(svc);
}
}
+
+ spin_unlock(lock);
+ spin_unlock_irqrestore(&prv->lock, flags);
}
static void
@@ -1798,8 +1810,11 @@ csched_dump(const struct scheduler *ops)
{
struct list_head *iter_sdom, *iter_svc;
struct csched_private *prv = CSCHED_PRIV(ops);
+ unsigned long flags;
int i, loop;
+ spin_lock_irqsave(&prv->lock, flags);
+
printk("Active queues: %d\n"
"\tdefault-weight = %d\n",
cpumask_weight(&prv->active_queues),
@@ -1822,7 +1837,6 @@ csched_dump(const struct scheduler *ops)
fraction);
}
- /* FIXME: Locking! */
printk("Domain info:\n");
loop = 0;
@@ -1831,10 +1845,10 @@ csched_dump(const struct scheduler *ops)
struct csched_dom *sdom;
sdom = list_entry(iter_sdom, struct csched_dom, sdom_elem);
- printk("\tDomain: %d w %d v %d\n\t",
- sdom->dom->domain_id,
- sdom->weight,
- sdom->nr_vcpus);
+ printk("\tDomain: %d w %d v %d\n\t",
+ sdom->dom->domain_id,
+ sdom->weight,
+ sdom->nr_vcpus);
list_for_each( iter_svc, &sdom->vcpu )
{
@@ -1842,9 +1856,13 @@ csched_dump(const struct scheduler *ops)
svc = list_entry(iter_svc, struct csched_vcpu, sdom_elem);
printk("\t%3d: ", ++loop);
+ vcpu_schedule_lock(svc->vcpu);
csched_dump_vcpu(svc);
+ vcpu_schedule_unlock(svc->vcpu);
}
}
+
+ spin_unlock_irqrestore(&prv->lock, flags);
}
static void activate_runqueue(struct csched_private *prv, int rqi)
diff -r 15ab61865ecb xen/common/sched_sedf.c
--- a/xen/common/sched_sedf.c Tue Jan 17 12:40:52 2012 +0000
+++ b/xen/common/sched_sedf.c Wed Jan 18 15:02:30 2012 +0000
@@ -1219,13 +1219,25 @@ static void sedf_dump_domain(struct vcpu
/* Dumps all domains on the specified cpu */
static void sedf_dump_cpu_state(const struct scheduler *ops, int i)
{
+ struct sedf_priv_info *prv = SEDF_PRIV(ops);
struct list_head *list, *queue, *tmp;
struct sedf_vcpu_info *d_inf;
struct domain *d;
struct vcpu *ed;
+ unsigned long flags;
int loop = 0;
+
+ /* Domains' parameters are changed under scheduler lock */
+ spin_lock_irqsave(&prv->lock, flags);
printk("now=%"PRIu64"\n",NOW());
+
+ /*
+ * We need runq lock as well, and as there's one runq per CPU,
+ * this is the correct one to take for this CPU/runq.
+ */
+ pcpu_schedule_lock(i);
+
queue = RUNQ(i);
printk("RUNQ rq %lx n: %lx, p: %lx\n", (unsigned long)queue,
(unsigned long) queue->next, (unsigned long) queue->prev);
@@ -1288,6 +1300,9 @@ static void sedf_dump_cpu_state(const st
}
}
rcu_read_unlock(&domlist_read_lock);
+
+ pcpu_schedule_unlock(i);
+ spin_unlock_irqrestore(&prv->lock, flags);
}
diff -r 15ab61865ecb xen/common/schedule.c
--- a/xen/common/schedule.c Tue Jan 17 12:40:52 2012 +0000
+++ b/xen/common/schedule.c Wed Jan 18 15:02:30 2012 +0000
@@ -1417,6 +1417,7 @@ void schedule_dump(struct cpupool *c)
struct scheduler *sched;
cpumask_t *cpus;
+ /* Proper locking is provided by each scheduler */
sched = (c == NULL) ? &ops : c->sched;
cpus = (c == NULL) ? &cpupool_free_cpus : c->cpu_valid;
printk("Scheduler: %s (%s)\n", sched->name, sched->opt_name);
@@ -1424,10 +1425,8 @@ void schedule_dump(struct cpupool *c)
for_each_cpu (i, cpus)
{
- pcpu_schedule_lock(i);
printk("CPU[%02d] ", i);
SCHED_OP(sched, dump_cpu_state, i);
- pcpu_schedule_unlock(i);
}
}
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)
[-- Attachment #1.1.2: rework-locking-for-dump-status.patch --]
[-- Type: text/x-patch, Size: 7879 bytes --]
# HG changeset patch
# Parent 15ab61865ecbd146f6ce65fbea5bf49bfd9c6cb1
sched: rework locking for dump debugkey.
As in all other paths, locking should be dealt with in the
specific schedulers, not in schedule.c.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
diff -r 15ab61865ecb xen/common/sched_credit.c
--- a/xen/common/sched_credit.c Tue Jan 17 12:40:52 2012 +0000
+++ b/xen/common/sched_credit.c Wed Jan 18 15:02:30 2012 +0000
@@ -1451,11 +1451,16 @@ static void
csched_dump_pcpu(const struct scheduler *ops, int cpu)
{
struct list_head *runq, *iter;
+ struct csched_private *prv = CSCHED_PRIV(ops);
struct csched_pcpu *spc;
struct csched_vcpu *svc;
+ unsigned long flags;
int loop;
#define cpustr keyhandler_scratch
+ /* Domains' parameters are changed under csched_priv lock */
+ spin_lock_irqsave(&prv->lock, flags);
+
spc = CSCHED_PCPU(cpu);
runq = &spc->runq;
@@ -1464,6 +1469,12 @@ csched_dump_pcpu(const struct scheduler
cpumask_scnprintf(cpustr, sizeof(cpustr), per_cpu(cpu_core_mask, cpu));
printk("core=%s\n", cpustr);
+ /*
+ * We need runq lock as well, and as there's one runq per CPU,
+ * this is the correct one to take for this CPU/runq.
+ */
+ pcpu_schedule_lock(cpu);
+
/* current VCPU */
svc = CSCHED_VCPU(per_cpu(schedule_data, cpu).curr);
if ( svc )
@@ -1482,6 +1493,9 @@ csched_dump_pcpu(const struct scheduler
csched_dump_vcpu(svc);
}
}
+
+ pcpu_schedule_unlock(cpu);
+ spin_unlock_irqrestore(&prv->lock, flags);
#undef cpustr
}
@@ -1493,7 +1507,7 @@ csched_dump(const struct scheduler *ops)
int loop;
unsigned long flags;
- spin_lock_irqsave(&(prv->lock), flags);
+ spin_lock_irqsave(&prv->lock, flags);
#define idlers_buf keyhandler_scratch
@@ -1537,12 +1551,14 @@ csched_dump(const struct scheduler *ops)
svc = list_entry(iter_svc, struct csched_vcpu, active_vcpu_elem);
printk("\t%3d: ", ++loop);
+ vcpu_schedule_lock(svc->vcpu);
csched_dump_vcpu(svc);
+ vcpu_schedule_unlock(svc->vcpu);
}
}
#undef idlers_buf
- spin_unlock_irqrestore(&(prv->lock), flags);
+ spin_unlock_irqrestore(&prv->lock, flags);
}
static int
diff -r 15ab61865ecb xen/common/sched_credit2.c
--- a/xen/common/sched_credit2.c Tue Jan 17 12:40:52 2012 +0000
+++ b/xen/common/sched_credit2.c Wed Jan 18 15:02:30 2012 +0000
@@ -53,8 +53,6 @@
* credit2 wiki page:
* http://wiki.xen.org/wiki/Credit2_Scheduler_Development
* TODO:
- * + Immediate bug-fixes
- * - Do per-runqueue, grab proper lock for dump debugkey
* + Multiple sockets
* - Detect cpu layout and make runqueue map, one per L2 (make_runq_map())
* - Simple load balancer / runqueue assignment
@@ -1759,12 +1757,16 @@ csched_dump_vcpu(struct csched_vcpu *svc
static void
csched_dump_pcpu(const struct scheduler *ops, int cpu)
{
+ struct csched_private *prv = CSCHED_PRIV(ops);
struct list_head *runq, *iter;
struct csched_vcpu *svc;
+ unsigned long flags;
+ spinlock_t *lock;
int loop;
char cpustr[100];
- /* FIXME: Do locking properly for access to runqueue structures */
+ /* Domains' parameters are changed under csched_priv lock */
+ spin_lock_irqsave(&prv->lock, flags);
runq = &RQD(ops, cpu)->runq;
@@ -1773,6 +1775,13 @@ csched_dump_pcpu(const struct scheduler
cpumask_scnprintf(cpustr, sizeof(cpustr), per_cpu(cpu_core_mask, cpu));
printk("core=%s\n", cpustr);
+ /*
+ * We need runq lock as well, and here's how we get to it
+ * for the requested cpu.
+ */
+ lock = per_cpu(schedule_data, cpu).schedule_lock;
+ spin_lock(lock);
+
/* current VCPU */
svc = CSCHED_VCPU(per_cpu(schedule_data, cpu).curr);
if ( svc )
@@ -1791,6 +1800,9 @@ csched_dump_pcpu(const struct scheduler
csched_dump_vcpu(svc);
}
}
+
+ spin_unlock(lock);
+ spin_unlock_irqrestore(&prv->lock, flags);
}
static void
@@ -1798,8 +1810,11 @@ csched_dump(const struct scheduler *ops)
{
struct list_head *iter_sdom, *iter_svc;
struct csched_private *prv = CSCHED_PRIV(ops);
+ unsigned long flags;
int i, loop;
+ spin_lock_irqsave(&prv->lock, flags);
+
printk("Active queues: %d\n"
"\tdefault-weight = %d\n",
cpumask_weight(&prv->active_queues),
@@ -1822,7 +1837,6 @@ csched_dump(const struct scheduler *ops)
fraction);
}
- /* FIXME: Locking! */
printk("Domain info:\n");
loop = 0;
@@ -1831,10 +1845,10 @@ csched_dump(const struct scheduler *ops)
struct csched_dom *sdom;
sdom = list_entry(iter_sdom, struct csched_dom, sdom_elem);
- printk("\tDomain: %d w %d v %d\n\t",
- sdom->dom->domain_id,
- sdom->weight,
- sdom->nr_vcpus);
+ printk("\tDomain: %d w %d v %d\n\t",
+ sdom->dom->domain_id,
+ sdom->weight,
+ sdom->nr_vcpus);
list_for_each( iter_svc, &sdom->vcpu )
{
@@ -1842,9 +1856,13 @@ csched_dump(const struct scheduler *ops)
svc = list_entry(iter_svc, struct csched_vcpu, sdom_elem);
printk("\t%3d: ", ++loop);
+ vcpu_schedule_lock(svc->vcpu);
csched_dump_vcpu(svc);
+ vcpu_schedule_unlock(svc->vcpu);
}
}
+
+ spin_unlock_irqrestore(&prv->lock, flags);
}
static void activate_runqueue(struct csched_private *prv, int rqi)
diff -r 15ab61865ecb xen/common/sched_sedf.c
--- a/xen/common/sched_sedf.c Tue Jan 17 12:40:52 2012 +0000
+++ b/xen/common/sched_sedf.c Wed Jan 18 15:02:30 2012 +0000
@@ -1219,13 +1219,25 @@ static void sedf_dump_domain(struct vcpu
/* Dumps all domains on the specified cpu */
static void sedf_dump_cpu_state(const struct scheduler *ops, int i)
{
+ struct sedf_priv_info *prv = SEDF_PRIV(ops);
struct list_head *list, *queue, *tmp;
struct sedf_vcpu_info *d_inf;
struct domain *d;
struct vcpu *ed;
+ unsigned long flags;
int loop = 0;
+
+ /* Domains' parameters are changed under scheduler lock */
+ spin_lock_irqsave(&prv->lock, flags);
printk("now=%"PRIu64"\n",NOW());
+
+ /*
+ * We need runq lock as well, and as there's one runq per CPU,
+ * this is the correct one to take for this CPU/runq.
+ */
+ pcpu_schedule_lock(i);
+
queue = RUNQ(i);
printk("RUNQ rq %lx n: %lx, p: %lx\n", (unsigned long)queue,
(unsigned long) queue->next, (unsigned long) queue->prev);
@@ -1288,6 +1300,9 @@ static void sedf_dump_cpu_state(const st
}
}
rcu_read_unlock(&domlist_read_lock);
+
+ pcpu_schedule_unlock(i);
+ spin_unlock_irqrestore(&prv->lock, flags);
}
diff -r 15ab61865ecb xen/common/schedule.c
--- a/xen/common/schedule.c Tue Jan 17 12:40:52 2012 +0000
+++ b/xen/common/schedule.c Wed Jan 18 15:02:30 2012 +0000
@@ -1417,6 +1417,7 @@ void schedule_dump(struct cpupool *c)
struct scheduler *sched;
cpumask_t *cpus;
+ /* Proper locking is provided by each scheduler */
sched = (c == NULL) ? &ops : c->sched;
cpus = (c == NULL) ? &cpupool_free_cpus : c->cpu_valid;
printk("Scheduler: %s (%s)\n", sched->name, sched->opt_name);
@@ -1424,10 +1425,8 @@ void schedule_dump(struct cpupool *c)
for_each_cpu (i, cpus)
{
- pcpu_schedule_lock(i);
printk("CPU[%02d] ", i);
SCHED_OP(sched, dump_cpu_state, i);
- pcpu_schedule_unlock(i);
}
}
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
next prev parent reply other threads:[~2012-01-18 16:24 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-18 16:13 [PATCH 0 of 1] sched: rework locking for dump debugkey Dario Faggioli
2012-01-18 16:22 ` Jan Beulich
2012-01-18 16:28 ` Dario Faggioli
2012-01-18 16:24 ` Dario Faggioli [this message]
2012-01-26 16:37 ` [PATCH 1 " George Dunlap
2012-01-26 16:55 ` Dario Faggioli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1326903894.5856.35.camel@Solace \
--to=raistlin@linux.it \
--cc=George.Dunlap@eu.citrix.com \
--cc=keir@xen.org \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.