Messages in this thread Patch in this message | | | Date | Tue, 14 Apr 2020 13:36:39 +0200 | From | Peter Zijlstra <> | Subject | Re: [RFC PATCH 03/13] sched: Core-wide rq->lock |
| |
On Wed, Mar 04, 2020 at 04:59:53PM +0000, vpillai wrote: > @@ -6400,8 +6464,15 @@ int sched_cpu_activate(unsigned int cpu) > /* > * When going up, increment the number of cores with SMT present. > */ > - if (cpumask_weight(cpu_smt_mask(cpu)) == 2) > + if (cpumask_weight(cpu_smt_mask(cpu)) == 2) { > static_branch_inc_cpuslocked(&sched_smt_present); > +#ifdef CONFIG_SCHED_CORE > + if (static_branch_unlikely(&__sched_core_enabled)) { > + rq->core_enabled = true; > + } > +#endif > + } > + > #endif > set_cpu_active(cpu, true); > > @@ -6447,8 +6518,16 @@ int sched_cpu_deactivate(unsigned int cpu) > /* > * When going down, decrement the number of cores with SMT present. > */ > - if (cpumask_weight(cpu_smt_mask(cpu)) == 2) > + if (cpumask_weight(cpu_smt_mask(cpu)) == 2) { > +#ifdef CONFIG_SCHED_CORE > + struct rq *rq = cpu_rq(cpu); > + if (static_branch_unlikely(&__sched_core_enabled)) { > + rq->core_enabled = false; > + } > +#endif > static_branch_dec_cpuslocked(&sched_smt_present); > + > + } > #endif > > if (!sched_smp_initialized)
Aside from the fact that it's probably much saner to write this as:
rq->core_enabled = static_key_enabled(&__sched_core_enabled);
I'm fairly sure I didn't write this part. And while I do somewhat see the point of disabling core scheduling for a core that has only a single thread on, I wonder why we care.
The thing is, this directly leads to the utter horror-show that is patch 6.
It should be perfectly possible to core schedule a core with only a single thread on. It might be a tad silly to do, but it beats the heck out of the trainwreck created here.
So how did this happen?
| |