| Message ID | 148467402295.27920.12760704062556867752.stgit@Solace.fritz.box (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 17.01.17 at 18:27, <dario.faggioli@citrix.com> wrote: > --- a/xen/include/xen/sched-if.h > +++ b/xen/include/xen/sched-if.h > @@ -47,6 +47,13 @@ DECLARE_PER_CPU(struct schedule_data, schedule_data); > DECLARE_PER_CPU(struct scheduler *, scheduler); > DECLARE_PER_CPU(struct cpupool *, cpupool); > > +/* > + * Scratch space, for avoiding having too many cpumask_var_t on the stack. Mind dropping the stray / misleading _var infix from here? There's no problem having many cpumask_var_t-s on the stack, as they're small. cpumask_t instances are problematic. Jan
On Wed, 2017-01-18 at 02:45 -0700, Jan Beulich wrote: > > > > On 17.01.17 at 18:27, <dario.faggioli@citrix.com> wrote: > > --- a/xen/include/xen/sched-if.h > > +++ b/xen/include/xen/sched-if.h > > @@ -47,6 +47,13 @@ DECLARE_PER_CPU(struct schedule_data, > > schedule_data); > > DECLARE_PER_CPU(struct scheduler *, scheduler); > > DECLARE_PER_CPU(struct cpupool *, cpupool); > > > > +/* > > + * Scratch space, for avoiding having too many cpumask_var_t on > > the stack. > > Mind dropping the stray / misleading _var infix from here? > Ah, sure! > There's > no problem having many cpumask_var_t-s on the stack, as they're > small. cpumask_t instances are problematic. > Of course. Will do. Thanks, Dario
On Tue, Jan 17, 2017 at 5:27 PM, Dario Faggioli <dario.faggioli@citrix.com> wrote: > It is ok to use just cpumask_scratch in csched_runq_steal(). > In fact, the cpu parameter comes from the cpu local variable > in csched_load_balance(), which in turn comes from cpu in > csched_schedule(), which is smp_processor_id(). > > While there, also: > - move the comment about cpumask_scratch in the header > where the scratch space is declared; > - spell more clearly (in that same comment) what are the > serialization rules. > > No functional change intended. > > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Acked-by: George Dunlap <george.dunlap@citrix.com> And... > +/* > + * Scratch space, for avoiding having too many cpumask_var_t on the stack. I can fix this up on check-in. -George
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c index dfe8545..ad20819 100644 --- a/xen/common/sched_credit.c +++ b/xen/common/sched_credit.c @@ -1636,9 +1636,8 @@ csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step) && !__vcpu_has_soft_affinity(vc, vc->cpu_hard_affinity) ) continue; - csched_balance_cpumask(vc, balance_step, cpumask_scratch_cpu(cpu)); - if ( __csched_vcpu_is_migrateable(vc, cpu, - cpumask_scratch_cpu(cpu)) ) + csched_balance_cpumask(vc, balance_step, cpumask_scratch); + if ( __csched_vcpu_is_migrateable(vc, cpu, cpumask_scratch) ) { /* We got a candidate. Grab it! */ TRACE_3D(TRC_CSCHED_STOLEN_VCPU, peer_cpu, diff --git a/xen/common/schedule.c b/xen/common/schedule.c index 36ff2e9..bee5d1f 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -65,12 +65,7 @@ static void poll_timer_fn(void *data); DEFINE_PER_CPU(struct schedule_data, schedule_data); DEFINE_PER_CPU(struct scheduler *, scheduler); -/* - * Scratch space, for avoiding having too many cpumask_var_t on the stack. - * Properly serializing access, if necessary, is responsibility of each - * scheduler (typically, one can expect this to be protected by the per pCPU - * or per runqueue lock). - */ +/* Scratch space for cpumasks. */ DEFINE_PER_CPU(cpumask_t, cpumask_scratch); extern const struct scheduler *__start_schedulers_array[], *__end_schedulers_array[]; diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h index bc0e794..c25cda6 100644 --- a/xen/include/xen/sched-if.h +++ b/xen/include/xen/sched-if.h @@ -47,6 +47,13 @@ DECLARE_PER_CPU(struct schedule_data, schedule_data); DECLARE_PER_CPU(struct scheduler *, scheduler); DECLARE_PER_CPU(struct cpupool *, cpupool); +/* + * Scratch space, for avoiding having too many cpumask_var_t on the stack. + * Within each scheduler, when using the scratch mask of one pCPU: + * - the pCPU must belong to the scheduler, + * - the caller must own the per-pCPU scheduler lock (a.k.a. runqueue + * lock). + */ DECLARE_PER_CPU(cpumask_t, cpumask_scratch); #define cpumask_scratch (&this_cpu(cpumask_scratch)) #define cpumask_scratch_cpu(c) (&per_cpu(cpumask_scratch, c))
It is ok to use just cpumask_scratch in csched_runq_steal(). In fact, the cpu parameter comes from the cpu local variable in csched_load_balance(), which in turn comes from cpu in csched_schedule(), which is smp_processor_id(). While there, also: - move the comment about cpumask_scratch in the header where the scratch space is declared; - spell more clearly (in that same comment) what are the serialization rules. No functional change intended. Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> --- Cc: George Dunlap <george.dunlap@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> --- xen/common/sched_credit.c | 5 ++--- xen/common/schedule.c | 7 +------ xen/include/xen/sched-if.h | 7 +++++++ 3 files changed, 10 insertions(+), 9 deletions(-)