From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: [RFC/RFT][PATCH 3 of 3] Introduce proper locking in sedf. Date: Wed, 23 Nov 2011 16:11:02 +0100 Message-ID: <1322061062.30168.22.camel@Abyss> References: <1322060131.30168.15.camel@Abyss> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0241393363426667974==" Return-path: In-Reply-To: <1322060131.30168.15.camel@Abyss> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com Cc: George Dunlap , Juergen Gross , Keir Fraser List-Id: xen-devel@lists.xenproject.org --===============0241393363426667974== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-IjfV40v9ap5/aVZ3f22v" --=-IjfV40v9ap5/aVZ3f22v Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Add a global scheduler lock in sedf, as we have in credit and credit2, mostly for preventing races while adjusting scheduling parameters. Also, add runq locking within sedf_adjust_weights since we are touching scheduling sensitive values in there. Signed-off-by: Dario Faggioli diff -r 17653f5c2995 xen/common/sched_sedf.c --- a/xen/common/sched_sedf.c Wed Nov 23 15:20:28 2011 +0100 +++ b/xen/common/sched_sedf.c Wed Nov 23 15:36:22 2011 +0100 @@ -61,6 +61,11 @@ struct sedf_dom_info { struct domain *domain; }; =20 +struct sedf_priv_info { + /* lock for the whole pluggable scheduler, nests inside cpupool_lock *= / + spinlock_t lock; +}; + struct sedf_vcpu_info { struct vcpu *vcpu; struct list_head list; @@ -115,6 +120,8 @@ struct sedf_cpu_info { s_time_t current_slice_expires; }; =20 +#define SEDF_PRIV(_ops) \ + ((struct sedf_priv_info *)((_ops)->sched_data)) #define EDOM_INFO(d) ((struct sedf_vcpu_info *)((d)->sched_priv)) #define CPU_INFO(cpu) \ ((struct sedf_cpu_info *)per_cpu(schedule_data, cpu).sched_priv) @@ -772,6 +779,31 @@ static struct task_slice sedf_do_extra_s } =20 +static int sedf_init(struct scheduler *ops) +{ + struct sedf_priv_info *prv; + + prv =3D xzalloc(struct sedf_priv_info); + if ( prv =3D=3D NULL ) + return -ENOMEM; + + ops->sched_data =3D prv; + spin_lock_init(&prv->lock); + + return 0; +} + + +static void sedf_deinit(const struct scheduler *ops) +{ + struct sedf_priv_info *prv; + + prv =3D SEDF_PRIV(ops); + if ( prv !=3D NULL ) + xfree(prv); +} + + /* Main scheduling function Reasons for calling this function are: -timeslice for the current period used up @@ -1247,11 +1279,18 @@ 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 =3D SEDF_PRIV(ops); struct list_head *list, *queue, *tmp; struct sedf_vcpu_info *d_inf; struct domain *d; struct vcpu *ed; int loop =3D 0; + unsigned long flags; + + /* Lock the scheduler, we want to be sure we dump a + * consistent set of parameters. + */ + spin_lock_irqsave(&prv->lock, flags); =20 printk("now=3D%"PRIu64"\n",NOW()); queue =3D RUNQ(i); @@ -1316,6 +1355,8 @@ static void sedf_dump_cpu_state(const st } } rcu_read_unlock(&domlist_read_lock); + + spin_unlock_irqrestore(&prv->lock, flags); } =20 @@ -1335,7 +1376,9 @@ static int sedf_adjust_weights(struct cp return -ENOMEM; } =20 - /* Sum across all weights. */ + /* Sum across all weights. Notice that no runq locking is needed + * here: the caller holds sedf_priv_info->lock and we're not changing + * anything that is accessed during scheduling. */ rcu_read_lock(&domlist_read_lock); for_each_domain_in_cpupool( d, c ) { @@ -1365,7 +1408,9 @@ static int sedf_adjust_weights(struct cp } rcu_read_unlock(&domlist_read_lock); =20 - /* Adjust all slices (and periods) to the new weight. */ + /* Adjust all slices (and periods) to the new weight. Unlike above, we + * need to take thr runq lock for the various VCPUs: we're modyfing + * slice and period which are referenced during scheduling. */ rcu_read_lock(&domlist_read_lock); for_each_domain_in_cpupool( d, c ) { @@ -1375,12 +1420,15 @@ static int sedf_adjust_weights(struct cp continue; if ( EDOM_INFO(p)->weight ) { + /* Interrupts already off */ + vcpu_schedule_lock(p); EDOM_INFO(p)->period_orig =3D=20 EDOM_INFO(p)->period =3D WEIGHT_PERIOD; EDOM_INFO(p)->slice_orig =3D EDOM_INFO(p)->slice =3D=20 (EDOM_INFO(p)->weight * (WEIGHT_PERIOD - WEIGHT_SAFETY - sumt[cpu])) / sumw[c= pu]; + vcpu_schedule_unlock(p); } } } @@ -1396,6 +1444,8 @@ static int sedf_adjust_weights(struct cp /* set or fetch domain scheduling parameters */ static int sedf_adjust(const struct scheduler *ops, struct domain *p, stru= ct xen_domctl_scheduler_op *op) { + struct sedf_priv_info *prv =3D SEDF_PRIV(ops); + unsigned long flags; struct vcpu *v; int rc =3D 0; =20 @@ -1404,11 +1454,12 @@ static int sedf_adjust(const struct sche p->domain_id, op->u.sedf.period, op->u.sedf.slice, op->u.sedf.latency, (op->u.sedf.extratime)?"yes":"no"); =20 - /* Serialize everything against runq lock to prevent concurrent update - * (notice all non-current VCPUs of the domain have been paused in the - * caller). */ - if ( p =3D=3D current->domain ) - vcpu_schedule_lock_irq(current); + /* Serialize against the pluggable scheduler lock to protect from + * concurrent updates. We need to take the runq lock for the VCPUs + * as well, since we are touching extraweight, weight, slice and + * period. As in sched_credit2.c, runq locks nest inside the + * pluggable scheduler lock. */ + spin_lock_irqsave(&prv->lock, flags); =20 if ( op->cmd =3D=3D XEN_DOMCTL_SCHEDOP_putinfo ) { @@ -1427,43 +1478,52 @@ static int sedf_adjust(const struct sche /* Weight-driven domains with extratime only. */ for_each_vcpu ( p, v ) { + /* (Here and everywhere in the following) IRQs are alr= eady off, + * hence vcpu_spin_lock() is the one. */ + vcpu_schedule_lock(v); EDOM_INFO(v)->extraweight =3D op->u.sedf.weight; EDOM_INFO(v)->weight =3D 0; EDOM_INFO(v)->slice =3D 0; EDOM_INFO(v)->period =3D WEIGHT_PERIOD; + vcpu_schedule_unlock(v); } } else { /* Weight-driven domains with real-time execution. */ - for_each_vcpu ( p, v ) + for_each_vcpu ( p, v ) { + vcpu_schedule_lock(v); EDOM_INFO(v)->weight =3D op->u.sedf.weight; + vcpu_schedule_lock(v); + } } } else { + /* + * Sanity checking: note that disabling extra weight requires + * that we set a non-zero slice. + */ + if ( (op->u.sedf.period > PERIOD_MAX) || + (op->u.sedf.period < PERIOD_MIN) || + (op->u.sedf.slice > op->u.sedf.period) || + (op->u.sedf.slice < SLICE_MIN) ) + { + rc =3D -EINVAL; + goto out; + } + /* Time-driven domains. */ for_each_vcpu ( p, v ) { - /* - * Sanity checking: note that disabling extra weight requi= res - * that we set a non-zero slice. - */ - if ( (op->u.sedf.period > PERIOD_MAX) || - (op->u.sedf.period < PERIOD_MIN) || - (op->u.sedf.slice > op->u.sedf.period) || - (op->u.sedf.slice < SLICE_MIN) ) - { - rc =3D -EINVAL; - goto out; - } - + vcpu_schedule_lock(v); EDOM_INFO(v)->weight =3D 0; EDOM_INFO(v)->extraweight =3D 0; EDOM_INFO(v)->period_orig =3D=20 EDOM_INFO(v)->period =3D op->u.sedf.period; EDOM_INFO(v)->slice_orig =3D=20 EDOM_INFO(v)->slice =3D op->u.sedf.slice; + vcpu_schedule_unlock(v); } } =20 @@ -1473,11 +1533,13 @@ static int sedf_adjust(const struct sche =20 for_each_vcpu ( p, v ) { + vcpu_schedule_lock(v); EDOM_INFO(v)->status =3D=20 (EDOM_INFO(v)->status & ~EXTRA_AWARE) | (op->u.sedf.extratime & EXTRA_AWARE); EDOM_INFO(v)->latency =3D op->u.sedf.latency; extraq_check(v); + vcpu_schedule_lock(v); } } else if ( op->cmd =3D=3D XEN_DOMCTL_SCHEDOP_getinfo ) @@ -1496,17 +1558,19 @@ static int sedf_adjust(const struct sche } =20 out: - if ( p =3D=3D current->domain ) - vcpu_schedule_unlock_irq(current); + spin_unlock_irqrestore(&prv->lock, flags); =20 PRINT(2,"sedf_adjust_finished with return code %d\n", rc); return rc; } =20 +static struct sedf_priv_info _sedf_priv; + const struct scheduler sched_sedf_def =3D { - .name =3D "Simple EDF Scheduler", - .opt_name =3D "sedf", - .sched_id =3D XEN_SCHEDULER_SEDF, + .name =3D "Simple EDF Scheduler", + .opt_name =3D "sedf", + .sched_id =3D XEN_SCHEDULER_SEDF, + .sched_data =3D &_sedf_priv, =20 .init_domain =3D sedf_init_domain, .destroy_domain =3D sedf_destroy_domain, @@ -1520,6 +1584,9 @@ const struct scheduler sched_sedf_def =3D=20 .alloc_domdata =3D sedf_alloc_domdata, .free_domdata =3D sedf_free_domdata, =20 + .init =3D sedf_init, + .deinit =3D sedf_deinit, + .do_schedule =3D sedf_do_schedule, .pick_cpu =3D sedf_pick_cpu, .dump_cpu_state =3D sedf_dump_cpu_state, --=20 <> (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) --=-IjfV40v9ap5/aVZ3f22v Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iEYEABECAAYFAk7NDQYACgkQk4XaBE3IOsTwSACfZG19agcDoI2QpTUucS6PzrFv HI8An0lOVe9XzYwMQbzTGduA0j0SE2XF =Egjy -----END PGP SIGNATURE----- --=-IjfV40v9ap5/aVZ3f22v-- --===============0241393363426667974== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --===============0241393363426667974==--