|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 3/7] xen: rework locking for dump of scheduler info (debug-key r)
On 03/16/2015 05:05 PM, Dario Faggioli wrote:
> such as it is taken care of by the various schedulers, rather
> than happening in schedule.c. In fact, it is the schedulers
> that know better which locks are necessary for the specific
> dumping operations.
>
> While there, fix a few style issues (indentation, trailing
> whitespace, parentheses and blank line after var declarations)
>
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> Cc: Meng Xu <xumengpanda@xxxxxxxxx>
> Cc: Jan Beulich <JBeulich@xxxxxxxx>
> Cc: Keir Fraser <keir@xxxxxxx>
Thanks!
Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> ---
> xen/common/sched_credit.c | 42 ++++++++++++++++++++++++++++++++++++++++--
> xen/common/sched_credit2.c | 40 ++++++++++++++++++++++++++++++++--------
> xen/common/sched_rt.c | 7 +++++--
> xen/common/schedule.c | 5 ++---
> 4 files changed, 79 insertions(+), 15 deletions(-)
>
> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> index bec67ff..953ecb0 100644
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -26,6 +26,23 @@
>
>
> /*
> + * Locking:
> + * - Scheduler-lock (a.k.a. runqueue lock):
> + * + is per-runqueue, and there is one runqueue per-cpu;
> + * + serializes all runqueue manipulation operations;
> + * - Private data lock (a.k.a. private scheduler lock):
> + * + serializes accesses to the scheduler global state (weight,
> + * credit, balance_credit, etc);
> + * + serializes updates to the domains' scheduling parameters.
> + *
> + * Ordering is "private lock always comes first":
> + * + if we need both locks, we must acquire the private
> + * scheduler lock for first;
> + * + if we already own a runqueue lock, we must never acquire
> + * the private scheduler lock.
> + */
> +
> +/*
> * Basic constants
> */
> #define CSCHED_DEFAULT_WEIGHT 256
> @@ -1750,11 +1767,24 @@ 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;
> + spinlock_t *lock = lock;
> + unsigned long flags;
> int loop;
> #define cpustr keyhandler_scratch
>
> + /*
> + * We need both locks:
> + * - csched_dump_vcpu() wants to access domains' scheduling
> + * parameters, which are protected by the private scheduler lock;
> + * - we scan through the runqueue, so we need the proper runqueue
> + * lock (the one of the runqueue of this cpu).
> + */
> + spin_lock_irqsave(&prv->lock, flags);
> + lock = pcpu_schedule_lock(cpu);
> +
> spc = CSCHED_PCPU(cpu);
> runq = &spc->runq;
>
> @@ -1781,6 +1811,9 @@ csched_dump_pcpu(const struct scheduler *ops, int cpu)
> csched_dump_vcpu(svc);
> }
> }
> +
> + pcpu_schedule_unlock(lock, cpu);
> + spin_unlock_irqrestore(&prv->lock, flags);
> #undef cpustr
> }
>
> @@ -1792,7 +1825,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
>
> @@ -1835,15 +1868,20 @@ csched_dump(const struct scheduler *ops)
> list_for_each( iter_svc, &sdom->active_vcpu )
> {
> struct csched_vcpu *svc;
> + spinlock_t *lock;
> +
> svc = list_entry(iter_svc, struct csched_vcpu, active_vcpu_elem);
> + lock = vcpu_schedule_lock(svc->vcpu);
>
> printk("\t%3d: ", ++loop);
> csched_dump_vcpu(svc);
> +
> + vcpu_schedule_unlock(lock, svc->vcpu);
> }
> }
> #undef idlers_buf
>
> - spin_unlock_irqrestore(&(prv->lock), flags);
> + spin_unlock_irqrestore(&prv->lock, flags);
> }
>
> static int
> diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
> index f0e2c82..564f890 100644
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -51,8 +51,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
> @@ -1800,12 +1798,24 @@ csched2_dump_vcpu(struct csched2_vcpu *svc)
> static void
> csched2_dump_pcpu(const struct scheduler *ops, int cpu)
> {
> + struct csched2_private *prv = CSCHED2_PRIV(ops);
> struct list_head *runq, *iter;
> struct csched2_vcpu *svc;
> + unsigned long flags;
> + spinlock_t *lock;
> int loop;
> char cpustr[100];
>
> - /* FIXME: Do locking properly for access to runqueue structures */
> + /*
> + * We need both locks:
> + * - csched2_dump_vcpu() wants to access domains' scheduling
> + * parameters, which are protected by the private scheduler lock;
> + * - we scan through the runqueue, so we need the proper runqueue
> + * lock (the one of the runqueue this cpu is associated to).
> + */
> + spin_lock_irqsave(&prv->lock, flags);
> + lock = per_cpu(schedule_data, cpu).schedule_lock;
> + spin_lock(lock);
>
> runq = &RQD(ops, cpu)->runq;
>
> @@ -1832,6 +1842,9 @@ csched2_dump_pcpu(const struct scheduler *ops, int cpu)
> csched2_dump_vcpu(svc);
> }
> }
> +
> + spin_unlock(lock);
> + spin_unlock_irqrestore(&prv->lock, flags);
> }
>
> static void
> @@ -1839,8 +1852,13 @@ csched2_dump(const struct scheduler *ops)
> {
> struct list_head *iter_sdom, *iter_svc;
> struct csched2_private *prv = CSCHED2_PRIV(ops);
> + unsigned long flags;
> int i, loop;
>
> + /* We need the private lock as we access global scheduler data
> + * and (below) the list of active domains. */
> + spin_lock_irqsave(&prv->lock, flags);
> +
> printk("Active queues: %d\n"
> "\tdefault-weight = %d\n",
> cpumask_weight(&prv->active_queues),
> @@ -1863,7 +1881,6 @@ csched2_dump(const struct scheduler *ops)
> fraction);
>
> }
> - /* FIXME: Locking! */
>
> printk("Domain info:\n");
> loop = 0;
> @@ -1872,20 +1889,27 @@ csched2_dump(const struct scheduler *ops)
> struct csched2_dom *sdom;
> sdom = list_entry(iter_sdom, struct csched2_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 )
> {
> struct csched2_vcpu *svc;
> + spinlock_t *lock;
> +
> svc = list_entry(iter_svc, struct csched2_vcpu, sdom_elem);
> + lock = vcpu_schedule_lock(svc->vcpu);
>
> printk("\t%3d: ", ++loop);
> csched2_dump_vcpu(svc);
> +
> + vcpu_schedule_unlock(lock, svc->vcpu);
> }
> }
> +
> + spin_unlock_irqrestore(&prv->lock, flags);
> }
>
> static void activate_runqueue(struct csched2_private *prv, int rqi)
> diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
> index 3e37e62..167a484 100644
> --- a/xen/common/sched_rt.c
> +++ b/xen/common/sched_rt.c
> @@ -253,9 +253,12 @@ rt_dump_vcpu(const struct scheduler *ops, const struct
> rt_vcpu *svc)
> static void
> rt_dump_pcpu(const struct scheduler *ops, int cpu)
> {
> - struct rt_vcpu *svc = rt_vcpu(curr_on_cpu(cpu));
> + struct rt_private *prv = rt_priv(ops);
> + unsigned long flags;
>
> - rt_dump_vcpu(ops, svc);
> + spin_lock_irqsave(&prv->lock, flags);
> + rt_dump_vcpu(ops, rt_vcpu(curr_on_cpu(cpu)));
> + spin_unlock_irqrestore(&prv->lock, flags);
> }
>
> static void
> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
> index ef79847..30eec0f 100644
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -1515,6 +1515,8 @@ void schedule_dump(struct cpupool *c)
> struct scheduler *sched;
> cpumask_t *cpus;
>
> + /* Locking, if necessary, must be handled withing each scheduler */
> +
> sched = (c == NULL) ? &ops : c->sched;
> cpus = cpupool_scheduler_cpumask(c);
> printk("Scheduler: %s (%s)\n", sched->name, sched->opt_name);
> @@ -1522,11 +1524,8 @@ void schedule_dump(struct cpupool *c)
>
> for_each_cpu (i, cpus)
> {
> - spinlock_t *lock = pcpu_schedule_lock(i);
> -
> printk("CPU[%02d] ", i);
> SCHED_OP(sched, dump_cpu_state, i);
> - pcpu_schedule_unlock(lock, i);
> }
> }
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |